Re: Are there u32 atomic bitops? (or dealing w/ i_flags)

From: Andy Lutomirski
Date: Thu Dec 20 2012 - 15:05:26 EST


On Wed, Dec 19, 2012 at 11:03 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
>
> The fact you are conerned about this function tells me something
> important - that you aren't having problems with i_mutex (show me
> where i_mutex is taken on the page_mkwrite path ;), but you are
> having latency problems with the ext4 .dirty_inode method starting a
> new transaction when it is called from mark_inode_dirty_sync().
>
> So, a filesystem specific problem, perhaps?

i_mutex isn't involved. On ext4, it looks like this (from latencytop):

start_this_handle jbd2__journal_start jbd2_journal_start
ext4_journal_start_sb ext4_dirty_inode __mark_inode_dirty update_time
file_update_time ext4_page_mkwrite do_wp_page handle_pte_fault
handle_mm_fault

This is a showstopper for my software -- I'm running on a kernel with
the call to file_update_time commented out.

>
>> Filesystems that haven't been converted can will continue to update
>> times in ->page_mkwrite.
>
> You don't need to change this at all. If you have ext4
> implement .update_timestamp to do whatever timestamp trickery you
> want and avoid ext4 starting a new transaction in .dirty_inode for
> pure timestamp updates, you can move the timestamp update into the
> ext4 writeback path. The ext4 writeback path is already quite
> special, so I'm sure people would welcome another weird behaviour
> being added to it :)
>
> IOWs, what you want to do doesn't seem to require any changes to the
> generic code. Just make it do timestamp updates in a manner similar
> to XFS and btrfs, and you can handle it all completely internally to
> the filesystem...

Are XFS and btrfs really better? XFS does:

tp = xfs_trans_alloc(mp, XFS_TRANS_FSYNC_TS);
error = xfs_trans_reserve(tp, 0, XFS_FSYNC_TS_LOG_RES(mp), 0, 0, 0);
if (error) {
xfs_trans_cancel(tp, 0);
return -error;
}

xfs_ilock(ip, XFS_ILOCK_EXCL);

This looks like it could sleep in a couple of places. I admit I
haven't actually tried it.


In any case, I have an alternative approach that I'm currently playing
with. If it survives a bit of testing, I'll send patches.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/