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

[Omar Sandoval <osandov@osandov.com>]

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.
         */