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

Kyoji Ogasawara
2025-05-25T17:45:16Z

Sorry for sending piecemeal updates.

I've realized that if the log goes into btrfs_emit_options(), it'll
only show up during remount operations.

So, I was thinking of adding the following code to open_ctree(), right
after the btrfs_check_options() call.

if (btrfs_test_opt(fs_info, NOBARRIER))
btrfs_info(fs_info, "turning off barriers, use with care");

What do you think of this approach?

We could also set the log level to warning if that's preferred.

2025年5月25日(日) 11:32 Kyoji Ogasawara <sawara04.o@gmail.com>:
>
> Got it, thanks.
>
> I'll add the "turning off barriers, use with care" log at the info
> level in btrfs_emit_options().
>
> Since this is part of a patch set and "[2/2] btrfs: Fix incorrect log
> message related barrier"
> doesn't need a Fixes tag, should I just resubmit the corrected version
> of this patch?
>
> Thanks,
> Kyoji
>
> 2025年5月24日(土) 12:49 Qu Wenruo <wqu@suse.com>:
> >
> >
> >
> > 在 2025/5/24 11:51, Kyoji Ogasawara 写道:
> > > 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?
> >
> > I think info level is good enough.
> >
> > As the main purpose of that message line is still just to show we're
> > using barrier or not, the extra "use with care" is just something good
> > to have.
> >
> > Thus no need to go warning IMHO.
> >
> > Thanks,
> > Qu
> >
> >
> > >
> > > 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
> > >
> >