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

From: Dave Chinner
Date: Thu Dec 20 2012 - 18:36:23 EST


On Thu, Dec 20, 2012 at 12:05:09PM -0800, Andy Lutomirski wrote:
> On Wed, Dec 19, 2012 at 11:03 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> 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

Yup, as I suspected. It's a filesystem specific problem.

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

Which means you are effectively running with O_CMTIME on all mmapped
files....

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

It certainly can if there is no log space available, but that's a
filesystem specific problem, not a problem with the way the VFS does
timestamp updates.

Indeed, it was only recently we changed this code to use a
transaction, previously it was doing exactly what you are proposing
completely internally to XFS. i.e. it copied the timestamp and set a
dirty flag that then trigger the next transaction that dirtied the
inode or triggered inode writeback to copy the new timestamps into
the inode with a transaction.

Cheers,

Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
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/