[PATCH v2] btrfs: allow setting per extent compression

[stijn rutjens <rutjensstijn@gmail.com>]

""Hi,
I've added an ioctl flag to notify the interface we want to use the
upper 4 bits for the compression level (which has significantly
decreased the size of the patch). How do you want to split the patch
up?

> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index bb374042d..e1603e1cf 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -67,7 +67,7 @@ int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
> >       btrfs_set_file_extent_ram_bytes(leaf, item, ram_bytes);
> >       btrfs_set_file_extent_generation(leaf, item, trans->transid);
> >       btrfs_set_file_extent_type(leaf, item, BTRFS_FILE_EXTENT_REG);
> > -     btrfs_set_file_extent_compression(leaf, item, compression);
> > +     btrfs_set_file_extent_compression(leaf, item, compression & 0xF);
>
> As a general comment, open coding the level as "& 0xf" should be replaced
> with a helper.

I've now changed this to use the already written "btrfs_compress_type"
and "btrfs_compress_level" helpers

> But in all cases where btrfs_set_file_extent_compression is called,
> there shluld be already only the type. Ie. it's up to the calling code
> to pass the correct value. An assert would be good.
>
> The number of entry points where the type and level are available is
> limited to ioctls or properties and the level is not part of the on-disk
> format so all code that handles the format should assume the value is
> valid.
this is also fixed

> > +                     ret = btrfs_compress_pages(
> > +                             compress_type | (compress_level << 4),
> > +                             inode->i_mapping, start,
> > +                             pages,
> > +                             &nr_pages,
> > +                             &total_in,
> > +                             &total_compressed);
> > +             }
> >               if (!ret) {
> >                       unsigned long offset = offset_in_page(total_compressed);
> >                       struct page *page = pages[nr_pages - 1];
> > @@ -2362,7 +2379,7 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
> >       btrfs_set_file_extent_offset(leaf, fi, 0);
> >       btrfs_set_file_extent_num_bytes(leaf, fi, num_bytes);
> >       btrfs_set_file_extent_ram_bytes(leaf, fi, ram_bytes);
> > -     btrfs_set_file_extent_compression(leaf, fi, compression);
> > +     btrfs_set_file_extent_compression(leaf, fi, compression & 0xF);
> >       btrfs_set_file_extent_encryption(leaf, fi, encryption);
> >       btrfs_set_file_extent_other_encoding(leaf, fi, other_encoding);
> >
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index 0fa1c386d..2a9c1f312 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -1414,7 +1414,11 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
> >               return -EINVAL;
> >
> >       if (do_compress) {
> > -             if (range->compress_type >= BTRFS_NR_COMPRESS_TYPES)
> > +             /*
> > +             * The bottom 4 bits of compress_type are for used for the
> > +             * compression type, the other bits for the compression level
> > +             */
> > +             if ((range->compress_type & 0xF) >= BTRFS_NR_COMPRESS_TYPES)
> >                       return -EINVAL;
>
> For the backward compatibility, the check without "& 0xf" is sufficient,
> but a new flag is needed to actually tell the ioctl code to read the
> level. I'm not sure that relying on automatic parsing of the high 4 bits
> is a good idea, usually all new data are accompanied by new flags.

fixed

> >               if (range->compress_type)
> >                       compress_type = range->compress_type;
> > @@ -1572,9 +1576,9 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
> >                       filemap_flush(inode->i_mapping);
> >       }
> >
> > -     if (range->compress_type == BTRFS_COMPRESS_LZO) {
> > +     if ((range->compress_type & 0xF) == BTRFS_COMPRESS_LZO) {
> >               btrfs_set_fs_incompat(fs_info, COMPRESS_LZO);
> > -     } else if (range->compress_type == BTRFS_COMPRESS_ZSTD) {
> > +     } else if ((range->compress_type & 0xF) == BTRFS_COMPRESS_ZSTD) {
> >               btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD);
> >       }
>
> For further iterations of the patch, I suggest to split it into more
> patches by logical change, like adding helpers, extending the ioctl,
> adding asserts, etc.

The ioctl interface is now extended (see patch-extend-ioctl.patch)
for the other patch, see patch-extend-inode.patch
Where should asserts be added?