linux-ext4 - Re: [PATCH] ext4: turn on i_version updates by default

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:	Tue, 15 May 2012 19:59:11 +0200
From:	Marco Stornelli <marco.stornelli@...il.com>
To:	Josef Bacik <josef@...hat.com>
CC:	Andreas Dilger <adilger@...ger.ca>,
	"J. Bruce Fields" <bfields@...ldses.org>, Ted Ts'o <tytso@....edu>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
	"linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH] ext4: turn on i_version updates by default

Il 15/05/2012 15:28, Josef Bacik ha scritto:
> On Mon, May 14, 2012 at 03:27:47PM -0600, Andreas Dilger wrote:
>> On 2012-05-14, at 1:05 PM, Josef Bacik wrote:
>>> On Mon, May 14, 2012 at 02:54:00PM -0400, J. Bruce Fields wrote:
>>>> I don't think they're worried about the inode_inc_iversion() calls
>>>> themselves, but the behavior of file_update_time():
>>>>
>>>>         if (!timespec_equal(&inode->i_mtime,&now))
>>>>                 sync_it = S_MTIME;
>>>>
>>>>         if (!timespec_equal(&inode->i_ctime,&now))
>>>>                 sync_it |= S_CTIME;
>>>>
>>>>         if (IS_I_VERSION(inode))
>>>>                 sync_it |= S_VERSION;
>>>>
>>>>         if (!sync_it)
>>>>                 return;
>>>> 	...
>>>> 	mark_inode_dirty_sync(inode);
>>>>
>>>> So now mark_inode_dirty_sync() is called on every update, instead of
>>>> merely on every update that sees a time change (so at most once a
>>>> jiffy).
>>>>
>>>> So mark_inode_dirty_sync (and hence ->dirty_inode = ext4_dirty_inode)
>>>> may get called more often if you're writing very frequently.
>>>>
>>>> I'm a bit surprised that's expected to add significant overhead to the
>>>> write.
>>>
>>> It shouldn't, let's be honest, most systems aren't going to have such
>>> a coarse jiffie counter that they'll be able to get away with doing
>>> 2 calls to write() or ->page_mkwrite() in the same jiffie and skip the
>>> update to mtime/ctime anyway.  If they do they are damned lucky, and
>>> again the amount of overhead added even if they are should be
>>> negligible since 99% of us all incur the overhead from having
>>> to update mtime/ctime anyway.  Thanks,
>>
>> Seriously?  The whole reason the above checks for timespec_equal()
>> are there is to avoid calling mark_inode_dirty_sync() thousands of
>> times per second.  If doing write() calls in the same jiffie were
>> so rare as you suggest then I don't think such an optimization
>> would ever have appeared in the first place.
>>
>

Only a really really stupid question (I don't know NFS protocol well 
enough). In 3.3 kernel, I see that only ext4 uses MS_I_VERSION, so I 
wonder: if i_version change it's needed for exportable fs and so for 
nfs, other exportable fs? Is this only a particular problem for ext4? I 
mean, it doesn't seems a blocking problem (or we could have a lot of 
traffic on fs-devel :) ), it seems a "more compliant behavior". If this 
considerations is right, I think the current behavior of ext4 is ok.

Marco

Marco
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html