Re: [PATCH] btrfs/defrag: implement compression levels

[Daniel Vacek]

On Wed, 5 Mar 2025 at 08:05, David Sterba <dsterba@xxxxxxx> 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.