All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
To: Josef Bacik <josef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Andreas Dilger <adilger-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>,
	Boaz Harrosh <bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	tytso-3s7WtUTddSA@public.gmane.org,
	jack-AlSwsSmVLrQ@public.gmane.org
Subject: Re: [PATCH] ext4: fix how i_version is modified and turn it on by default V2
Date: Tue, 15 May 2012 17:00:29 -0400	[thread overview]
Message-ID: <20120515210029.GA11932@fieldses.org> (raw)
In-Reply-To: <20120515200533.GD1907-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

On Tue, May 15, 2012 at 04:05:34PM -0400, Josef Bacik wrote:
> On Tue, May 15, 2012 at 01:55:33PM -0600, Andreas Dilger wrote:
> > It should be fairly straight forward to have a flag set in the ext4
> > superblock (s_state flag?) that indicates that the filesystem has
> > been exported via NFS.  There might be other optimizations that can
> > be done based on this (e.g. avoid some of the directory cookie
> > hijinx that are only needed if NFS has exported the filesystem and
> > needs to keep persistent cookies across reboots).
> > 
> > I think that the ext4_mark_inode_dirty() performance problem could
> > be at least partially fixed by deferring the copy of in-core inode
> > to on-disk inode to use a journal commit callback.  This is far more
> > work than just setting a flag in the superblock, but it has the
> > potential to _improve_ performance rather than make it worse.

Could you give any more pointers for an ext4 ignoramus?  (Where *is* the
journal commit code that would need the callback?  And where is the copy
currently done?)

> Yeah Btrfs doesn't have this sort of problem since we delay inode
> updating sinc it is so costly, we simply let it hang around in the
> in-core inode until we feel like updating it at some point down the
> road.  I'll put together a feature flag or something to make it be
> enabled for always if somebody turns it on.

Thanks for looking at this.

A feature flag would be an improvement over a mount option.

If the flag makes a noticeable difference to performance, then it makes
me nervous toggling it automatically.  And what will we do if statx
starts returning i_version to userspace?

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Josef Bacik <josef@redhat.com>
Cc: Andreas Dilger <adilger@dilger.ca>,
	Boaz Harrosh <bharrosh@panasas.com>,
	linux-ext4@vger.kernel.org, linux-nfs@vger.kernel.org,
	tytso@mit.edu, jack@suse.cz
Subject: Re: [PATCH] ext4: fix how i_version is modified and turn it on by default V2
Date: Tue, 15 May 2012 17:00:29 -0400	[thread overview]
Message-ID: <20120515210029.GA11932@fieldses.org> (raw)
In-Reply-To: <20120515200533.GD1907@localhost.localdomain>

On Tue, May 15, 2012 at 04:05:34PM -0400, Josef Bacik wrote:
> On Tue, May 15, 2012 at 01:55:33PM -0600, Andreas Dilger wrote:
> > It should be fairly straight forward to have a flag set in the ext4
> > superblock (s_state flag?) that indicates that the filesystem has
> > been exported via NFS.  There might be other optimizations that can
> > be done based on this (e.g. avoid some of the directory cookie
> > hijinx that are only needed if NFS has exported the filesystem and
> > needs to keep persistent cookies across reboots).
> > 
> > I think that the ext4_mark_inode_dirty() performance problem could
> > be at least partially fixed by deferring the copy of in-core inode
> > to on-disk inode to use a journal commit callback.  This is far more
> > work than just setting a flag in the superblock, but it has the
> > potential to _improve_ performance rather than make it worse.

Could you give any more pointers for an ext4 ignoramus?  (Where *is* the
journal commit code that would need the callback?  And where is the copy
currently done?)

> Yeah Btrfs doesn't have this sort of problem since we delay inode
> updating sinc it is so costly, we simply let it hang around in the
> in-core inode until we feel like updating it at some point down the
> road.  I'll put together a feature flag or something to make it be
> enabled for always if somebody turns it on.

Thanks for looking at this.

A feature flag would be an improvement over a mount option.

If the flag makes a noticeable difference to performance, then it makes
me nervous toggling it automatically.  And what will we do if statx
starts returning i_version to userspace?

--b.

  parent reply	other threads:[~2012-05-15 21:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-15 14:33 [PATCH] ext4: fix how i_version is modified and turn it on by default V2 Josef Bacik
2012-05-15 14:33 ` Josef Bacik
2012-05-15 15:18 ` Jan Kara
2012-05-15 17:53 ` Josef Bacik
     [not found]   ` <20120515175308.GB1907-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2012-05-15 18:17     ` Boaz Harrosh
2012-05-15 18:17       ` Boaz Harrosh
2012-05-15 18:29       ` Josef Bacik
2012-05-15 18:29         ` Josef Bacik
2012-05-15 19:55         ` Andreas Dilger
     [not found]           ` <AB91C817-DFEE-415F-8769-78831D72C6B7-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
2012-05-15 20:05             ` Josef Bacik
2012-05-15 20:05               ` Josef Bacik
     [not found]               ` <20120515200533.GD1907-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2012-05-15 21:00                 ` J. Bruce Fields [this message]
2012-05-15 21:00                   ` J. Bruce Fields
     [not found]                   ` <20120515210029.GA11932-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2012-05-15 21:08                     ` Josef Bacik
2012-05-15 21:08                       ` Josef Bacik
2012-05-15 22:19                     ` Andreas Dilger
2012-05-15 22:19                       ` Andreas Dilger
     [not found]         ` <20120515182914.GC1907-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2012-05-15 20:24           ` Boaz Harrosh
2012-05-15 20:24             ` Boaz Harrosh
2012-05-16  1:36   ` Ted Ts'o

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=20120515210029.GA11932@fieldses.org \
    --to=bfields-uc3wqj2krung9huczpvpmw@public.gmane.org \
    --cc=adilger-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org \
    --cc=bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org \
    --cc=jack-AlSwsSmVLrQ@public.gmane.org \
    --cc=josef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=tytso-3s7WtUTddSA@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.