Re: [PATCH 04/13] fsverity: add per-sb workqueue for post read processing

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

On Thu, Apr 04, 2024 at 10:39:53PM -0400, Eric Biggers wrote:
> On Fri, Mar 29, 2024 at 05:33:43PM -0700, Darrick J. Wong wrote:
> > diff --git a/fs/super.c b/fs/super.c
> > index 71d9779c42b10..aaa75131f6795 100644
> > --- a/fs/super.c
> > +++ b/fs/super.c
> > @@ -637,6 +637,13 @@ void generic_shutdown_super(struct super_block *sb)
> >  			sb->s_dio_done_wq = NULL;
> >  		}
> >  
> > +#ifdef CONFIG_FS_VERITY
> > +		if (sb->s_verify_wq) {
> > +			destroy_workqueue(sb->s_verify_wq);
> > +			sb->s_verify_wq = NULL;
> > +		}
> > +#endif
> 
> Should it maybe be s_verity_wq with a t?
> 
> I guess it could be argued that the actual data verification is just part of
> fsverity, and there could be other workqueues related to fsverity, e.g. one for
> enabling fsverity.  But in practice this is the only one.

Yeah.  If someone wants to add another wq for enabling verity then we
can deal with that

> Also, the runtime name of the workqueue is "fsverity/$s_id", which is more
> consistent with s_verity_wq than s_verify_wq.

Ok, s_verity_wq (with a t) it is.

I'll change __fsverity_init_verify_wq to __fsverity_init_wq since we're
getting rid of the "verify" wording.

> > +int __fsverity_init_verify_wq(struct super_block *sb)
> > +{
> > +	struct workqueue_struct *wq, *old;
> > +
> > +	wq = alloc_workqueue("fsverity/%s", WQ_MEM_RECLAIM | WQ_FREEZABLE, 0,
> > +			sb->s_id);
> > +	if (!wq)
> > +		return -ENOMEM;
> > +
> > +	/*
> > +	 * This has to be atomic as readaheads can race to create the
> > +	 * workqueue.  If someone created workqueue before us, we drop ours.
> > +	 */
> > +	old = cmpxchg(&sb->s_verify_wq, NULL, wq);
> > +	if (old)
> > +		destroy_workqueue(wq);
> > +	return 0;
> > +}
> 
> The cmpxchg thing makes no sense because this is only called at mount time, when
> there is no chance of a race condition.

Aha, right.  That was a dumb leftover from an earlier version that could
turn it on at runtime.  Removed.

--D

> - Eric
>