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
>
