linux-kernel - Re: [PATCH v4 2/2] virtio_balloon: Use a workqueue instead of "vballoon" kthread

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160102213603.GA1703@redhat.com>
Date:	Sat, 2 Jan 2016 23:36:03 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	Petr Mladek <pmladek@...e.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Jeff Epler <jepler@...ythonic.net>,
	Jiri Kosina <jkosina@...e.cz>,
	virtualization@...ts.linux-foundation.org,
	linux-kernel@...r.kernel.org, Petr Mladek <pmladek@...e.cz>
Subject: Re: [PATCH v4 2/2] virtio_balloon: Use a workqueue instead of
 "vballoon" kthread

On Sat, Jan 02, 2016 at 06:43:16AM -0500, Tejun Heo wrote:
> Hello,
> 
> On Fri, Jan 01, 2016 at 12:18:17PM +0200, Michael S. Tsirkin wrote:
> > > My initial idea was to use a dedicated workqueue. Michael S. Tsirkin
> > > suggested using a system one. Tejun Heo confirmed that the system
> > > workqueue has a pretty high concurrency level (256) by default.
> > > Therefore we need not be afraid of too long blocking.
> > 
> > Right but fill has a 1/5 second sleep on failure - *that*
> > is problematic for a system queue.
> 
> Why so?  As long as the maximum concurrently used workers are not
> high, 1/5 second or even a lot longer sleeps are completely fine.

I always thought the right way to defer executing a work queue item
is to queue delayed work, not sleep + queue work.

Doing a sleep ties up one thread for 1/5 of a second, does it not?
If so, as long as it's the only driver doing this, we'll be fine,
but if many others copy this pattern, things will
start to break, will they not?

> > > @@ -563,7 +534,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
> > >  	struct virtio_balloon *vb = vdev->priv;
> > >  
> > >  	unregister_oom_notifier(&vb->nb);
> > > -	kthread_stop(vb->thread);
> > > +	cancel_work_sync(&vb->wq_work);
> > 
> > OK but since job requeues itself, cancelling like this might not be enough.
> 
> As long as there's no further external queueing, cancel_work_sync() is
> guaranteed to kill a self-requeueing work item.
> 
> Thanks.

I didn't realise this. Thanks!

Unfortunately in this case, there can be further requeueing
if a stats request arrives.

> -- 
> tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/