linux-kernel - Re: lock i_mutex for fallocate?

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: <4E5FC533.1060409@linux.vnet.ibm.com>
Date:	Thu, 01 Sep 2011 10:47:31 -0700
From:	Allison Henderson <achender@...ux.vnet.ibm.com>
To:	Christoph Hellwig <hch@...radead.org>
CC:	linux-kernel@...r.kernel.org,
	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	Andreas Dilger <adilger@...ger.ca>
Subject: Re: lock i_mutex for fallocate?

On 09/01/2011 12:31 AM, Christoph Hellwig wrote:
> On Wed, Aug 31, 2011 at 05:33:25PM -0700, Allison Henderson wrote:
>> Hi All,
>>
>> In ext4 punch hole, we realized that the punch hole operation needs
>> to be done under i_mutex just like truncate.  i_mutex for truncate
>> is held in the vfs layer, so we dont need to lock it at the file
>> system layer, but vfs does not lock i_mutex for fallocate.  We can
>> lock i_mutex for fallocate at the fs layer, but question was raised
>> then: should i_mutex for fallocate be held in the vfs layer instead?
>> I do not know if other file systems need i_mutex to be locked for
>> fallocate, or if they might be locking it already, so I am doing
>> some investigating on this idea, and also the appropriate use of
>> i_mutex in general.  Can someone provide some insight this topic?
>
> Don't do it.
>
> i_mutex is already overloaded, and this does not fit into any
> of the somewhat reasonable uses cases for it, which are:
>
>   a) for directories the VFS uses it to protect the tree topology
>   b) for regular files all generic I/O code currently uses it to
>      serialize writers.
>   c) the VFS uses it around truncate, and setxattr updates
>   d) filesystems abuse it for internal metadata in various places
>
> As you can see right now we do not hold it over any file operation,
> and I'm absolutely against adding that.  I'd rather untange the
> current uses, specificly:
>
>   - push synchronization of setattr into the filesystems
>   - push synchronization of xattr write operations into the filesystems
>   - move the read/write synchronization to a separate shared/exclusive
>     lock like it's already done in XFS, and like Lukas proposed for
>     ext4.  This fixes the Posix compliance corner cases about reads
>     beeing atomic vs writes, simplifies direct I/O locking a lot,
>     and allows for more parallel direct I/O support like XFS supports.
>   - try to get rid of the abuses inside filesystems as much as possible.
>

Alrighty, this helps explain things!  Thx all for the feedback!  :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/