[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPjX3Fetm8jmPoaP84PC9ck_XtRch9TPny6tKcn2EJL6ZPaXug@mail.gmail.com>
Date: Wed, 2 Apr 2025 12:29:58 +0200
From: Daniel Vacek <neelx@...e.com>
To: Qu Wenruo <quwenruo.btrfs@....com>
Cc: dsterba@...e.cz, Qu Wenruo <wqu@...e.com>, Chris Mason <clm@...com>,
Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>, Nick Terrell <terrelln@...com>,
linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: zstd: add `zstd-fast` alias mount option for fast modes
On Wed, 2 Apr 2025 at 11:08, Qu Wenruo <quwenruo.btrfs@....com> wrote:
> 在 2025/4/2 19:07, Daniel Vacek 写道:
> > On Tue, 1 Apr 2025 at 00:57, David Sterba <dsterba@...e.cz> wrote:
> [...]
> >>>> I thought this was solid. Or would you rather bail out with -EINVAL?
> >>>
> >>> I prefer to bail out with -EINVAL, but it's only my personal choice.
> >>
> >> I tend to agree with you, the idea for the alias was based on feedback
> >> that upstream zstd calls the levels fast, not by the negative numbers.
> >> So I think we stick to the zstd: and zstd-fast: prefixes followed only
> >> by the positive numbers.
> >
> > Hmm, so for zlib and zstd if the level is out of range, it's just
> > clipped and not failed as invalid. I guess zstd-fast should also do
> > the same to be consistent.
>
> Or we can change the zlib/zstd level checks so that it can return
> -EINVAL when invalid levels are provided.
>
> But to avoid huge surprise, I'd recommend to add warning/error messages
> first.
>
> I'm not a huge fan when invalid values are silently clamped, even it's
> just an optional level parameter.
I agree. Well, one by one. Let's nail the `zstd-fast` first and clean
up in subsequent patches.
I already plan to fix another issue I noticed. For example
`compress=zlibfoo` is still accepted as zlib, etc.
> Thanks,
> Qu
>
> >
> >> We can make this change before 6.15 final so it's not in any released
> >> kernel and we don't have to deal with compatibility.
> >
>
Powered by blists - more mailing lists