* [PATCH RFC] iomap: introduce IOMAP_TAIL @ 2019-06-29 7:30 Chao Yu 2019-06-29 9:34 ` Gao Xiang 2019-06-30 23:19 ` Darrick J. Wong 0 siblings, 2 replies; 12+ messages in thread From: Chao Yu @ 2019-06-29 7:30 UTC (permalink / raw) To: hch, darrick.wong; +Cc: linux-xfs, linux-fsdevel, gaoxiang25, chao, Chao Yu Some filesystems like erofs/reiserfs have the ability to pack tail data into metadata, however iomap framework can only support mapping inline data with IOMAP_INLINE type, it restricts that: - inline data should be locating at page #0. - inline size should equal to .i_size So we can not use IOMAP_INLINE to handle tail-packing case. This patch introduces new mapping type IOMAP_TAIL to map tail-packed data for further use of erofs. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/iomap.c | 22 ++++++++++++++++++++++ include/linux/iomap.h | 1 + 2 files changed, 23 insertions(+) diff --git a/fs/iomap.c b/fs/iomap.c index 12654c2e78f8..ae7777ce77d0 100644 --- a/fs/iomap.c +++ b/fs/iomap.c @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, SetPageUptodate(page); } +static void +iomap_read_tail_data(struct inode *inode, struct page *page, + struct iomap *iomap) +{ + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); + void *addr; + + if (PageUptodate(page)) + return; + + addr = kmap_atomic(page); + memcpy(addr, iomap->inline_data, size); + memset(addr + size, 0, PAGE_SIZE - size); + kunmap_atomic(addr); + SetPageUptodate(page); +} + static loff_t iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, struct iomap *iomap) @@ -298,6 +315,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, return PAGE_SIZE; } + if (iomap->type == IOMAP_TAIL) { + iomap_read_tail_data(inode, page, iomap); + return PAGE_SIZE; + } + /* zero post-eof blocks as the page may be mapped */ iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); if (plen == 0) diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 2103b94cb1bf..7e1ee48e3db7 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -25,6 +25,7 @@ struct vm_fault; #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */ #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */ #define IOMAP_INLINE 0x05 /* data inline in the inode */ +#define IOMAP_TAIL 0x06 /* tail data packed in metdata */ /* * Flags for all iomap mappings: -- 2.18.0.rc1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-06-29 7:30 [PATCH RFC] iomap: introduce IOMAP_TAIL Chao Yu @ 2019-06-29 9:34 ` Gao Xiang 2019-07-01 6:40 ` Chao Yu 2019-06-30 23:19 ` Darrick J. Wong 1 sibling, 1 reply; 12+ messages in thread From: Gao Xiang @ 2019-06-29 9:34 UTC (permalink / raw) To: Chao Yu; +Cc: hch, darrick.wong, linux-xfs, linux-fsdevel, chao Hi Chao, On 2019/6/29 15:30, Chao Yu wrote: > Some filesystems like erofs/reiserfs have the ability to pack tail > data into metadata, however iomap framework can only support mapping > inline data with IOMAP_INLINE type, it restricts that: > - inline data should be locating at page #0. > - inline size should equal to .i_size > So we can not use IOMAP_INLINE to handle tail-packing case. > > This patch introduces new mapping type IOMAP_TAIL to map tail-packed > data for further use of erofs. > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fs/iomap.c | 22 ++++++++++++++++++++++ > include/linux/iomap.h | 1 + > 2 files changed, 23 insertions(+) > > diff --git a/fs/iomap.c b/fs/iomap.c > index 12654c2e78f8..ae7777ce77d0 100644 > --- a/fs/iomap.c > +++ b/fs/iomap.c > @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, > SetPageUptodate(page); > } > > +static void > +iomap_read_tail_data(struct inode *inode, struct page *page, > + struct iomap *iomap) > +{ > + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); > + void *addr; > + > + if (PageUptodate(page)) > + return; > + > + addr = kmap_atomic(page); > + memcpy(addr, iomap->inline_data, size); > + memset(addr + size, 0, PAGE_SIZE - size); need flush_dcache_page(page) here for new page cache page since it's generic iomap code (althrough not necessary for x86, arm), I am not sure... see commit d2b2c6dd227b and c01778001a4f... Thanks, Gao Xiang > + kunmap_atomic(addr); > + SetPageUptodate(page); > +} > + > static loff_t > iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > struct iomap *iomap) > @@ -298,6 +315,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > return PAGE_SIZE; > } > > + if (iomap->type == IOMAP_TAIL) { > + iomap_read_tail_data(inode, page, iomap); > + return PAGE_SIZE; > + } > + > /* zero post-eof blocks as the page may be mapped */ > iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); > if (plen == 0) > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index 2103b94cb1bf..7e1ee48e3db7 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -25,6 +25,7 @@ struct vm_fault; > #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */ > #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */ > #define IOMAP_INLINE 0x05 /* data inline in the inode */ > +#define IOMAP_TAIL 0x06 /* tail data packed in metdata */ > > /* > * Flags for all iomap mappings: > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-06-29 9:34 ` Gao Xiang @ 2019-07-01 6:40 ` Chao Yu 2019-07-01 7:03 ` Gao Xiang 0 siblings, 1 reply; 12+ messages in thread From: Chao Yu @ 2019-07-01 6:40 UTC (permalink / raw) To: Gao Xiang; +Cc: hch, darrick.wong, linux-xfs, linux-fsdevel, chao Hi Xiang, On 2019/6/29 17:34, Gao Xiang wrote: > Hi Chao, > > On 2019/6/29 15:30, Chao Yu wrote: >> Some filesystems like erofs/reiserfs have the ability to pack tail >> data into metadata, however iomap framework can only support mapping >> inline data with IOMAP_INLINE type, it restricts that: >> - inline data should be locating at page #0. >> - inline size should equal to .i_size >> So we can not use IOMAP_INLINE to handle tail-packing case. >> >> This patch introduces new mapping type IOMAP_TAIL to map tail-packed >> data for further use of erofs. >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> fs/iomap.c | 22 ++++++++++++++++++++++ >> include/linux/iomap.h | 1 + >> 2 files changed, 23 insertions(+) >> >> diff --git a/fs/iomap.c b/fs/iomap.c >> index 12654c2e78f8..ae7777ce77d0 100644 >> --- a/fs/iomap.c >> +++ b/fs/iomap.c >> @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, >> SetPageUptodate(page); >> } >> >> +static void >> +iomap_read_tail_data(struct inode *inode, struct page *page, >> + struct iomap *iomap) >> +{ >> + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); >> + void *addr; >> + >> + if (PageUptodate(page)) >> + return; >> + >> + addr = kmap_atomic(page); >> + memcpy(addr, iomap->inline_data, size); >> + memset(addr + size, 0, PAGE_SIZE - size); > > need flush_dcache_page(page) here for new page cache page since > it's generic iomap code (althrough not necessary for x86, arm), I am not sure... > see commit d2b2c6dd227b and c01778001a4f... Thanks for your reminding, these all codes were copied from iomap_read_inline_data(), so I think we need a separated patch to fix this issue if necessary. Thanks, > > Thanks, > Gao Xiang > >> + kunmap_atomic(addr); >> + SetPageUptodate(page); >> +} >> + >> static loff_t >> iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, >> struct iomap *iomap) >> @@ -298,6 +315,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, >> return PAGE_SIZE; >> } >> >> + if (iomap->type == IOMAP_TAIL) { >> + iomap_read_tail_data(inode, page, iomap); >> + return PAGE_SIZE; >> + } >> + >> /* zero post-eof blocks as the page may be mapped */ >> iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); >> if (plen == 0) >> diff --git a/include/linux/iomap.h b/include/linux/iomap.h >> index 2103b94cb1bf..7e1ee48e3db7 100644 >> --- a/include/linux/iomap.h >> +++ b/include/linux/iomap.h >> @@ -25,6 +25,7 @@ struct vm_fault; >> #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */ >> #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */ >> #define IOMAP_INLINE 0x05 /* data inline in the inode */ >> +#define IOMAP_TAIL 0x06 /* tail data packed in metdata */ >> >> /* >> * Flags for all iomap mappings: >> > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-07-01 6:40 ` Chao Yu @ 2019-07-01 7:03 ` Gao Xiang 2019-07-01 9:49 ` Andreas Grünbacher 0 siblings, 1 reply; 12+ messages in thread From: Gao Xiang @ 2019-07-01 7:03 UTC (permalink / raw) To: Chao Yu; +Cc: hch, darrick.wong, linux-xfs, linux-fsdevel, chao On 2019/7/1 14:40, Chao Yu wrote: > Hi Xiang, > > On 2019/6/29 17:34, Gao Xiang wrote: >> Hi Chao, >> >> On 2019/6/29 15:30, Chao Yu wrote: >>> Some filesystems like erofs/reiserfs have the ability to pack tail >>> data into metadata, however iomap framework can only support mapping >>> inline data with IOMAP_INLINE type, it restricts that: >>> - inline data should be locating at page #0. >>> - inline size should equal to .i_size >>> So we can not use IOMAP_INLINE to handle tail-packing case. >>> >>> This patch introduces new mapping type IOMAP_TAIL to map tail-packed >>> data for further use of erofs. >>> >>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>> --- >>> fs/iomap.c | 22 ++++++++++++++++++++++ >>> include/linux/iomap.h | 1 + >>> 2 files changed, 23 insertions(+) >>> >>> diff --git a/fs/iomap.c b/fs/iomap.c >>> index 12654c2e78f8..ae7777ce77d0 100644 >>> --- a/fs/iomap.c >>> +++ b/fs/iomap.c >>> @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, >>> SetPageUptodate(page); >>> } >>> >>> +static void >>> +iomap_read_tail_data(struct inode *inode, struct page *page, >>> + struct iomap *iomap) >>> +{ >>> + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); >>> + void *addr; >>> + >>> + if (PageUptodate(page)) >>> + return; >>> + >>> + addr = kmap_atomic(page); >>> + memcpy(addr, iomap->inline_data, size); >>> + memset(addr + size, 0, PAGE_SIZE - size); >> >> need flush_dcache_page(page) here for new page cache page since >> it's generic iomap code (althrough not necessary for x86, arm), I am not sure... >> see commit d2b2c6dd227b and c01778001a4f... > > Thanks for your reminding, these all codes were copied from > iomap_read_inline_data(), so I think we need a separated patch to fix this issue > if necessary. Yes, just a reminder, it is good as it-is. Thanks, Gao Xiang > > Thanks, > >> >> Thanks, >> Gao Xiang >> >>> + kunmap_atomic(addr); >>> + SetPageUptodate(page); >>> +} >>> + >>> static loff_t >>> iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, >>> struct iomap *iomap) >>> @@ -298,6 +315,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, >>> return PAGE_SIZE; >>> } >>> >>> + if (iomap->type == IOMAP_TAIL) { >>> + iomap_read_tail_data(inode, page, iomap); >>> + return PAGE_SIZE; >>> + } >>> + >>> /* zero post-eof blocks as the page may be mapped */ >>> iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); >>> if (plen == 0) >>> diff --git a/include/linux/iomap.h b/include/linux/iomap.h >>> index 2103b94cb1bf..7e1ee48e3db7 100644 >>> --- a/include/linux/iomap.h >>> +++ b/include/linux/iomap.h >>> @@ -25,6 +25,7 @@ struct vm_fault; >>> #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */ >>> #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */ >>> #define IOMAP_INLINE 0x05 /* data inline in the inode */ >>> +#define IOMAP_TAIL 0x06 /* tail data packed in metdata */ >>> >>> /* >>> * Flags for all iomap mappings: >>> >> . >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-07-01 7:03 ` Gao Xiang @ 2019-07-01 9:49 ` Andreas Grünbacher 2019-07-01 10:07 ` Chao Yu 0 siblings, 1 reply; 12+ messages in thread From: Andreas Grünbacher @ 2019-07-01 9:49 UTC (permalink / raw) To: Gao Xiang Cc: Chao Yu, Christoph Hellwig, Darrick J. Wong, linux-xfs, Linux FS-devel Mailing List, chao Am Mo., 1. Juli 2019 um 10:04 Uhr schrieb Gao Xiang <gaoxiang25@huawei.com>: > On 2019/7/1 14:40, Chao Yu wrote: > > Hi Xiang, > > > > On 2019/6/29 17:34, Gao Xiang wrote: > >> Hi Chao, > >> > >> On 2019/6/29 15:30, Chao Yu wrote: > >>> Some filesystems like erofs/reiserfs have the ability to pack tail > >>> data into metadata, however iomap framework can only support mapping > >>> inline data with IOMAP_INLINE type, it restricts that: > >>> - inline data should be locating at page #0. > >>> - inline size should equal to .i_size > >>> So we can not use IOMAP_INLINE to handle tail-packing case. > >>> > >>> This patch introduces new mapping type IOMAP_TAIL to map tail-packed > >>> data for further use of erofs. > >>> > >>> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >>> --- > >>> fs/iomap.c | 22 ++++++++++++++++++++++ > >>> include/linux/iomap.h | 1 + > >>> 2 files changed, 23 insertions(+) > >>> > >>> diff --git a/fs/iomap.c b/fs/iomap.c > >>> index 12654c2e78f8..ae7777ce77d0 100644 > >>> --- a/fs/iomap.c > >>> +++ b/fs/iomap.c > >>> @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, > >>> SetPageUptodate(page); > >>> } > >>> > >>> +static void > >>> +iomap_read_tail_data(struct inode *inode, struct page *page, > >>> + struct iomap *iomap) > >>> +{ > >>> + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); > >>> + void *addr; > >>> + > >>> + if (PageUptodate(page)) > >>> + return; > >>> + > >>> + addr = kmap_atomic(page); > >>> + memcpy(addr, iomap->inline_data, size); > >>> + memset(addr + size, 0, PAGE_SIZE - size); > >> > >> need flush_dcache_page(page) here for new page cache page since > >> it's generic iomap code (althrough not necessary for x86, arm), I am not sure... > >> see commit d2b2c6dd227b and c01778001a4f... > > > > Thanks for your reminding, these all codes were copied from > > iomap_read_inline_data(), so I think we need a separated patch to fix this issue > > if necessary. > > Yes, just a reminder, it is good as it-is. Not sure if that means that IOMAP_INLINE as is works for you now. In any case, if the inline data isn't transparently copied into the page cache at index 0, memory-mapped I/O isn't going to work. The code further assumes that "packed" files consist of exactly one IOMAP_INLINE mapping; no IOMAP_MAPPED or other mappings may follow. Is it that assumption that's causing you trouble? If so, what's the layout at the filesystem level you want to support? Thanks, Andreas > Thanks, > Gao Xiang > > > > > Thanks, > > > >> > >> Thanks, > >> Gao Xiang > >> > >>> + kunmap_atomic(addr); > >>> + SetPageUptodate(page); > >>> +} > >>> + > >>> static loff_t > >>> iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > >>> struct iomap *iomap) > >>> @@ -298,6 +315,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > >>> return PAGE_SIZE; > >>> } > >>> > >>> + if (iomap->type == IOMAP_TAIL) { > >>> + iomap_read_tail_data(inode, page, iomap); > >>> + return PAGE_SIZE; > >>> + } > >>> + > >>> /* zero post-eof blocks as the page may be mapped */ > >>> iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); > >>> if (plen == 0) > >>> diff --git a/include/linux/iomap.h b/include/linux/iomap.h > >>> index 2103b94cb1bf..7e1ee48e3db7 100644 > >>> --- a/include/linux/iomap.h > >>> +++ b/include/linux/iomap.h > >>> @@ -25,6 +25,7 @@ struct vm_fault; > >>> #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */ > >>> #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */ > >>> #define IOMAP_INLINE 0x05 /* data inline in the inode */ > >>> +#define IOMAP_TAIL 0x06 /* tail data packed in metdata */ > >>> > >>> /* > >>> * Flags for all iomap mappings: > >>> > >> . > >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-07-01 9:49 ` Andreas Grünbacher @ 2019-07-01 10:07 ` Chao Yu 2019-07-01 10:13 ` Andreas Grünbacher 0 siblings, 1 reply; 12+ messages in thread From: Chao Yu @ 2019-07-01 10:07 UTC (permalink / raw) To: Andreas Grünbacher, Gao Xiang Cc: Christoph Hellwig, Darrick J. Wong, linux-xfs, Linux FS-devel Mailing List, chao On 2019/7/1 17:49, Andreas Grünbacher wrote: > Am Mo., 1. Juli 2019 um 10:04 Uhr schrieb Gao Xiang <gaoxiang25@huawei.com>: >> On 2019/7/1 14:40, Chao Yu wrote: >>> Hi Xiang, >>> >>> On 2019/6/29 17:34, Gao Xiang wrote: >>>> Hi Chao, >>>> >>>> On 2019/6/29 15:30, Chao Yu wrote: >>>>> Some filesystems like erofs/reiserfs have the ability to pack tail >>>>> data into metadata, however iomap framework can only support mapping >>>>> inline data with IOMAP_INLINE type, it restricts that: >>>>> - inline data should be locating at page #0. >>>>> - inline size should equal to .i_size >>>>> So we can not use IOMAP_INLINE to handle tail-packing case. >>>>> >>>>> This patch introduces new mapping type IOMAP_TAIL to map tail-packed >>>>> data for further use of erofs. >>>>> >>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>>> --- >>>>> fs/iomap.c | 22 ++++++++++++++++++++++ >>>>> include/linux/iomap.h | 1 + >>>>> 2 files changed, 23 insertions(+) >>>>> >>>>> diff --git a/fs/iomap.c b/fs/iomap.c >>>>> index 12654c2e78f8..ae7777ce77d0 100644 >>>>> --- a/fs/iomap.c >>>>> +++ b/fs/iomap.c >>>>> @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, >>>>> SetPageUptodate(page); >>>>> } >>>>> >>>>> +static void >>>>> +iomap_read_tail_data(struct inode *inode, struct page *page, >>>>> + struct iomap *iomap) >>>>> +{ >>>>> + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); >>>>> + void *addr; >>>>> + >>>>> + if (PageUptodate(page)) >>>>> + return; >>>>> + >>>>> + addr = kmap_atomic(page); >>>>> + memcpy(addr, iomap->inline_data, size); >>>>> + memset(addr + size, 0, PAGE_SIZE - size); >>>> >>>> need flush_dcache_page(page) here for new page cache page since >>>> it's generic iomap code (althrough not necessary for x86, arm), I am not sure... >>>> see commit d2b2c6dd227b and c01778001a4f... >>> >>> Thanks for your reminding, these all codes were copied from >>> iomap_read_inline_data(), so I think we need a separated patch to fix this issue >>> if necessary. >> >> Yes, just a reminder, it is good as it-is. > > Not sure if that means that IOMAP_INLINE as is works for you now. In > any case, if the inline data isn't transparently copied into the page > cache at index 0, memory-mapped I/O isn't going to work. > > The code further assumes that "packed" files consist of exactly one > IOMAP_INLINE mapping; no IOMAP_MAPPED or other mappings may follow. Is > it that assumption that's causing you trouble? If so, what's the Yes, that's the problem we met. > layout at the filesystem level you want to support? The layout which can support tail-merge one, it means if the data locating at the tail of file has small enough size, we can inline the tail data into metadata area. e.g.: IOMAP_MAPPED [0, 8192] IOMAP_INLINE (or maybe IOMAP_TAIL) [8192, 8200] Thanks, > > Thanks, > Andreas > >> Thanks, >> Gao Xiang >> >>> >>> Thanks, >>> >>>> >>>> Thanks, >>>> Gao Xiang >>>> >>>>> + kunmap_atomic(addr); >>>>> + SetPageUptodate(page); >>>>> +} >>>>> + >>>>> static loff_t >>>>> iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, >>>>> struct iomap *iomap) >>>>> @@ -298,6 +315,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, >>>>> return PAGE_SIZE; >>>>> } >>>>> >>>>> + if (iomap->type == IOMAP_TAIL) { >>>>> + iomap_read_tail_data(inode, page, iomap); >>>>> + return PAGE_SIZE; >>>>> + } >>>>> + >>>>> /* zero post-eof blocks as the page may be mapped */ >>>>> iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); >>>>> if (plen == 0) >>>>> diff --git a/include/linux/iomap.h b/include/linux/iomap.h >>>>> index 2103b94cb1bf..7e1ee48e3db7 100644 >>>>> --- a/include/linux/iomap.h >>>>> +++ b/include/linux/iomap.h >>>>> @@ -25,6 +25,7 @@ struct vm_fault; >>>>> #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */ >>>>> #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */ >>>>> #define IOMAP_INLINE 0x05 /* data inline in the inode */ >>>>> +#define IOMAP_TAIL 0x06 /* tail data packed in metdata */ >>>>> >>>>> /* >>>>> * Flags for all iomap mappings: >>>>> >>>> . >>>> > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-07-01 10:07 ` Chao Yu @ 2019-07-01 10:13 ` Andreas Grünbacher 2019-07-01 10:22 ` Chao Yu 0 siblings, 1 reply; 12+ messages in thread From: Andreas Grünbacher @ 2019-07-01 10:13 UTC (permalink / raw) To: Chao Yu Cc: Gao Xiang, Christoph Hellwig, Darrick J. Wong, linux-xfs, Linux FS-devel Mailing List, chao Am Mo., 1. Juli 2019 um 12:09 Uhr schrieb Chao Yu <yuchao0@huawei.com>: > On 2019/7/1 17:49, Andreas Grünbacher wrote: > > Am Mo., 1. Juli 2019 um 10:04 Uhr schrieb Gao Xiang <gaoxiang25@huawei.com>: > >> On 2019/7/1 14:40, Chao Yu wrote: > >>> Hi Xiang, > >>> > >>> On 2019/6/29 17:34, Gao Xiang wrote: > >>>> Hi Chao, > >>>> > >>>> On 2019/6/29 15:30, Chao Yu wrote: > >>>>> Some filesystems like erofs/reiserfs have the ability to pack tail > >>>>> data into metadata, however iomap framework can only support mapping > >>>>> inline data with IOMAP_INLINE type, it restricts that: > >>>>> - inline data should be locating at page #0. > >>>>> - inline size should equal to .i_size > >>>>> So we can not use IOMAP_INLINE to handle tail-packing case. > >>>>> > >>>>> This patch introduces new mapping type IOMAP_TAIL to map tail-packed > >>>>> data for further use of erofs. > >>>>> > >>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >>>>> --- > >>>>> fs/iomap.c | 22 ++++++++++++++++++++++ > >>>>> include/linux/iomap.h | 1 + > >>>>> 2 files changed, 23 insertions(+) > >>>>> > >>>>> diff --git a/fs/iomap.c b/fs/iomap.c > >>>>> index 12654c2e78f8..ae7777ce77d0 100644 > >>>>> --- a/fs/iomap.c > >>>>> +++ b/fs/iomap.c > >>>>> @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, > >>>>> SetPageUptodate(page); > >>>>> } > >>>>> > >>>>> +static void > >>>>> +iomap_read_tail_data(struct inode *inode, struct page *page, > >>>>> + struct iomap *iomap) > >>>>> +{ > >>>>> + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); > >>>>> + void *addr; > >>>>> + > >>>>> + if (PageUptodate(page)) > >>>>> + return; > >>>>> + > >>>>> + addr = kmap_atomic(page); > >>>>> + memcpy(addr, iomap->inline_data, size); > >>>>> + memset(addr + size, 0, PAGE_SIZE - size); > >>>> > >>>> need flush_dcache_page(page) here for new page cache page since > >>>> it's generic iomap code (althrough not necessary for x86, arm), I am not sure... > >>>> see commit d2b2c6dd227b and c01778001a4f... > >>> > >>> Thanks for your reminding, these all codes were copied from > >>> iomap_read_inline_data(), so I think we need a separated patch to fix this issue > >>> if necessary. > >> > >> Yes, just a reminder, it is good as it-is. > > > > Not sure if that means that IOMAP_INLINE as is works for you now. In > > any case, if the inline data isn't transparently copied into the page > > cache at index 0, memory-mapped I/O isn't going to work. > > > > The code further assumes that "packed" files consist of exactly one > > IOMAP_INLINE mapping; no IOMAP_MAPPED or other mappings may follow. Is > > it that assumption that's causing you trouble? If so, what's the > > Yes, that's the problem we met. > > > layout at the filesystem level you want to support? > > The layout which can support tail-merge one, it means if the data locating at > the tail of file has small enough size, we can inline the tail data into > metadata area. e.g.: > > IOMAP_MAPPED [0, 8192] > IOMAP_INLINE (or maybe IOMAP_TAIL) [8192, 8200] Ah, now I see. Let's generalize the IOMAP_INLINE code to support that and rename it to IOMAP_TAIL then. Thanks, Andreas ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-07-01 10:13 ` Andreas Grünbacher @ 2019-07-01 10:22 ` Chao Yu 0 siblings, 0 replies; 12+ messages in thread From: Chao Yu @ 2019-07-01 10:22 UTC (permalink / raw) To: Andreas Grünbacher Cc: Gao Xiang, Christoph Hellwig, Darrick J. Wong, linux-xfs, Linux FS-devel Mailing List, chao On 2019/7/1 18:13, Andreas Grünbacher wrote: > Am Mo., 1. Juli 2019 um 12:09 Uhr schrieb Chao Yu <yuchao0@huawei.com>: >> On 2019/7/1 17:49, Andreas Grünbacher wrote: >>> Am Mo., 1. Juli 2019 um 10:04 Uhr schrieb Gao Xiang <gaoxiang25@huawei.com>: >>>> On 2019/7/1 14:40, Chao Yu wrote: >>>>> Hi Xiang, >>>>> >>>>> On 2019/6/29 17:34, Gao Xiang wrote: >>>>>> Hi Chao, >>>>>> >>>>>> On 2019/6/29 15:30, Chao Yu wrote: >>>>>>> Some filesystems like erofs/reiserfs have the ability to pack tail >>>>>>> data into metadata, however iomap framework can only support mapping >>>>>>> inline data with IOMAP_INLINE type, it restricts that: >>>>>>> - inline data should be locating at page #0. >>>>>>> - inline size should equal to .i_size >>>>>>> So we can not use IOMAP_INLINE to handle tail-packing case. >>>>>>> >>>>>>> This patch introduces new mapping type IOMAP_TAIL to map tail-packed >>>>>>> data for further use of erofs. >>>>>>> >>>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>>>>> --- >>>>>>> fs/iomap.c | 22 ++++++++++++++++++++++ >>>>>>> include/linux/iomap.h | 1 + >>>>>>> 2 files changed, 23 insertions(+) >>>>>>> >>>>>>> diff --git a/fs/iomap.c b/fs/iomap.c >>>>>>> index 12654c2e78f8..ae7777ce77d0 100644 >>>>>>> --- a/fs/iomap.c >>>>>>> +++ b/fs/iomap.c >>>>>>> @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, >>>>>>> SetPageUptodate(page); >>>>>>> } >>>>>>> >>>>>>> +static void >>>>>>> +iomap_read_tail_data(struct inode *inode, struct page *page, >>>>>>> + struct iomap *iomap) >>>>>>> +{ >>>>>>> + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); >>>>>>> + void *addr; >>>>>>> + >>>>>>> + if (PageUptodate(page)) >>>>>>> + return; >>>>>>> + >>>>>>> + addr = kmap_atomic(page); >>>>>>> + memcpy(addr, iomap->inline_data, size); >>>>>>> + memset(addr + size, 0, PAGE_SIZE - size); >>>>>> >>>>>> need flush_dcache_page(page) here for new page cache page since >>>>>> it's generic iomap code (althrough not necessary for x86, arm), I am not sure... >>>>>> see commit d2b2c6dd227b and c01778001a4f... >>>>> >>>>> Thanks for your reminding, these all codes were copied from >>>>> iomap_read_inline_data(), so I think we need a separated patch to fix this issue >>>>> if necessary. >>>> >>>> Yes, just a reminder, it is good as it-is. >>> >>> Not sure if that means that IOMAP_INLINE as is works for you now. In >>> any case, if the inline data isn't transparently copied into the page >>> cache at index 0, memory-mapped I/O isn't going to work. >>> >>> The code further assumes that "packed" files consist of exactly one >>> IOMAP_INLINE mapping; no IOMAP_MAPPED or other mappings may follow. Is >>> it that assumption that's causing you trouble? If so, what's the >> >> Yes, that's the problem we met. >> >>> layout at the filesystem level you want to support? >> >> The layout which can support tail-merge one, it means if the data locating at >> the tail of file has small enough size, we can inline the tail data into >> metadata area. e.g.: >> >> IOMAP_MAPPED [0, 8192] >> IOMAP_INLINE (or maybe IOMAP_TAIL) [8192, 8200] > > Ah, now I see. Let's generalize the IOMAP_INLINE code to support that > and rename it to IOMAP_TAIL then. Okay, it's fine to me, let me refactor the RFC patch. ;) Thanks, > > Thanks, > Andreas > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-06-29 7:30 [PATCH RFC] iomap: introduce IOMAP_TAIL Chao Yu 2019-06-29 9:34 ` Gao Xiang @ 2019-06-30 23:19 ` Darrick J. Wong 2019-07-01 6:08 ` Christoph Hellwig 2019-07-01 7:28 ` Chao Yu 1 sibling, 2 replies; 12+ messages in thread From: Darrick J. Wong @ 2019-06-30 23:19 UTC (permalink / raw) To: Chao Yu; +Cc: hch, linux-xfs, linux-fsdevel, gaoxiang25, chao On Sat, Jun 29, 2019 at 03:30:20PM +0800, Chao Yu wrote: > Some filesystems like erofs/reiserfs have the ability to pack tail > data into metadata, however iomap framework can only support mapping > inline data with IOMAP_INLINE type, it restricts that: > - inline data should be locating at page #0. > - inline size should equal to .i_size Wouldn't it be easier simply to fix the meaning of IOMAP_INLINE so that it can be used at something other than offset 0 and length == isize? IOWs, make it mean "use the *inline_data pointer to read/write data as a direct memory access"? I also don't really like the idea of leaving the write paths unimplemented in core code, though I suppose as an erofs developer you're not likely to have a good means for testing... :/ /me starts wondering if a better solution would be to invent iomaptestfs which exists solely to test all iomap code with as little other intelligence as possible... --D > So we can not use IOMAP_INLINE to handle tail-packing case. > > This patch introduces new mapping type IOMAP_TAIL to map tail-packed > data for further use of erofs. > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fs/iomap.c | 22 ++++++++++++++++++++++ > include/linux/iomap.h | 1 + > 2 files changed, 23 insertions(+) > > diff --git a/fs/iomap.c b/fs/iomap.c > index 12654c2e78f8..ae7777ce77d0 100644 > --- a/fs/iomap.c > +++ b/fs/iomap.c > @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, > SetPageUptodate(page); > } > > +static void > +iomap_read_tail_data(struct inode *inode, struct page *page, > + struct iomap *iomap) > +{ > + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); > + void *addr; > + > + if (PageUptodate(page)) > + return; > + > + addr = kmap_atomic(page); > + memcpy(addr, iomap->inline_data, size); > + memset(addr + size, 0, PAGE_SIZE - size); > + kunmap_atomic(addr); > + SetPageUptodate(page); > +} > + > static loff_t > iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > struct iomap *iomap) > @@ -298,6 +315,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > return PAGE_SIZE; > } > > + if (iomap->type == IOMAP_TAIL) { > + iomap_read_tail_data(inode, page, iomap); > + return PAGE_SIZE; > + } > + > /* zero post-eof blocks as the page may be mapped */ > iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); > if (plen == 0) > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index 2103b94cb1bf..7e1ee48e3db7 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -25,6 +25,7 @@ struct vm_fault; > #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */ > #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */ > #define IOMAP_INLINE 0x05 /* data inline in the inode */ > +#define IOMAP_TAIL 0x06 /* tail data packed in metdata */ > > /* > * Flags for all iomap mappings: > -- > 2.18.0.rc1 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-06-30 23:19 ` Darrick J. Wong @ 2019-07-01 6:08 ` Christoph Hellwig 2019-07-01 7:38 ` Chao Yu 2019-07-01 7:28 ` Chao Yu 1 sibling, 1 reply; 12+ messages in thread From: Christoph Hellwig @ 2019-07-01 6:08 UTC (permalink / raw) To: Darrick J. Wong Cc: Chao Yu, hch, linux-xfs, linux-fsdevel, gaoxiang25, chao, Andreas Gruenbacher On Sun, Jun 30, 2019 at 04:19:32PM -0700, Darrick J. Wong wrote: > On Sat, Jun 29, 2019 at 03:30:20PM +0800, Chao Yu wrote: > > Some filesystems like erofs/reiserfs have the ability to pack tail > > data into metadata, however iomap framework can only support mapping > > inline data with IOMAP_INLINE type, it restricts that: > > - inline data should be locating at page #0. > > - inline size should equal to .i_size > > Wouldn't it be easier simply to fix the meaning of IOMAP_INLINE so that > it can be used at something other than offset 0 and length == isize? > IOWs, make it mean "use the *inline_data pointer to read/write data > as a direct memory access"? Yes. I vaguely remember Andreas pointed out some issues with a general scheme, which is why we put the limits in. If that is not just me making things up we'll need to address them. Either way just copy and pasting code isn't very helpful. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-07-01 6:08 ` Christoph Hellwig @ 2019-07-01 7:38 ` Chao Yu 0 siblings, 0 replies; 12+ messages in thread From: Chao Yu @ 2019-07-01 7:38 UTC (permalink / raw) To: Christoph Hellwig, Andreas Gruenbacher Cc: Darrick J. Wong, linux-xfs, linux-fsdevel, gaoxiang25, chao On 2019/7/1 14:08, Christoph Hellwig wrote: > On Sun, Jun 30, 2019 at 04:19:32PM -0700, Darrick J. Wong wrote: >> On Sat, Jun 29, 2019 at 03:30:20PM +0800, Chao Yu wrote: >>> Some filesystems like erofs/reiserfs have the ability to pack tail >>> data into metadata, however iomap framework can only support mapping >>> inline data with IOMAP_INLINE type, it restricts that: >>> - inline data should be locating at page #0. >>> - inline size should equal to .i_size >> >> Wouldn't it be easier simply to fix the meaning of IOMAP_INLINE so that >> it can be used at something other than offset 0 and length == isize? >> IOWs, make it mean "use the *inline_data pointer to read/write data >> as a direct memory access"? > > Yes. I vaguely remember Andreas pointed out some issues with a @Andreas, could you please help to explain which issue we may encounter without those limits? Thanks, > general scheme, which is why we put the limits in. If that is not just > me making things up we'll need to address them. Either way just copy > and pasting code isn't very helpful. > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] iomap: introduce IOMAP_TAIL 2019-06-30 23:19 ` Darrick J. Wong 2019-07-01 6:08 ` Christoph Hellwig @ 2019-07-01 7:28 ` Chao Yu 1 sibling, 0 replies; 12+ messages in thread From: Chao Yu @ 2019-07-01 7:28 UTC (permalink / raw) To: Darrick J. Wong; +Cc: hch, linux-xfs, linux-fsdevel, gaoxiang25, chao On 2019/7/1 7:19, Darrick J. Wong wrote: > On Sat, Jun 29, 2019 at 03:30:20PM +0800, Chao Yu wrote: >> Some filesystems like erofs/reiserfs have the ability to pack tail >> data into metadata, however iomap framework can only support mapping >> inline data with IOMAP_INLINE type, it restricts that: >> - inline data should be locating at page #0. >> - inline size should equal to .i_size > > Wouldn't it be easier simply to fix the meaning of IOMAP_INLINE so that > it can be used at something other than offset 0 and length == isize? > IOWs, make it mean "use the *inline_data pointer to read/write data > as a direct memory access"? I thought about that, finally I implemented this by adding a new type because: - I checked fs.h, noticing that there are two separated flags: FS_INLINE_DATA_FL and FS_NOTAIL_FL, I guess they are separated features, but not sure about it... - If we keep those restriction of inline data, maybe we can help to do sanity check on inline data for most inline function callers additionally, since most filesystems implement inline_data feature with restriction by default. Anyway, I can change IOMAP_INLINE's restriction, and adjust erofs to use it if there is no further concern on those restrictions. > > I also don't really like the idea of leaving the write paths > unimplemented in core code, though I suppose as an erofs developer > you're not likely to have a good means for testing... :/ Yes, I don't like it too, but previously I didn't add it because I'm not sure that, recently we have potential user of IOMAP_TAIL's write path except reiserfs, it may be out-of-{maintain,test} due to lack of user later.... > > /me starts wondering if a better solution would be to invent iomaptestfs > which exists solely to test all iomap code with as little other > intelligence as possible... Good idea, any plan on this fs? :) Now, for erofs, as we don't support mapping hole, so I just inject code to force covering IOMAP_HOLE path. :P Thanks, > > --D > >> So we can not use IOMAP_INLINE to handle tail-packing case. >> >> This patch introduces new mapping type IOMAP_TAIL to map tail-packed >> data for further use of erofs. >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> fs/iomap.c | 22 ++++++++++++++++++++++ >> include/linux/iomap.h | 1 + >> 2 files changed, 23 insertions(+) >> >> diff --git a/fs/iomap.c b/fs/iomap.c >> index 12654c2e78f8..ae7777ce77d0 100644 >> --- a/fs/iomap.c >> +++ b/fs/iomap.c >> @@ -280,6 +280,23 @@ iomap_read_inline_data(struct inode *inode, struct page *page, >> SetPageUptodate(page); >> } >> >> +static void >> +iomap_read_tail_data(struct inode *inode, struct page *page, >> + struct iomap *iomap) >> +{ >> + size_t size = i_size_read(inode) & (PAGE_SIZE - 1); >> + void *addr; >> + >> + if (PageUptodate(page)) >> + return; >> + >> + addr = kmap_atomic(page); >> + memcpy(addr, iomap->inline_data, size); >> + memset(addr + size, 0, PAGE_SIZE - size); >> + kunmap_atomic(addr); >> + SetPageUptodate(page); >> +} >> + >> static loff_t >> iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, >> struct iomap *iomap) >> @@ -298,6 +315,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, >> return PAGE_SIZE; >> } >> >> + if (iomap->type == IOMAP_TAIL) { >> + iomap_read_tail_data(inode, page, iomap); >> + return PAGE_SIZE; >> + } >> + >> /* zero post-eof blocks as the page may be mapped */ >> iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); >> if (plen == 0) >> diff --git a/include/linux/iomap.h b/include/linux/iomap.h >> index 2103b94cb1bf..7e1ee48e3db7 100644 >> --- a/include/linux/iomap.h >> +++ b/include/linux/iomap.h >> @@ -25,6 +25,7 @@ struct vm_fault; >> #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */ >> #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */ >> #define IOMAP_INLINE 0x05 /* data inline in the inode */ >> +#define IOMAP_TAIL 0x06 /* tail data packed in metdata */ >> >> /* >> * Flags for all iomap mappings: >> -- >> 2.18.0.rc1 >> > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-07-01 10:22 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-06-29 7:30 [PATCH RFC] iomap: introduce IOMAP_TAIL Chao Yu 2019-06-29 9:34 ` Gao Xiang 2019-07-01 6:40 ` Chao Yu 2019-07-01 7:03 ` Gao Xiang 2019-07-01 9:49 ` Andreas Grünbacher 2019-07-01 10:07 ` Chao Yu 2019-07-01 10:13 ` Andreas Grünbacher 2019-07-01 10:22 ` Chao Yu 2019-06-30 23:19 ` Darrick J. Wong 2019-07-01 6:08 ` Christoph Hellwig 2019-07-01 7:38 ` Chao Yu 2019-07-01 7:28 ` Chao Yu
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).