* [PATCH 0/2] iomap: make inline data support more flexible @ 2021-07-24 3:44 Matthew Wilcox (Oracle) 2021-07-24 3:44 ` [PATCH 1/2] iomap: Support file tail packing Matthew Wilcox (Oracle) 2021-07-24 3:44 ` [PATCH 2/2] iomap: Support inline data with block size < page size Matthew Wilcox (Oracle) 0 siblings, 2 replies; 8+ messages in thread From: Matthew Wilcox (Oracle) @ 2021-07-24 3:44 UTC (permalink / raw) To: linux-fsdevel, linux-erofs, linux-xfs; +Cc: Matthew Wilcox (Oracle) The first patch seems to be where Gao Xiang, Christoph and I have settled for immediate support of EROFS tails. The second is a prequel to the folio work. Because folios are variable size, if we have, eg, an 8.1KiB file, we must support reading the first two pages of the folio from blocks and then the last 100 bytes from the tailpack. That means effectively supporting block size < page size. I thought it cleaner to separate it out, and maybe that can go in this round. The folio patches are rebased on top of all this; if you're hungry to see them, they can be found at https://git.infradead.org/users/willy/pagecache.git/shortlog/refs/heads/folio as usual. I haven't applied all the R-b yet (and I should probably figure out which ones still apply since I did some substantial changes to a couple of the patches). Gao Xiang (1): iomap: Support file tail packing Matthew Wilcox (Oracle) (1): iomap: Support inline data with block size < page size fs/iomap/buffered-io.c | 46 +++++++++++++++++++++++++----------------- fs/iomap/direct-io.c | 10 +++++---- include/linux/iomap.h | 14 +++++++++++++ 3 files changed, 47 insertions(+), 23 deletions(-) -- 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] iomap: Support file tail packing 2021-07-24 3:44 [PATCH 0/2] iomap: make inline data support more flexible Matthew Wilcox (Oracle) @ 2021-07-24 3:44 ` Matthew Wilcox (Oracle) 2021-07-24 3:44 ` [PATCH 2/2] iomap: Support inline data with block size < page size Matthew Wilcox (Oracle) 1 sibling, 0 replies; 8+ messages in thread From: Matthew Wilcox (Oracle) @ 2021-07-24 3:44 UTC (permalink / raw) To: linux-fsdevel, linux-erofs, linux-xfs; +Cc: Matthew Wilcox From: Gao Xiang <hsiangkao@linux.alibaba.com> Add support for reading inline data content into the page cache from nonzero page-aligned file offsets. This enables the EROFS tailpacking mode where the last few bytes of the file are stored right after the inode. The buffered write path remains untouched since EROFS cannot be used for testing. It'd be better to be implemented if upcoming real users care and provide a real pattern rather than leave untested dead code around. Tested-by: Huang Jianan <huangjianan@oppo.com> # erofs Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/iomap/buffered-io.c | 42 ++++++++++++++++++++++++++---------------- fs/iomap/direct-io.c | 10 ++++++---- include/linux/iomap.h | 14 ++++++++++++++ 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index a463b41c0a16..7bd8e5de996d 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -205,25 +205,29 @@ struct iomap_readpage_ctx { struct readahead_control *rac; }; -static void -iomap_read_inline_data(struct inode *inode, struct page *page, - struct iomap *iomap) +static int iomap_read_inline_data(struct inode *inode, struct page *page, + struct iomap *iomap, loff_t pos) { - size_t size = i_size_read(inode); + size_t size = iomap->length + iomap->offset - pos; void *addr; if (PageUptodate(page)) - return; + return PAGE_SIZE; - BUG_ON(page_has_private(page)); - BUG_ON(page->index); - BUG_ON(size > PAGE_SIZE - offset_in_page(iomap->inline_data)); + /* inline data must start page aligned in the file */ + if (WARN_ON_ONCE(offset_in_page(pos))) + return -EIO; + if (WARN_ON_ONCE(!iomap_inline_data_size_valid(iomap))) + return -EIO; + if (WARN_ON_ONCE(page_has_private(page))) + return -EIO; addr = kmap_atomic(page); - memcpy(addr, iomap->inline_data, size); + memcpy(addr, iomap_inline_buf(iomap, pos), size); memset(addr + size, 0, PAGE_SIZE - size); kunmap_atomic(addr); SetPageUptodate(page); + return PAGE_SIZE; } static inline bool iomap_block_needs_zeroing(struct inode *inode, @@ -245,11 +249,8 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, unsigned poff, plen; sector_t sector; - if (iomap->type == IOMAP_INLINE) { - WARN_ON_ONCE(pos); - iomap_read_inline_data(inode, page, iomap); - return PAGE_SIZE; - } + if (iomap->type == IOMAP_INLINE) + return iomap_read_inline_data(inode, page, iomap, pos); /* zero post-eof blocks as the page may be mapped */ iop = iomap_page_create(inode, page); @@ -581,6 +582,15 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, return 0; } +static int iomap_write_begin_inline(struct inode *inode, + struct page *page, struct iomap *srcmap) +{ + /* needs more work for the tailpacking case, disable for now */ + if (WARN_ON_ONCE(srcmap->offset != 0)) + return -EIO; + return iomap_read_inline_data(inode, page, srcmap, 0); +} + static int iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags, struct page **pagep, struct iomap *iomap, struct iomap *srcmap) @@ -610,14 +620,14 @@ iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags, } if (srcmap->type == IOMAP_INLINE) - iomap_read_inline_data(inode, page, srcmap); + status = iomap_write_begin_inline(inode, page, srcmap); else if (iomap->flags & IOMAP_F_BUFFER_HEAD) status = __block_write_begin_int(page, pos, len, NULL, srcmap); else status = __iomap_write_begin(inode, pos, len, flags, page, srcmap); - if (unlikely(status)) + if (unlikely(status < 0)) goto out_unlock; *pagep = page; diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 9398b8c31323..a6aaea2764a5 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -378,23 +378,25 @@ iomap_dio_inline_actor(struct inode *inode, loff_t pos, loff_t length, struct iomap_dio *dio, struct iomap *iomap) { struct iov_iter *iter = dio->submit.iter; + void *dst = iomap_inline_buf(iomap, pos); size_t copied; - BUG_ON(pos + length > PAGE_SIZE - offset_in_page(iomap->inline_data)); + if (WARN_ON_ONCE(!iomap_inline_data_size_valid(iomap))) + return -EIO; if (dio->flags & IOMAP_DIO_WRITE) { loff_t size = inode->i_size; if (pos > size) - memset(iomap->inline_data + size, 0, pos - size); - copied = copy_from_iter(iomap->inline_data + pos, length, iter); + memset(iomap_inline_buf(iomap, size), 0, pos - size); + copied = copy_from_iter(dst, length, iter); if (copied) { if (pos + copied > size) i_size_write(inode, pos + copied); mark_inode_dirty(inode); } } else { - copied = copy_to_iter(iomap->inline_data + pos, length, iter); + copied = copy_to_iter(dst, length, iter); } dio->size += copied; return copied; diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 479c1da3e221..3cfe787fbf50 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -97,6 +97,20 @@ iomap_sector(struct iomap *iomap, loff_t pos) return (iomap->addr + pos - iomap->offset) >> SECTOR_SHIFT; } +static inline void *iomap_inline_buf(const struct iomap *iomap, loff_t pos) +{ + return iomap->inline_data - iomap->offset + pos; +} + +/* + * iomap->inline_data is a potentially kmapped page, ensure it never crosses a + * page boundary. + */ +static inline bool iomap_inline_data_size_valid(const struct iomap *iomap) +{ + return iomap->length <= PAGE_SIZE - offset_in_page(iomap->inline_data); +} + /* * When a filesystem sets page_ops in an iomap mapping it returns, page_prepare * and page_done will be called for each page written to. This only applies to -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] iomap: Support inline data with block size < page size 2021-07-24 3:44 [PATCH 0/2] iomap: make inline data support more flexible Matthew Wilcox (Oracle) 2021-07-24 3:44 ` [PATCH 1/2] iomap: Support file tail packing Matthew Wilcox (Oracle) @ 2021-07-24 3:44 ` Matthew Wilcox (Oracle) 2021-07-24 4:46 ` Gao Xiang 1 sibling, 1 reply; 8+ messages in thread From: Matthew Wilcox (Oracle) @ 2021-07-24 3:44 UTC (permalink / raw) To: linux-fsdevel, linux-erofs, linux-xfs; +Cc: Matthew Wilcox (Oracle) Remove the restriction that inline data must start on a page boundary in a file. This allows, for example, the first 2KiB to be stored out of line and the trailing 30 bytes to be stored inline. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/iomap/buffered-io.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 7bd8e5de996d..d7d6af29af7f 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -209,25 +209,23 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page, struct iomap *iomap, loff_t pos) { size_t size = iomap->length + iomap->offset - pos; + size_t poff = offset_in_page(pos); void *addr; if (PageUptodate(page)) - return PAGE_SIZE; + return PAGE_SIZE - poff; - /* inline data must start page aligned in the file */ - if (WARN_ON_ONCE(offset_in_page(pos))) - return -EIO; if (WARN_ON_ONCE(!iomap_inline_data_size_valid(iomap))) return -EIO; - if (WARN_ON_ONCE(page_has_private(page))) - return -EIO; + if (poff > 0) + iomap_page_create(inode, page); - addr = kmap_atomic(page); + addr = kmap_atomic(page) + poff; memcpy(addr, iomap_inline_buf(iomap, pos), size); - memset(addr + size, 0, PAGE_SIZE - size); + memset(addr + size, 0, PAGE_SIZE - poff - size); kunmap_atomic(addr); - SetPageUptodate(page); - return PAGE_SIZE; + iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff); + return PAGE_SIZE - poff; } static inline bool iomap_block_needs_zeroing(struct inode *inode, -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] iomap: Support inline data with block size < page size 2021-07-24 3:44 ` [PATCH 2/2] iomap: Support inline data with block size < page size Matthew Wilcox (Oracle) @ 2021-07-24 4:46 ` Gao Xiang 2021-07-24 5:11 ` Gao Xiang 2021-07-24 14:03 ` Matthew Wilcox 0 siblings, 2 replies; 8+ messages in thread From: Gao Xiang @ 2021-07-24 4:46 UTC (permalink / raw) To: Matthew Wilcox (Oracle); +Cc: linux-fsdevel, linux-xfs, linux-erofs Hi Matthew, On Sat, Jul 24, 2021 at 04:44:35AM +0100, Matthew Wilcox (Oracle) wrote: > Remove the restriction that inline data must start on a page boundary > in a file. This allows, for example, the first 2KiB to be stored out > of line and the trailing 30 bytes to be stored inline. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/iomap/buffered-io.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 7bd8e5de996d..d7d6af29af7f 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -209,25 +209,23 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page, > struct iomap *iomap, loff_t pos) > { > size_t size = iomap->length + iomap->offset - pos; > + size_t poff = offset_in_page(pos); > void *addr; > > if (PageUptodate(page)) > - return PAGE_SIZE; > + return PAGE_SIZE - poff; > > - /* inline data must start page aligned in the file */ > - if (WARN_ON_ONCE(offset_in_page(pos))) > - return -EIO; > if (WARN_ON_ONCE(!iomap_inline_data_size_valid(iomap))) > return -EIO; > - if (WARN_ON_ONCE(page_has_private(page))) > - return -EIO; > + if (poff > 0) > + iomap_page_create(inode, page); Thanks for the patch! Previously I'd like to skip the leading uptodate blocks and update the extent it covers that is due to already exist iop. If we get rid of the offset_in_page(pos) restriction like this, I'm not sure if we (or some other fs users) could face something like below (due to somewhat buggy fs users likewise): [0 - 4096) plain block [4096 - 4608) tail INLINE 1 (e.g. by mistake or just splitted .iomap_begin() report.) [4608 - 5120] tail INLINE 2 with this code iomap_set_range_uptodate() wouldn't behave correctly. In addition, currently EROFS cannot test such path (since EROFS is page-sized block only for now) as Darrick said in the previous reply, so I think it would be better together with the folio patchset and targeted for the corresponding merge window, so I can test iomap supported EROFS with the new folio support together (That also give me some time to support sub-pagesized uncompressed blocks...) > > - addr = kmap_atomic(page); > + addr = kmap_atomic(page) + poff; > memcpy(addr, iomap_inline_buf(iomap, pos), size); > - memset(addr + size, 0, PAGE_SIZE - size); > + memset(addr + size, 0, PAGE_SIZE - poff - size); > kunmap_atomic(addr); As my limited understanding, this may need to be fixed, since it doesn't match kmap_atomic(page)... Thanks, Gao Xiang > - SetPageUptodate(page); > - return PAGE_SIZE; > + iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff); > + return PAGE_SIZE - poff; > } > > static inline bool iomap_block_needs_zeroing(struct inode *inode, > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] iomap: Support inline data with block size < page size 2021-07-24 4:46 ` Gao Xiang @ 2021-07-24 5:11 ` Gao Xiang 2021-07-24 7:52 ` Gao Xiang 2021-07-24 14:03 ` Matthew Wilcox 1 sibling, 1 reply; 8+ messages in thread From: Gao Xiang @ 2021-07-24 5:11 UTC (permalink / raw) To: Matthew Wilcox (Oracle); +Cc: linux-fsdevel, linux-xfs, linux-erofs On Sat, Jul 24, 2021 at 12:46:45PM +0800, Gao Xiang wrote: > Hi Matthew, > > On Sat, Jul 24, 2021 at 04:44:35AM +0100, Matthew Wilcox (Oracle) wrote: > > Remove the restriction that inline data must start on a page boundary > > in a file. This allows, for example, the first 2KiB to be stored out > > of line and the trailing 30 bytes to be stored inline. > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > --- > > fs/iomap/buffered-io.c | 18 ++++++++---------- > > 1 file changed, 8 insertions(+), 10 deletions(-) > > > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > > index 7bd8e5de996d..d7d6af29af7f 100644 > > --- a/fs/iomap/buffered-io.c > > +++ b/fs/iomap/buffered-io.c > > @@ -209,25 +209,23 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page, > > struct iomap *iomap, loff_t pos) > > { > > size_t size = iomap->length + iomap->offset - pos; > > + size_t poff = offset_in_page(pos); > > void *addr; > > > > if (PageUptodate(page)) > > - return PAGE_SIZE; > > + return PAGE_SIZE - poff; > > > > - /* inline data must start page aligned in the file */ > > - if (WARN_ON_ONCE(offset_in_page(pos))) > > - return -EIO; > > if (WARN_ON_ONCE(!iomap_inline_data_size_valid(iomap))) > > return -EIO; > > - if (WARN_ON_ONCE(page_has_private(page))) > > - return -EIO; > > + if (poff > 0) > > + iomap_page_create(inode, page); > > Thanks for the patch! > > Previously I'd like to skip the leading uptodate blocks and update the > extent it covers that is due to already exist iop. If we get rid of the > offset_in_page(pos) restriction like this, I'm not sure if we (or some > other fs users) could face something like below (due to somewhat buggy > fs users likewise): > > [0 - 4096) plain block > > [4096 - 4608) tail INLINE 1 (e.g. by mistake or just splitted > .iomap_begin() report.) > [4608 - 5120] tail INLINE 2 > (cont.) So I think without the !offset_in_page(pos) restriction, at least we may need to add something like: if (WARN_ON_ONCE(size != i_size_read(inode) - iomap->offset)) return -EIO; to the approach to detect such cases at least but that is no need with page-sized and !offset_in_page(pos) restriction. > with this code iomap_set_range_uptodate() wouldn't behave correctly. > > In addition, currently EROFS cannot test such path (since EROFS is > page-sized block only for now) as Darrick said in the previous reply, > so I think it would be better together with the folio patchset and > targeted for the corresponding merge window, so I can test iomap > supported EROFS with the new folio support together (That also give > me some time to support sub-pagesized uncompressed blocks...) > > > > > - addr = kmap_atomic(page); > > + addr = kmap_atomic(page) + poff; > > memcpy(addr, iomap_inline_buf(iomap, pos), size); > > - memset(addr + size, 0, PAGE_SIZE - size); > > + memset(addr + size, 0, PAGE_SIZE - poff - size); > > kunmap_atomic(addr); > > As my limited understanding, this may need to be fixed, since it > doesn't match kmap_atomic(page)... > > Thanks, > Gao Xiang > > > - SetPageUptodate(page); > > - return PAGE_SIZE; > > + iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff); > > + return PAGE_SIZE - poff; > > } > > > > static inline bool iomap_block_needs_zeroing(struct inode *inode, > > -- > > 2.30.2 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] iomap: Support inline data with block size < page size 2021-07-24 5:11 ` Gao Xiang @ 2021-07-24 7:52 ` Gao Xiang 0 siblings, 0 replies; 8+ messages in thread From: Gao Xiang @ 2021-07-24 7:52 UTC (permalink / raw) To: Matthew Wilcox (Oracle); +Cc: linux-fsdevel, linux-xfs, linux-erofs On Sat, Jul 24, 2021 at 01:11:09PM +0800, Gao Xiang wrote: > On Sat, Jul 24, 2021 at 12:46:45PM +0800, Gao Xiang wrote: ... > > > > Thanks for the patch! > > > > Previously I'd like to skip the leading uptodate blocks and update the > > extent it covers that is due to already exist iop. If we get rid of the > > offset_in_page(pos) restriction like this, I'm not sure if we (or some > > other fs users) could face something like below (due to somewhat buggy > > fs users likewise): > > > > [0 - 4096) plain block > > > > [4096 - 4608) tail INLINE 1 (e.g. by mistake or just splitted > > .iomap_begin() report.) > > [4608 - 5120] tail INLINE 2 > > > > (cont.) > > So I think without the !offset_in_page(pos) restriction, at least we > may need to add something like: > > if (WARN_ON_ONCE(size != i_size_read(inode) - iomap->offset)) > return -EIO; > > to the approach to detect such cases at least but that is no need with > page-sized and !offset_in_page(pos) restriction. > Never mind, after rethinking with clear head, I think I was overthinking this part and it shouldn't behave like this. Sorry about the noise above. > > > > > > > > - addr = kmap_atomic(page); > > > + addr = kmap_atomic(page) + poff; > > > memcpy(addr, iomap_inline_buf(iomap, pos), size); > > > - memset(addr + size, 0, PAGE_SIZE - size); > > > + memset(addr + size, 0, PAGE_SIZE - poff - size); > > > kunmap_atomic(addr); > > > > As my limited understanding, this may need to be fixed, since it > > doesn't match kmap_atomic(page)... > > > > Thanks, > > Gao Xiang > > > > > - SetPageUptodate(page); > > > - return PAGE_SIZE; > > > + iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff); > > > + return PAGE_SIZE - poff; > > > } > > > > > > static inline bool iomap_block_needs_zeroing(struct inode *inode, > > > -- > > > 2.30.2 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] iomap: Support inline data with block size < page size 2021-07-24 4:46 ` Gao Xiang 2021-07-24 5:11 ` Gao Xiang @ 2021-07-24 14:03 ` Matthew Wilcox 2021-07-24 15:14 ` Gao Xiang 1 sibling, 1 reply; 8+ messages in thread From: Matthew Wilcox @ 2021-07-24 14:03 UTC (permalink / raw) To: linux-fsdevel, linux-erofs, linux-xfs On Sat, Jul 24, 2021 at 12:46:45PM +0800, Gao Xiang wrote: > Hi Matthew, > > On Sat, Jul 24, 2021 at 04:44:35AM +0100, Matthew Wilcox (Oracle) wrote: > > Remove the restriction that inline data must start on a page boundary > > in a file. This allows, for example, the first 2KiB to be stored out > > of line and the trailing 30 bytes to be stored inline. > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > --- > > fs/iomap/buffered-io.c | 18 ++++++++---------- > > 1 file changed, 8 insertions(+), 10 deletions(-) > > > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > > index 7bd8e5de996d..d7d6af29af7f 100644 > > --- a/fs/iomap/buffered-io.c > > +++ b/fs/iomap/buffered-io.c > > @@ -209,25 +209,23 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page, > > struct iomap *iomap, loff_t pos) > > { > > size_t size = iomap->length + iomap->offset - pos; > > + size_t poff = offset_in_page(pos); > > void *addr; > > > > if (PageUptodate(page)) > > - return PAGE_SIZE; > > + return PAGE_SIZE - poff; > > > > - /* inline data must start page aligned in the file */ > > - if (WARN_ON_ONCE(offset_in_page(pos))) > > - return -EIO; > > if (WARN_ON_ONCE(!iomap_inline_data_size_valid(iomap))) > > return -EIO; > > - if (WARN_ON_ONCE(page_has_private(page))) > > - return -EIO; > > + if (poff > 0) > > + iomap_page_create(inode, page); > > Thanks for the patch! > > Previously I'd like to skip the leading uptodate blocks and update the > extent it covers that is due to already exist iop. If we get rid of the > offset_in_page(pos) restriction like this, I'm not sure if we (or some > other fs users) could face something like below (due to somewhat buggy > fs users likewise): > > [0 - 4096) plain block > > [4096 - 4608) tail INLINE 1 (e.g. by mistake or just splitted > .iomap_begin() report.) > [4608 - 5120] tail INLINE 2 My assumption is that an INLINE extent is <= block_size. Otherwise the first part of the extent would be not-inline. So this would be a bug in the filesystem; [4096-4608) should not be an inline extent. > with this code iomap_set_range_uptodate() wouldn't behave correctly. > > In addition, currently EROFS cannot test such path (since EROFS is > page-sized block only for now) as Darrick said in the previous reply, > so I think it would be better together with the folio patchset and > targeted for the corresponding merge window, so I can test iomap > supported EROFS with the new folio support together (That also give > me some time to support sub-pagesized uncompressed blocks...) Do you want to test erofs with multi-page folios? That might be even more interesting than block size < page size. > > - addr = kmap_atomic(page); > > + addr = kmap_atomic(page) + poff; > > memcpy(addr, iomap_inline_buf(iomap, pos), size); > > - memset(addr + size, 0, PAGE_SIZE - size); > > + memset(addr + size, 0, PAGE_SIZE - poff - size); > > kunmap_atomic(addr); > > As my limited understanding, this may need to be fixed, since it > doesn't match kmap_atomic(page)... void kunmap_local_indexed(void *vaddr) { unsigned long addr = (unsigned long) vaddr & PAGE_MASK; so it's fine to unmap any address in the page. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] iomap: Support inline data with block size < page size 2021-07-24 14:03 ` Matthew Wilcox @ 2021-07-24 15:14 ` Gao Xiang 0 siblings, 0 replies; 8+ messages in thread From: Gao Xiang @ 2021-07-24 15:14 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-fsdevel, linux-xfs, linux-erofs On Sat, Jul 24, 2021 at 03:03:56PM +0100, Matthew Wilcox wrote: > On Sat, Jul 24, 2021 at 12:46:45PM +0800, Gao Xiang wrote: > > Hi Matthew, > > > > On Sat, Jul 24, 2021 at 04:44:35AM +0100, Matthew Wilcox (Oracle) wrote: > > > Remove the restriction that inline data must start on a page boundary > > > in a file. This allows, for example, the first 2KiB to be stored out > > > of line and the trailing 30 bytes to be stored inline. > > > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > > --- > > > fs/iomap/buffered-io.c | 18 ++++++++---------- > > > 1 file changed, 8 insertions(+), 10 deletions(-) > > > > > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > > > index 7bd8e5de996d..d7d6af29af7f 100644 > > > --- a/fs/iomap/buffered-io.c > > > +++ b/fs/iomap/buffered-io.c > > > @@ -209,25 +209,23 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page, > > > struct iomap *iomap, loff_t pos) > > > { > > > size_t size = iomap->length + iomap->offset - pos; > > > + size_t poff = offset_in_page(pos); > > > void *addr; > > > > > > if (PageUptodate(page)) > > > - return PAGE_SIZE; > > > + return PAGE_SIZE - poff; > > > > > > - /* inline data must start page aligned in the file */ > > > - if (WARN_ON_ONCE(offset_in_page(pos))) > > > - return -EIO; > > > if (WARN_ON_ONCE(!iomap_inline_data_size_valid(iomap))) > > > return -EIO; > > > - if (WARN_ON_ONCE(page_has_private(page))) > > > - return -EIO; > > > + if (poff > 0) > > > + iomap_page_create(inode, page); > > > > Thanks for the patch! > > > > Previously I'd like to skip the leading uptodate blocks and update the > > extent it covers that is due to already exist iop. If we get rid of the > > offset_in_page(pos) restriction like this, I'm not sure if we (or some > > other fs users) could face something like below (due to somewhat buggy > > fs users likewise): > > > > [0 - 4096) plain block > > > > [4096 - 4608) tail INLINE 1 (e.g. by mistake or just splitted > > .iomap_begin() report.) > > [4608 - 5120] tail INLINE 2 > > My assumption is that an INLINE extent is <= block_size. Otherwise > the first part of the extent would be not-inline. So this would be > a bug in the filesystem; [4096-4608) should not be an inline extent. Yes, never mind. Sorry about again. > > > with this code iomap_set_range_uptodate() wouldn't behave correctly. > > > > In addition, currently EROFS cannot test such path (since EROFS is > > page-sized block only for now) as Darrick said in the previous reply, > > so I think it would be better together with the folio patchset and > > targeted for the corresponding merge window, so I can test iomap > > supported EROFS with the new folio support together (That also give > > me some time to support sub-pagesized uncompressed blocks...) > > Do you want to test erofs with multi-page folios? That might be > even more interesting than block size < page size. Hmm.. I'm busy in developing for some new scenario. Will look into that after the current busy period. > > > > - addr = kmap_atomic(page); > > > + addr = kmap_atomic(page) + poff; > > > memcpy(addr, iomap_inline_buf(iomap, pos), size); > > > - memset(addr + size, 0, PAGE_SIZE - size); > > > + memset(addr + size, 0, PAGE_SIZE - poff - size); > > > kunmap_atomic(addr); > > > > As my limited understanding, this may need to be fixed, since it > > doesn't match kmap_atomic(page)... > > void kunmap_local_indexed(void *vaddr) > { > unsigned long addr = (unsigned long) vaddr & PAGE_MASK; > > so it's fine to unmap any address in the page. I already checked this (in practice it has no problem due to the current implementation), yet I'm not quite sure if it matches the API usage, and not quite sure how many in-kernel users use like this. Thanks, Gao Xiang ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-24 15:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-24 3:44 [PATCH 0/2] iomap: make inline data support more flexible Matthew Wilcox (Oracle) 2021-07-24 3:44 ` [PATCH 1/2] iomap: Support file tail packing Matthew Wilcox (Oracle) 2021-07-24 3:44 ` [PATCH 2/2] iomap: Support inline data with block size < page size Matthew Wilcox (Oracle) 2021-07-24 4:46 ` Gao Xiang 2021-07-24 5:11 ` Gao Xiang 2021-07-24 7:52 ` Gao Xiang 2021-07-24 14:03 ` Matthew Wilcox 2021-07-24 15:14 ` Gao Xiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).