Re: [PATCH v11 07/14] btrfs: add definitions + documentation for encoded I/O ioctls

Omar Sandoval
2021-10-18T18:16:48Z

On Fri, Oct 15, 2021 at 12:42:37PM +0300, Nikolay Borisov wrote:
>
>
> On 1.09.21 г. 20:01, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > In order to allow sending and receiving compressed data without
> > decompressing it, we need an interface to write pre-compressed data
> > directly to the filesystem and the matching interface to read compressed
> > data without decompressing it. This adds the definitions for ioctls to
> > do that and detailed explanations of how to use them.
> >
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
>
> One minor nit below but otherwise LGTM:
>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>
> <snip>
>
> > +struct btrfs_ioctl_encoded_io_args {
> > + /* Input parameters for both reads and writes. */
> > +
> > + /*
> > + * iovecs containing encoded data.
> > + *
> > + * For reads, if the size of the encoded data is larger than the sum of
> > + * iov[n].iov_len for 0 <= n < iovcnt, then the ioctl fails with
> > + * ENOBUFS.
> > + *
> > + * For writes, the size of the encoded data is the sum of iov[n].iov_len
> > + * for 0 <= n < iovcnt. This must be less than 128 KiB (this limit may
> > + * increase in the future). This must also be less than or equal to
> > + * unencoded_len.
> > + */
> > + const struct iovec __user *iov;
> > + /* Number of iovecs. */
> > + unsigned long iovcnt;
> > + /*
> > + * Offset in file.
> > + *
> > + * For writes, must be aligned to the sector size of the filesystem.
> > + */
> > + __s64 offset;
> > + /* Currently must be zero. */
> > + __u64 flags;
> > +
>
> nit: A comment stating that the output params begin here could be added.

There is a comment in this spot:

/*
* For reads, the following members are filled in with the metadata for
* the encoded data.
* For writes, the following members must be set to the metadata for the
* encoded data.
*/

I'll clarify it to:

/*
* For reads, the following members are output parameters that will
* contain the returned metadata for the encoded data.
* For writes, the following members must be set to the metadata for the
* encoded data.
*/