xfs
[Top] [All Lists]

Re: Cleancache support in XFS

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: Cleancache support in XFS
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Tue, 26 Nov 2013 11:35:48 -0500
Cc: Ben Myers <bpm@xxxxxxx>, James Dingwall <james.dingwall@xxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131121221214.GC6502@dastard>
References: <20130522192834.GD10617@xxxxxxxxxxxxxxxxxxx> <519F1708.10603@xxxxxxxxxxx> <20130607170826.GA6925@xxxxxxxxxxxxxxxxxxx> <51E8E834.6080509@xxxxxxxxxxx> <20130722154812.GC30300@xxxxxxxxxxxxxxxxxxx> <51EE2F54.80701@xxxxxxxxxxx> <20130723082700.GK19986@dastard> <528E0C3F.3010401@xxxxxxxxxxx> <20131121160741.GK1935@xxxxxxx> <20131121221214.GC6502@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Nov 22, 2013 at 09:12:14AM +1100, Dave Chinner wrote:
> On Thu, Nov 21, 2013 at 10:07:41AM -0600, Ben Myers wrote:
> > Hi James,
> > 
> > On Thu, Nov 21, 2013 at 01:35:59PM +0000, James Dingwall wrote:
> > > Dave Chinner wrote:
> > > >On Tue, Jul 23, 2013 at 08:23:00AM +0100, James Dingwall wrote:
> > > >>Konrad Rzeszutek Wilk wrote:
> > > >>>On Fri, Jul 19, 2013 at 08:18:12AM +0100, James Dingwall wrote:
> > > >>>>Konrad Rzeszutek Wilk wrote:
> > > >>>>>On Fri, May 24, 2013 at 08:30:16AM +0100, James Dingwall wrote:
> > > >>>>>>Konrad Rzeszutek Wilk wrote:
> > > >>>>>>>On Thu, May 02, 2013 at 09:24:51AM +0100, James Dingwall wrote:
> > > >>>>>>>>Dave Chinner wrote:
> > > >>>>>>>>>On Wed, May 01, 2013 at 11:20:44AM -0500, Ben Myers wrote:
> > > >>>>>>>>>>Hi James,
> > > >>>>>>>Hey folks,
> > > >>>>>>>I am walking through my vacation-emails-mbox.
> > > >>>>>>>
> > > >>>>>>>>>>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:
> > > >>>>>>>>I had started to look at these too but I feel very out of my 
> > > >>>>>>>>depth!
> > > >>>>>>>>I had similar conclusions to what Dave wrote but I don't think my
> > > >>>>>>>>thoughts should carry very much (any) weight.  Anyway I gambled 
> > > >>>>>>>>and
> > > >>>>>>>>booted my xen domU with this patch and so far so good...  xen top
> > > >>>>>>>>shows that tmem is now being used where previously it wasn't.  
> > > >>>>>>>>I'll
> > > >>>>>>>>try running the xfstests at the weekend after a couple more days 
> > > >>>>>>>>up
> > > >>>>>>>>time to see what happens.
> > > >>>>>>>And how did it go?
> > > >>>>>>I am running the patch I created on 3.9.3 on half of my xen guests
> > > >>>>>>now and have not noticed any stability or filesystem problems.  xl
> > > >>>>>>top with 'T' shows that the guests running with it are using
> > > >>>>>>ephemeral pages were those without do not.  I did do some runs with
> > > >>>>>>xfstests which had some failures but they were present with and
> > > >>>>>>without the patch.  The best I can really offer is that it works for
> > > >>>>>>me, ymmv.  The patch is available as commit
> > > >>>>>>c725011c4fc5d47e12d131f61bd91a58a40036b5 in
> > > >>>>>>https://github.com/JKDingwall/linux.git xfs-enable-cleancache or in
> > > >>>>>>the first message of this thread.
> > > >>>>>Hey James,
> > > >>>>>
> > > >>>>>I've run this patch on my local tree and it looks to work right. I am
> > > >>>>>saying "looks" as I am hitting some other issue that I believe are
> > > >>>>>unralted to the patch - but I need to figure them out before I can
> > > >>>>>comfortably say: "Yes, this looks right and works for me as well."
> > > >>>>>
> > > >>>>>Stay tuned.
> > > >>>>Just to add that I have also had no observable problems running this
> > > >>>>patch on 3.10.0 or 3.10.1.
> > > >>>Yup. And I tested it as well. In other words if you would like to add
> > > >>>Acked-by from me that would be super. Thanks!
> > > >>My patch is now available in https://github.com/JKDingwall/linux.git
> > > >>xfs-enable-cleancache as commit id
> > > >>6d50663e4ec88b7e1fd872b12ac310b1f4bb38c6.  I have rebased it on 3.10
> > > >>with KRW's Acked-by and my Signed-off-by in the commit message.  Is
> > > >>it reasonable for this to be considered for 3.12 or would further
> > > >>testing be required?
> > > >I think that's sufficient. Send it to the list ;)
> > > >
> > > >Cheers,
> > > >
> > > >Dave.
> > > I had a quick look through the 3.12 sources today and noted that
> > > this patch did not seem to get picked up.  I'm still running with it
> > > and am now on 3.12 with no observed problems.  I'm happy to rebase
> > > against 3.12 if required but it seems simple enough it should go on
> > > cleanly to the current development tree.
> > 
> > It hasn't been picked up because I'm not quite convinced it's ok to do this.
> > See if you can get a Dave, Christoph, or Eric to offer a Reviewed-by with a
> > little explanation of why this is ok and I'll pull it in.  ;)
> 
> Like I said back in July, it's OK because they performed a
> substantial amount of testing like we asked them to it didn't show
> up any obvious regressions.
> 
> Reviewed-by: Dave Chinner <david@xxxxxxxxxxxxx>

Yeey! (and I should have so read this before I sent the other email out).

Should the patch be rebased and resent? Thanks.
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>