linux-kernel - Re: [PATCH] btrfs/defrag: implement compression levels

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPjX3Fe-_vcVvkMvmgaVCoVU3kAXVH84rN=_77NpMUMSYw3G7A@mail.gmail.com>
Date: Wed, 5 Mar 2025 08:21:18 +0100
From: Daniel Vacek <neelx@...e.com>
To: dsterba@...e.cz
Cc: Qu Wenruo <quwenruo.btrfs@....com>, Chris Mason <clm@...com>, 
	Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs/defrag: implement compression levels

On Wed, 5 Mar 2025 at 08:05, David Sterba <dsterba@...e.cz> wrote:
>
> On Wed, Mar 05, 2025 at 08:02:28AM +0100, Daniel Vacek wrote:
> > > > @@ -1376,10 +1377,19 @@ int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
> > > >               return -EINVAL;
> > > >
> > > >       if (do_compress) {
> > > > -             if (range->compress_type >= BTRFS_NR_COMPRESS_TYPES)
> > > > -                     return -EINVAL;
> > > > -             if (range->compress_type)
> > > > -                     compress_type = range->compress_type;
> > > > +             if (range->flags & BTRFS_DEFRAG_RANGE_COMPRESS_LEVEL) {
> > > > +                     if (range->compress.type >= BTRFS_NR_COMPRESS_TYPES)
> > > > +                             return -EINVAL;
> > > > +                     if (range->compress.type) {
> > > > +                             compress_type = range->compress.type;
> > > > +                             compress_level= range->compress.level;
> > > > +                     }
> > >
> > > I am not familiar with the compress level, but
> > > btrfs_compress_set_level() does extra clamping, maybe we also want to do
> > > that too?
> >
> > This is intentionally left to be limited later. There's no need to do
> > it at this point and the code is simpler. It's also compression
> > type/method agnostic.
>
> This is input parameter validation so we should not postpone it until
> the whole process starts. The complexity can be wrapped in helpers, we
> already have that for various purposes like
> compression_decompress_bio().

OK, that makes sense. I'll add the check in v2.

Thaks guys.