xfs
[Top] [All Lists]

Re: [PATCH v3] Add test 249: Check filesystem FITRIM implementation

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH v3] Add test 249: Check filesystem FITRIM implementation
From: Lukas Czerner <lczerner@xxxxxxxxxx>
Date: Wed, 9 Feb 2011 13:13:09 +0100 (CET)
Cc: Lukas Czerner <lczerner@xxxxxxxxxx>, xfs@xxxxxxxxxxx, hch@xxxxxxxxxxxxx, esandeen@xxxxxxxxxx
In-reply-to: <1297167619.7351.29.camel@doink>
References: <1296762207-17265-1-git-send-email-lczerner@xxxxxxxxxx> <1297167619.7351.29.camel@doink>
User-agent: Alpine 2.00 (LFD 1167 2008-08-23)
On Tue, 8 Feb 2011, Alex Elder wrote:

> On Thu, 2011-02-03 at 20:43 +0100, Lukas Czerner wrote:
> > FITRIM ioctl  is used on a mounted filesystem to discard (or "trim")
> > blocks which are not in use by the filesystem.  This is useful for
> > solid-state drives (SSDs) and thinly-provi-sioned storage. This test
> > helps to verify filesystem FITRIM implementation to assure that it
> > does not corrupts data.
> > 
> > This test creates checksums of all files in xfstests directory and
> > run several processes which clear its working directory on SCRATCH_MNT,
> > then copy everything from xfstests into its working directory, create
> > list of files in working directory and its checksums and compare it with the
> > original list of checksums. Every process works in the loop so it repeat
> > remove->copy->check, while fstrim tool is running simultaneously.
> > 
> > Fstrim is just a helper tool which uses FITRIM ioctl to actually do the
> > filesystem discard.
> > 
> > I found this very useful because when the FITRIM is really buggy (thus
> > data-destroying) the 249 test will notice, because checksums will most
> > likely change.
> 
> This sounds like a good test.  I ran it and it failed, but I
> couldn't really tell why.  Turns out the ioctl() returned
> ENOTSUP, which is fine.  But the test shouldn't work quite
> that way.
> 
> Based on this very small experience, I have a few comments,
> below.  I glanced at the C code also, and have a few suggestions
> but they're not that important.
> 
>                                       -Alex
> 
> > Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx>
> > ---
> >  249          |  170 ++++++++++++++++++++++++++++++++++++++
> >  249.out      |    3 +
> >  group        |    3 +-
> >  src/Makefile |    2 +-
> >  src/fstrim.c |  257 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 433 insertions(+), 2 deletions(-)
> >  create mode 100644 249
> >  create mode 100644 249.out
> >  create mode 100644 src/fstrim.c
> > 
> > diff --git a/249 b/249
> > new file mode 100644
> > index 0000000..9943176
> > --- /dev/null
> > +++ b/249
> > @@ -0,0 +1,170 @@
> > +#!/bin/bash
> > +# FS QA Test No. 248
> 
> Unfortunately for you, you'll need to change the number yet again.
> (When the time comes to commit it I'll update the number for you
> if needed.)  I made it number 252 for my own testing.  Remember
> to change the name as well as the content of the test and its
> output file.
> 

One more thing though. Where did you get the number 252 ? This patch
still apply cleanly on the top of the master branch. I am using this
repo: 

git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git

Shall I use a different one ? Which one is the most up-to-date ?

Thanks!
-Lukas

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