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.
Also, the runtime name of the workqueue is "fsverity/$s_id", which is more
consistent with s_verity_wq than s_verify_wq.
> +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.
- Eric
