Re: [PATCH 1/2] virtio: abstract virtqueue related methods
Hi zhenwei, kernel test robot noticed the following build warnings: [auto build test WARNING on mst-vhost/linux-next] [also build test WARNING on linus/master v6.4-rc1 next-20230512] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/zhenwei-pi/virtio-abstract-virtqueue-related-methods/20230512-174928 base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next patch link: https://lore.kernel.org/r/20230512094618.433707-2-pizhenwei%40bytedance.com patch subject: [PATCH 1/2] virtio: abstract virtqueue related methods reproduce: # https://github.com/intel-lab-lkp/linux/commit/372bc1a0371968752fe0f5ec6e81edee6f9c44dd git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review zhenwei-pi/virtio-abstract-virtqueue-related-methods/20230512-174928 git checkout 372bc1a0371968752fe0f5ec6e81edee6f9c44dd make menuconfig # enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS make htmldocs If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202305140142.c0qqq9wz-...@intel.com/ All warnings (new ones prefixed by >>): >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_add_inbuf' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_add_outbuf' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_add_sgs' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_get_buf_ctx' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_disable_cb' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_enable_cb' not found vim +/virtqueue_add_inbuf +1 ./drivers/virtio/virtio_ring.c fd534e9b5fdcf9 Thomas Gleixner 2019-05-23 @1 // SPDX-License-Identifier: GPL-2.0-or-later 0a8a69dd77ddbd Rusty Russell 2007-10-22 2 /* Virtio ring implementation. 0a8a69dd77ddbd Rusty Russell 2007-10-22 3 * 0a8a69dd77ddbd Rusty Russell 2007-10-22 4 * Copyright 2007 Rusty Russell IBM Corporation 0a8a69dd77ddbd Rusty Russell 2007-10-22 5 */ 0a8a69dd77ddbd Rusty Russell 2007-10-22 6 #include 0a8a69dd77ddbd Rusty Russell 2007-10-22 7 #include e34f87256794b8 Rusty Russell 2008-07-25 8 #include 0a8a69dd77ddbd Rusty Russell 2007-10-22 9 #include 5a0e3ad6af8660 Tejun Heo 2010-03-24 10 #include b5a2c4f1996d1d Paul Gortmaker 2011-07-03 11 #include e93300b1afc7cd Rusty Russell 2012-01-12 12 #include 780bc7903a32ed Andy Lutomirski 2016-02-02 13 #include 88938359e2dfe1 Alexander Potapenko 2022-09-15 14 #include f8ce72632fa7ed Michael S. Tsirkin 2021-08-10 15 #include 78fe39872378b0 Andy Lutomirski 2016-02-02 16 #include 0a8a69dd77ddbd Rusty Russell 2007-10-22 17 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads
On Sat, May 13, 2023 at 7:39 AM Thorsten Leemhuis wrote: > > Jumping in here, as I found another problem with that patch: it broke > s2idle on my laptop when a qemu-kvm VM is running, as freezing user > space processes now fails for me: Hmm. kthreads have PF_NOFREEZE by default, which is probably the reason. Adding current->flags |= PF_NOFREEZE; to the vhost_task setup might just fix it, but it feels a bit off. The way io_uring does this is to do if (signal_pending(current)) { struct ksignal ksig; if (!get_signal()) continue; break; } in the main loop, which ends up handling the freezer situation too. But it should handle things like SIGSTOP etc as well, and also exit on actual signals. I get the feeling that the whole "vhost_task_should_stop()" logic should have the exact logic above, and basically make those threads killable as well. Hmm? Linus ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads
[CCing the regression list] On 06.05.23 00:37, Mike Christie wrote: > On 5/5/23 1:22 PM, Linus Torvalds wrote: >> On Fri, May 5, 2023 at 6:40 AM Nicolas Dichtel >> wrote: >>> >>> Is this an intended behavior? >>> This breaks some of our scripts. Jumping in here, as I found another problem with that patch: it broke s2idle on my laptop when a qemu-kvm VM is running, as freezing user space processes now fails for me: ``` [ 195.442949] PM: suspend entry (s2idle) [ 195.641271] Filesystems sync: 0.198 seconds [ 195.833828] Freezing user space processes [ 215.841084] Freezing user space processes failed after 20.007 seconds (1 tasks refusing to freeze, wq_busy=0): [ 215.841255] task:vhost-3221 state:R stack:0 pid:3250 ppid:3221 flags:0x4006 [ 215.841264] Call Trace: [ 215.841266] [ 215.841270] ? update_rq_clock+0x39/0x270 [ 215.841283] ? _raw_spin_unlock+0x19/0x40 [ 215.841290] ? __schedule+0x3f/0x1510 [ 215.841296] ? sysvec_apic_timer_interrupt+0xaf/0xd0 [ 215.841306] ? schedule+0x61/0xe0 [ 215.841313] ? vhost_worker+0x87/0xb0 [vhost] [ 215.841329] ? vhost_task_fn+0x1a/0x30 [ 215.841336] ? __pfx_vhost_task_fn+0x10/0x10 [ 215.841341] ? ret_from_fork+0x2c/0x50 [ 215.841352] [ 215.841936] OOM killer enabled. [ 215.841938] Restarting tasks ... done. [ 215.844204] random: crng reseeded on system resumption [ 215.957095] PM: suspend exit [ 215.957185] PM: suspend entry (s2idle) [ 215.967646] Filesystems sync: 0.010 seconds [ 215.971326] Freezing user space processes [ 235.974400] Freezing user space processes failed after 20.003 seconds (1 tasks refusing to freeze, wq_busy=0): [ 235.974574] task:vhost-3221 state:R stack:0 pid:3250 ppid:3221 flags:0x4806 [ 235.974583] Call Trace: [ 235.974586] [ 235.974593] ? __schedule+0x184/0x1510 [ 235.974605] ? sysvec_apic_timer_interrupt+0xaf/0xd0 [ 235.974616] ? schedule+0x61/0xe0 [ 235.974624] ? vhost_worker+0x87/0xb0 [vhost] [ 235.974648] ? vhost_task_fn+0x1a/0x30 [ 235.974656] ? __pfx_vhost_task_fn+0x10/0x10 [ 235.974662] ? ret_from_fork+0x2c/0x50 [ 235.974673] [ 235.975190] OOM killer enabled. [ 235.975192] Restarting tasks ... done. [ 235.978131] random: crng reseeded on system resumption [ 236.091219] PM: suspend exit ``` After running into the problem I booted 6.3.1-rc1 again and there s2idle still worked. Didn't do a bisection, just looked at the vhost commits during the latest merge window; 6e890c5d502 ("vhost: use vhost_tasks for worker threads") looked suspicious, so I reverted it on top of latest mainline and then things work again. Through a search on lore I arrived in this thread and found below patch from Mike. Gave it a try on top of latest mainline, but it didn't help. Ciao, Thorsten > [...] > If it's ok to change the behavior of "ps -u root", then we can do this patch: > (Nicolas, I confirmed it fixes the 'ps a' case, but couldn't replicate the > 'ps' > case. If you could test the ps only case or give me info on what > /usr/bin/example > was doing I can replicate and test here): > > > diff --git a/kernel/fork.c b/kernel/fork.c > index ed4e01daccaa..eb9ffc58e211 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -2269,8 +2269,14 @@ __latent_entropy struct task_struct *copy_process( > /* >* Thread groups must share signals as well, and detached threads >* can only be started up within the thread group. > + * > + * A userworker's parent thread will normally have a signal handler > + * that performs management operations, but the worker will not > + * because the parent will handle the signal then user a worker > + * specific interface to manage the thread and related resources. >*/ > - if ((clone_flags & CLONE_THREAD) && !(clone_flags & CLONE_SIGHAND)) > + if ((clone_flags & CLONE_THREAD) && !(clone_flags & CLONE_SIGHAND) && > + !args->user_worker && !args->ignore_signals) > return ERR_PTR(-EINVAL); > > /* > diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c > index b7cbd66f889e..3700c21ea39d 100644 > --- a/kernel/vhost_task.c > +++ b/kernel/vhost_task.c > @@ -75,7 +78,8 @@ struct vhost_task *vhost_task_create(int (*fn)(void *), > void *arg, >const char *name) > { > struct kernel_clone_args args = { > - .flags = CLONE_FS | CLONE_UNTRACED | CLONE_VM, > + .flags = CLONE_FS | CLONE_THREAD | CLONE_VM | > + CLONE_UNTRACED, > .exit_signal= 0, > .fn = vhost_task_fn, > .name = name ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/3] MAINTAINERS: Update maintainers for paravirt-ops
On 12.05.23 18:49, Srivatsa S. Bhat wrote: From: "Srivatsa S. Bhat (VMware)" I have decided to change employers and I'm not sure if I'll be able to spend as much time on the paravirt-ops subsystem going forward. So, I would like to remove myself from the maintainer role for paravirt-ops. Remove Srivatsa from the maintainers entry and add Ajay Kaher as an additional reviewer for paravirt-ops. Also, add an entry to CREDITS for Srivatsa. Signed-off-by: Srivatsa S. Bhat (VMware) Acked-by: Alexey Makhalov Acked-by: Ajay Kaher Acked-by: Juergen Gross Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization