Re: [PATCH 1/2] btrfs: Raise nobarrier log level to warn

Kyoji Ogasawara
2025-05-24T02:21:47Z

Thanks for the explanation. I understand the issue with btrfs_parse_param()
being triggered multiple times.

If I move the log into btrfs_parse_param(), it would currently use
btrfs_info_if_set(),
resulting in an info level log.

Is an info level acceptable for this warning, or would you prefer a
warn level log?

If so, should I create a new helper function like btrfs_warn_if_set()?

2025年5月21日(水) 13:13 Qu Wenruo <quwenruo.btrfs@gmx.com>:
>
>
>
> 在 2025/5/21 12:57, sawara04.o@gmail.com 写道:
> > From: Kyoji Ogasawara <sawara04.o@gmail.com>
> >
> > The nobarrier option disables barriers, improving performance at the cost
> > of potential data loss during crashes or power failures especially on devices
> > without reliable write-back caching.
> > To better reflect this risk, the log level has been raised to warn.
> >
> > Signed-off-by: Kyoji Ogasawara <sawara04.o@gmail.com>
> > ---
> > fs/btrfs/super.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index 7310e2fa8526..012b63a07ab1 100644
> > --- a/fs/btrfs/super.c
> > +++ b/fs/btrfs/super.c
> > @@ -407,10 +407,12 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
> > }
> > break;
> > case Opt_barrier:
> > - if (result.negated)
> > + if (result.negated) {
> > btrfs_set_opt(ctx->mount_opt, NOBARRIER);
> > - else
> > + btrfs_warn(NULL, "turning off barriers, use with care");
> > + } else {
> > btrfs_clear_opt(ctx->mount_opt, NOBARRIER);
> > + }
> > break;
> > case Opt_thread_pool:
> > if (result.uint_32 == 0) {
> > @@ -1439,7 +1441,6 @@ static void btrfs_emit_options(struct btrfs_fs_info *info,
> > btrfs_info_if_set(info, old, NODATASUM, "setting nodatasum");
> > btrfs_info_if_set(info, old, SSD, "enabling ssd optimizations");
> > btrfs_info_if_set(info, old, SSD_SPREAD, "using spread ssd allocation scheme");
> > - btrfs_info_if_set(info, old, NOBARRIER, "turning off barriers");
>
> I know you want to avoid duplicated messages about the nobarrier.
>
> But it is better to use btrfs_emit_options() to add the warning.
>
> The problem of showing the error in btrfs_parse_param() is, it can be
> triggered multiple times.
>
> E.g. mount -o nobarrier,nobarrier,nobarrier $dev $mnt
>
> Then there will be 3 lines of "turning of barrier, use with care".
> But inside btrfs_emit_options() it will be only once.
>
> In fact this also affect the warning for excessive commit mount option,
> but there is no better solution for that option, but we can do better here.
>
> Thanks,
> Qu