Re: [PATCH v6 4/9] iomap: Generic inline data handling

[Christoph Hellwig <hch@lst.de>]

On Sat, Jun 02, 2018 at 11:57:12AM +0200, Andreas Gruenbacher wrote:
> Add generic inline data handling by adding a pointer to the inline data
> region to struct iomap.  When handling a buffered IOMAP_INLINE write,
> iomap_write_begin will copy the current inline data from the inline data
> region into the page cache, and iomap_write_end will copy the changes in
> the page cache back to the inline data region.

This approach looks good.  A few comments below:

> 
> This doesn't cover inline data reads and direct I/O yet because so far,
> we have no users.

I'm fine with that as a first step, but gfs2 should be able to do
proper direct I/O to inline data and use by new iomap_readpage(s)
easily at least for blocksize == PAGE_SIZE, so this should be added
soon.

> -int generic_write_end(struct file *file, struct address_space *mapping,
> +void __generic_write_end(struct file *file, struct address_space *mapping,
>  			loff_t pos, unsigned len, unsigned copied,
> -			struct page *page, void *fsdata)
> +			struct page *page, void *fsdata, bool dirty_inode)

This is going to clash with

http://git.infradead.org/users/hch/xfs.git/commitdiff/2733909d6b40046ce9c7302c2e742c5e993a0108

It should also be a separate prep patch with a good explanation

>  iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
> -		unsigned copied, struct page *page)
> +		unsigned copied, struct page *page, struct iomap *iomap)

Note that I have another patch adding this parameter.  I think we'll need
a common iomap base tree for the gfs2 and xfs changes for the next
merge window.  I'd be happy to one one up.


> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 918f14075702..c61113c71a60 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -47,6 +47,7 @@ struct iomap {
>  	u64			length;	/* length of mapping, bytes */
>  	u16			type;	/* type of mapping */
>  	u16			flags;	/* flags for mapping */
> +	void			*inline_data;  /* inline data buffer */
>  	struct block_device	*bdev;	/* block device for I/O */
>  	struct dax_device	*dax_dev; /* dax_dev for dax operations */
>  };

Eventually we need to find a way to union the inline_data, bdev and
dax_dev fields, but that can be left to later.