mbox series

[v4,0/9] Test the new fanotify FAN_FS_ERROR event

Message ID 20211118235744.802584-1-krisman@collabora.com
Headers show
Series Test the new fanotify FAN_FS_ERROR event | expand

Message

Gabriel Krisman Bertazi Nov. 18, 2021, 11:57 p.m. UTC
Hi,

FAN_FS_ERROR was merged into Linus tree, and the PIDFD testcases reached
LTP.  Therefore, I'm sending a new version of the FAN_FS_ERROR LTP
tests.  This is the v4 of this patchset, and it applies the feedback of
the previous version.

Thanks,

---

Original cover letter:

FAN_FS_ERROR is a new (still unmerged) fanotify event to monitor
fileystem errors.  This patchset introduces a new LTP test for this
feature.

Testing file system errors is slightly tricky, in particular because
they are mostly file system dependent.  Since there are only patches for
ext4, I choose to make the test around it, since there wouldn't be much
to do with other file systems.  The second challenge is how we cause the
file system errors, since there is no error injection for ext4 in Linux.
In this series, this is done by corrupting specific data in the
test device with the help of debugfs.

The FAN_FS_ERROR feature is flying around linux-ext4 and fsdevel, and
the latest version is available on the branch below:

    https://gitlab.collabora.com/krisman/linux -b fanotify-notifications-v9

A proper manpage description is also available on the respective mailing
list, or in the branch below:

    https://gitlab.collabora.com/krisman/man-pages.git -b fan-fs-error

Please, let me know your thoughts.

Gabriel Krisman Bertazi (9):
  syscalls: fanotify: Add macro to require specific mark types
  syscalls: fanotify: Add macro to require specific events
  syscalls/fanotify22: Introduce FAN_FS_ERROR test
  syscalls/fanotify22: Validate the generic error info
  syscalls/fanotify22: Validate incoming FID in FAN_FS_ERROR
  syscalls/fanotify22: Support submission of debugfs commands
  syscalls/fanotify22: Create a corrupted file
  syscalls/fanotify22: Test file event with broken inode
  syscalls/fanotify22: Test capture of multiple errors

 configure.ac                                  |   3 +-
 testcases/kernel/syscalls/fanotify/.gitignore |   1 +
 testcases/kernel/syscalls/fanotify/fanotify.h |  65 +++-
 .../kernel/syscalls/fanotify/fanotify03.c     |   4 +-
 .../kernel/syscalls/fanotify/fanotify10.c     |   3 +-
 .../kernel/syscalls/fanotify/fanotify12.c     |   3 +-
 .../kernel/syscalls/fanotify/fanotify22.c     | 314 ++++++++++++++++++
 7 files changed, 385 insertions(+), 8 deletions(-)
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify22.c

Comments

Amir Goldstein Nov. 19, 2021, 5:48 a.m. UTC | #1
On Fri, Nov 19, 2021 at 1:57 AM Gabriel Krisman Bertazi
<krisman@collabora.com> wrote:
>
> Hi,
>
> FAN_FS_ERROR was merged into Linus tree, and the PIDFD testcases reached
> LTP.  Therefore, I'm sending a new version of the FAN_FS_ERROR LTP
> tests.  This is the v4 of this patchset, and it applies the feedback of
> the previous version.
>
> Thanks,
>
> ---
>
> Original cover letter:
>
> FAN_FS_ERROR is a new (still unmerged) fanotify event to monitor
> fileystem errors.  This patchset introduces a new LTP test for this
> feature.
>
> Testing file system errors is slightly tricky, in particular because
> they are mostly file system dependent.  Since there are only patches for
> ext4, I choose to make the test around it, since there wouldn't be much
> to do with other file systems.  The second challenge is how we cause the
> file system errors, since there is no error injection for ext4 in Linux.
> In this series, this is done by corrupting specific data in the
> test device with the help of debugfs.
>
> The FAN_FS_ERROR feature is flying around linux-ext4 and fsdevel, and
> the latest version is available on the branch below:
>
>     https://gitlab.collabora.com/krisman/linux -b fanotify-notifications-v9
>
> A proper manpage description is also available on the respective mailing
> list, or in the branch below:
>
>     https://gitlab.collabora.com/krisman/man-pages.git -b fan-fs-error
>
> Please, let me know your thoughts.
>

Gabriel,

Can you please push these v4 patches to your gitlab tree?

Thanks,
Amir.
Gabriel Krisman Bertazi Nov. 19, 2021, 7:29 p.m. UTC | #2
Amir Goldstein <amir73il@gmail.com> writes:

> On Fri, Nov 19, 2021 at 1:57 AM Gabriel Krisman Bertazi
> <krisman@collabora.com> wrote:
>>
>> Hi,
>>
>> FAN_FS_ERROR was merged into Linus tree, and the PIDFD testcases reached
>> LTP.  Therefore, I'm sending a new version of the FAN_FS_ERROR LTP
>> tests.  This is the v4 of this patchset, and it applies the feedback of
>> the previous version.
>>
>> Thanks,
>>
>> ---
>>
>> Original cover letter:
>>
>> FAN_FS_ERROR is a new (still unmerged) fanotify event to monitor
>> fileystem errors.  This patchset introduces a new LTP test for this
>> feature.
>>
>> Testing file system errors is slightly tricky, in particular because
>> they are mostly file system dependent.  Since there are only patches for
>> ext4, I choose to make the test around it, since there wouldn't be much
>> to do with other file systems.  The second challenge is how we cause the
>> file system errors, since there is no error injection for ext4 in Linux.
>> In this series, this is done by corrupting specific data in the
>> test device with the help of debugfs.
>>
>> The FAN_FS_ERROR feature is flying around linux-ext4 and fsdevel, and
>> the latest version is available on the branch below:
>>
>>     https://gitlab.collabora.com/krisman/linux -b fanotify-notifications-v9
>>
>> A proper manpage description is also available on the respective mailing
>> list, or in the branch below:
>>
>>     https://gitlab.collabora.com/krisman/man-pages.git -b fan-fs-error
>>
>> Please, let me know your thoughts.
>>
>
> Gabriel,
>
> Can you please push these v4 patches to your gitlab tree?

Hi Amir,

I have pushed v4 to :

https://gitlab.collabora.com/krisman/ltp.git -b fan-fs-error_v4

Thank you!
Amir Goldstein Nov. 20, 2021, 10:43 a.m. UTC | #3
> >> A proper manpage description is also available on the respective mailing
> >> list, or in the branch below:
> >>
> >>     https://gitlab.collabora.com/krisman/man-pages.git -b fan-fs-error
> >>
> >> Please, let me know your thoughts.
> >>
> >
> > Gabriel,
> >
> > Can you please push these v4 patches to your gitlab tree?
>
> Hi Amir,
>
> I have pushed v4 to :
>
> https://gitlab.collabora.com/krisman/ltp.git -b fan-fs-error_v4
>

Thanks. I've based my fan_rename ltp branch on this.
I would like to do the same for the man-page update branch.
However, Matthew's man page updates for v5.15 are conflicting
with your changes, so after Matthew posts v2 of his man page patch,
please rebase your changes on top of his branch.

Ideally, you could have waited until Matthew's changes are merged
upstream, like you did for ltp before rebasing, but man-pages maintainers
are quite behind on merging updates, so we will need to collaborate
between us in the meanwhile.

Thanks,
Amir.
Petr Vorel Nov. 22, 2021, 7:47 a.m. UTC | #4
Hi all,

<snip>
> Hi Amir,

> I have pushed v4 to :

> https://gitlab.collabora.com/krisman/ltp.git -b fan-fs-error_v4

FYI I've rebased it on my fix 3b2ea2e00 ("configure.ac: Add struct
fanotify_event_info_pidfd check")

https://github.com/linux-test-project/ltp.git -b gertazi/fanotify21.v4.fixes

diff to krisman/fan-fs-error_v4:

diff --git configure.ac configure.ac
index a9dc25249..d25183368 100644
--- configure.ac
+++ configure.ac
@@ -160,8 +160,8 @@ AC_CHECK_MEMBERS([struct utsname.domainname],,,[
 AC_CHECK_TYPES([enum kcmp_type],,,[#include <linux/kcmp.h>])
 AC_CHECK_TYPES([struct acct_v3],,,[#include <sys/acct.h>])
 AC_CHECK_TYPES([struct af_alg_iv, struct sockaddr_alg],,,[# include <linux/if_alg.h>])
-AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_header,
-		struct fanotify_event_info_error],[],[],[#include <sys/fanotify.h>])
+AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_error,
+		struct fanotify_event_info_header, struct fanotify_event_info_pidfd],,,[#include <sys/fanotify.h>])
 AC_CHECK_TYPES([struct file_dedupe_range],,,[#include <linux/fs.h>])
 
 AC_CHECK_TYPES([struct file_handle],,,[

Kind regards,
Petr
Gabriel Krisman Bertazi Nov. 22, 2021, 5:35 p.m. UTC | #5
Petr Vorel <pvorel@suse.cz> writes:

> Hi all,
>
> <snip>
>> Hi Amir,
>
>> I have pushed v4 to :
>
>> https://gitlab.collabora.com/krisman/ltp.git -b fan-fs-error_v4
>
> FYI I've rebased it on my fix 3b2ea2e00 ("configure.ac: Add struct
> fanotify_event_info_pidfd check")
>
> https://github.com/linux-test-project/ltp.git -b gertazi/fanotify21.v4.fixes
>
> diff to krisman/fan-fs-error_v4:

Petr,

Should I send a v5 or is v4 getting picked up and merged with the fixup
hunk?

Thanks,
Petr Vorel Nov. 22, 2021, 9:09 p.m. UTC | #6
Hi all,
> Petr Vorel <pvorel@suse.cz> writes:

> > Hi all,

> > <snip>
> >> Hi Amir,

> >> I have pushed v4 to :

> >> https://gitlab.collabora.com/krisman/ltp.git -b fan-fs-error_v4

> > FYI I've rebased it on my fix 3b2ea2e00 ("configure.ac: Add struct
> > fanotify_event_info_pidfd check")

> > https://github.com/linux-test-project/ltp.git -b gertazi/fanotify21.v4.fixes

> > diff to krisman/fan-fs-error_v4:

> Petr,

> Should I send a v5 or is v4 getting picked up and merged with the fixup
> hunk?
No need to sent v4, I'll merge it from my branch. This is info for Amir, which
wanted to use your git tree to base his patchset on (if it wasn't relevant only
to patches for man-pages).

Kind regards,
Petr
Petr Vorel Nov. 24, 2021, 10:39 a.m. UTC | #7
Hi all,

<snip>
> > Hi Amir,

> > I have pushed v4 to :

> > https://gitlab.collabora.com/krisman/ltp.git -b fan-fs-error_v4

> FYI I've rebased it on my fix 3b2ea2e00 ("configure.ac: Add struct
> fanotify_event_info_pidfd check")

> https://github.com/linux-test-project/ltp.git -b gertazi/fanotify21.v4.fixes

FYI I removed branch from official LTP repository and put it to my fork
https://github.com/pevik/ltp.git -b fan-fs-error_v4.fixes

Kind regards,
Petr

> diff to krisman/fan-fs-error_v4:

> diff --git configure.ac configure.ac
> index a9dc25249..d25183368 100644
> --- configure.ac
> +++ configure.ac
> @@ -160,8 +160,8 @@ AC_CHECK_MEMBERS([struct utsname.domainname],,,[
>  AC_CHECK_TYPES([enum kcmp_type],,,[#include <linux/kcmp.h>])
>  AC_CHECK_TYPES([struct acct_v3],,,[#include <sys/acct.h>])
>  AC_CHECK_TYPES([struct af_alg_iv, struct sockaddr_alg],,,[# include <linux/if_alg.h>])
> -AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_header,
> -		struct fanotify_event_info_error],[],[],[#include <sys/fanotify.h>])
> +AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_error,
> +		struct fanotify_event_info_header, struct fanotify_event_info_pidfd],,,[#include <sys/fanotify.h>])
>  AC_CHECK_TYPES([struct file_dedupe_range],,,[#include <linux/fs.h>])

>  AC_CHECK_TYPES([struct file_handle],,,[

> Kind regards,
> Petr
Amir Goldstein Dec. 20, 2021, 11:48 a.m. UTC | #8
On Wed, Nov 24, 2021 at 12:40 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi all,
>
> <snip>
> > > Hi Amir,
>
> > > I have pushed v4 to :
>
> > > https://gitlab.collabora.com/krisman/ltp.git -b fan-fs-error_v4
>
> > FYI I've rebased it on my fix 3b2ea2e00 ("configure.ac: Add struct
> > fanotify_event_info_pidfd check")
>
> > https://github.com/linux-test-project/ltp.git -b gertazi/fanotify21.v4.fixes
>
> FYI I removed branch from official LTP repository and put it to my fork
> https://github.com/pevik/ltp.git -b fan-fs-error_v4.fixes
>

Hi Petr,

Are you waiting with this merge for after release of v5.16?
or is it just waiting behind other work?

Just asking out of curiosity.
I've based my tests for fan_rename (queued for v5.17) on top of your branch.

Thanks,
Amir.
Petr Vorel Dec. 20, 2021, 6:07 p.m. UTC | #9
Hi Amir,

[ Cc Cyril and Richie ]

> On Wed, Nov 24, 2021 at 12:40 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi all,

> > <snip>
> > > > Hi Amir,

> > > > I have pushed v4 to :

> > > > https://gitlab.collabora.com/krisman/ltp.git -b fan-fs-error_v4

> > > FYI I've rebased it on my fix 3b2ea2e00 ("configure.ac: Add struct
> > > fanotify_event_info_pidfd check")

> > > https://github.com/linux-test-project/ltp.git -b gertazi/fanotify21.v4.fixes

> > FYI I removed branch from official LTP repository and put it to my fork
> > https://github.com/pevik/ltp.git -b fan-fs-error_v4.fixes


> Hi Petr,

> Are you waiting with this merge for after release of v5.16?
> or is it just waiting behind other work?
Yes. Thanks for this input, we're just discussing our policy about tests for new
(kernel) release functionality. First we agreed to wait [1] (due problems
described in [2]), Richie is suggesting to merge earlier [2], although Cyril
had doubts it's worth of the work [3].

Kind regards,
Petr

> Just asking out of curiosity.
> I've based my tests for fan_rename (queued for v5.17) on top of your branch.

> Thanks,
> Amir.

[1] https://lore.kernel.org/ltp/20211210134556.26091-1-pvorel@suse.cz/
[2] https://lore.kernel.org/ltp/87lf0ffw1y.fsf@suse.de/
[3] https://lore.kernel.org/ltp/Ybc5QJSZM3YIji70@yuki/
Petr Vorel Jan. 10, 2022, 3:16 p.m. UTC | #10
Hi all,

v5.16 released => patchset merged.
Thanks!

Kind regards,
Petr
Amir Goldstein Feb. 2, 2022, 1:49 p.m. UTC | #11
On Mon, Jan 10, 2022 at 5:16 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi all,
>
> v5.16 released => patchset merged.
> Thanks!
>

Guys,

Looks like we have a regression.
With kernel v5.17-rc1, test fanotify22 blocks on the first test case,
because the expected ECORRUPTED event on remount,abort is never received.
The multiple error test cases also fail for the same reason.

Gabriel,

Are you aware of any ext4 change that could explain this regression?

In any case, Petr, I suggest adding a short timeout to the test
instead of the default 5min.
Test takes less than 1 second on my VM on v5.16, so...

Thanks,
Amir.
Jan Stancek Feb. 2, 2022, 2:10 p.m. UTC | #12
On Wed, Feb 2, 2022 at 2:49 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Jan 10, 2022 at 5:16 PM Petr Vorel <pvorel@suse.cz> wrote:
> >
> > Hi all,
> >
> > v5.16 released => patchset merged.
> > Thanks!
> >
>
> Guys,
>
> Looks like we have a regression.

agreed, "abort" option stopped working:
https://gitlab.com/cki-project/kernel-tests/-/issues/945

Lukas pointed out this patch that didn't make it in yet:
https://lkml.org/lkml/2021/12/21/384
This should be new version:
https://lore.kernel.org/linux-ext4/YcSYvk5DdGjjB9q%2F@mit.edu/T/#t

> With kernel v5.17-rc1, test fanotify22 blocks on the first test case,
> because the expected ECORRUPTED event on remount,abort is never received.
> The multiple error test cases also fail for the same reason.
>
> Gabriel,
>
> Are you aware of any ext4 change that could explain this regression?
>
> In any case, Petr, I suggest adding a short timeout to the test
> instead of the default 5min.
> Test takes less than 1 second on my VM on v5.16, so...
>
> Thanks,
> Amir.
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
Petr Vorel Feb. 2, 2022, 2:22 p.m. UTC | #13
Hi Amir,

> On Mon, Jan 10, 2022 at 5:16 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi all,

> > v5.16 released => patchset merged.
> > Thanks!


> Guys,

> Looks like we have a regression.
> With kernel v5.17-rc1, test fanotify22 blocks on the first test case,
> because the expected ECORRUPTED event on remount,abort is never received.
> The multiple error test cases also fail for the same reason.

> Gabriel,

> Are you aware of any ext4 change that could explain this regression?

> In any case, Petr, I suggest adding a short timeout to the test
> instead of the default 5min.
> Test takes less than 1 second on my VM on v5.16, so...
We usually don't lower the default timeout, but here it's a good idea since here
it blocks. It's similar speed on my machine, but I'd be conservative and put
timeout 10 sec. Sending patch.

Kind regards,
Petr


> Thanks,
> Amir.
Cyril Hrubis Feb. 2, 2022, 4:13 p.m. UTC | #14
Hi!
> > In any case, Petr, I suggest adding a short timeout to the test
> > instead of the default 5min.
> > Test takes less than 1 second on my VM on v5.16, so...
> We usually don't lower the default timeout, but here it's a good idea since here
> it blocks. It's similar speed on my machine, but I'd be conservative and put
> timeout 10 sec. Sending patch.

Actually I hope to change the default timeout once I finish my runtime
patchset to something more reasonable as majority of LTP tests finish
under one second.
Gabriel Krisman Bertazi Feb. 2, 2022, 4:45 p.m. UTC | #15
Jan Stancek <jstancek@redhat.com> writes:

> On Wed, Feb 2, 2022 at 2:49 PM Amir Goldstein <amir73il@gmail.com> wrote:
>> Guys,
>>
>> Looks like we have a regression.
>
> agreed, "abort" option stopped working:
> https://gitlab.com/cki-project/kernel-tests/-/issues/945
>
> Lukas pointed out this patch that didn't make it in yet:
> https://lkml.org/lkml/2021/12/21/384
> This should be new version:
> https://lore.kernel.org/linux-ext4/YcSYvk5DdGjjB9q%2F@mit.edu/T/#t

I gave this patch a try and verified it fixes the test case regression.
Let me ask Ted to make sure it is merged for the 5.17 release.

Thanks,
Petr Vorel Feb. 2, 2022, 4:57 p.m. UTC | #16
> Hi!
> > > In any case, Petr, I suggest adding a short timeout to the test
> > > instead of the default 5min.
> > > Test takes less than 1 second on my VM on v5.16, so...
> > We usually don't lower the default timeout, but here it's a good idea since here
> > it blocks. It's similar speed on my machine, but I'd be conservative and put
> > timeout 10 sec. Sending patch.

> Actually I hope to change the default timeout once I finish my runtime
> patchset to something more reasonable as majority of LTP tests finish
> under one second.

+1. We can then revert that temporary fix for fanotify22.

Kind regards,
Petr