[patch 0/2] xfs: xfs_tosspages() bug

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch 0/2] xfs: xfs_tosspages() bug

Andrew Dahl
Hi everyone,

This patch set fixes a bug in xfs with the xfs_tosspages() function and adds a tosspages test to xfstests.

xfs_tosspages() tries to avoid tossing a partial tail page, but it gets the calculation wrong.  As a result, if we pass in an interval [0, PAGE-SIZE-1] we end up tossing all pages.  This patch set resolves this issue.

Thanks,

Andrew



_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

[patch 1/2] xfs: xfs_tosspages() bug

Andrew Dahl
xfs_tosspages() takes a closed interval as an argument, take
this into account when rounding down to the last byte of the
last complete page. If the request consists of a single
partial page, there will be nothing to toss.

Signed-off-by: Andrew Dahl <[hidden email]>

---

Index: xfs/fs/xfs/xfs_fs_subr.c
===================================================================
--- xfs.orig/fs/xfs/xfs_fs_subr.c
+++ xfs/fs/xfs/xfs_fs_subr.c
@@ -32,9 +32,17 @@ xfs_tosspages(
  xfs_off_t last,
  int fiopt)
 {
- /* can't toss partial tail pages, so mask them out */
- last &= ~(PAGE_SIZE - 1);
- truncate_inode_pages_range(VFS_I(ip)->i_mapping, first, last - 1);
+ /*
+ * Can't toss partial tail pages, so mask them out.  If the only
+ * page to toss was a partial tail, there will be nothing left
+ * to do.
+ */
+ if (last != -1) {
+        last = ((last + 1) & PAGE_MASK) - 1;
+        if (last < first)
+                return;
+ }
+ truncate_inode_pages_range(VFS_I(ip)->i_mapping, first, last);
 }
 
 int
Index: xfs/fs/xfs/xfs_vnodeops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_vnodeops.c
+++ xfs/fs/xfs/xfs_vnodeops.c
@@ -2172,7 +2172,7 @@ xfs_change_file_space(
  switch (cmd) {
  case XFS_IOC_ZERO_RANGE:
  prealloc_type |= XFS_BMAPI_CONVERT;
- xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
+ xfs_tosspages(ip, startoffset, bf->l_len ? startoffset + llen : -1, 0);
  /* FALLTHRU */
  case XFS_IOC_RESVSP:
  case XFS_IOC_RESVSP64:



_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

[patch 2/2] xfstests: xfs_tosspages() test addition

Andrew Dahl
In reply to this post by Andrew Dahl
Tests xfs_tosspages() to ensure the expected output of tossing various
ranges. It uses the XFS_IOC_ZERO_RANGE ioctl by way of "xfs_io zero"

The ranges tested are [0,4095], [0,4096], [4096,8192], [1024,5120]

Signed-off-by: Andrew Dahl <[hidden email]>

---

Index: xfstests/290
===================================================================
--- /dev/null
+++ xfstests/290
@@ -0,0 +1,86 @@
+#! /bin/bash
+# FS QA Test No. 290
+#
+# check xfs_tosspages to ensure it's tossing proper pages
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2012 SGI.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+# creator
+owner=[hidden email]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    rm -f $TEST_DIR/290.*
+}
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs xfs
+_supported_os Linux
+
+_test_io_zero()
+{
+        $XFS_IO_PROG -c "zero help" 2>&1 | \
+                        grep 'command "zero" not found' > /dev/null
+        echo $?
+}
+
+[ $(_test_io_zero) -eq 0 ] && _notrun "zero command not supported"
+
+testfile=$TEST_DIR/290.$$
+
+$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
+                      -c "pwrite -S 0x42 4096 4096" \
+                      -c "zero 0 4095" \
+                      -c "pread -v 0 8192" \
+                      $testfile | _filter_xfs_io_unique
+
+$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
+                      -c "pwrite -S 0x42 4096 4096" \
+                      -c "zero 0 4096" \
+                      -c "pread -v 0 8192" \
+                      $testfile | _filter_xfs_io_unique
+
+$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
+                      -c "pwrite -S 0x42 4096 4096" \
+                      -c "zero 4096 4096" \
+                      -c "pread -v 0 8192" \
+                      $testfile | _filter_xfs_io_unique
+
+$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
+                      -c "pwrite -S 0x42 4096 4096" \
+                      -c "zero 1024 4096" \
+                      -c "pread -v 0 8192" \
+                      $testfile | _filter_xfs_io_unique
+
+# success, all done
+status=0
+exit
Index: xfstests/290.out
===================================================================
--- /dev/null
+++ xfstests/290.out
@@ -0,0 +1,43 @@
+QA output created by 290
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000400:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Index: xfstests/group
===================================================================
--- xfstests.orig/group
+++ xfstests/group
@@ -408,3 +408,4 @@ deprecated
 287 auto dump quota quick
 288 auto quick ioctl trim
 289 auto quick
+290 auto quick ioctl



_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 1/2] xfs: xfs_tosspages() bug

Dave Chinner
In reply to this post by Andrew Dahl
On Thu, Nov 08, 2012 at 04:23:16PM -0600, Andrew Dahl wrote:

> xfs_tosspages() takes a closed interval as an argument, take
> this into account when rounding down to the last byte of the
> last complete page. If the request consists of a single
> partial page, there will be nothing to toss.
>
> Signed-off-by: Andrew Dahl <[hidden email]>
>
> ---
>
> Index: xfs/fs/xfs/xfs_fs_subr.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_fs_subr.c
> +++ xfs/fs/xfs/xfs_fs_subr.c
> @@ -32,9 +32,17 @@ xfs_tosspages(
>   xfs_off_t last,
>   int fiopt)
>  {
> - /* can't toss partial tail pages, so mask them out */
> - last &= ~(PAGE_SIZE - 1);
> - truncate_inode_pages_range(VFS_I(ip)->i_mapping, first, last - 1);
> + /*
> + * Can't toss partial tail pages, so mask them out.  If the only
> + * page to toss was a partial tail, there will be nothing left
> + * to do.
> + */
> + if (last != -1) {
> +        last = ((last + 1) & PAGE_MASK) - 1;
> +        if (last < first)
> +                return;
> + }
> + truncate_inode_pages_range(VFS_I(ip)->i_mapping, first, last);

Ok, lets look at critical ranges:

                passed to truncate_inode_pages_range
first,last current patched
0,4095 0,0xffffffff 0,4095
0,4096 0,4095 0,4095
0,4097 0,4095 0,4095

Yup, that's needed.

0,1 0,0xffffffff aborts (0,0xffffffff)

Big assumption: xfs_off_t is signed.

0xfffffffe 0xffffefff 0xfffeffff
0xffffffff 0xffffefff 0xffffffff

So the change is good.

However, there's a bigger issue here. We've planned to remove these
wrappers for a long time, just never got around to doing it. Seeing
as there is a bug in this wrapper and it needs to be fixed, now
seems like the right time to remove it.

Hence I'd suggest that fixing this particular bug should just
remove xfs_tosspages() and call truncate_inode_pages_range()
directly. There are only two calls to this function, so it should be
a simple conversion.  That can then be followed up with more patches
to remove the other wrappers in xfs_fs_subr.c and hence remove the
file completely...

>  int
> Index: xfs/fs/xfs/xfs_vnodeops.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_vnodeops.c
> +++ xfs/fs/xfs/xfs_vnodeops.c
> @@ -2172,7 +2172,7 @@ xfs_change_file_space(
>   switch (cmd) {
>   case XFS_IOC_ZERO_RANGE:
>   prealloc_type |= XFS_BMAPI_CONVERT;
> - xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> + xfs_tosspages(ip, startoffset, bf->l_len ? startoffset + llen : -1, 0);
>   /* FALLTHRU */
>   case XFS_IOC_RESVSP:
>   case XFS_IOC_RESVSP64:

What's this hunk for? Indeed, one of the first things that the
xfs_alloc_file_space() checks is this:

        if (len <= 0)
                return XFS_ERROR(EINVAL);

xfs_free_file_space() does the same check, so it is invalid to pass
a bf_len <= 0 for any of these specific functions. Hence this change
is wrong regardless of what the comment on the struct xfs_flock64_t
says - preallocation and hole punch operations must have a positive
length associated with them.

Cheers,

Dave.
--
Dave Chinner
[hidden email]

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 2/2] xfstests: xfs_tosspages() test addition

Dave Chinner
In reply to this post by Andrew Dahl
On Thu, Nov 08, 2012 at 04:23:17PM -0600, Andrew Dahl wrote:
> Tests xfs_tosspages() to ensure the expected output of tossing various
> ranges. It uses the XFS_IOC_ZERO_RANGE ioctl by way of "xfs_io zero"
>
> The ranges tested are [0,4095], [0,4096], [4096,8192], [1024,5120]

I'd add [0,1] [0,4097], [4095,8191] [4095,8192] and [4095,8193] as
well for good measure, as they exercise other off-by-one corner
cases not tested here.

> Signed-off-by: Andrew Dahl <[hidden email]>
>
> ---
>
> Index: xfstests/290
> ===================================================================
> --- /dev/null
> +++ xfstests/290
> @@ -0,0 +1,86 @@
> +#! /bin/bash
> +# FS QA Test No. 290
> +#
> +# check xfs_tosspages to ensure it's tossing proper pages

A better description about the corner cases it is test woul dbe
helpful.

> +_cleanup()
> +{
> +    rm -f $TEST_DIR/290.*
> +}

Leave them there - the test dir is supposed to grow and age over
time. Also, removing them here means on a failure the files re
removed and there is no corpse left to dissect. Besides, you are
already using O_TRUNC for opening the files, so if they already
exist the test handles it just fine.

> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs xfs
> +_supported_os Linux
> +
> +_test_io_zero()
> +{
> +        $XFS_IO_PROG -c "zero help" 2>&1 | \
> +                        grep 'command "zero" not found' > /dev/null
> +        echo $?
> +}
> +
> +[ $(_test_io_zero) -eq 0 ] && _notrun "zero command not supported"

You need to test whether the filesystem supports it. See, for
example, _require_xfs_io_falloc. Also, moving it to common.rc and
naming it _require_xfs_io_zero woul dbe a good idea.

Yes, I know you copied this from test 242, and that was probably my
fault in the first place, but rather than propagate something that
is wrong, let's fix it properly before it becomes widespread....


> +testfile=$TEST_DIR/290.$$
> +
> +$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
> +                      -c "pwrite -S 0x42 4096 4096" \
> +                      -c "zero 0 4095" \
> +                      -c "pread -v 0 8192" \
> +                      $testfile | _filter_xfs_io_unique

Nice! Someone's been listening to me about filtering. :)

> +
> +$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
> +                      -c "pwrite -S 0x42 4096 4096" \
> +                      -c "zero 0 4096" \
> +                      -c "pread -v 0 8192" \
> +                      $testfile | _filter_xfs_io_unique
> +
> +$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
> +                      -c "pwrite -S 0x42 4096 4096" \
> +                      -c "zero 4096 4096" \
> +                      -c "pread -v 0 8192" \
> +                      $testfile | _filter_xfs_io_unique
> +
> +$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
> +                      -c "pwrite -S 0x42 4096 4096" \
> +                      -c "zero 1024 4096" \
> +                      -c "pread -v 0 8192" \
> +                      $testfile | _filter_xfs_io_unique

A little wrapper function might be in order here - it makes it much
easier to see what is being tested. i.e.:

test_zero()
{
        zero_start=$1
        zero_len=$2

        $XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
                              -c "pwrite -S 0x42 4096 4096" \
                              -c "zero $zero_start $zero_end" \
                              -c "pread -v 0 8192" \
                              $testfile | _filter_xfs_io_unique
}

#  Zero Range
test_zero    0 4095
test_zero    0 4096
test_zero 4096 4096
test_zero 1024 4096

That also makes it much easier to add more ranges to be tested. :)

> Index: xfstests/group
> ===================================================================
> --- xfstests.orig/group
> +++ xfstests/group
> @@ -408,3 +408,4 @@ deprecated
>  287 auto dump quota quick
>  288 auto quick ioctl trim
>  289 auto quick
> +290 auto quick ioctl

prealloc and rw groups make sense, too.

Cheers,

Dave.
--
Dave Chinner
[hidden email]

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 1/2] xfs: xfs_tosspages() bug

Ben Myers
In reply to this post by Dave Chinner
Hey Dave,

On Fri, Nov 09, 2012 at 10:06:49AM +1100, Dave Chinner wrote:
> On Thu, Nov 08, 2012 at 04:23:16PM -0600, Andrew Dahl wrote:
> > xfs_tosspages() takes a closed interval as an argument, take
> > this into account when rounding down to the last byte of the
> > last complete page. If the request consists of a single
> > partial page, there will be nothing to toss.
> >
> > Signed-off-by: Andrew Dahl <[hidden email]>
> >
> > ---

...

> So the change is good.
>
> However, there's a bigger issue here. We've planned to remove these
> wrappers for a long time, just never got around to doing it. Seeing
> as there is a bug in this wrapper and it needs to be fixed, now
> seems like the right time to remove it.

The removal of the wrappers would not be appropriate for -stable.  This fix
needs to go in separately from any refactoring so that it can be pulled back
within the rules outlined in Documentation/stable_kernel_rules.txt.

> Hence I'd suggest that fixing this particular bug should just
> remove xfs_tosspages() and call truncate_inode_pages_range()
> directly. There are only two calls to this function, so it should be
> a simple conversion.  That can then be followed up with more patches
> to remove the other wrappers in xfs_fs_subr.c and hence remove the
> file completely...

I have no objection to doing so in a followup series, and I don't consider it
to be a high priority either.

> >  int
> > Index: xfs/fs/xfs/xfs_vnodeops.c
> > ===================================================================
> > --- xfs.orig/fs/xfs/xfs_vnodeops.c
> > +++ xfs/fs/xfs/xfs_vnodeops.c
> > @@ -2172,7 +2172,7 @@ xfs_change_file_space(
> >   switch (cmd) {
> >   case XFS_IOC_ZERO_RANGE:
> >   prealloc_type |= XFS_BMAPI_CONVERT;
> > - xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> > + xfs_tosspages(ip, startoffset, bf->l_len ? startoffset + llen : -1, 0);
> >   /* FALLTHRU */
> >   case XFS_IOC_RESVSP:
> >   case XFS_IOC_RESVSP64:
>
> What's this hunk for? Indeed, one of the first things that the
> xfs_alloc_file_space() checks is this:
>
>         if (len <= 0)
> return XFS_ERROR(EINVAL);
>
> xfs_free_file_space() does the same check, so it is invalid to pass
> a bf_len <= 0 for any of these specific functions. Hence this change
> is wrong regardless of what the comment on the struct xfs_flock64_t
> says - preallocation and hole punch operations must have a positive
> length associated with them.

Andrew, if you agree that this second change is unnecessary go ahead and remove
it and repost.  Otherwise,

Reviewed-by: Ben Myers <[hidden email]>

Welcome to the XFS community!

-Ben

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 1/2] xfs: xfs_tosspages() bug

Dave Chinner
On Thu, Nov 08, 2012 at 05:46:42PM -0600, Ben Myers wrote:

> Hey Dave,
>
> On Fri, Nov 09, 2012 at 10:06:49AM +1100, Dave Chinner wrote:
> > On Thu, Nov 08, 2012 at 04:23:16PM -0600, Andrew Dahl wrote:
> > > xfs_tosspages() takes a closed interval as an argument, take
> > > this into account when rounding down to the last byte of the
> > > last complete page. If the request consists of a single
> > > partial page, there will be nothing to toss.
> > >
> > > Signed-off-by: Andrew Dahl <[hidden email]>
> > >
> > > ---
>
> ...
>
> > So the change is good.
> >
> > However, there's a bigger issue here. We've planned to remove these
> > wrappers for a long time, just never got around to doing it. Seeing
> > as there is a bug in this wrapper and it needs to be fixed, now
> > seems like the right time to remove it.
>
> The removal of the wrappers would not be appropriate for -stable.  This fix
> needs to go in separately from any refactoring so that it can be pulled back
> within the rules outlined in Documentation/stable_kernel_rules.txt.

You're acting like I've never read those rules before. I know
exactly what they say, and patch that removes a busted helper is
well and truly within the scope of a stable backport. Quoting rules
at me like I'm a newbie only serves to annoy me....

As it is, looking at what xfs_tosspages is supposed to be doing,
calling truncate_inode_pages_range() is actually the wrong thing to
do.  We should be calling truncate_pagecache_range(), because we
should be unmapping pages before truncating them away. And for that
same reason, xfs_flushinvalidate() is also wrong and broken.

That is, the call in xfs_swap_extents() changes to:

- xfs_tosspages(ip, 0, -1, FI_REMAPF);
+ truncate_pagecache_range(VFS_I(ip), 0, -1);

And the one in xfs_change_file_space becomes:

- xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
+ truncate_pagecache_range(VFS_I(ip), startoffset,
+ startoffset + bf->l_len);

and xfs_tosspages() goes away. That's a far better fix for the
problem than what has been proposed, IMO, and in no way is
inappropriate for -stable.

As it is, I wouldn't even consider this a fix that is needed for
stable kernels - XFS_IOC_ZERO is an obscure interface, and
xfs_swap_extents works just fine as it stands....

....

> > >   prealloc_type |= XFS_BMAPI_CONVERT;
> > > - xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> > > + xfs_tosspages(ip, startoffset, bf->l_len ? startoffset + llen : -1, 0);
> > >   /* FALLTHRU */
> > >   case XFS_IOC_RESVSP:
> > >   case XFS_IOC_RESVSP64:
> >
> > What's this hunk for? Indeed, one of the first things that the
> > xfs_alloc_file_space() checks is this:
> >
> >         if (len <= 0)
> > return XFS_ERROR(EINVAL);
> >
> > xfs_free_file_space() does the same check, so it is invalid to pass
> > a bf_len <= 0 for any of these specific functions. Hence this change
> > is wrong regardless of what the comment on the struct xfs_flock64_t
> > says - preallocation and hole punch operations must have a positive
> > length associated with them.
>
> Andrew, if you agree that this second change is unnecessary go ahead and remove
> it and repost.  Otherwise,

I didn't say it was unnecessary - I said it was wrong. We shouldn't
even be getting as far as the xfs_tosspages() call if bf_len is zero
or negative. That's the bug that needs fixing in this function.

Cheers,

Dave.
--
Dave Chinner
[hidden email]

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

xfs_quota -x -c 'report -u /dev/sde5' shows the duplicate result.

yyq@eisoo.com
In reply to this post by Andrew Dahl
Hi,

    When I use "xfs_quota -x -c 'report -u /dev/sde5'" to show /dev/sde5's quotas, and the result have two same block as below:

[root@localhost ~]# xfs_quota -x -c 'report -u /dev/sde5'
User quota on /root/mntpath2 (/dev/sde5)
                               Blocks                    
User ID          Used       Soft       Hard    Warn/Grace    
---------- --------------------------------------------------
root                0          0          0     00 [--------]
user2               0     102400     409600     00 [--------]
user3               0     204800     512000     00 [--------]

User quota on /root/mntpath2 (/dev/sde5)
                               Blocks                    
User ID          Used       Soft       Hard    Warn/Grace    
---------- --------------------------------------------------
root                0          0          0     00 [--------]
user2               0     102400     409600     00 [--------]
user3               0     204800     512000     00 [--------]


    Is this correct or what does this output mean?


    My kernel is 3.4.4 and xfs filesystem version is just in the version of this kernel.
_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: xfs_quota -x -c 'report -u /dev/sde5' shows the duplicate result.

Dave Chinner
On Fri, Nov 09, 2012 at 09:12:37AM +0800, [hidden email] wrote:
> Hi,
>
>     When I use "xfs_quota -x -c 'report -u /dev/sde5'" to show
> /dev/sde5's quotas, and the result have two same block as below:

What version of xfs_quota are you using? What is the output of
/proc/mounts and /etc/mtab say?

And does the latest version of xfs_quota have the same problem?

Cheers,

Dave.
--
Dave Chinner
[hidden email]

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: xfs_quota -x -c 'report -u /dev/sde5' shows the duplicate result.

yyq@eisoo.com

于 2012-11-9 9:24, Dave Chinner 写道:
On Fri, Nov 09, 2012 at 09:12:37AM +0800, [hidden email] wrote:
Hi,

    When I use "xfs_quota -x -c 'report -u /dev/sde5'" to show
/dev/sde5's quotas, and the result have two same block as below:
What version of xfs_quota are you using? What is the output of
/proc/mounts and /etc/mtab say?
xfs_quota's version is 3.1.1 and output of /proc/mounts like this:

.....
/dev/sde3 /root/mnt_path xfs rw,relatime,attr2,usrquota 0 0
/dev/sde5 /root/mntpath2 xfs rw,relatime,attr2,usrquota 0 0
/dev/sde7 /root/mntpath3 xfs rw,relatime,attr2,usrquota 0 0


/etc/mtab:

....
/dev/sde3 /root/mnt_path xfs rw,uquota 0 0
/dev/sde5 /root/mntpath2 xfs rw,uquota 0 0
/dev/sde7 /root/mntpath3 xfs rw,uquota 0 0


in the original post "/dev/sde5" was removed and try "/dev/sde7", the remain the same problem.

And does the latest version of xfs_quota have the same problem?
version 3.1.8's output have the same problem,and even worse, there are three same blocks :

User quota on /root/mntpath3 (/dev/sde7)
                               Blocks                    
User ID          Used       Soft       Hard    Warn/Grace    
---------- --------------------------------------------------
root                0          0          0     00 [--------]
user2               0     102400     409600     00 [--------]
user3               0     204800     512000     00 [--------]

User quota on /root/mntpath3 (/dev/sde7)
                               Blocks                    
User ID          Used       Soft       Hard    Warn/Grace    
---------- --------------------------------------------------
root                0          0          0     00 [--------]
user2               0     102400     409600     00 [--------]
user3               0     204800     512000     00 [--------]

User quota on /root/mntpath3 (/dev/sde7)
                               Blocks                    
User ID          Used       Soft       Hard    Warn/Grace    
---------- --------------------------------------------------
root                0          0          0     00 [--------]
user2               0     102400     409600     00 [--------]
user3               0     204800     512000     00 [--------]




Cheers,

Dave.


_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: xfs_quota -x -c 'report -u /dev/sde5' shows the duplicate result.

Dave Chinner
On Fri, Nov 09, 2012 at 11:27:33AM +0800, [hidden email] wrote:

>
> 于 2012-11-9 9:24, Dave Chinner 写道:
> >On Fri, Nov 09, 2012 at 09:12:37AM +0800, [hidden email] wrote:
> >>Hi,
> >>
> >>     When I use "xfs_quota -x -c 'report -u /dev/sde5'" to show
> >>/dev/sde5's quotas, and the result have two same block as below:
> >What version of xfs_quota are you using? What is the output of
> >/proc/mounts and /etc/mtab say?
> xfs_quota's version is 3.1.1 and output of /proc/mounts like this:
>
> .....
> /dev/sde3 /root/mnt_path xfs rw,relatime,attr2,usrquota 0 0
> /dev/sde5 /root/mntpath2 xfs rw,relatime,attr2,usrquota 0 0
> /dev/sde7 /root/mntpath3 xfs rw,relatime,attr2,usrquota 0 0
>
> /etc/mtab:
>
> ....
> /dev/sde3 /root/mnt_path xfs rw,uquota 0 0
> /dev/sde5 /root/mntpath2 xfs rw,uquota 0 0
> /dev/sde7 /root/mntpath3 xfs rw,uquota 0 0
>
> in the original post "/dev/sde5" was removed and try "/dev/sde7",
> the remain the same problem.
> >
> >And does the latest version of xfs_quota have the same problem?
> version 3.1.8's output have the same problem,and even worse, there
> are three same blocks :
>
> User quota on /root/mntpath3 (/dev/sde7)
>                                Blocks
> User ID          Used       Soft       Hard    Warn/Grace
> ---------- --------------------------------------------------
> root                0          0          0     00 [--------]
> user2               0     102400     409600     00 [--------]
> user3               0     204800     512000     00 [--------]
>
> User quota on /root/mntpath3 (/dev/sde7)
>                                Blocks
> User ID          Used       Soft       Hard    Warn/Grace
> ---------- --------------------------------------------------
> root                0          0          0     00 [--------]
> user2               0     102400     409600     00 [--------]
> user3               0     204800     512000     00 [--------]
>
> User quota on /root/mntpath3 (/dev/sde7)
>                                Blocks
> User ID          Used       Soft       Hard    Warn/Grace
> ---------- --------------------------------------------------
> root                0          0          0     00 [--------]
> user2               0     102400     409600     00 [--------]
> user3               0     204800     512000     00 [--------]

Can you attach the strace output of the command?

Cheers,

Dave.
--
Dave Chinner
[hidden email]

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: xfs_quota -x -c 'report -u /dev/sde5' shows the duplicate result.

Dave Chinner
On Fri, Nov 09, 2012 at 02:52:31PM +0800, [hidden email] wrote:

> 于 2012-11-9 13:39, Dave Chinner 写道:
> >On Fri, Nov 09, 2012 at 11:27:33AM +0800, [hidden email] wrote:
> >>于 2012-11-9 9:24, Dave Chinner 写道:
> >>>On Fri, Nov 09, 2012 at 09:12:37AM +0800, [hidden email] wrote:
> >>>>Hi,
> >>>>
> >>>>     When I use "xfs_quota -x -c 'report -u /dev/sde5'" to show
> >>>>/dev/sde5's quotas, and the result have two same block as below:
> >>>What version of xfs_quota are you using? What is the output of
> >>>/proc/mounts and /etc/mtab say?
> >>xfs_quota's version is 3.1.1 and output of /proc/mounts like this:
....
> >Can you attach the strace output of the command?

Thanks. The patch below should fix it. Can you try it?

Cheers,

Dave.
--
Dave Chinner
[hidden email]

xfs_quota: fix report command parsing

From: Dave Chinner <[hidden email]>

The report command line needs to be parsed as a whole not as
individual elements - report_f() is set up to do this correctly.
When treated as non-global command line, the report function is
called once for each command line arg, resulting in reports being
issued multiple times.

Set the command to be a global command so that it is only called
once.

Signed-off-by: Dave Chinner <[hidden email]>
---
 quota/report.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/quota/report.c b/quota/report.c
index a1e165b..70894a2 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -653,6 +653,7 @@ report_init(void)
  report_cmd.cfunc = report_f;
  report_cmd.argmin = 0;
  report_cmd.argmax = -1;
+ report_cmd.flags = CMD_FLAG_GLOBAL;
  report_cmd.args = _("[-bir] [-gpu] [-ahnt] [-f file]");
  report_cmd.oneline = _("report filesystem quota information");
  report_cmd.help = report_help;

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 1/2] xfs: xfs_tosspages() bug

Ben Myers
In reply to this post by Dave Chinner
Hey Dave,

On Fri, Nov 09, 2012 at 12:05:17PM +1100, Dave Chinner wrote:

> On Thu, Nov 08, 2012 at 05:46:42PM -0600, Ben Myers wrote:
> > Hey Dave,
> >
> > On Fri, Nov 09, 2012 at 10:06:49AM +1100, Dave Chinner wrote:
> > > On Thu, Nov 08, 2012 at 04:23:16PM -0600, Andrew Dahl wrote:
> > > > xfs_tosspages() takes a closed interval as an argument, take
> > > > this into account when rounding down to the last byte of the
> > > > last complete page. If the request consists of a single
> > > > partial page, there will be nothing to toss.
> > > >
> > > > Signed-off-by: Andrew Dahl <[hidden email]>
> > > >
> > > > ---
> >
> > ...
> >
> > > So the change is good.
> > >
> > > However, there's a bigger issue here. We've planned to remove these
> > > wrappers for a long time, just never got around to doing it. Seeing
> > > as there is a bug in this wrapper and it needs to be fixed, now
> > > seems like the right time to remove it.
> >
> > The removal of the wrappers would not be appropriate for -stable.  This fix
> > needs to go in separately from any refactoring so that it can be pulled back
> > within the rules outlined in Documentation/stable_kernel_rules.txt.
>
> You're acting like I've never read those rules before. I know
> exactly what they say, and patch that removes a busted helper is
> well and truly within the scope of a stable backport. Quoting rules
> at me like I'm a newbie only serves to annoy me....

Whoa there, relax!  There's no intent to annoy you here.  It's Andrew who is
the newbie.  I am adressing you here but I'm also keeping in mind that he'll
read it too.  You're both in the To: line.

> As it is, looking at what xfs_tosspages is supposed to be doing,
> calling truncate_inode_pages_range() is actually the wrong thing to
> do.  We should be calling truncate_pagecache_range(), because we
> should be unmapping pages before truncating them away. And for that
> same reason, xfs_flushinvalidate() is also wrong and broken.
>
> That is, the call in xfs_swap_extents() changes to:
>
> - xfs_tosspages(ip, 0, -1, FI_REMAPF);
> + truncate_pagecache_range(VFS_I(ip), 0, -1);
>
> And the one in xfs_change_file_space becomes:
>
> - xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> + truncate_pagecache_range(VFS_I(ip), startoffset,
> + startoffset + bf->l_len);
>
> and xfs_tosspages() goes away. That's a far better fix for the
> problem than what has been proposed, IMO, and in no way is
> inappropriate for -stable.

Sounds reasonable.

> As it is, I wouldn't even consider this a fix that is needed for
> stable kernels - XFS_IOC_ZERO is an obscure interface, and
> xfs_swap_extents works just fine as it stands....

We support even the obscure interfaces.

> ....
> > > >   prealloc_type |= XFS_BMAPI_CONVERT;
> > > > - xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> > > > + xfs_tosspages(ip, startoffset, bf->l_len ? startoffset + llen : -1, 0);
> > > >   /* FALLTHRU */
> > > >   case XFS_IOC_RESVSP:
> > > >   case XFS_IOC_RESVSP64:
> > >
> > > What's this hunk for? Indeed, one of the first things that the
> > > xfs_alloc_file_space() checks is this:
> > >
> > >         if (len <= 0)
> > > return XFS_ERROR(EINVAL);
> > >
> > > xfs_free_file_space() does the same check, so it is invalid to pass
> > > a bf_len <= 0 for any of these specific functions. Hence this change
> > > is wrong regardless of what the comment on the struct xfs_flock64_t
> > > says - preallocation and hole punch operations must have a positive
> > > length associated with them.
> >
> > Andrew, if you agree that this second change is unnecessary go ahead and remove
> > it and repost.  Otherwise,
>
> I didn't say it was unnecessary - I said it was wrong. We shouldn't
> even be getting as far as the xfs_tosspages() call if bf_len is zero
> or negative. That's the bug that needs fixing in this function.

Aha.  I think I see it now.

Thanks,
        Ben

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 2/2 V2] xfstests: xfs_tosspages() test addition

Andrew Dahl
In reply to this post by Dave Chinner


Tests the XFS_IOC_ZERO_RANGE ioctl by way of the "xfs_io zero" utility
to ensure it is tossing the expected ranges.

The ranges tested are [0,1] [0,4095] [0,4096] [0,4097] [4095,8191]
  [4095,8192] [4095,8193] [4096,8192] [1024,4096]

Signed-off-by: Andrew Dahl <[hidden email]>

---

Index: xfstests/290
===================================================================
--- /dev/null
+++ xfstests/290
@@ -0,0 +1,94 @@
+#! /bin/bash
+# FS QA Test No. 290
+#
+# Makes calls to XFS_IOC_ZERO_RANGE and checks tossed ranges
+#
+# Nothing should be tossed unless the range includes a page boundry
+#
+# Primarily tests page boundries and boundries that are
+#  off-by-one to ensure we're only tossing what's expected
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2012 SGI.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+# creator
+owner=[hidden email]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs xfs
+_supported_os Linux
+
+_require_xfs_io_zero
+
+testfile=$TEST_DIR/290.$$
+
+test_zero()
+{
+ zero_start=$1
+ zero_len=$2
+
+ $XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
+                      -c "pwrite -S 0x42 4096 4096" \
+                      -c "zero $zero_start $zero_len" \
+                      -c "pread -v 0 8192" \
+                      $testfile | _filter_xfs_io_unique
+}
+
+# [0,1] -- Shouldn't toss anything
+test_zero    0    1
+
+#[0,4095] -- Shouldn't toss anything
+test_zero    0 4095
+
+#[0,4096] -- Should toss first page
+test_zero    0 4096
+
+#[0,4097] -- Should toss first page
+test_zero    0 4097
+
+#[4095,8191] -- Should toss last byte of first page
+test_zero 4095 4096
+
+#[4095,8192] -- Should toss second page & last byte of first page
+test_zero 4095 4097
+
+#[4095,8193] -- Should toss second page & last byte of first page
+test_zero 4095 4098
+
+#[4096,8192] -- Should toss second page
+test_zero 4096 4096
+
+#[1024,5120] -- Should toss from 1024 to end of first page
+test_zero 1024 4096
+
+# success, all done
+status=0
+exit
Index: xfstests/290.out
===================================================================
--- /dev/null
+++ xfstests/290.out
@@ -0,0 +1,96 @@
+QA output created by 290
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41
AAAAAAAAAAAAAAAA
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42
BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41
AAAAAAAAAAAAAAAA
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42
BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42
BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42
BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41
AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00
AAAAAAAAAAAAAAA.
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42
BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41
AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00
AAAAAAAAAAAAAAA.
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41
AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00
AAAAAAAAAAAAAAA.
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41
AAAAAAAAAAAAAAAA
+*
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41
AAAAAAAAAAAAAAAA
+*
+00000400:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42
BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Index: xfstests/group
===================================================================
--- xfstests.orig/group
+++ xfstests/group
@@ -408,3 +408,4 @@ deprecated
 287 auto dump quota quick
 288 auto quick ioctl trim
 289 auto quick
+290 auto rw prealloc quick ioctl


_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs

xfstests_zero (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch 2/2 V2] xfstests: xfs_tosspages() test addition

Mark Tinguely-3
On 11/12/12 19:13, Andrew Dahl wrote:

>
> Tests the XFS_IOC_ZERO_RANGE ioctl by way of the "xfs_io zero" utility
> to ensure it is tossing the expected ranges.
>
> The ranges tested are [0,1] [0,4095] [0,4096] [0,4097] [4095,8191]
>    [4095,8192] [4095,8193] [4096,8192] [1024,4096]
>
> Signed-off-by: Andrew Dahl<[hidden email]>
>
> ---
>

...


> +
> + $XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \

-F is deprecated in xfs_io.

Looks good.

Reviewed-by: Mark Tinguely <[hidden email]>

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 2/2 V2] xfstests: xfs_tosspages() test addition

Andrew Dahl
On 11/14/2012 12:00 PM, Mark Tinguely wrote:

> On 11/12/12 19:13, Andrew Dahl wrote:
>>
>> Tests the XFS_IOC_ZERO_RANGE ioctl by way of the "xfs_io zero" utility
>> to ensure it is tossing the expected ranges.
>>
>> The ranges tested are [0,1] [0,4095] [0,4096] [0,4097] [4095,8191]
>>    [4095,8192] [4095,8193] [4096,8192] [1024,4096]
>>
>> Signed-off-by: Andrew Dahl<[hidden email]>
>>
>> ---
>>
>
> ...
>
>
>> +
>> +    $XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
>
> -F is deprecated in xfs_io.
>
> Looks good.
>
> Reviewed-by: Mark Tinguely <[hidden email]>

Actually... I just realized I neglected to add one change in there. The
call to _require_xfs_io_zero doesn't exist, but should in common.rc

I'll repost the patch with this included.

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 2/2 V3] xfstests: xfs_tosspages() test addition

Andrew Dahl


_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs

xfstests_zero (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch 2/2 V3] xfstests: xfs_tosspages() test addition

Dave Chinner
On Wed, Nov 14, 2012 at 12:57:35PM -0600, Andrew Dahl wrote:
>
>
> --
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
>

> Tests the XFS_IOC_ZERO_RANGE ioctl by way of the "xfs_io zero" utility
> to ensure it is tossing the expected ranges.
>
> The ranges tested are [0,1] [0,4095] [0,4096] [0,4097] [4095,8191]
>   [4095,8192] [4095,8193] [4096,8192] [1024,4096]
>
> Signed-off-by: Andrew Dahl <[hidden email]>

looks good.

Reviewed-by: Dave Chinner <[hidden email]>

Cheers,

Dave.

--
Dave Chinner
[hidden email]

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs
Reply | Threaded
Open this post in threaded view
|

Re: [patch 2/2 V3] xfstests: xfs_tosspages() test addition

Mark Tinguely-3
In reply to this post by Andrew Dahl
On 11/14/12 12:57, Andrew Dahl wrote:
>
> Tests the XFS_IOC_ZERO_RANGE ioctl by way of the "xfs_io zero" utility
> to ensure it is tossing the expected ranges.
>
> The ranges tested are [0,1] [0,4095] [0,4096] [0,4097] [4095,8191]
>    [4095,8192] [4095,8193] [4096,8192] [1024,4096]
>
> Signed-off-by: Andrew Dahl<[hidden email]>

This patch has been committed to git://oss.sgi.com/xfs/cmds/xfstests
master branch, commit 82846 (test 290).

--Mark.

_______________________________________________
xfs mailing list
[hidden email]
http://oss.sgi.com/mailman/listinfo/xfs