Re: [PATCH 2/6] fs: report filesystem and file I/O errors to fsnotify

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Dec 17, 2025 at 09:21:52PM -0800, Christoph Hellwig wrote:
> >  	long					s_min_writeback_pages;
> > +
> > +	/* number of fserrors that are being sent to fsnotify/filesystems */
> > +	refcount_t		s_pending_errors;
> 
> Use the same tab-alignment as the fields above?  Also is this really

Will fix.

> a refcount?  It's a counter, but not really a reference?  I guess
> that doesn't matter too much.

<shrug> It does count pending events, but you could also look upon it as
a count of weak "references" to the super_block since the fserror_event
object does point to the super_block.

(Not sure what happens if we get more than 2^31 errors...)

> > +static inline void fserror_unmount(struct super_block *sb)
> > +{
> > +	/*
> > +	 * If we don't drop the pending error count to zero, then wait for it
> > +	 * to drop below 1, which means that the pending errors cleared or
> > +	 * that we saturated the system with 1 billion+ concurrent events.
> > +	 */
> > +	if (!refcount_dec_and_test(&sb->s_pending_errors))
> > +		wait_var_event(&sb->s_pending_errors,
> > +			       refcount_read(&sb->s_pending_errors) < 1);
> > +}
> 
> Should this be out of line?

Yes, it's better to hide these details (particularly since there's a fat
comment about the refcount bias) in fserror.c.

> > +/**
> > + * fserror_report - report a filesystem error of some kind
> > + *
> > + * Report details of a filesystem error to the super_operations::report_error
> > + * callback if present; and to fsnotify for distribution to userspace.  @sb,
> > + * @gfp, @type, and @error must all be specified.  For file I/O errors, the
> > + * @inode, @pos, and @len fields must also be specified.  For file metadata
> > + * errors, @inode must be specified.  If @inode is not NULL, then @inode->i_sb
> > + * must point to @sb.
> > + *
> > + * Reporting work is deferred to a workqueue to ensure that ->report_error is
> > + * called from process context without any locks held.  An active reference to
> > + * the inode is maintained until event handling is complete, and unmount will
> > + * wait for queued events to drain.
> > + *
> > + * @sb:		superblock of the filesystem
> 
> The normal convention is to have the arguments documented above the
> long description.  Any reason to deviate from that here?

Nope.  Fixed.

--D