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.
