All of lore.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 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.