Re: [PATCH v2 1/7] vhost: Add a new modparam to allow userspace select vhost_task

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 04, 2024 at 09:58:15AM GMT, Cindy Lu wrote:
The vhost is now using vhost_task and working as a child of the owner thread.
While this makes sense from containerization POV, some old userspace is
confused, as previously vhost not

not what?

and so was allowed to steal cpu resources
from outside the container. So we add the kthread API support back

Sorry, but it's not clear the reason.

I understand that we want to provide a way to bring back the previous behavior when we had kthreads, but why do we want that?
Do you have examples where the new mechanism is causing problems?


Add a new module parameter to allow userspace to select behaviour
between using kthread and task

Signed-off-by: Cindy Lu <lulu@xxxxxxxxxx>
---
drivers/vhost/vhost.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 9ac25d08f473..a4a0bc34f59b 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -41,6 +41,10 @@ static int max_iotlb_entries = 2048;
module_param(max_iotlb_entries, int, 0444);
MODULE_PARM_DESC(max_iotlb_entries,
	"Maximum number of iotlb entries. (default: 2048)");
+bool enforce_inherit_owner = true;
       ^
This should be static:

$ make -j6 O=build C=2 drivers/vhost/
...
  CHECK   ../drivers/vhost/vhost.c
../drivers/vhost/vhost.c:45:6: warning: symbol 'enforce_inherit_owner' was not declared. Should it be static?

+module_param(enforce_inherit_owner, bool, 0444);
+MODULE_PARM_DESC(enforce_inherit_owner,
+		 "enforce vhost use vhost_task(default: Y)");

I would follow the style of the other 2 parameters:
                 "Enforce vhost use vhost_task. (default: Y)"

With a view to simplifying bisection, we added this parameter in this patch, but it does nothing, so IMHO we should only add it at the end of the series when we have all the code ready. Maybe you can just add `enforce_inherit_owner` here or in the first patch where you need it, but I'd expose it with module_param() only when we have all the pieces in place.

About the param name, I'm not sure "enforce" is the right word, since IIUC the user can still change it using the ioctl. It would seem that set `enforce_inherit_owner` to true, it is always forced, but instead ioctl allows you to change it, right?

Is it more of a default behavior?
Something like `inherit_owner_default` ?

Thanks,
Stefano





[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux