| Message ID | 20220809124816.2880990-1-suzuki.poulose@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvmtool] net: Use vfork() instead of fork() for script execution | expand |
Hi Suzuki, On Tue, Aug 09, 2022 at 01:48:16PM +0100, Suzuki K Poulose wrote: > When a script is specified for a guest nic setup, we fork() and execl()s > the script when it is time to execute the script. However this is not > optimal, given we are running a VM. The fork() will trigger marking the > entire page-table of the current process as CoW, which will trigger > unmapping the entire stage2 page tables from the guest. Anyway, the This looks correct to me, virtio_notify_status() is called when the guest writes to the status register, which in turn can end up calling virtio_net_exec_script(). This happens when the guest is up and running, when stage 2 has been created and populated. > child process will exec the script as soon as we fork(), making all > these mm operations moot. Also, this operation could be problematic > for confidential compute VMs, where it may be expensive (and sometimes > destructive) to make changes to the stage2 page tables. > > So, instead we could use vfork() and avoid the CoW and unmap of the stage2. > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > virtio/net.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/virtio/net.c b/virtio/net.c > index c4e302bd..a5e0cea5 100644 > --- a/virtio/net.c > +++ b/virtio/net.c > @@ -295,7 +295,7 @@ static int virtio_net_exec_script(const char* script, const char *tap_name) > pid_t pid; > int status; > > - pid = fork(); > + pid = vfork(); > if (pid == 0) { > execl(script, script, tap_name, NULL); This matches the man 2 page for vfork, which basically says that vfork can be used only if the child immediately issues one of the exec family of functions. Which is what is happening here. The man page for vfork also says this: "vfork() differs from fork(2) in that the calling thread is suspended until the child terminates (either normally, by calling _exit(2), or abnormally, after delivery of a fatal signal), or it makes a call to execve(2)". waitpid (which is invoked in the parent in the else branch) waits until the child has changed state, which is defined in man 2 waitpid as: "A state change is considered to be: the child terminated; the child was stopped by a signal; or the child was resumed by a signal." It doesn't mention anything about waitpid returning after the child make a call to execve, so I guess it's correct to keep the call to waitpid in the parent: Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, Alex > _exit(1); > -- > 2.37.1 >
On Tue, 9 Aug 2022 13:48:16 +0100, Suzuki K Poulose wrote: > When a script is specified for a guest nic setup, we fork() and execl()s > the script when it is time to execute the script. However this is not > optimal, given we are running a VM. The fork() will trigger marking the > entire page-table of the current process as CoW, which will trigger > unmapping the entire stage2 page tables from the guest. Anyway, the > child process will exec the script as soon as we fork(), making all > these mm operations moot. Also, this operation could be problematic > for confidential compute VMs, where it may be expensive (and sometimes > destructive) to make changes to the stage2 page tables. > > [...] Applied to kvmtool (master), thanks! [1/1] net: Use vfork() instead of fork() for script execution https://git.kernel.org/will/kvmtool/c/9987a37cfc57 Cheers,
diff --git a/virtio/net.c b/virtio/net.c index c4e302bd..a5e0cea5 100644 --- a/virtio/net.c +++ b/virtio/net.c @@ -295,7 +295,7 @@ static int virtio_net_exec_script(const char* script, const char *tap_name) pid_t pid; int status; - pid = fork(); + pid = vfork(); if (pid == 0) { execl(script, script, tap_name, NULL); _exit(1);
When a script is specified for a guest nic setup, we fork() and execl()s the script when it is time to execute the script. However this is not optimal, given we are running a VM. The fork() will trigger marking the entire page-table of the current process as CoW, which will trigger unmapping the entire stage2 page tables from the guest. Anyway, the child process will exec the script as soon as we fork(), making all these mm operations moot. Also, this operation could be problematic for confidential compute VMs, where it may be expensive (and sometimes destructive) to make changes to the stage2 page tables. So, instead we could use vfork() and avoid the CoW and unmap of the stage2. Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- virtio/net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)