linux-kernel - Re: [PATCH] floppy: convert to delayed work and single-thread wq

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20120517150649.GG21275@google.com>
Date:	Thu, 17 May 2012 08:06:49 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Jiri Kosina <jkosina@...e.cz>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Stephen Hemminger <shemminger@...tta.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] floppy: convert to delayed work and single-thread wq

On Wed, May 16, 2012 at 10:44:31PM +0200, Jiri Kosina wrote:
> On Wed, 16 May 2012, Tejun Heo wrote:
> 
> > > In floppy_grab_irq_and_dma() the point is to drain the workqueue 
> > > completely (before the conversion, we were just using 
> > > flush_work_sync(&floppy_work) for particular work item), and for that 
> > > flush_work() is not sufficient any more.
> > > 
> > > So I am really considering going back to driver-specific singlethreaded 
> > > workqueue.
> > 
> > Ummm... still confused.  flush_work_sync() is fine too.  If you have
> > two, two calls to flush_work_sync() are equivalent to flushing the
> > workqueue in effect.  You just need to avoid flush_workqueue() because
> > system workqueues may be hosting work items which can run arbitrarily
> > long.
> 
> Before the conversion, we do
> 
> 	flush_work_sync(&floppy_work);
> 
> in floppy_grab_irq_and_dma(). After the conversion, the single-threaded 
> workqueue is used to queue more than just floppy_work, and we want all 
> this to be flushed before proceeding, so neither flush_work() nor 
> flush_work_sync() is enough, as there might be floppy_work, fd_timer or 
> fd_timeout queued. This all has to be flushed.
> 
> If this still doesn't seem to make sense, I'll get back to it tomorrow, it 
> might be just too late and my brain cells might already be dreaming.

So, AFAICS, there are only three work items in question here,
floppy_work, fd_timer and fd_timeout, so three calls to
flush[_delayed]_work_sync() is functionally equivalent to
flush_workqueue().  While flushing separately may take longer, AFAICS,
it doesn't seem to be on the hot path (not much in floppy can be
considered hot these days tho) and I don't think it would make any
noticeable performance difference.

If having a separate flush domain is beneficial, please create a
non-reentrant workqueue without rescuer - alloc_workqueue("floppy",
WQ_NON_REENTRANT, 0).  If serialization among different work items is
necessary, single thread workqueue without rescuer can be used -
alloc_workqueue("floppy", WQ_UNBOUND, 1).

create_*workqueue() are left there for backwards compatibility and
they all have rescuer for that reason.  Gotta clean them up and maybe
create different wrappers, I guess.

Thanks.

-- 
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/