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 18:04:06 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [git pull] vfs.git - including i_mutex wrappers

On Sun, Jan 24, 2016 at 01:41:12AM +0000, Al Viro wrote:
> On Sun, Jan 24, 2016 at 11:53:04AM +1100, Dave Chinner wrote:
> > > readdir() is another potential target for weaker exclusion (i.e. switching
> > > it to taking that thing shared), but that's a separate story and I'd prefer
> > > to deal with ->lookup() first.  There are potentially hairy issues around
> > > the instances that pre-seed dcache and I don't want to mix them into the
> > > initial series.
> > 
> > So you're doing this for purely to enable lookup concurrency, not
> > for anyone else to be able to use the inode lock as a read/write
> > lock? Can anyone use the inode rwsem as a read/write lock for their
> > own purposes? If so, we can probably use it to replace the XFS
> > IOLOCK and so effectively remove a layer of locking in various
> > XFS IO paths. What's the policy you are proposing here?
> 
> Depends...  I definitely want to keep directory modifiers with that thing
> taken exclusive, with lookup and possibly readdir - shared.  Non-directories...
> it's mostly up to filesystems; the only place where VFS cares is setattr
> and {set,remove}xattr, and that probably should stay exclusive (or be
> separated, for that matter, but I hadn't looked into implications of that;
> we probably can do that, but there might be dragons).

Separated is the model that XFS uses. i.e. the ILOCK is the lock
that serialises access to inode metadata, nests inside the IOLOCK.

Essentially that means a ->setattr operation (e.g. truncate) is

i_mutex
IOLOCK (exclusive)
<do IO serialisation>
<do data manipulation (e.g. page cache invalidation)>
start transaction
ILOCK (exclusive)
<do metadata modification>
commit transaction
<unwind locking>

Hence even for ->setattr, we can remove the IOLOCK usage if the
vfs takes the the new i_rwsem in exclusive mode because we would
still have a functional IO submission barrier....

> For data operations on regular files it's probably up to filesystems, as
> i_mutex is now.  Not sure if IOLOCK would map well on that; can you live with
> that thing taken outside of transaction?

Yes. IOLOCK has the same scope as i_mutex in the IO path.

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com