Re: [PATCH 5/6] bcachefs: BCH_IOCTL_FSCK_OFFLINE

[Brian Foster <bfoster@redhat.com>]

On Wed, Dec 06, 2023 at 03:33:09PM -0500, Kent Overstreet wrote:
> This adds a new ioctl for running fsck on a list of devices.
> 
> Normally, if we wish to use the kernel's implementation of fsck we'd run
> it at mount time with -o fsck. This ioctl lets us run fsck without
> mounting, so that userspace bcachefs-tools can transparently switch to
> the kernel's implementation of fsck when appropriate - primarily if the
> kernel version of bcachefs better matches the filesystem on disk.
> 
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> ---
>  fs/bcachefs/bcachefs_ioctl.h |  13 +++
>  fs/bcachefs/chardev.c        | 206 ++++++++++++++++++++++++++++++++++-
>  fs/bcachefs/recovery.c       |   6 +-
>  3 files changed, 219 insertions(+), 6 deletions(-)
> 
...
> diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
> index 8e3ac2d32298..03082a001036 100644
> --- a/fs/bcachefs/chardev.c
> +++ b/fs/bcachefs/chardev.c
...
> @@ -193,8 +196,196 @@ static long bch2_ioctl_incremental(struct bch_ioctl_incremental __user *user_arg
>  }
>  #endif
>  
...
> +
> +static const struct file_operations fsck_thread_ops = {
> +	.release	= bch2_fsck_thread_release,
> +	.read		= bch2_fsck_thread_read,
> +	.llseek		= no_llseek,
> +};
> +
> +static int bch2_fsck_offline_thread_fn(void *arg)
> +{
> +	struct fsck_thread *thr = container_of(arg, struct fsck_thread, thr);
> +	struct bch_fs *c = bch2_fs_open(thr->devs, thr->nr_devs, thr->opts);
> +
> +	thr->thr.ret = PTR_ERR_OR_ZERO(c);
> +	if (!thr->thr.ret)
> +		bch2_fs_stop(c);
> +

Looks pretty neat. Mainly just curious if I'm following this right...
Basically it looks like we wire up the log output redirect to c->output
via options, then start/stop the fs which runs "mount time" fsck (?),
but feeding the message output to the redirect (and then fed to
userspace via the fd/read handler just above). Hm?

Brian

> +	thr->thr.done = true;
> +	wake_up(&thr->output.wait);
> +	return 0;
> +}
> +
> +static long bch2_ioctl_fsck_offline(struct bch_ioctl_fsck_offline __user *user_arg)
> +{
> +	struct bch_ioctl_fsck_offline arg;
> +	struct fsck_thread *thr = NULL;
> +	u64 *devs = NULL;
> +	long ret = 0;
> +
> +	if (copy_from_user(&arg, user_arg, sizeof(arg)))
> +		return -EFAULT;
> +
> +	if (arg.flags)
> +		return -EINVAL;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (!(devs = kcalloc(arg.nr_devs, sizeof(*devs), GFP_KERNEL)) ||
> +	    !(thr = kzalloc(sizeof(*thr), GFP_KERNEL)) ||
> +	    !(thr->devs = kcalloc(arg.nr_devs, sizeof(*thr->devs), GFP_KERNEL))) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	thr->nr_devs = arg.nr_devs;
> +	thr->output.buf	= PRINTBUF;
> +	thr->output.buf.atomic++;
> +	spin_lock_init(&thr->output.lock);
> +	init_waitqueue_head(&thr->output.wait);
> +	darray_init(&thr->output2);
> +
> +	if (copy_from_user(devs, &user_arg->devs[0], sizeof(user_arg->devs[0]) * arg.nr_devs)) {
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	for (size_t i = 0; i < arg.nr_devs; i++) {
> +		thr->devs[i] = strndup_user((char __user *)(unsigned long) devs[i], PATH_MAX);
> +		ret = PTR_ERR_OR_ZERO(thr->devs[i]);
> +		if (ret)
> +			goto err;
> +	}
> +
> +	if (arg.opts) {
> +		char *optstr = strndup_user((char __user *)(unsigned long) arg.opts, 1 << 16);
> +
> +		ret =   PTR_ERR_OR_ZERO(optstr) ?:
> +			bch2_parse_mount_opts(NULL, &thr->opts, optstr);
> +		kfree(optstr);
> +
> +		if (ret)
> +			goto err;
> +	}
> +
> +	opt_set(thr->opts, log_output, (u64)(unsigned long)&thr->output);
> +
> +	ret = run_thread_with_file(&thr->thr,
> +				   &fsck_thread_ops,
> +				   bch2_fsck_offline_thread_fn,
> +				   "bch-fsck");
> +err:
> +	if (ret < 0) {
> +		if (thr)
> +			bch2_fsck_thread_free(thr);
> +		pr_err("ret %s", bch2_err_str(ret));
> +	}
> +	kfree(devs);
> +	return ret;
> +}
> +
>  static long bch2_global_ioctl(unsigned cmd, void __user *arg)
>  {
> +	long ret;
> +
>  	switch (cmd) {
>  #if 0
>  	case BCH_IOCTL_ASSEMBLE:
> @@ -202,9 +393,18 @@ static long bch2_global_ioctl(unsigned cmd, void __user *arg)
>  	case BCH_IOCTL_INCREMENTAL:
>  		return bch2_ioctl_incremental(arg);
>  #endif
> +	case BCH_IOCTL_FSCK_OFFLINE: {
> +		ret = bch2_ioctl_fsck_offline(arg);
> +		break;
> +	}
>  	default:
> -		return -ENOTTY;
> +		ret = -ENOTTY;
> +		break;
>  	}
> +
> +	if (ret < 0)
> +		ret = bch2_err_class(ret);
> +	return ret;
>  }
>  
>  static long bch2_ioctl_query_uuid(struct bch_fs *c,
> diff --git a/fs/bcachefs/recovery.c b/fs/bcachefs/recovery.c
> index 262c923b2f1a..2f5daecfbcf7 100644
> --- a/fs/bcachefs/recovery.c
> +++ b/fs/bcachefs/recovery.c
> @@ -655,13 +655,13 @@ static int bch2_run_recovery_pass(struct bch_fs *c, enum bch_recovery_pass pass)
>  	int ret;
>  
>  	if (!(p->when & PASS_SILENT))
> -		printk(KERN_INFO bch2_log_msg(c, "%s..."),
> -		       bch2_recovery_passes[pass]);
> +		bch2_print(c, KERN_INFO bch2_log_msg(c, "%s..."),
> +			   bch2_recovery_passes[pass]);
>  	ret = p->fn(c);
>  	if (ret)
>  		return ret;
>  	if (!(p->when & PASS_SILENT))
> -		printk(KERN_CONT " done\n");
> +		bch2_print(c, KERN_CONT " done\n");
>  
>  	return 0;
>  }
> -- 
> 2.42.0
> 
>