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]

Message-Id: <60F0B94D-FDB9-4401-B0EA-1A1C6DE4086F@dilger.ca>
Date:	Mon, 14 May 2012 15:27:47 -0600
From:	Andreas Dilger <adilger@...ger.ca>
To:	Josef Bacik <josef@...hat.com>
Cc:	"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

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.

For writes to a high-IOPS device (e.g. SSD) can run far higher than
1000 IOPS, and this is an important use case that people care about
today, so why add useless overhead when it isn't needed?

Cheers, Andreas





--
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