[LTP] [PATCH v1] fanotify22.c: handle multiple asynchronous error events

[Wei Gao]

On Thu, Mar 05, 2026 at 10:36:04AM +0100, Jan Kara wrote:
> On Wed 04-03-26 13:38:07, Wei Gao wrote:
> > Since the introduction of the asynchronous fserror reporting framework
> > (kernel commit 81d2e13a57c9), fanotify22 has encountered sporadic failures
> > due to the non-deterministic nature of event delivery and merging:
> > 
> > 1) tcase3 failure: A race condition occurs when the test reads the
> >    notification fd between two events. Adding a short delay
> >    (usleep) ensures all events are dispatched and ready before the
> >    read() call.
> 
> OK, but please add a comment in the code why this is needed.
> 

Thanks for your quick feedback, i will add comments in next version.

> > 2) tcase4 failure: The kernel may deliver errors as independent events
> >    instead of a single merged event, The test logic is updated to
> >    validate the expected error_count by either a single merged event
> >    or the accumulation of multiple independent events in the buffer.
> 
> Did you investigate why the events didn't get merged in the kernel? If they
> are against the same filesystem they should get merged AFAICS.
> 
> 								Honza
Sorry i have no idea why this happen, I just add debug code into LTP case and 
found the event not lost but deliver independent, this leads me to believe that
the LTP should handle both scenarios—merged and independent events.
I also not sure my patch is correct or not, that's also the reason i
CC the patch to you :)

> 
> > 
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Closes: https://lore.kernel.org/oe-lkp/202602042124.87bd00e3-lkp@intel.com
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> >  .../kernel/syscalls/fanotify/fanotify22.c     | 32 ++++++++++++++++---
> >  1 file changed, 28 insertions(+), 4 deletions(-)
> > 
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify22.c b/testcases/kernel/syscalls/fanotify/fanotify22.c
> > index 6578474a7..82eed7ba9 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify22.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify22.c
> > @@ -53,6 +53,8 @@ static struct fanotify_fid_t null_fid;
> >  static struct fanotify_fid_t bad_file_fid;
> >  static struct fanotify_fid_t bad_link_fid;
> >  
> > +static int event_count;
> > +
> >  static void trigger_fs_abort(void)
> >  {
> >  	SAFE_MOUNT(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
> > @@ -88,7 +90,6 @@ static void trigger_bad_link_lookup(void)
> >  			ret, BAD_LINK, errno, EUCLEAN);
> >  }
> >  
> > -
> >  static void tcase3_trigger(void)
> >  {
> >  	trigger_bad_link_lookup();
> > @@ -176,9 +177,10 @@ static int check_error_event_info_error(struct fanotify_event_info_error *info_e
> >  {
> >  	int fail = 0;
> >  
> > -	if (info_error->error_count != ex->error_count) {
> > -		tst_res(TFAIL, "%s: Unexpected error_count (%d!=%d)",
> > -			ex->name, info_error->error_count, ex->error_count);
> > +	if (info_error->error_count != ex->error_count && event_count != ex->error_count) {
> > +		tst_res(TFAIL, "%s: Unexpected error_count (%d!=%d && %d!=%d)",
> > +			ex->name, info_error->error_count, ex->error_count,
> > +			event_count, ex->error_count);
> >  		fail++;
> >  	}
> >  
> > @@ -255,8 +257,30 @@ static void do_test(unsigned int i)
> >  
> >  	tcase->trigger_error();
> >  
> > +	usleep(100000);
> > +
> 
> OK, but can you please add a comment why the sleep is here.
> 
Sure!
> >  	read_len = SAFE_READ(0, fd_notify, event_buf, BUF_SIZE);
> >  
> > +	struct fanotify_event_metadata *metadata;
> > +	size_t len = read_len;
> > +
> > +	event_count = 0;
> > +
> > +	for (metadata = (struct fanotify_event_metadata *)event_buf;
> > +			FAN_EVENT_OK(metadata, len);
> > +			metadata = FAN_EVENT_NEXT(metadata, len)) {
> > +		event_count++;
> > +		struct fanotify_event_info_error *info_error = get_event_info_error(metadata);
> > +
> > +		if (info_error) {
> > +			tst_res(TINFO, "Event [%d]: errno=%d (expected %d), error_count=%d (expected total %d)",
> > +					event_count, info_error->error, tcase->error,
> > +					info_error->error_count, tcase->error_count);
> > +		} else {
> > +			tst_res(TINFO, "Event [%d]: No error info record found", event_count);
> > +		}
> > +	}
> > +
> 
> This looks too lax to me. I think
> 
I guess your mean this is workaround for the issue? Are you suggesting that I should reconstruct the test case 
to properly handle independent events rather than relying on the existing logic? 
If so, I’d be happy to explore that and implement a more robust solution.

> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR