Re: [git pull] vfs.git - including i_mutex wrappers

[Dave Chinner]

On Sat, Jan 23, 2016 at 03:48:30PM -0800, Linus Torvalds wrote:
> On Sat, Jan 23, 2016 at 2:44 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> >
> > FWIW, I'm not opposed to making such a locking change - I'm more
> > concerned about the fact I'm finding out about plans for such a
> > fundamental locking change from a pull request on the last day of a
> > merge window....
> 
> It's actually not a new patch - the patch (or rather, the script to
> generate it - there's just a few small manual fixups for whitespace
> etc that went along with it iirc) goes back almost a year by now, and
> came about from a NFS "who owns the locking" discussion back then. It
> was mainly with Neil Brown (who brought up a NFS performance issue).
> 
> I know you were involved in that thread at least tangentially too - we
> talked about readdir() and how annoying the i_mutex is.
> 
> So the original script and patch were a "how about this" from me back
> last spring.

Right, but that was a year ago, and I don't recall there being any
clear conclusion from that discussion. Certainly not that I'm going
to remember when I'm reading a pull request that has a vague
commit message.

> And the whole "last day of the merge window" is actually intentional -
> it's behind all the filesystem merges on purpose, so that there aren't
> any merge conflicts from an almost entirely automated patch.

That's fair enough. However, compare this to how core locking
changes occur in the mm subsystem - they go through multiple patch
postings and review so there's no surprise when the pull request
comes.

> I know filesystem developers always think the buffering and the
> caching that the vfs layer does is "not important", but that's because
> you don't see the real work.

That's not true. We do see all the real loads, an more-so than
you think - most of the performance issues are solved long before
they come to the attention of the mm/page cache people because the
usual bug report is "filesystem X is slow" to the relevant
filesystem list. i.e. you don't see (and don't want to see) most of
the issues we deal with around users/applications doing terrible
things to the IO subsystems. :)

> Also, do note that the patch itself doesn't actually change locking at
> all.

Yes, but that's not the sort of warning I want when something
fundamental is about to change....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx