On Mon, 7 Oct 2024 at 21:31, Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote: > > 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 > thanks, Stefano, I will change this thanks cindy
