linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	linux-kernel@vger.kernel.org,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>
Subject: Re: Are there u32 atomic bitops? (or dealing w/ i_flags)
Date: Fri, 21 Dec 2012 10:36:24 +1100	[thread overview]
Message-ID: <20121220233624.GX15182@dastard> (raw)
In-Reply-To: <CALCETrUxFHHVPO=EC0jmV9SSKJiOm-mc-xwsd6Ruk3p5nC4fFQ@mail.gmail.com>

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@fromorbit.com> 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@fromorbit.com

  parent reply	other threads:[~2012-12-20 23:36 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-18  1:10 Are there u32 atomic bitops? (or dealing w/ i_flags) Andy Lutomirski
2012-12-18  1:34 ` Ming Lei
2012-12-18  1:57 ` Al Viro
2012-12-18  2:42   ` Andy Lutomirski
2012-12-18 21:30     ` Dave Chinner
2012-12-18 22:20       ` Andy Lutomirski
2012-12-20  7:03         ` Dave Chinner
2012-12-20 20:05           ` Andy Lutomirski
2012-12-20 23:10             ` [RFC PATCH 0/4] Rework mtime and ctime updates on mmaped writes Andy Lutomirski
2012-12-20 23:10               ` [RFC PATCH 1/4] mm: Explicitly track when the page dirty bit is transferred from a pte Andy Lutomirski
2012-12-20 23:10               ` [RFC PATCH 2/4] mm: Update file times when inodes are written after mmaped writes Andy Lutomirski
2012-12-21  0:14                 ` Dave Chinner
2012-12-21  0:58                   ` Jan Kara
2012-12-21  1:12                     ` Dave Chinner
2012-12-21  1:36                       ` Jan Kara
2012-12-21  5:36                   ` Andy Lutomirski
2012-12-21 10:51                     ` Jan Kara
2012-12-21 18:26                       ` Andy Lutomirski
2012-12-21  0:34                 ` Jan Kara
2012-12-21  5:42                   ` Andy Lutomirski
2012-12-21 11:03                     ` Jan Kara
2012-12-20 23:10               ` [RFC PATCH 3/4] Remove file_update_time from all mkwrite paths Andy Lutomirski
2012-12-20 23:10               ` [RFC PATCH 4/4] ext4: Fix an incorrect comment about i_mutex Andy Lutomirski
2012-12-20 23:42                 ` Jan Kara
2012-12-20 23:36             ` Dave Chinner [this message]
2012-12-20 23:42               ` Are there u32 atomic bitops? (or dealing w/ i_flags) Andy Lutomirski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121220233624.GX15182@dastard \
    --to=david@fromorbit.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).