linux-kernel - Re: [GIT PULL] killable rwsems for v4.7

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]

Date:	Tue, 17 May 2016 19:05:52 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Michal Hocko <mhocko@...nel.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] killable rwsems for v4.7

On Tue, May 17, 2016 at 09:08:38AM +0200, Michal Hocko wrote:
> On Mon 16-05-16 13:32:28, Linus Torvalds wrote:
> > On Mon, May 16, 2016 at 7:55 AM, Ingo Molnar <mingo@...nel.org> wrote:
> > >
> > > This tree, by Michal Hocko, implements down_write_killable(). The main usecase
> > > will be to update mm_sem usage sites to use this new API,
> > 
> > Hmm. Is somebody (Michal?) looking at down_read_killable() too?
> 
> I wasn't because I didn't need it for my oom_reaper use case.
> 
> > The VFS layer will want it with the pending parallel lookup code - the
> > inode semaphore is being converted to a rwsem, and there's a couple of
> > "killable" users.. The first step actually just wants to the exclusive
> > case (ie the write case that this adds), but I think the readdir code
> > could really use a reading version too..
> 
> This is more a question for Al but I do not think adding killable read
> lock is a big deal. I can certainly help with it.

I'm not sure - killable write is needed in a bunch of places there (and the
only reason it's not used in #work.lookups is to avoid even more merge
headache; as soon as both are merged, I'll post a trivial followup switching
half a dozen places to it), killable read...  Do we really need it?
The only plausible user right now (->i_rwsem one, that is) is parallel readdir.
And I'm not convinced that we need to make that one killable.  We can
(down_read_killable seems to be easy to put together), but is it worth
using it in that usecase?

IIRC, the original motivation had been "what if anything gets stuck on NFS
server and we are left with unkillable processes on client sitting there in
attempts to lock the directory".  Seeing that lookups would be just as prone
to getting stuck (and had been all along)...  It doesn't look like there
could be a deadlock scenario avoided by having that down_read (or mutex_lock)
killable.  I can't find the original thread, unfortunately; does anyone
remember the details?