Re: [PATCH 04/11] xfs: convey filesystem unmount events to the health monitor

[Christoph Hellwig <hch@lst.de>]

> +	event = kzalloc(sizeof(struct xfs_healthmon_event), GFP_NOFS);
> +	mutex_lock(&hm->lock);
> +
> +	trace_xfs_healthmon_report_unmount(hm);
> +
> +	if (event) {
> +		/*
> +		 * Insert the unmount notification at the start of the event
> +		 * queue so that userspace knows the filesystem went away as
> +		 * soon as possible.  There's nothing actionable for userspace
> +		 * after an unmount.
> +		 */
> +		event->type = XFS_HEALTHMON_UNMOUNT;
> +		event->domain = XFS_HEALTHMON_MOUNT;
> +
> +		__xfs_healthmon_insert(hm, event);

The allocation (and locking) could move into __xfs_healthmon_insert
sharing this code with xfs_ioc_health_monitor().  That means the wake_up
would not be covered by the lock, but I can't see how that would need
it.

> +	} else {
> +		/*
> +		 * Wake up the reader directly in case we didn't have enough
> +		 * memory to queue the unmount event.  The filesystem is about
> +		 * to go away so we don't care about reporting previously lost
> +		 * events.
> +		 */
> +		wake_up(&hm->wait);
> +	}

And then even after reading this many times I'm still not understanding
it.  Yes, this wakes up the reader direct, but why is waking it up
without an even ok?  And if it is ok, why do we even bother with the
unmount even?  If OTOH the unmount event is actually important, should
we preallocate the event?  (and render my above suggestion impractical).