public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Ben Myers <bpm@sgi.com>
Cc: James Dingwall <james.dingwall@zynstra.com>, xfs@oss.sgi.com
Subject: Re: Cleancache support in XFS
Date: Thu, 2 May 2013 08:30:22 +1000	[thread overview]
Message-ID: <20130501223022.GQ10481@dastard> (raw)
In-Reply-To: <20130501162044.GN29359@sgi.com>

On Wed, May 01, 2013 at 11:20:44AM -0500, Ben Myers wrote:
> Hi James, 
> 
> On Wed, May 01, 2013 at 01:39:09PM +0100, James Dingwall wrote:
> > In reference to: http://oss.sgi.com/archives/xfs/2012-05/msg00046.html
> > 
> > $ grep -r cleancache fs/xfs
> > on the 3.9 kernel source suggests that no patch was submitted to
> > enable cleancache for the XFS filesystem.  Since it was suggested
> > that this could be a one liner I've had a go and my first effort is
> > inline below.  While this seems to compile OK I have no experience
> > in filesystems so I would appreciate it if anyone can point out that
> > it is obviously wrong and likely to eat my data before I try booting
> > the kernel.
> > 
> > If it seems a reasonable attempt what would be the best way to check
> > that it isn't doing nasty things?
> 
> Hrm.. Looks like there is a doc in Documentation/vm/cleancache.txt which
> includes a list of attributes the filesystem needs to have to work properly
> with cleancache.

So, those points are:

| Some points for a filesystem to consider:
|
| - The FS should be block-device-based (e.g. a ram-based FS such
|  as tmpfs should not enable cleancache)

OK.

|- To ensure coherency/correctness, the FS must ensure that all
|  file removal or truncation operations either go through VFS or
|  add hooks to do the equivalent cleancache "invalidate" operations

There be dragons - do all the XFS ioctls do the right thing?

|- To ensure coherency/correctness, either inode numbers must
|  be unique across the lifetime of the on-disk file OR the
|  FS must provide an "encode_fh" function.

Ok.

|- The FS must call the VFS superblock alloc and deactivate routines
|  or add hooks to do the equivalent cleancache calls done there.

OK.

|- To maximize performance, all pages fetched from the FS should
|  go through the do_mpag_readpage routine or the FS should add
|  hooks to do the equivalent (cf. btrfs)

xfs uses mpage_readpages() so should be fine.

|- Currently, the FS blocksize must be the same as PAGESIZE.  This
|  is not an architectural restriction, but no backends currently
|  support anything different.

Which means that we need hooks in the mount path to determine if
this is the case or not. I note that neither ext3/ext4 do this check
so I can't determine why this restriction is mentioned, and I'm not
sure if it has any relevance to btrfs.

IOWs, I'd like to know why this restriction exists - what does
cleancache care about how the filesystem maps blocks to the page in
the page cache - any way the filesystem does this it uses
page->private to hide this fact from the page cache....

|- A clustered FS should invoke the "shared_init_fs" cleancache
|  hook to get best performance for some backends.

Not a problem.

So there's a couple of things that need to be explained and
explored, and a bunch of testing to be done....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2013-05-01 22:30 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-01 12:39 Cleancache support in XFS James Dingwall
2013-05-01 16:20 ` Ben Myers
2013-05-01 22:30   ` Dave Chinner [this message]
2013-05-02  8:24     ` James Dingwall
2013-05-22 19:28       ` Konrad Rzeszutek Wilk
2013-05-24  7:30         ` James Dingwall
2013-06-07 17:08           ` Konrad Rzeszutek Wilk
2013-07-19  7:18             ` James Dingwall
2013-07-22 15:48               ` Konrad Rzeszutek Wilk
2013-07-23  7:23                 ` James Dingwall
2013-07-23  8:27                   ` Dave Chinner
2013-07-23  8:53                     ` James Dingwall
2013-11-21 13:35                     ` James Dingwall
2013-11-21 16:07                       ` Ben Myers
2013-11-21 22:12                         ` Dave Chinner
2013-11-26 16:35                           ` Konrad Rzeszutek Wilk
2013-11-26 16:35                         ` Konrad Rzeszutek Wilk
2013-05-13  8:59 ` James Dingwall
  -- strict thread matches above, loose matches on Subject: below --
2012-04-30 17:31 Alexey Vlasov
2012-04-30 19:12 ` Christoph Hellwig
2012-05-02  8:02   ` Alexey Vlasov
2012-05-05 23:40     ` Dave Chinner

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=20130501223022.GQ10481@dastard \
    --to=david@fromorbit.com \
    --cc=bpm@sgi.com \
    --cc=james.dingwall@zynstra.com \
    --cc=xfs@oss.sgi.com \
    /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