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]

Message-ID: <CA+55aFwEYux859r4yH9jpfEB+UVjULMpTmPb7pTCf8M5Mdgx8Q@mail.gmail.com>
Date:	Sat, 23 Jan 2016 15:48:30 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Dave Chinner <david@...morbit.com>
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 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.

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.

Side note: a large reason for the patch is exactly the fact that
particularly for things like filename lookup, the vfs layer really
needs to be in control of locking, and I disagreed violently with Neil
who thought the filesystem should be in control. We really have had
much better luck with trying to make sure that the vfs layer does a
reasonable job at locking than have each filesystem decide to do its
own thing.

But obviously, the inode semaphore is one of the weakest areas of the
vfs locking. I agree that it needs fixing, I just disagree violently
when somebody says "the vfs layer should get out of the way".

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. Outside of some very specialized loads,
the cached cases are generally 99% of pretty much all loads, and it's
why doing things at the vfs layer (and the mm layer - the two are kind
of incestuous when it comes to things like the page cache) has been so
successful and important, and why I completely dismiss any "filesystem
should be in control" arguments. We used to do that, and it was a
nightmare.

Also, do note that the patch itself doesn't actually change locking at
all. It's purely about making it easier to slowly start changing the
locking by adding this abstraction layer that makes it possible to
change the lock type eventually. Al has some plans for (maybe) next
merge window, but for now it's all entirely syntactic preparation for
the future, no real change at all.

             Linus