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

From: Al Viro
Date: Tue May 17 2016 - 14:06:00 EST


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@xxxxxxxxxx> 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?