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

[Jan Kara]

On Thu 05-03-26 14:36:31, Wei Gao wrote:
> On Thu, Mar 05, 2026 at 10:36:04AM +0100, Jan Kara wrote:
> > > 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.
> > 
> 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 :)

OK :). How easily is this reproducible? Because in principle event merging
is not *guaranteed* so it isn't wrong for the LTP test to handle split
events but before we complicate the test too much it would be good to
figure out why the kernel behaves in unexpected way and doesn't merge the
events...

								Honza

> > > 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
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR