linux-kernel - Re: [git pull] vfs.git - including i

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:	Sun, 24 Jan 2016 11:26:58 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Al Viro <viro@...iv.linux.org.uk>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [git pull] vfs.git - including i_mutex wrappers

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@...morbit.com> 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@...morbit.com