Re: [PATCH 1/5] xfs: always set xfs_healthmon::first_event when inserting at front of list

[Christoph Hellwig <hch@lst.de>]

On Tue, Aug 25, 2026 at 11:33:08AM -0700, Darrick J. Wong wrote:
> Let me try again:
> 
> "LOLLM complains that while __xfs_healthmon_insert is supposed to insert
> an event at the head of the list, it doesn't do that correctly if the
> list isn't empty.  In that case it *should* make our new event point to
> the current head, and then make the head point to the new event, but
> it doesn't actually update the head."

> > event is the newly inserted event.  We want to queue it at the
> > head of the list (why, btw?).
> 
> The only event that gets inserted at the front is the unmount event.
> This signals that the filesystem is being completely unmounted (not just
> detached from a mount ns) and there's nothing further that xfs_healer
> can do to fix the filesystem.  Therefore we put the unmount item first
> in the list so that xfs_healer won't waste time trying to find the
> mount to do repairs that it won't be able to make.

Ok, me forgetting about two different ways to insert events was the
reason I had a really hard time with the commit log.

> 
> > The next point in event points to
> > first_event (which can be NULL).  And first should always point
> > to event, otherwise we potentially never queue anything up?  I.e.
> > we never ever actually set first?  Not sure how that is related
> > to umount.
> 
> xfs_healthmon_push will set first_event if the list is empty.
> 
> (for context, all other events are _pushed on to the end of the list)
> 
> > Maybe this should just use standard list_head-based lists even
> > if they waste an extra pointer in the event structure?
> 
> Yes, that at least wouldn't increase the size of xfs_healthmon_event.

I think we should do this.  And merge __xfs_healthmon_push and
__xfs_healthmon_insert that has a at_head argument to switch between
list_add vs list_add_tail.  Thart would make this code much easier
to understand and maintain.