Re: [PATCH 1/2] xfs: Add realtime group

Chandan Babu R
2020-09-17T04:10:15Z

On Wednesday 16 September 2020 10:21:42 PM IST Darrick J. Wong wrote:
> On Wed, Sep 16, 2020 at 11:04:06AM +0530, Chandan Babu R wrote:
> > This commit adds a new group to classify tests that can work with
> > realtime devices.
> >
> > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> > ---
> > tests/xfs/group | 52 ++++++++++++++++++++++++-------------------------
> > 1 file changed, 26 insertions(+), 26 deletions(-)
> >
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index ed0d389e..3bb0f674 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -67,7 +67,7 @@
> > 067 acl attr auto quick
> > 068 auto stress dump
> > 069 ioctl auto quick
> > -070 auto quick repair
> > +070 auto quick repair realtime
>
> This test has an open-coded call to repair + rt volume, but is not
> itself a test of rt functionality.

That is true. I had decided to include this since it executed xfs_repair if
the scratch fs had a realtime device associated with it. I will remove it.

>
> > 071 rw auto
> > 072 rw auto prealloc quick
> > 073 copy auto
> > @@ -87,11 +87,11 @@
> > 087 fuzzers
> > 088 fuzzers
> > 089 fuzzers
> > -090 rw auto
> > +090 rw auto realtime
> > 091 fuzzers
> > 092 other auto quick
> > 093 fuzzers
> > -094 metadata dir ioctl auto
> > +094 metadata dir ioctl auto realtime
> > 095 log v2log auto
> > 096 mkfs v2log auto quick
> > 097 fuzzers
> > @@ -119,7 +119,7 @@
> > 119 log v2log auto freeze
> > 120 fuzzers
> > 121 shutdown log auto quick
> > -122 other auto quick clone
> > +122 other auto quick clone realtime
>
> This is an ondisk structure size check. It doesn't test rt
> functionality, but I guess it doesn't really harm things to throw it on
> the 'realtime' pile. I'm not objecting to this; it's just a funny
> thought I had while reading this patch.

I had decided to add this test to "realtime" group since it was checking sizes
of structures associated with realtime functionality. For
e.g. tests/xfs/122.out has,

sizeof(struct xfs_rtrmap_key) = 24
sizeof(struct xfs_rtrmap_rec) = 32
sizeof(struct xfs_rtrmap_root) = 4

However, I think these structures are associated with a Btree that hasn't been
merged into the mainline kernel yet since I couldn't find them in the source
code. I was of the opinion that adding this test will be useful when the
corresponding patches get merged into mainline and test suite is invoked for
"realtime" group only.

>
> (Not sure why it's in 'clone' either...)
>
> > 123 fuzzers
> > 124 fuzzers
> > 125 fuzzers
> > @@ -128,7 +128,7 @@
> > 128 auto quick clone fsr
> > 129 auto quick clone
> > 130 fuzzers clone
> > -131 auto quick clone
> > +131 auto quick clone realtime
> > 132 auto quick
> > 133 dangerous_fuzzers
> > 134 dangerous_fuzzers
> > @@ -188,7 +188,7 @@
> > 188 ci dir auto
> > 189 mount auto quick
> > 190 rw auto quick
> > -191-input-validation auto quick mkfs
> > +191-input-validation auto quick mkfs realtime
> > 192 auto quick clone
> > 193 auto quick clone
> > 194 rw auto
> > @@ -272,7 +272,7 @@
> > 273 auto rmap fsmap
> > 274 auto quick rmap fsmap
> > 275 auto quick rmap fsmap
> > -276 auto quick rmap fsmap
> > +276 auto quick rmap fsmap realtime
> > 277 auto quick rmap fsmap
> > 278 repair auto
> > 279 auto mkfs
> > @@ -287,7 +287,7 @@
> > 288 auto quick repair fuzzers
> > 289 growfs auto quick
> > 290 auto rw prealloc quick ioctl zero
> > -291 auto repair
> > +291 auto repair realtime
>
> This is a directory repair test, which doesn't exercise any rt volume
> functionality.
>
> ...
>
> FWIW I checked all the other tests that you added to the realtime group,
> and the changes I don't have any comments about all look ok to me.

Thanks for going through the list of tests.

--
chandan