All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Gao Xiang <hsiangkao@linux.alibaba.com>
Cc: Trond Myklebust <trondmy@hammerspace.com>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"joseph.qi@linux.alibaba.com" <joseph.qi@linux.alibaba.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"anna.schumaker@netapp.com" <anna.schumaker@netapp.com>
Subject: Re: [PATCH] nfs: set block size according to pnfs_blksize first
Date: Thu, 17 Jun 2021 09:08:41 -0400	[thread overview]
Message-ID: <YMtJWT7hWnHZhrmm@mit.edu> (raw)
In-Reply-To: <YMq104Ps9nTnzE9R@B-P7TQMD6M-0146.local>

On Thu, Jun 17, 2021 at 10:39:15AM +0800, Gao Xiang wrote:
> What I said was the original testcase strictly addressing the original
> regression report, which converts from shortform to single-block
> leaf format, see:
> 
> https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/attr_replace_test.c#n40

So the question is really is this a generic test or an xfs test?  If
it's an xfs test, then you can make non-portable assumptions about the
returned values from statfs(2).

If it's a generic test, then you can't really do that.  And given the
name --- generic/486, it's a generic test.

What we probably *can* do is to test a series of xattr value
replacements --- e.g., find out what is the maximum xattr size
supported, and then try going from an 8 byte xattr value to an xattr
value of size N, where N might be 16, 32, 64, 128, 256, .. up to the
max xattr size supported.

One could also imagine testing starting with an xattr size of 16, 32,
64, 128, 256, ... max_size and replacing with a small xattr, which
would test the reverse case.

That would ba a truly general, generic test.

In any case, I'd suggest making a proposed patch to the fstests@
mailing list, since I'll note that the current set of lists where this
is being discussed may not guarantee that XFS developers will be
paying attention to this thread.

> > > And I found another problem in the test, it fails on 1k/2k block size
> > > extN filesystems, because 2k xattr doesn't fit in single block.. e.g.
> > > 
> > >     -Attribute "world" has a 2048 byte value for SCRATCH_MNT/hello
> > >     +No space left on device
> > >     +error=22 at line 46
> > >     +Attribute "world" has a 1 byte value for
> > > 
> > > We probably need to check the block size of SCRATCH_DEV and _notrun if
> > > it's smaller than 4k.

I think you mean using a 1k or 2k extN file system when testing over
NFS.  It works fine directly on a 1k block file system, since that's
one of my regular test configs and I would have noticed if it was
breaking.

BEGIN TEST 1k (1 test): Ext4 1k block Thu Jun 17 09:05:09 EDT 2021
DEVICE: /dev/vdd
EXT_MKFS_OPTIONS: -b 1024
EXT_MOUNT_OPTIONS: -o block_validity
FSTYP         -- ext4
PLATFORM      -- Linux/x86_64 kvm-xfstests 5.13.0-rc5-xfstests-00008-ga3f5d4136d5a #171 SMP Wed Jun 16 20:40:09 EDT 2021
MKFS_OPTIONS  -- -q -b 1024 /dev/vdc
MOUNT_OPTIONS -- -o acl,user_xattr -o block_validity /dev/vdc /vdc

generic/486             [09:05:10][   19.128541] run fstests generic/486 at 2021-06-17 09:05:10
 [09:05:10] 0s
Ran: generic/486
Passed all 1 tests
Xunit report: /results/ext4/results-1k/result.xml

Again, my suggestion of using binary search to find the largest size
xattr supported by the file system is really the way to go.

Cheers,

						- Ted

      reply	other threads:[~2021-06-17 13:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16 12:44 Gao Xiang
2021-06-16 13:47 ` Trond Myklebust
2021-06-16 14:06   ` Gao Xiang
2021-06-16 14:20     ` Trond Myklebust
2021-06-16 14:41       ` Gao Xiang
2021-06-16 15:14         ` Trond Myklebust
2021-06-16 17:08           ` Theodore Ts'o
2021-06-16 17:17           ` Frank van der Linden
2021-06-16 22:51             ` Theodore Ts'o
2021-06-16 16:14         ` Theodore Ts'o
2021-06-16 17:51           ` Gao Xiang
2021-06-16 18:51             ` Trond Myklebust
2021-06-16 22:55             ` Theodore Ts'o
2021-06-17  2:39               ` Gao Xiang
2021-06-17 13:08                 ` Theodore Ts'o [this message]

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=YMtJWT7hWnHZhrmm@mit.edu \
    --to=tytso@mit.edu \
    --cc=anna.schumaker@netapp.com \
    --cc=hsiangkao@linux.alibaba.com \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@hammerspace.com \
    --subject='Re: [PATCH] nfs: set block size according to pnfs_blksize first' \
    /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

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.