mbox series

[v2,00/13] rename & split tests

Message ID 20220512165250.450989-1-brauner@kernel.org (mailing list archive)
Headers show
Series rename & split tests | expand

Message

Christian Brauner May 12, 2022, 4:52 p.m. UTC
From: "Christian Brauner (Microsoft)" <brauner@kernel.org>

Hey everyone,

Please note that this patch series contains patches that will be
rejected by the fstests mailing list because of the amount of changes
they contain. So tools like b4 will not be able to find the whole patch
series on a mailing list. In case it's helpful I've added the
"fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
possible to simply use the patch series as it appears in your inbox.

All vfstests pass:

#### btrfs ####
ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
FSTYP         -- btrfs
PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
MKFS_OPTIONS  -- /dev/sda4
MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch

btrfs/245 52s ...  54s
generic/633 58s ...  51s
generic/644 60s ...  49s
generic/645 161s ...  143s
generic/656 63s ...  55s
Ran: btrfs/245 generic/633 generic/644 generic/645 generic/656
Passed all 5 tests

#### ext4 ####
ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
FSTYP         -- ext4
PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
MKFS_OPTIONS  -- /dev/sda4
MOUNT_OPTIONS -- -o acl,user_xattr /dev/sda4 /mnt/scratch

generic/633 47s ...  50s
generic/644 46s ...  49s
generic/645 135s ...  139s
generic/656 53s ...  54s
Ran: generic/633 generic/644 generic/645 generic/656
Passed all 4 tests

#### xfs ####
ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
FSTYP         -- xfs (debug)
PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
MKFS_OPTIONS  -- -f /dev/sda4
MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch

generic/633 58s ...  58s
generic/644 62s ...  60s
generic/645 161s ...  161s
generic/656 62s ...  63s
xfs/152 133s ...  133s
xfs/153 94s ...  92s
Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
Passed all 6 tests

/* v2 */
* This rebases the patchset on top the for-next branch.
* Last week we merged 858a19c5e9b0 ("idmapped_mounts: Prepare for
  support for more features"). The patch switched feature checking from
  a boolean to a flag. It failed convert all tests. This adds a patch to
  fix this in patch 01/13.
* A patch has been added to remove an invalid test. The semantics for a
  specific corner-case where we allowed a mount's idmapping to change
  while there were active writers will be altered.

/* v1 */
As announced multiple times already we need to rename and split the
idmapped mount testsuite into separate source files and also give it a
better name to reflect the fact that it covers a lot more than just
idmapped mounts.

I have decided against compiling different binaries for now. Instead we
compile a single vfstest binary that can be called with various command
line switches to run the various test suites. This is not different than
what we did for the idmapped-mounts binary. Of course, nothing prevents
us from using multiple binaries in the future.

Thanks!
Christian

Christian Brauner (13):
  idmapped-mounts: make all tests set their required feature flags
  src: rename idmapped-mounts folder
  src/vfs: rename idmapped-mounts.c file
  vfstest: rename struct t_idmapped_mounts
  utils: add missing global.h include
  utils: add struct vfstest_info
  utils: move helpers into utils
  missing: move sys_execveat() to missing.h
  utils: add struct test_suite
  vfstests: split idmapped mount tests into separate suite
  vfstest: split out btrfs idmapped mounts test
  vfstest: split out remaining idmapped mount tests
  vfs/idmapped-mounts: remove invalid test

 .gitignore                                    |     4 +-
 common/rc                                     |    32 +-
 src/Makefile                                  |     2 +-
 src/detached_mounts_propagation.c             |     2 +-
 src/feature.c                                 |     2 +-
 src/idmapped-mounts/idmapped-mounts.c         | 14625 ----------------
 src/idmapped-mounts/utils.c                   |   425 -
 src/idmapped-mounts/utils.h                   |   130 -
 src/{idmapped-mounts => vfs}/Makefile         |    14 +-
 src/vfs/btrfs-idmapped-mounts.c               |  3854 ++++
 src/vfs/btrfs-idmapped-mounts.h               |    15 +
 src/vfs/idmapped-mounts.c                     |  7747 ++++++++
 src/vfs/idmapped-mounts.h                     |    18 +
 src/{idmapped-mounts => vfs}/missing.h        |    11 +
 src/{idmapped-mounts => vfs}/mount-idmapped.c |     0
 src/vfs/utils.c                               |  1050 ++
 src/vfs/utils.h                               |   373 +
 src/vfs/vfstest.c                             |  2073 +++
 tests/btrfs/245                               |     2 +-
 tests/generic/633                             |     2 +-
 tests/generic/644                             |     2 +-
 tests/generic/645                             |     2 +-
 tests/generic/656                             |     2 +-
 tests/xfs/152                                 |     4 +-
 tests/xfs/153                                 |     2 +-
 25 files changed, 15177 insertions(+), 15216 deletions(-)
 delete mode 100644 src/idmapped-mounts/idmapped-mounts.c
 delete mode 100644 src/idmapped-mounts/utils.c
 delete mode 100644 src/idmapped-mounts/utils.h
 rename src/{idmapped-mounts => vfs}/Makefile (59%)
 create mode 100644 src/vfs/btrfs-idmapped-mounts.c
 create mode 100644 src/vfs/btrfs-idmapped-mounts.h
 create mode 100644 src/vfs/idmapped-mounts.c
 create mode 100644 src/vfs/idmapped-mounts.h
 rename src/{idmapped-mounts => vfs}/missing.h (93%)
 rename src/{idmapped-mounts => vfs}/mount-idmapped.c (100%)
 create mode 100644 src/vfs/utils.c
 create mode 100644 src/vfs/utils.h
 create mode 100644 src/vfs/vfstest.c


base-commit: 87cf32ad3fa234e3d5ec501e0f86516bef91d805

Comments

Zorro Lang May 12, 2022, 8:19 p.m. UTC | #1
On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
> 
> Hey everyone,
> 
> Please note that this patch series contains patches that will be
> rejected by the fstests mailing list because of the amount of changes
> they contain. So tools like b4 will not be able to find the whole patch
> series on a mailing list. In case it's helpful I've added the
> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
> possible to simply use the patch series as it appears in your inbox.

Thanks Christian! I've merged this patchset to my local testing branch, and
haven't found regressions for now. That's good to me.

Due to it *blocks* all other idmapped related patches, so if VFS (idmapped)
forks don't have objections, and if no big regressions either, I'd like to merge
this patchset at first, to free this *blocking*, then we can fix small issues
bit by bit. If anyone has review points, please point out ASAP :)

Thanks,
Zorro

> 
> All vfstests pass:
> 
> #### btrfs ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- /dev/sda4
> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> 
> btrfs/245 52s ...  54s
> generic/633 58s ...  51s
> generic/644 60s ...  49s
> generic/645 161s ...  143s
> generic/656 63s ...  55s
> Ran: btrfs/245 generic/633 generic/644 generic/645 generic/656
> Passed all 5 tests
> 
> #### ext4 ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- ext4
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- /dev/sda4
> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sda4 /mnt/scratch
> 
> generic/633 47s ...  50s
> generic/644 46s ...  49s
> generic/645 135s ...  139s
> generic/656 53s ...  54s
> Ran: generic/633 generic/644 generic/645 generic/656
> Passed all 4 tests
> 
> #### xfs ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- -f /dev/sda4
> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> 
> generic/633 58s ...  58s
> generic/644 62s ...  60s
> generic/645 161s ...  161s
> generic/656 62s ...  63s
> xfs/152 133s ...  133s
> xfs/153 94s ...  92s
> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
> Passed all 6 tests
> 
> /* v2 */
> * This rebases the patchset on top the for-next branch.
> * Last week we merged 858a19c5e9b0 ("idmapped_mounts: Prepare for
>   support for more features"). The patch switched feature checking from
>   a boolean to a flag. It failed convert all tests. This adds a patch to
>   fix this in patch 01/13.
> * A patch has been added to remove an invalid test. The semantics for a
>   specific corner-case where we allowed a mount's idmapping to change
>   while there were active writers will be altered.
> 
> /* v1 */
> As announced multiple times already we need to rename and split the
> idmapped mount testsuite into separate source files and also give it a
> better name to reflect the fact that it covers a lot more than just
> idmapped mounts.
> 
> I have decided against compiling different binaries for now. Instead we
> compile a single vfstest binary that can be called with various command
> line switches to run the various test suites. This is not different than
> what we did for the idmapped-mounts binary. Of course, nothing prevents
> us from using multiple binaries in the future.
> 
> Thanks!
> Christian
> 
> Christian Brauner (13):
>   idmapped-mounts: make all tests set their required feature flags
>   src: rename idmapped-mounts folder
>   src/vfs: rename idmapped-mounts.c file
>   vfstest: rename struct t_idmapped_mounts
>   utils: add missing global.h include
>   utils: add struct vfstest_info
>   utils: move helpers into utils
>   missing: move sys_execveat() to missing.h
>   utils: add struct test_suite
>   vfstests: split idmapped mount tests into separate suite
>   vfstest: split out btrfs idmapped mounts test
>   vfstest: split out remaining idmapped mount tests
>   vfs/idmapped-mounts: remove invalid test
> 
>  .gitignore                                    |     4 +-
>  common/rc                                     |    32 +-
>  src/Makefile                                  |     2 +-
>  src/detached_mounts_propagation.c             |     2 +-
>  src/feature.c                                 |     2 +-
>  src/idmapped-mounts/idmapped-mounts.c         | 14625 ----------------
>  src/idmapped-mounts/utils.c                   |   425 -
>  src/idmapped-mounts/utils.h                   |   130 -
>  src/{idmapped-mounts => vfs}/Makefile         |    14 +-
>  src/vfs/btrfs-idmapped-mounts.c               |  3854 ++++
>  src/vfs/btrfs-idmapped-mounts.h               |    15 +
>  src/vfs/idmapped-mounts.c                     |  7747 ++++++++
>  src/vfs/idmapped-mounts.h                     |    18 +
>  src/{idmapped-mounts => vfs}/missing.h        |    11 +
>  src/{idmapped-mounts => vfs}/mount-idmapped.c |     0
>  src/vfs/utils.c                               |  1050 ++
>  src/vfs/utils.h                               |   373 +
>  src/vfs/vfstest.c                             |  2073 +++
>  tests/btrfs/245                               |     2 +-
>  tests/generic/633                             |     2 +-
>  tests/generic/644                             |     2 +-
>  tests/generic/645                             |     2 +-
>  tests/generic/656                             |     2 +-
>  tests/xfs/152                                 |     4 +-
>  tests/xfs/153                                 |     2 +-
>  25 files changed, 15177 insertions(+), 15216 deletions(-)
>  delete mode 100644 src/idmapped-mounts/idmapped-mounts.c
>  delete mode 100644 src/idmapped-mounts/utils.c
>  delete mode 100644 src/idmapped-mounts/utils.h
>  rename src/{idmapped-mounts => vfs}/Makefile (59%)
>  create mode 100644 src/vfs/btrfs-idmapped-mounts.c
>  create mode 100644 src/vfs/btrfs-idmapped-mounts.h
>  create mode 100644 src/vfs/idmapped-mounts.c
>  create mode 100644 src/vfs/idmapped-mounts.h
>  rename src/{idmapped-mounts => vfs}/missing.h (93%)
>  rename src/{idmapped-mounts => vfs}/mount-idmapped.c (100%)
>  create mode 100644 src/vfs/utils.c
>  create mode 100644 src/vfs/utils.h
>  create mode 100644 src/vfs/vfstest.c
> 
> 
> base-commit: 87cf32ad3fa234e3d5ec501e0f86516bef91d805
> -- 
> 2.34.1
>
Zorro Lang May 14, 2022, 5:59 p.m. UTC | #2
On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
> 
> Hey everyone,
> 
> Please note that this patch series contains patches that will be
> rejected by the fstests mailing list because of the amount of changes
> they contain. So tools like b4 will not be able to find the whole patch
> series on a mailing list. In case it's helpful I've added the
> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
> possible to simply use the patch series as it appears in your inbox.

Hi Christian,

After a full round regression test, I just found this patchset cause
generic/689 fail as [1], so I think it still need below fix [2]. If
you won't send a fixed version before fstests release this sunday, I
will fix it by myself, to help this patchset be merged this week. Due
to it's not worth waiting one more week for this.

Thanks,
Zorro

[1]
/var/lib/xfstests/tests/generic/689: line 29: /var/lib/xfstests/src/idmapped-mounts/idmapped-mounts: No such file or directory

[2]
diff --git a/tests/generic/689 b/tests/generic/689
index 670f8e5a..10dc58ed 100755
--- a/tests/generic/689
+++ b/tests/generic/689
@@ -11,7 +11,7 @@
 # 705191b03d50 ("fs: fix acl translation")
 #
 . ./common/preamble
-_begin_fstest auto quick perms
+_begin_fstest auto quick idmapped perms
 
 # Import common functions.
 . ./common/filter
@@ -26,7 +26,7 @@ _require_group fsgqa
 
 echo "Silence is golden"
 
-$here/src/idmapped-mounts/idmapped-mounts --test-setxattr-fix-705191b03d50 \
+$here/src/vfs/vfstest --test-setxattr-fix-705191b03d50 \
        --device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
 
 status=$?

> 
> All vfstests pass:
> 
> #### btrfs ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- /dev/sda4
> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> 
> btrfs/245 52s ...  54s
> generic/633 58s ...  51s
> generic/644 60s ...  49s
> generic/645 161s ...  143s
> generic/656 63s ...  55s
> Ran: btrfs/245 generic/633 generic/644 generic/645 generic/656
> Passed all 5 tests
> 
> #### ext4 ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- ext4
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- /dev/sda4
> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sda4 /mnt/scratch
> 
> generic/633 47s ...  50s
> generic/644 46s ...  49s
> generic/645 135s ...  139s
> generic/656 53s ...  54s
> Ran: generic/633 generic/644 generic/645 generic/656
> Passed all 4 tests
> 
> #### xfs ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- -f /dev/sda4
> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> 
> generic/633 58s ...  58s
> generic/644 62s ...  60s
> generic/645 161s ...  161s
> generic/656 62s ...  63s
> xfs/152 133s ...  133s
> xfs/153 94s ...  92s
> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
> Passed all 6 tests
> 
> /* v2 */
> * This rebases the patchset on top the for-next branch.
> * Last week we merged 858a19c5e9b0 ("idmapped_mounts: Prepare for
>   support for more features"). The patch switched feature checking from
>   a boolean to a flag. It failed convert all tests. This adds a patch to
>   fix this in patch 01/13.
> * A patch has been added to remove an invalid test. The semantics for a
>   specific corner-case where we allowed a mount's idmapping to change
>   while there were active writers will be altered.
> 
> /* v1 */
> As announced multiple times already we need to rename and split the
> idmapped mount testsuite into separate source files and also give it a
> better name to reflect the fact that it covers a lot more than just
> idmapped mounts.
> 
> I have decided against compiling different binaries for now. Instead we
> compile a single vfstest binary that can be called with various command
> line switches to run the various test suites. This is not different than
> what we did for the idmapped-mounts binary. Of course, nothing prevents
> us from using multiple binaries in the future.
> 
> Thanks!
> Christian
> 
> Christian Brauner (13):
>   idmapped-mounts: make all tests set their required feature flags
>   src: rename idmapped-mounts folder
>   src/vfs: rename idmapped-mounts.c file
>   vfstest: rename struct t_idmapped_mounts
>   utils: add missing global.h include
>   utils: add struct vfstest_info
>   utils: move helpers into utils
>   missing: move sys_execveat() to missing.h
>   utils: add struct test_suite
>   vfstests: split idmapped mount tests into separate suite
>   vfstest: split out btrfs idmapped mounts test
>   vfstest: split out remaining idmapped mount tests
>   vfs/idmapped-mounts: remove invalid test
> 
>  .gitignore                                    |     4 +-
>  common/rc                                     |    32 +-
>  src/Makefile                                  |     2 +-
>  src/detached_mounts_propagation.c             |     2 +-
>  src/feature.c                                 |     2 +-
>  src/idmapped-mounts/idmapped-mounts.c         | 14625 ----------------
>  src/idmapped-mounts/utils.c                   |   425 -
>  src/idmapped-mounts/utils.h                   |   130 -
>  src/{idmapped-mounts => vfs}/Makefile         |    14 +-
>  src/vfs/btrfs-idmapped-mounts.c               |  3854 ++++
>  src/vfs/btrfs-idmapped-mounts.h               |    15 +
>  src/vfs/idmapped-mounts.c                     |  7747 ++++++++
>  src/vfs/idmapped-mounts.h                     |    18 +
>  src/{idmapped-mounts => vfs}/missing.h        |    11 +
>  src/{idmapped-mounts => vfs}/mount-idmapped.c |     0
>  src/vfs/utils.c                               |  1050 ++
>  src/vfs/utils.h                               |   373 +
>  src/vfs/vfstest.c                             |  2073 +++
>  tests/btrfs/245                               |     2 +-
>  tests/generic/633                             |     2 +-
>  tests/generic/644                             |     2 +-
>  tests/generic/645                             |     2 +-
>  tests/generic/656                             |     2 +-
>  tests/xfs/152                                 |     4 +-
>  tests/xfs/153                                 |     2 +-
>  25 files changed, 15177 insertions(+), 15216 deletions(-)
>  delete mode 100644 src/idmapped-mounts/idmapped-mounts.c
>  delete mode 100644 src/idmapped-mounts/utils.c
>  delete mode 100644 src/idmapped-mounts/utils.h
>  rename src/{idmapped-mounts => vfs}/Makefile (59%)
>  create mode 100644 src/vfs/btrfs-idmapped-mounts.c
>  create mode 100644 src/vfs/btrfs-idmapped-mounts.h
>  create mode 100644 src/vfs/idmapped-mounts.c
>  create mode 100644 src/vfs/idmapped-mounts.h
>  rename src/{idmapped-mounts => vfs}/missing.h (93%)
>  rename src/{idmapped-mounts => vfs}/mount-idmapped.c (100%)
>  create mode 100644 src/vfs/utils.c
>  create mode 100644 src/vfs/utils.h
>  create mode 100644 src/vfs/vfstest.c
> 
> 
> base-commit: 87cf32ad3fa234e3d5ec501e0f86516bef91d805
> -- 
> 2.34.1
>
Christian Brauner May 16, 2022, 8:02 a.m. UTC | #3
On Sun, May 15, 2022 at 01:59:43AM +0800, Zorro Lang wrote:
> On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
> > From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
> > 
> > Hey everyone,
> > 
> > Please note that this patch series contains patches that will be
> > rejected by the fstests mailing list because of the amount of changes
> > they contain. So tools like b4 will not be able to find the whole patch
> > series on a mailing list. In case it's helpful I've added the
> > "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
> > possible to simply use the patch series as it appears in your inbox.
> 
> Hi Christian,

Hi Zorro,

> 
> After a full round regression test, I just found this patchset cause
> generic/689 fail as [1], so I think it still need below fix [2]. If
> you won't send a fixed version before fstests release this sunday, I
> will fix it by myself, to help this patchset be merged this week. Due
> to it's not worth waiting one more week for this.

Thank you!

> 
> Thanks,
> Zorro
> 
> [1]
> /var/lib/xfstests/tests/generic/689: line 29: /var/lib/xfstests/src/idmapped-mounts/idmapped-mounts: No such file or directory
> 
> [2]
> diff --git a/tests/generic/689 b/tests/generic/689
> index 670f8e5a..10dc58ed 100755
> --- a/tests/generic/689
> +++ b/tests/generic/689
> @@ -11,7 +11,7 @@
>  # 705191b03d50 ("fs: fix acl translation")
>  #
>  . ./common/preamble
> -_begin_fstest auto quick perms
> +_begin_fstest auto quick idmapped perms

Ah, I see. I didn't catch this because I forgot to add the test to the
idmapped group.

Thanks,
Christian

>  
>  # Import common functions.
>  . ./common/filter
> @@ -26,7 +26,7 @@ _require_group fsgqa
>  
>  echo "Silence is golden"
>  
> -$here/src/idmapped-mounts/idmapped-mounts --test-setxattr-fix-705191b03d50 \
> +$here/src/vfs/vfstest --test-setxattr-fix-705191b03d50 \
>         --device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
>  
>  status=$?
> 
> > 
> > All vfstests pass:
> > 
> > #### btrfs ####
> > ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> > FSTYP         -- btrfs
> > PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> > MKFS_OPTIONS  -- /dev/sda4
> > MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> > 
> > btrfs/245 52s ...  54s
> > generic/633 58s ...  51s
> > generic/644 60s ...  49s
> > generic/645 161s ...  143s
> > generic/656 63s ...  55s
> > Ran: btrfs/245 generic/633 generic/644 generic/645 generic/656
> > Passed all 5 tests
> > 
> > #### ext4 ####
> > ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> > FSTYP         -- ext4
> > PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> > MKFS_OPTIONS  -- /dev/sda4
> > MOUNT_OPTIONS -- -o acl,user_xattr /dev/sda4 /mnt/scratch
> > 
> > generic/633 47s ...  50s
> > generic/644 46s ...  49s
> > generic/645 135s ...  139s
> > generic/656 53s ...  54s
> > Ran: generic/633 generic/644 generic/645 generic/656
> > Passed all 4 tests
> > 
> > #### xfs ####
> > ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> > FSTYP         -- xfs (debug)
> > PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> > MKFS_OPTIONS  -- -f /dev/sda4
> > MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> > 
> > generic/633 58s ...  58s
> > generic/644 62s ...  60s
> > generic/645 161s ...  161s
> > generic/656 62s ...  63s
> > xfs/152 133s ...  133s
> > xfs/153 94s ...  92s
> > Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
> > Passed all 6 tests
> > 
> > /* v2 */
> > * This rebases the patchset on top the for-next branch.
> > * Last week we merged 858a19c5e9b0 ("idmapped_mounts: Prepare for
> >   support for more features"). The patch switched feature checking from
> >   a boolean to a flag. It failed convert all tests. This adds a patch to
> >   fix this in patch 01/13.
> > * A patch has been added to remove an invalid test. The semantics for a
> >   specific corner-case where we allowed a mount's idmapping to change
> >   while there were active writers will be altered.
> > 
> > /* v1 */
> > As announced multiple times already we need to rename and split the
> > idmapped mount testsuite into separate source files and also give it a
> > better name to reflect the fact that it covers a lot more than just
> > idmapped mounts.
> > 
> > I have decided against compiling different binaries for now. Instead we
> > compile a single vfstest binary that can be called with various command
> > line switches to run the various test suites. This is not different than
> > what we did for the idmapped-mounts binary. Of course, nothing prevents
> > us from using multiple binaries in the future.
> > 
> > Thanks!
> > Christian
> > 
> > Christian Brauner (13):
> >   idmapped-mounts: make all tests set their required feature flags
> >   src: rename idmapped-mounts folder
> >   src/vfs: rename idmapped-mounts.c file
> >   vfstest: rename struct t_idmapped_mounts
> >   utils: add missing global.h include
> >   utils: add struct vfstest_info
> >   utils: move helpers into utils
> >   missing: move sys_execveat() to missing.h
> >   utils: add struct test_suite
> >   vfstests: split idmapped mount tests into separate suite
> >   vfstest: split out btrfs idmapped mounts test
> >   vfstest: split out remaining idmapped mount tests
> >   vfs/idmapped-mounts: remove invalid test
> > 
> >  .gitignore                                    |     4 +-
> >  common/rc                                     |    32 +-
> >  src/Makefile                                  |     2 +-
> >  src/detached_mounts_propagation.c             |     2 +-
> >  src/feature.c                                 |     2 +-
> >  src/idmapped-mounts/idmapped-mounts.c         | 14625 ----------------
> >  src/idmapped-mounts/utils.c                   |   425 -
> >  src/idmapped-mounts/utils.h                   |   130 -
> >  src/{idmapped-mounts => vfs}/Makefile         |    14 +-
> >  src/vfs/btrfs-idmapped-mounts.c               |  3854 ++++
> >  src/vfs/btrfs-idmapped-mounts.h               |    15 +
> >  src/vfs/idmapped-mounts.c                     |  7747 ++++++++
> >  src/vfs/idmapped-mounts.h                     |    18 +
> >  src/{idmapped-mounts => vfs}/missing.h        |    11 +
> >  src/{idmapped-mounts => vfs}/mount-idmapped.c |     0
> >  src/vfs/utils.c                               |  1050 ++
> >  src/vfs/utils.h                               |   373 +
> >  src/vfs/vfstest.c                             |  2073 +++
> >  tests/btrfs/245                               |     2 +-
> >  tests/generic/633                             |     2 +-
> >  tests/generic/644                             |     2 +-
> >  tests/generic/645                             |     2 +-
> >  tests/generic/656                             |     2 +-
> >  tests/xfs/152                                 |     4 +-
> >  tests/xfs/153                                 |     2 +-
> >  25 files changed, 15177 insertions(+), 15216 deletions(-)
> >  delete mode 100644 src/idmapped-mounts/idmapped-mounts.c
> >  delete mode 100644 src/idmapped-mounts/utils.c
> >  delete mode 100644 src/idmapped-mounts/utils.h
> >  rename src/{idmapped-mounts => vfs}/Makefile (59%)
> >  create mode 100644 src/vfs/btrfs-idmapped-mounts.c
> >  create mode 100644 src/vfs/btrfs-idmapped-mounts.h
> >  create mode 100644 src/vfs/idmapped-mounts.c
> >  create mode 100644 src/vfs/idmapped-mounts.h
> >  rename src/{idmapped-mounts => vfs}/missing.h (93%)
> >  rename src/{idmapped-mounts => vfs}/mount-idmapped.c (100%)
> >  create mode 100644 src/vfs/utils.c
> >  create mode 100644 src/vfs/utils.h
> >  create mode 100644 src/vfs/vfstest.c
> > 
> > 
> > base-commit: 87cf32ad3fa234e3d5ec501e0f86516bef91d805
> > -- 
> > 2.34.1
> > 
>
Dave Chinner May 21, 2022, 11:13 p.m. UTC | #4
[cc io_uring]

On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
> 
> Hey everyone,
> 
> Please note that this patch series contains patches that will be
> rejected by the fstests mailing list because of the amount of changes
> they contain. So tools like b4 will not be able to find the whole patch
> series on a mailing list. In case it's helpful I've added the
> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
> possible to simply use the patch series as it appears in your inbox.
> 
> All vfstests pass:

[...]

> #### xfs ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- -f /dev/sda4
> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> 
> generic/633 58s ...  58s
> generic/644 62s ...  60s
> generic/645 161s ...  161s
> generic/656 62s ...  63s
> xfs/152 133s ...  133s
> xfs/153 94s ...  92s
> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
> Passed all 6 tests

I'm not sure if it's this series that has introduced a test bug or
triggered a latent issue in the kernel, but I've started seeing
generic/633 throw audit subsystem warnings on a single test machine
as of late Friday:

[ 7285.015888] WARNING: CPU: 3 PID: 2147118 at kernel/auditsc.c:2035 __audit_syscall_entry+0x113/0x140
[ 7285.019973] Modules linked in:
[ 7285.021281] CPU: 3 PID: 2147118 Comm: vfstest Not tainted 5.18.0-rc7-dgc+ #1250
[ 7285.024341] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
[ 7285.027782] RIP: 0010:__audit_syscall_entry+0x113/0x140
[ 7285.029923] Code: 24 e8 c1 6b ff ff 48 8b 34 24 85 c0 48 8b 54 24 08 48 8b 4c 24 10 4c 8b 44 24 18 0f 84 72 ff ff ff 48 83 c4 20 5b 5d 41 5c c3 <0f> 0b 85 c0 75 14 48 83 c4 20 48 c7 c7 70 45 7f 82 5b 5d 41 5c e9
[ 7285.037563] RSP: 0018:ffffc900012f7ed0 EFLAGS: 00010202
[ 7285.039748] RAX: 0000000000000000 RBX: ffff8880aaf18800 RCX: 000000000000003c
[ 7285.043126] RDX: 00000000000000e7 RSI: 0000000000000000 RDI: ffff888102104f00
[ 7285.046120] RBP: 00000000000000e7 R08: fffffffffffffe2c R09: 0000000000000002
[ 7285.049108] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
[ 7285.052058] R13: ffffc900012f7f58 R14: 00000000000000e7 R15: 0000000000000000
[ 7285.055030] FS:  00007f7906d6c740(0000) GS:ffff88813bd80000(0000) knlGS:0000000000000000
[ 7285.058396] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 7285.060788] CR2: 00007f3ffa7e9bb8 CR3: 000000010bb00000 CR4: 00000000000006e0
[ 7285.063735] Call Trace:
[ 7285.064796]  <TASK>
[ 7285.065759]  syscall_trace_enter.constprop.0+0x122/0x1a0
[ 7285.067978]  do_syscall_64+0x16/0x80
[ 7285.069497]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 7285.071590] RIP: 0033:0x7f7906e35f49
[ 7285.073118] Code: 00 4c 8b 05 29 6f 10 00 be e7 00 00 00 ba 3c 00 00 00 eb 12 0f 1f 44 00 00 89 d0 0f 05 48 3d 00 f0 ff ff 77 1c f4 89 f0 0f 05 <48> 3d 00 f0 ff ff 76 e7 f7 d8 64 41 89 00 eb df 0f 1f 80 00 00 00
[ 7285.078021] RSP: 002b:00007ffeee52db88 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
[ 7285.079995] RAX: ffffffffffffffda RBX: 00007f7906f39920 RCX: 00007f7906e35f49
[ 7285.081869] RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
[ 7285.083729] RBP: 0000000000000000 R08: ffffffffffffff88 R09: 0000000000000001
[ 7285.085594] R10: fffffffffffffe2c R11: 0000000000000246 R12: 00007f7906f39920
[ 7285.087457] R13: 0000000000000001 R14: 00007f7906f3ee28 R15: 0000000000000000
[ 7285.089320]  </TASK>
[ 7285.089949] ---[ end trace 0000000000000000 ]---
[ 7285.091182] audit_panic: 22 callbacks suppressed
[ 7285.091185] audit: unrecoverable error in audit_syscall_entry()

Adn faddr_to_line tells me it is this:

void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
                           unsigned long a3, unsigned long a4)
{
        struct audit_context *context = audit_context();
        enum audit_state     state;

        if (!audit_enabled || !context)
                return;

>>>>>>  WARN_ON(context->context != AUDIT_CTX_UNUSED);  <<<<<<
        WARN_ON(context->name_count);
        if (context->context != AUDIT_CTX_UNUSED || context->name_count) {
                audit_panic("unrecoverable error in audit_syscall_entry()");
                return;
        }
.....

I have no clue how the audit subsystem works, I don't even know how
to enable it, and I've never seen audit messages on the console of
this test machine. I have other test machines that have audit
enabled, and they do not dump warnings like this - the only thing I
see in the logs for those machines is this:

 run xfstest generic/633
 process 'vfstest' launched '/dev/fd/4/file1' with NULL argv: empty string added
 XFS (pmem0): Unmounting Filesystem
 XFS (pmem0): Mounting V5 Filesystem
 XFS (pmem0): Ending clean mount
 run xfstest generic/634

That's waht I was seeing from this test machine earlier in the week,
too - I've been running 5.18-rc7 as the base kernel all week - so
I'm not sure .....

Oooooohhhh.

/* The per-task audit context. */
struct audit_context {
        int                 dummy;      /* must be the first element */
        enum {
                AUDIT_CTX_UNUSED,       /* audit_context is currently unused */
                AUDIT_CTX_SYSCALL,      /* in use by syscall */
                AUDIT_CTX_URING,        /* in use by io_uring */
        } context;
....

And that reminded me of something. I commented on #xfs on Friday
afternoon:

[20/5/22 15:04] <dchinner> so of the 3.5 hours run time on the machine that jsut completed, it looks like about a dozen tests are responsible for an hour of that runtime...
[20/5/22 15:05] <dchinner> but it was a clean run with no failures in 1055 tests run.
[20/5/22 15:06] <dchinner> But there's some WTFs like this in it:
[20/5/22 15:06] <dchinner> generic/678     [not run] kernel does not support IO_URING
[20/5/22 15:08] <dchinner> yet the same kernel on a different machine:
[20/5/22 15:08] <dchinner> generic/678 11s ...  3s
[20/5/22 15:08] <dchinner> and they have the same userspace, too....

Yeah, the machine that complained about "kernel does not support
IO_URING" is the one that is throwing these warnings now. It wasn't
that the kernel didn't support io-uring, it was that the machine was
missing the liburing-dev library. I installed it and rebuilt
fstests. These audit failures co-incide with the first test runs
with io-uring enabled. And vfstest uses io_uring if fstests enables
it.

Hence this now smells like a pre-existing issue -  either a test bug
or an io_uring task audit context leak. Anyone got any ideas?

Cheers,

Dave.
Jens Axboe May 22, 2022, 1:07 a.m. UTC | #5
On 5/21/22 5:13 PM, Dave Chinner wrote:
> [cc io_uring]
> 
> On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
>> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
>>
>> Hey everyone,
>>
>> Please note that this patch series contains patches that will be
>> rejected by the fstests mailing list because of the amount of changes
>> they contain. So tools like b4 will not be able to find the whole patch
>> series on a mailing list. In case it's helpful I've added the
>> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
>> possible to simply use the patch series as it appears in your inbox.
>>
>> All vfstests pass:
> 
> [...]
> 
>> #### xfs ####
>> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
>> FSTYP         -- xfs (debug)
>> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
>> MKFS_OPTIONS  -- -f /dev/sda4
>> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
>>
>> generic/633 58s ...  58s
>> generic/644 62s ...  60s
>> generic/645 161s ...  161s
>> generic/656 62s ...  63s
>> xfs/152 133s ...  133s
>> xfs/153 94s ...  92s
>> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
>> Passed all 6 tests
> 
> I'm not sure if it's this series that has introduced a test bug or
> triggered a latent issue in the kernel, but I've started seeing
> generic/633 throw audit subsystem warnings on a single test machine
> as of late Friday:
> 
> [ 7285.015888] WARNING: CPU: 3 PID: 2147118 at kernel/auditsc.c:2035 __audit_syscall_entry+0x113/0x140

Does your kernel have this commit?

commit 69e9cd66ae1392437234a63a3a1d60b6655f92ef
Author: Julian Orth <ju.orth@gmail.com>
Date:   Tue May 17 12:32:53 2022 +0200

    audit,io_uring,io-wq: call __audit_uring_exit for dummy contexts
Jens Axboe May 22, 2022, 2:19 a.m. UTC | #6
On 5/21/22 7:07 PM, Jens Axboe wrote:
> On 5/21/22 5:13 PM, Dave Chinner wrote:
>> [cc io_uring]
>>
>> On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
>>> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
>>>
>>> Hey everyone,
>>>
>>> Please note that this patch series contains patches that will be
>>> rejected by the fstests mailing list because of the amount of changes
>>> they contain. So tools like b4 will not be able to find the whole patch
>>> series on a mailing list. In case it's helpful I've added the
>>> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
>>> possible to simply use the patch series as it appears in your inbox.
>>>
>>> All vfstests pass:
>>
>> [...]
>>
>>> #### xfs ####
>>> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
>>> FSTYP         -- xfs (debug)
>>> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
>>> MKFS_OPTIONS  -- -f /dev/sda4
>>> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
>>>
>>> generic/633 58s ...  58s
>>> generic/644 62s ...  60s
>>> generic/645 161s ...  161s
>>> generic/656 62s ...  63s
>>> xfs/152 133s ...  133s
>>> xfs/153 94s ...  92s
>>> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
>>> Passed all 6 tests
>>
>> I'm not sure if it's this series that has introduced a test bug or
>> triggered a latent issue in the kernel, but I've started seeing
>> generic/633 throw audit subsystem warnings on a single test machine
>> as of late Friday:
>>
>> [ 7285.015888] WARNING: CPU: 3 PID: 2147118 at kernel/auditsc.c:2035 __audit_syscall_entry+0x113/0x140
> 
> Does your kernel have this commit?
> 
> commit 69e9cd66ae1392437234a63a3a1d60b6655f92ef
> Author: Julian Orth <ju.orth@gmail.com>
> Date:   Tue May 17 12:32:53 2022 +0200
> 
>     audit,io_uring,io-wq: call __audit_uring_exit for dummy contexts

I could not reproduce either with or without your patch when I finally
got that test going and figure out how to turn on audit and get it
enabled. I don't run with that.

But looking at your line numbers, I think you're missing the above
commit. The WARN_ON_ONCE() matches up with it NOT being applied, which
is most likely why it triggers for you. It's in Linus's tree, but not in
-rc7.
Dave Chinner May 23, 2022, 12:13 a.m. UTC | #7
On Sat, May 21, 2022 at 08:19:51PM -0600, Jens Axboe wrote:
> On 5/21/22 7:07 PM, Jens Axboe wrote:
> > On 5/21/22 5:13 PM, Dave Chinner wrote:
> >> [cc io_uring]
> >>
> >> On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
> >>> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
> >>>
> >>> Hey everyone,
> >>>
> >>> Please note that this patch series contains patches that will be
> >>> rejected by the fstests mailing list because of the amount of changes
> >>> they contain. So tools like b4 will not be able to find the whole patch
> >>> series on a mailing list. In case it's helpful I've added the
> >>> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
> >>> possible to simply use the patch series as it appears in your inbox.
> >>>
> >>> All vfstests pass:
> >>
> >> [...]
> >>
> >>> #### xfs ####
> >>> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> >>> FSTYP         -- xfs (debug)
> >>> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> >>> MKFS_OPTIONS  -- -f /dev/sda4
> >>> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> >>>
> >>> generic/633 58s ...  58s
> >>> generic/644 62s ...  60s
> >>> generic/645 161s ...  161s
> >>> generic/656 62s ...  63s
> >>> xfs/152 133s ...  133s
> >>> xfs/153 94s ...  92s
> >>> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
> >>> Passed all 6 tests
> >>
> >> I'm not sure if it's this series that has introduced a test bug or
> >> triggered a latent issue in the kernel, but I've started seeing
> >> generic/633 throw audit subsystem warnings on a single test machine
> >> as of late Friday:
> >>
> >> [ 7285.015888] WARNING: CPU: 3 PID: 2147118 at kernel/auditsc.c:2035 __audit_syscall_entry+0x113/0x140
> > 
> > Does your kernel have this commit?
> > 
> > commit 69e9cd66ae1392437234a63a3a1d60b6655f92ef
> > Author: Julian Orth <ju.orth@gmail.com>
> > Date:   Tue May 17 12:32:53 2022 +0200
> > 
> >     audit,io_uring,io-wq: call __audit_uring_exit for dummy contexts

No, that wasn't in -rc7.

> I could not reproduce either with or without your patch when I finally
> got that test going and figure out how to turn on audit and get it
> enabled. I don't run with that.

Ok. Given that this has been broken for over a year and nobody
has noticed until late .18-rcX, it might be worth adding an audit
enabled VM to your io-uring test farm....

Cheers,

Dve.
Jens Axboe May 23, 2022, 12:57 a.m. UTC | #8
On 5/22/22 6:13 PM, Dave Chinner wrote:
> On Sat, May 21, 2022 at 08:19:51PM -0600, Jens Axboe wrote:
>> On 5/21/22 7:07 PM, Jens Axboe wrote:
>>> On 5/21/22 5:13 PM, Dave Chinner wrote:
>>>> [cc io_uring]
>>>>
>>>> On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
>>>>> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
>>>>>
>>>>> Hey everyone,
>>>>>
>>>>> Please note that this patch series contains patches that will be
>>>>> rejected by the fstests mailing list because of the amount of changes
>>>>> they contain. So tools like b4 will not be able to find the whole patch
>>>>> series on a mailing list. In case it's helpful I've added the
>>>>> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
>>>>> possible to simply use the patch series as it appears in your inbox.
>>>>>
>>>>> All vfstests pass:
>>>>
>>>> [...]
>>>>
>>>>> #### xfs ####
>>>>> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
>>>>> FSTYP         -- xfs (debug)
>>>>> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
>>>>> MKFS_OPTIONS  -- -f /dev/sda4
>>>>> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
>>>>>
>>>>> generic/633 58s ...  58s
>>>>> generic/644 62s ...  60s
>>>>> generic/645 161s ...  161s
>>>>> generic/656 62s ...  63s
>>>>> xfs/152 133s ...  133s
>>>>> xfs/153 94s ...  92s
>>>>> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
>>>>> Passed all 6 tests
>>>>
>>>> I'm not sure if it's this series that has introduced a test bug or
>>>> triggered a latent issue in the kernel, but I've started seeing
>>>> generic/633 throw audit subsystem warnings on a single test machine
>>>> as of late Friday:
>>>>
>>>> [ 7285.015888] WARNING: CPU: 3 PID: 2147118 at kernel/auditsc.c:2035 __audit_syscall_entry+0x113/0x140
>>>
>>> Does your kernel have this commit?
>>>
>>> commit 69e9cd66ae1392437234a63a3a1d60b6655f92ef
>>> Author: Julian Orth <ju.orth@gmail.com>
>>> Date:   Tue May 17 12:32:53 2022 +0200
>>>
>>>     audit,io_uring,io-wq: call __audit_uring_exit for dummy contexts
> 
> No, that wasn't in -rc7.
> 
>> I could not reproduce either with or without your patch when I finally
>> got that test going and figure out how to turn on audit and get it
>> enabled. I don't run with that.
> 
> Ok. Given that this has been broken for over a year and nobody
> has noticed until late .18-rcX, it might be worth adding an audit
> enabled VM to your io-uring test farm....

It was in the 5.16 release, so it's ~4 months ago. Don't disagree on the
testing, though I do think that's mostly on the audit side. I had no
hand in any of that code.

From my experience trying to reproduce it yesterday, my test distros
don't even enable it and you have to both fiddle the config and add a
boot parameter to even turn it on. And then it still didn't trigger for
me.

I'll see if I can add something to the testing mix for this.
Christian Brauner May 23, 2022, 9:44 a.m. UTC | #9
On Sun, May 22, 2022 at 09:13:50AM +1000, Dave Chinner wrote:
> [cc io_uring]
> 
> On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
> > From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
> > 
> > Hey everyone,
> > 
> > Please note that this patch series contains patches that will be
> > rejected by the fstests mailing list because of the amount of changes
> > they contain. So tools like b4 will not be able to find the whole patch
> > series on a mailing list. In case it's helpful I've added the
> > "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
> > possible to simply use the patch series as it appears in your inbox.
> > 
> > All vfstests pass:
> 
> [...]
> 
> > #### xfs ####
> > ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> > FSTYP         -- xfs (debug)
> > PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> > MKFS_OPTIONS  -- -f /dev/sda4
> > MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> > 
> > generic/633 58s ...  58s
> > generic/644 62s ...  60s
> > generic/645 161s ...  161s
> > generic/656 62s ...  63s
> > xfs/152 133s ...  133s
> > xfs/153 94s ...  92s
> > Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
> > Passed all 6 tests
> 
> I'm not sure if it's this series that has introduced a test bug or
> triggered a latent issue in the kernel, but I've started seeing
> generic/633 throw audit subsystem warnings on a single test machine
> as of late Friday:
> 
> [ 7285.015888] WARNING: CPU: 3 PID: 2147118 at kernel/auditsc.c:2035 __audit_syscall_entry+0x113/0x140
> [ 7285.019973] Modules linked in:
> [ 7285.021281] CPU: 3 PID: 2147118 Comm: vfstest Not tainted 5.18.0-rc7-dgc+ #1250
> [ 7285.024341] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> [ 7285.027782] RIP: 0010:__audit_syscall_entry+0x113/0x140
> [ 7285.029923] Code: 24 e8 c1 6b ff ff 48 8b 34 24 85 c0 48 8b 54 24 08 48 8b 4c 24 10 4c 8b 44 24 18 0f 84 72 ff ff ff 48 83 c4 20 5b 5d 41 5c c3 <0f> 0b 85 c0 75 14 48 83 c4 20 48 c7 c7 70 45 7f 82 5b 5d 41 5c e9
> [ 7285.037563] RSP: 0018:ffffc900012f7ed0 EFLAGS: 00010202
> [ 7285.039748] RAX: 0000000000000000 RBX: ffff8880aaf18800 RCX: 000000000000003c
> [ 7285.043126] RDX: 00000000000000e7 RSI: 0000000000000000 RDI: ffff888102104f00
> [ 7285.046120] RBP: 00000000000000e7 R08: fffffffffffffe2c R09: 0000000000000002
> [ 7285.049108] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
> [ 7285.052058] R13: ffffc900012f7f58 R14: 00000000000000e7 R15: 0000000000000000
> [ 7285.055030] FS:  00007f7906d6c740(0000) GS:ffff88813bd80000(0000) knlGS:0000000000000000
> [ 7285.058396] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 7285.060788] CR2: 00007f3ffa7e9bb8 CR3: 000000010bb00000 CR4: 00000000000006e0
> [ 7285.063735] Call Trace:
> [ 7285.064796]  <TASK>
> [ 7285.065759]  syscall_trace_enter.constprop.0+0x122/0x1a0
> [ 7285.067978]  do_syscall_64+0x16/0x80
> [ 7285.069497]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 7285.071590] RIP: 0033:0x7f7906e35f49
> [ 7285.073118] Code: 00 4c 8b 05 29 6f 10 00 be e7 00 00 00 ba 3c 00 00 00 eb 12 0f 1f 44 00 00 89 d0 0f 05 48 3d 00 f0 ff ff 77 1c f4 89 f0 0f 05 <48> 3d 00 f0 ff ff 76 e7 f7 d8 64 41 89 00 eb df 0f 1f 80 00 00 00
> [ 7285.078021] RSP: 002b:00007ffeee52db88 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> [ 7285.079995] RAX: ffffffffffffffda RBX: 00007f7906f39920 RCX: 00007f7906e35f49
> [ 7285.081869] RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
> [ 7285.083729] RBP: 0000000000000000 R08: ffffffffffffff88 R09: 0000000000000001
> [ 7285.085594] R10: fffffffffffffe2c R11: 0000000000000246 R12: 00007f7906f39920
> [ 7285.087457] R13: 0000000000000001 R14: 00007f7906f3ee28 R15: 0000000000000000
> [ 7285.089320]  </TASK>
> [ 7285.089949] ---[ end trace 0000000000000000 ]---
> [ 7285.091182] audit_panic: 22 callbacks suppressed
> [ 7285.091185] audit: unrecoverable error in audit_syscall_entry()
> 
> Adn faddr_to_line tells me it is this:
> 
> void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
>                            unsigned long a3, unsigned long a4)
> {
>         struct audit_context *context = audit_context();
>         enum audit_state     state;
> 
>         if (!audit_enabled || !context)
>                 return;
> 
> >>>>>>  WARN_ON(context->context != AUDIT_CTX_UNUSED);  <<<<<<
>         WARN_ON(context->name_count);
>         if (context->context != AUDIT_CTX_UNUSED || context->name_count) {
>                 audit_panic("unrecoverable error in audit_syscall_entry()");
>                 return;
>         }
> .....
> 
> I have no clue how the audit subsystem works, I don't even know how
> to enable it, and I've never seen audit messages on the console of
> this test machine. I have other test machines that have audit
> enabled, and they do not dump warnings like this - the only thing I
> see in the logs for those machines is this:
> 
>  run xfstest generic/633
>  process 'vfstest' launched '/dev/fd/4/file1' with NULL argv: empty string added
>  XFS (pmem0): Unmounting Filesystem
>  XFS (pmem0): Mounting V5 Filesystem
>  XFS (pmem0): Ending clean mount
>  run xfstest generic/634
> 
> That's waht I was seeing from this test machine earlier in the week,
> too - I've been running 5.18-rc7 as the base kernel all week - so
> I'm not sure .....
> 
> Oooooohhhh.
> 
> /* The per-task audit context. */
> struct audit_context {
>         int                 dummy;      /* must be the first element */
>         enum {
>                 AUDIT_CTX_UNUSED,       /* audit_context is currently unused */
>                 AUDIT_CTX_SYSCALL,      /* in use by syscall */
>                 AUDIT_CTX_URING,        /* in use by io_uring */
>         } context;
> ....
> 
> And that reminded me of something. I commented on #xfs on Friday
> afternoon:
> 
> [20/5/22 15:04] <dchinner> so of the 3.5 hours run time on the machine that jsut completed, it looks like about a dozen tests are responsible for an hour of that runtime...
> [20/5/22 15:05] <dchinner> but it was a clean run with no failures in 1055 tests run.
> [20/5/22 15:06] <dchinner> But there's some WTFs like this in it:
> [20/5/22 15:06] <dchinner> generic/678     [not run] kernel does not support IO_URING
> [20/5/22 15:08] <dchinner> yet the same kernel on a different machine:
> [20/5/22 15:08] <dchinner> generic/678 11s ...  3s
> [20/5/22 15:08] <dchinner> and they have the same userspace, too....
> 
> Yeah, the machine that complained about "kernel does not support
> IO_URING" is the one that is throwing these warnings now. It wasn't
> that the kernel didn't support io-uring, it was that the machine was
> missing the liburing-dev library. I installed it and rebuilt
> fstests. These audit failures co-incide with the first test runs
> with io-uring enabled. And vfstest uses io_uring if fstests enables
> it.
> 
> Hence this now smells like a pre-existing issue -  either a test bug
> or an io_uring task audit context leak. Anyone got any ideas?

I see this is unrelated to the test thankfully and can be considered
fixed afaict.

Thanks for taking care of this everyone!
Christian
Dave Chinner May 23, 2022, 10:41 a.m. UTC | #10
On Sun, May 22, 2022 at 06:57:04PM -0600, Jens Axboe wrote:
> On 5/22/22 6:13 PM, Dave Chinner wrote:
> > On Sat, May 21, 2022 at 08:19:51PM -0600, Jens Axboe wrote:
> >> On 5/21/22 7:07 PM, Jens Axboe wrote:
> >>> On 5/21/22 5:13 PM, Dave Chinner wrote:
> >>>> [cc io_uring]
> >>>>
> >>>> On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
> >>>>> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
> >>>>>
> >>>>> Hey everyone,
> >>>>>
> >>>>> Please note that this patch series contains patches that will be
> >>>>> rejected by the fstests mailing list because of the amount of changes
> >>>>> they contain. So tools like b4 will not be able to find the whole patch
> >>>>> series on a mailing list. In case it's helpful I've added the
> >>>>> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
> >>>>> possible to simply use the patch series as it appears in your inbox.
> >>>>>
> >>>>> All vfstests pass:
> >>>>
> >>>> [...]
> >>>>
> >>>>> #### xfs ####
> >>>>> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> >>>>> FSTYP         -- xfs (debug)
> >>>>> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> >>>>> MKFS_OPTIONS  -- -f /dev/sda4
> >>>>> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> >>>>>
> >>>>> generic/633 58s ...  58s
> >>>>> generic/644 62s ...  60s
> >>>>> generic/645 161s ...  161s
> >>>>> generic/656 62s ...  63s
> >>>>> xfs/152 133s ...  133s
> >>>>> xfs/153 94s ...  92s
> >>>>> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
> >>>>> Passed all 6 tests
> >>>>
> >>>> I'm not sure if it's this series that has introduced a test bug or
> >>>> triggered a latent issue in the kernel, but I've started seeing
> >>>> generic/633 throw audit subsystem warnings on a single test machine
> >>>> as of late Friday:
> >>>>
> >>>> [ 7285.015888] WARNING: CPU: 3 PID: 2147118 at kernel/auditsc.c:2035 __audit_syscall_entry+0x113/0x140
> >>>
> >>> Does your kernel have this commit?
> >>>
> >>> commit 69e9cd66ae1392437234a63a3a1d60b6655f92ef
> >>> Author: Julian Orth <ju.orth@gmail.com>
> >>> Date:   Tue May 17 12:32:53 2022 +0200
> >>>
> >>>     audit,io_uring,io-wq: call __audit_uring_exit for dummy contexts
> > 
> > No, that wasn't in -rc7.
> > 
> >> I could not reproduce either with or without your patch when I finally
> >> got that test going and figure out how to turn on audit and get it
> >> enabled. I don't run with that.
> > 
> > Ok. Given that this has been broken for over a year and nobody
> > has noticed until late .18-rcX, it might be worth adding an audit
> > enabled VM to your io-uring test farm....
> 
> It was in the 5.16 release, so it's ~4 months ago. Don't disagree on the

Huh. The commit that it fixes is dated Feb 2021:

commit 5bd2182d58e9d9c6279b7a8a2f9b41add0e7f9cb
Author: Paul Moore <paul@paul-moore.com>
Date:   Tue Feb 16 19:46:48 2021 -0500

    audit,io_uring,io-wq: add some basic audit support to io_uring

I guess it must have sat in a tree somewhere for 6 months before
before being merged.

> testing, though I do think that's mostly on the audit side. I had no
> hand in any of that code.

Fair enough.

> From my experience trying to reproduce it yesterday, my test distros
> don't even enable it and you have to both fiddle the config and add a
> boot parameter to even turn it on. And then it still didn't trigger for
> me.

I have machines with audit enabled as it seems to be the debian
default these days. I haven't explicitly turned it on - it's just
there. I guess it came along with selinux being enabled on these
test VMs - I have "selinux=1 security=selinux" on the kernel CLI for
these VMs.

Apart from that, I have no clue as to why this one particular
VM tripped this and none of the others with similar selinux/audit
configs have had any problems...

> I'll see if I can add something to the testing mix for this.

Thanks!

Cheers,

Dave.
Jens Axboe May 23, 2022, 12:28 p.m. UTC | #11
On 5/23/22 4:41 AM, Dave Chinner wrote:
>> From my experience trying to reproduce it yesterday, my test distros
>> don't even enable it and you have to both fiddle the config and add a
>> boot parameter to even turn it on. And then it still didn't trigger for
>> me.
> 
> I have machines with audit enabled as it seems to be the debian
> default these days. I haven't explicitly turned it on - it's just
> there. I guess it came along with selinux being enabled on these
> test VMs - I have "selinux=1 security=selinux" on the kernel CLI for
> these VMs.

Hmm, I am running debian on these. Anyway, I'll get one configured so
that it triggers this issue, and use that going forward. Maybe I need
selinux too, not just audit, and explicitly enable it with the boot
parameters.