* [PATCH v2 0/5] iomap & xfs support for large pages @ 2019-08-21 0:30 Matthew Wilcox 2019-08-21 0:30 ` [PATCH v2 1/5] fs: Introduce i_blocks_per_page Matthew Wilcox ` (4 more replies) 0 siblings, 5 replies; 19+ messages in thread From: Matthew Wilcox @ 2019-08-21 0:30 UTC (permalink / raw) To: linux-fsdevel; +Cc: Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm From: "Matthew Wilcox (Oracle)" <willy@infradead.org> In order to support large pages in the page cache, filesystems have to understand that they're being passed a large page and read or write the entire large page, rather than just the first page. This pair of patches adds that support to XFS. Still untested beyond compilation. v2: - Added a few helpers per Dave Chinner's suggestions - Use GFP_ZERO instead of individually zeroing each field of iop - Rewrite iomap_set_range_uptodate() to use bitmap functions instead of individual bit operations - Drop support for large pages being used for files with inline data (it didn't work anyway, because kmap_atomic() is only going to map the first page of a compound page) - Pass a struct page to xfs_finish_page_writeback instead of the bvec Matthew Wilcox (Oracle) (5): fs: Introduce i_blocks_per_page mm: Add file_offset_of_ helpers iomap: Support large pages xfs: Support large pages xfs: Pass a page to xfs_finish_page_writeback fs/iomap/buffered-io.c | 121 ++++++++++++++++++++++++++-------------- fs/jfs/jfs_metapage.c | 2 +- fs/xfs/xfs_aops.c | 37 ++++++------ include/linux/iomap.h | 2 +- include/linux/mm.h | 2 + include/linux/pagemap.h | 38 ++++++++++++- 6 files changed, 135 insertions(+), 67 deletions(-) -- 2.23.0.rc1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/5] fs: Introduce i_blocks_per_page 2019-08-21 0:30 [PATCH v2 0/5] iomap & xfs support for large pages Matthew Wilcox @ 2019-08-21 0:30 ` Matthew Wilcox 2019-08-23 12:26 ` kbuild test robot 2019-09-18 21:14 ` Darrick J. Wong 2019-08-21 0:30 ` [PATCH v2 2/5] mm: Add file_offset_of_ helpers Matthew Wilcox ` (3 subsequent siblings) 4 siblings, 2 replies; 19+ messages in thread From: Matthew Wilcox @ 2019-08-21 0:30 UTC (permalink / raw) To: linux-fsdevel; +Cc: Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm From: "Matthew Wilcox (Oracle)" <willy@infradead.org> This helper is useful for both large pages in the page cache and for supporting block size larger than page size. Convert some example users (we have a few different ways of writing this idiom). Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/iomap/buffered-io.c | 4 ++-- fs/jfs/jfs_metapage.c | 2 +- fs/xfs/xfs_aops.c | 8 ++++---- include/linux/pagemap.h | 13 +++++++++++++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index e25901ae3ff4..0e76a4b6d98a 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -24,7 +24,7 @@ iomap_page_create(struct inode *inode, struct page *page) { struct iomap_page *iop = to_iomap_page(page); - if (iop || i_blocksize(inode) == PAGE_SIZE) + if (iop || i_blocks_per_page(inode, page) <= 1) return iop; iop = kmalloc(sizeof(*iop), GFP_NOFS | __GFP_NOFAIL); @@ -128,7 +128,7 @@ iomap_set_range_uptodate(struct page *page, unsigned off, unsigned len) bool uptodate = true; if (iop) { - for (i = 0; i < PAGE_SIZE / i_blocksize(inode); i++) { + for (i = 0; i < i_blocks_per_page(inode, page); i++) { if (i >= first && i <= last) set_bit(i, iop->uptodate); else if (!test_bit(i, iop->uptodate)) diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c index a2f5338a5ea1..176580f54af9 100644 --- a/fs/jfs/jfs_metapage.c +++ b/fs/jfs/jfs_metapage.c @@ -473,7 +473,7 @@ static int metapage_readpage(struct file *fp, struct page *page) struct inode *inode = page->mapping->host; struct bio *bio = NULL; int block_offset; - int blocks_per_page = PAGE_SIZE >> inode->i_blkbits; + int blocks_per_page = i_blocks_per_page(inode, page); sector_t page_start; /* address of page in fs blocks */ sector_t pblock; int xlen; diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c index f16d5f196c6b..102cfd8a97d6 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -68,7 +68,7 @@ xfs_finish_page_writeback( mapping_set_error(inode->i_mapping, -EIO); } - ASSERT(iop || i_blocksize(inode) == PAGE_SIZE); + ASSERT(iop || i_blocks_per_page(inode, bvec->bv_page) <= 1); ASSERT(!iop || atomic_read(&iop->write_count) > 0); if (!iop || atomic_dec_and_test(&iop->write_count)) @@ -839,7 +839,7 @@ xfs_aops_discard_page( page, ip->i_ino, offset); error = xfs_bmap_punch_delalloc_range(ip, start_fsb, - PAGE_SIZE / i_blocksize(inode)); + i_blocks_per_page(inode, page)); if (error && !XFS_FORCED_SHUTDOWN(mp)) xfs_alert(mp, "page discard unable to remove delalloc mapping."); out_invalidate: @@ -877,7 +877,7 @@ xfs_writepage_map( uint64_t file_offset; /* file offset of page */ int error = 0, count = 0, i; - ASSERT(iop || i_blocksize(inode) == PAGE_SIZE); + ASSERT(iop || i_blocks_per_page(inode, page) <= 1); ASSERT(!iop || atomic_read(&iop->write_count) == 0); /* @@ -886,7 +886,7 @@ xfs_writepage_map( * one. */ for (i = 0, file_offset = page_offset(page); - i < (PAGE_SIZE >> inode->i_blkbits) && file_offset < end_offset; + i < i_blocks_per_page(inode, page) && file_offset < end_offset; i++, file_offset += len) { if (iop && !test_bit(i, iop->uptodate)) continue; diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index cf837d313b96..2728f20fbc49 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -644,4 +644,17 @@ static inline unsigned long dir_pages(struct inode *inode) PAGE_SHIFT; } +/** + * i_blocks_per_page - How many blocks fit in this page. + * @inode: The inode which contains the blocks. + * @page: The (potentially large) page. + * + * Context: Any context. + * Return: The number of filesystem blocks covered by this page. + */ +static inline +unsigned int i_blocks_per_page(struct inode *inode, struct page *page) +{ + return page_size(page) >> inode->i_blkbits; +} #endif /* _LINUX_PAGEMAP_H */ -- 2.23.0.rc1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/5] fs: Introduce i_blocks_per_page 2019-08-21 0:30 ` [PATCH v2 1/5] fs: Introduce i_blocks_per_page Matthew Wilcox @ 2019-08-23 12:26 ` kbuild test robot 2019-09-18 21:14 ` Darrick J. Wong 1 sibling, 0 replies; 19+ messages in thread From: kbuild test robot @ 2019-08-23 12:26 UTC (permalink / raw) To: Matthew Wilcox Cc: kbuild-all, linux-fsdevel, Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm [-- Attachment #1: Type: text/plain, Size: 5975 bytes --] Hi Matthew, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3-rc5 next-20190823] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Matthew-Wilcox/iomap-xfs-support-for-large-pages/20190823-191138 config: c6x-allyesconfig (attached as .config) compiler: c6x-elf-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=c6x If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from include/linux/blkdev.h:16:0, from include/linux/blk-cgroup.h:21, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/net/sock.h:53, from net//tipc/socket.h:38, from net//tipc/core.c:44: include/linux/pagemap.h: In function 'i_blocks_per_page': >> include/linux/pagemap.h:640:9: error: implicit declaration of function 'page_size'; did you mean 'msg_size'? [-Werror=implicit-function-declaration] return page_size(page) >> inode->i_blkbits; ^~~~~~~~~ msg_size cc1: some warnings being treated as errors -- In file included from include/linux/blkdev.h:16:0, from include/linux/blk-cgroup.h:21, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/net/sock.h:53, from net//tipc/socket.h:38, from net//tipc/trace.h:45, from net//tipc/trace.c:37: include/linux/pagemap.h: In function 'i_blocks_per_page': >> include/linux/pagemap.h:640:9: error: implicit declaration of function 'page_size'; did you mean 'msg_size'? [-Werror=implicit-function-declaration] return page_size(page) >> inode->i_blkbits; ^~~~~~~~~ msg_size In file included from net//tipc/trace.h:431:0, from net//tipc/trace.c:37: include/trace/define_trace.h: At top level: include/trace/define_trace.h:95:42: fatal error: ./trace.h: No such file or directory #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) ^ cc1: some warnings being treated as errors compilation terminated. -- In file included from include/linux/blkdev.h:16:0, from include/linux/blk-cgroup.h:21, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/net/sock.h:53, from include/linux/tcp.h:19, from include/linux/ipv6.h:87, from include/net/ipv6.h:12, from include/rdma/ib_verbs.h:51, from include/linux/lsm_audit.h:25, from security//apparmor/include/audit.h:16, from security//apparmor/include/policy.h:23, from security//apparmor/include/policy_ns.h:19, from security//apparmor/include/cred.h:19, from security//apparmor/task.c:15: include/linux/pagemap.h: In function 'i_blocks_per_page': >> include/linux/pagemap.h:640:9: error: implicit declaration of function 'page_size'; did you mean 'table_size'? [-Werror=implicit-function-declaration] return page_size(page) >> inode->i_blkbits; ^~~~~~~~~ table_size cc1: some warnings being treated as errors -- In file included from include/linux/blkdev.h:16:0, from include/linux/blk-cgroup.h:21, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/net/sock.h:53, from include/linux/tcp.h:19, from include/linux/ipv6.h:87, from include/net/ipv6.h:12, from include/rdma/ib_verbs.h:51, from include/linux/lsm_audit.h:25, from security//apparmor/include/audit.h:16, from security//apparmor/include/policy.h:23, from security//apparmor/include/policy_ns.h:19, from security//apparmor/include/cred.h:19, from security//apparmor/capability.c:18: include/linux/pagemap.h: In function 'i_blocks_per_page': >> include/linux/pagemap.h:640:9: error: implicit declaration of function 'page_size'; did you mean 'table_size'? [-Werror=implicit-function-declaration] return page_size(page) >> inode->i_blkbits; ^~~~~~~~~ table_size security//apparmor/capability.c: At top level: security//apparmor/capability.c:25:10: fatal error: capability_names.h: No such file or directory #include "capability_names.h" ^~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors compilation terminated. vim +640 include/linux/pagemap.h 628 629 /** 630 * i_blocks_per_page - How many blocks fit in this page. 631 * @inode: The inode which contains the blocks. 632 * @page: The (potentially large) page. 633 * 634 * Context: Any context. 635 * Return: The number of filesystem blocks covered by this page. 636 */ 637 static inline 638 unsigned int i_blocks_per_page(struct inode *inode, struct page *page) 639 { > 640 return page_size(page) >> inode->i_blkbits; --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 49703 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/5] fs: Introduce i_blocks_per_page 2019-08-21 0:30 ` [PATCH v2 1/5] fs: Introduce i_blocks_per_page Matthew Wilcox 2019-08-23 12:26 ` kbuild test robot @ 2019-09-18 21:14 ` Darrick J. Wong 2019-09-18 23:48 ` Matthew Wilcox 1 sibling, 1 reply; 19+ messages in thread From: Darrick J. Wong @ 2019-09-18 21:14 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-fsdevel, hch, linux-xfs, linux-mm On Tue, Aug 20, 2019 at 05:30:35PM -0700, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > This helper is useful for both large pages in the page cache and for > supporting block size larger than page size. Convert some example > users (we have a few different ways of writing this idiom). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Seems pretty straightforward, modulo whatever's going on with the kbuild robot complaint (is there something wrong, or is it just that obnoxious header check thing?) Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/iomap/buffered-io.c | 4 ++-- > fs/jfs/jfs_metapage.c | 2 +- > fs/xfs/xfs_aops.c | 8 ++++---- > include/linux/pagemap.h | 13 +++++++++++++ > 4 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index e25901ae3ff4..0e76a4b6d98a 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -24,7 +24,7 @@ iomap_page_create(struct inode *inode, struct page *page) > { > struct iomap_page *iop = to_iomap_page(page); > > - if (iop || i_blocksize(inode) == PAGE_SIZE) > + if (iop || i_blocks_per_page(inode, page) <= 1) > return iop; > > iop = kmalloc(sizeof(*iop), GFP_NOFS | __GFP_NOFAIL); > @@ -128,7 +128,7 @@ iomap_set_range_uptodate(struct page *page, unsigned off, unsigned len) > bool uptodate = true; > > if (iop) { > - for (i = 0; i < PAGE_SIZE / i_blocksize(inode); i++) { > + for (i = 0; i < i_blocks_per_page(inode, page); i++) { > if (i >= first && i <= last) > set_bit(i, iop->uptodate); > else if (!test_bit(i, iop->uptodate)) > diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c > index a2f5338a5ea1..176580f54af9 100644 > --- a/fs/jfs/jfs_metapage.c > +++ b/fs/jfs/jfs_metapage.c > @@ -473,7 +473,7 @@ static int metapage_readpage(struct file *fp, struct page *page) > struct inode *inode = page->mapping->host; > struct bio *bio = NULL; > int block_offset; > - int blocks_per_page = PAGE_SIZE >> inode->i_blkbits; > + int blocks_per_page = i_blocks_per_page(inode, page); > sector_t page_start; /* address of page in fs blocks */ > sector_t pblock; > int xlen; > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index f16d5f196c6b..102cfd8a97d6 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -68,7 +68,7 @@ xfs_finish_page_writeback( > mapping_set_error(inode->i_mapping, -EIO); > } > > - ASSERT(iop || i_blocksize(inode) == PAGE_SIZE); > + ASSERT(iop || i_blocks_per_page(inode, bvec->bv_page) <= 1); > ASSERT(!iop || atomic_read(&iop->write_count) > 0); > > if (!iop || atomic_dec_and_test(&iop->write_count)) > @@ -839,7 +839,7 @@ xfs_aops_discard_page( > page, ip->i_ino, offset); > > error = xfs_bmap_punch_delalloc_range(ip, start_fsb, > - PAGE_SIZE / i_blocksize(inode)); > + i_blocks_per_page(inode, page)); > if (error && !XFS_FORCED_SHUTDOWN(mp)) > xfs_alert(mp, "page discard unable to remove delalloc mapping."); > out_invalidate: > @@ -877,7 +877,7 @@ xfs_writepage_map( > uint64_t file_offset; /* file offset of page */ > int error = 0, count = 0, i; > > - ASSERT(iop || i_blocksize(inode) == PAGE_SIZE); > + ASSERT(iop || i_blocks_per_page(inode, page) <= 1); > ASSERT(!iop || atomic_read(&iop->write_count) == 0); > > /* > @@ -886,7 +886,7 @@ xfs_writepage_map( > * one. > */ > for (i = 0, file_offset = page_offset(page); > - i < (PAGE_SIZE >> inode->i_blkbits) && file_offset < end_offset; > + i < i_blocks_per_page(inode, page) && file_offset < end_offset; > i++, file_offset += len) { > if (iop && !test_bit(i, iop->uptodate)) > continue; > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index cf837d313b96..2728f20fbc49 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -644,4 +644,17 @@ static inline unsigned long dir_pages(struct inode *inode) > PAGE_SHIFT; > } > > +/** > + * i_blocks_per_page - How many blocks fit in this page. > + * @inode: The inode which contains the blocks. > + * @page: The (potentially large) page. > + * > + * Context: Any context. > + * Return: The number of filesystem blocks covered by this page. > + */ > +static inline > +unsigned int i_blocks_per_page(struct inode *inode, struct page *page) > +{ > + return page_size(page) >> inode->i_blkbits; > +} > #endif /* _LINUX_PAGEMAP_H */ > -- > 2.23.0.rc1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/5] fs: Introduce i_blocks_per_page 2019-09-18 21:14 ` Darrick J. Wong @ 2019-09-18 23:48 ` Matthew Wilcox 0 siblings, 0 replies; 19+ messages in thread From: Matthew Wilcox @ 2019-09-18 23:48 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-fsdevel, hch, linux-xfs, linux-mm On Wed, Sep 18, 2019 at 02:14:39PM -0700, Darrick J. Wong wrote: > On Tue, Aug 20, 2019 at 05:30:35PM -0700, Matthew Wilcox wrote: > > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > > > This helper is useful for both large pages in the page cache and for > > supporting block size larger than page size. Convert some example > > users (we have a few different ways of writing this idiom). > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > Seems pretty straightforward, modulo whatever's going on with the kbuild > robot complaint (is there something wrong, or is it just that obnoxious > header check thing?) It doesn't apply patches on top of the -mm tree for some reason. So it has no idea about the page_size() macro that's sitting in -mm at the moment. > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/5] mm: Add file_offset_of_ helpers 2019-08-21 0:30 [PATCH v2 0/5] iomap & xfs support for large pages Matthew Wilcox 2019-08-21 0:30 ` [PATCH v2 1/5] fs: Introduce i_blocks_per_page Matthew Wilcox @ 2019-08-21 0:30 ` Matthew Wilcox 2019-08-23 12:49 ` kbuild test robot ` (2 more replies) 2019-08-21 0:30 ` [PATCH v2 3/5] iomap: Support large pages Matthew Wilcox ` (2 subsequent siblings) 4 siblings, 3 replies; 19+ messages in thread From: Matthew Wilcox @ 2019-08-21 0:30 UTC (permalink / raw) To: linux-fsdevel; +Cc: Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm From: "Matthew Wilcox (Oracle)" <willy@infradead.org> The page_offset function is badly named for people reading the functions which call it. The natural meaning of a function with this name would be 'offset within a page', not 'page offset in bytes within a file'. Dave Chinner suggests file_offset_of_page() as a replacement function name and I'm also adding file_offset_of_next_page() as a helper for the large page work. Also add kernel-doc for these functions so they show up in the kernel API book. page_offset() is retained as a compatibility define for now. --- include/linux/pagemap.h | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 2728f20fbc49..84f341109710 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -436,14 +436,33 @@ static inline pgoff_t page_to_pgoff(struct page *page) return page_to_index(page); } -/* - * Return byte-offset into filesystem object for page. +/** + * file_offset_of_page - File offset of this page. + * @page: Page cache page. + * + * Context: Any context. + * Return: The offset of the first byte of this page. */ -static inline loff_t page_offset(struct page *page) +static inline loff_t file_offset_of_page(struct page *page) { return ((loff_t)page->index) << PAGE_SHIFT; } +/* Legacy; please convert callers */ +#define page_offset(page) file_offset_of_page(page) + +/** + * file_offset_of_next_page - File offset of the next page. + * @page: Page cache page. + * + * Context: Any context. + * Return: The offset of the first byte after this page. + */ +static inline loff_t file_offset_of_next_page(struct page *page) +{ + return ((loff_t)page->index + compound_nr(page)) << PAGE_SHIFT; +} + static inline loff_t page_file_offset(struct page *page) { return ((loff_t)page_index(page)) << PAGE_SHIFT; -- 2.23.0.rc1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] mm: Add file_offset_of_ helpers 2019-08-21 0:30 ` [PATCH v2 2/5] mm: Add file_offset_of_ helpers Matthew Wilcox @ 2019-08-23 12:49 ` kbuild test robot 2019-08-24 11:48 ` kbuild test robot 2019-09-18 21:17 ` Darrick J. Wong 2 siblings, 0 replies; 19+ messages in thread From: kbuild test robot @ 2019-08-23 12:49 UTC (permalink / raw) To: Matthew Wilcox Cc: kbuild-all, linux-fsdevel, Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm [-- Attachment #1: Type: text/plain, Size: 2696 bytes --] Hi Matthew, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3-rc5 next-20190823] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Matthew-Wilcox/iomap-xfs-support-for-large-pages/20190823-191138 config: i386-defconfig (attached as .config) compiler: gcc-7 (Debian 7.4.0-10) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from include/linux/blkdev.h:16:0, from include/linux/blk-cgroup.h:21, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/linux/swap.h:9, from include/linux/suspend.h:5, from arch/x86/kernel/asm-offsets.c:13: include/linux/pagemap.h: In function 'file_offset_of_next_page': >> include/linux/pagemap.h:445:32: error: implicit declaration of function 'compound_nr'; did you mean 'compound_order'? [-Werror=implicit-function-declaration] return ((loff_t)page->index + compound_nr(page)) << PAGE_SHIFT; ^~~~~~~~~~~ compound_order include/linux/pagemap.h: In function 'i_blocks_per_page': include/linux/pagemap.h:659:9: error: implicit declaration of function 'page_size'; did you mean 'page_zone'? [-Werror=implicit-function-declaration] return page_size(page) >> inode->i_blkbits; ^~~~~~~~~ page_zone cc1: some warnings being treated as errors make[2]: *** [arch/x86/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [sub-make] Error 2 16 real 6 user 2 sys 56.01% cpu make prepare vim +445 include/linux/pagemap.h 435 436 /** 437 * file_offset_of_next_page - File offset of the next page. 438 * @page: Page cache page. 439 * 440 * Context: Any context. 441 * Return: The offset of the first byte after this page. 442 */ 443 static inline loff_t file_offset_of_next_page(struct page *page) 444 { > 445 return ((loff_t)page->index + compound_nr(page)) << PAGE_SHIFT; 446 } 447 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 28074 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] mm: Add file_offset_of_ helpers 2019-08-21 0:30 ` [PATCH v2 2/5] mm: Add file_offset_of_ helpers Matthew Wilcox 2019-08-23 12:49 ` kbuild test robot @ 2019-08-24 11:48 ` kbuild test robot 2019-08-24 15:28 ` Matthew Wilcox 2019-09-18 21:17 ` Darrick J. Wong 2 siblings, 1 reply; 19+ messages in thread From: kbuild test robot @ 2019-08-24 11:48 UTC (permalink / raw) To: Matthew Wilcox Cc: kbuild-all, linux-fsdevel, Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm [-- Attachment #1: Type: text/plain, Size: 2284 bytes --] Hi Matthew, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [cannot apply to v5.3-rc5 next-20190823] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Matthew-Wilcox/iomap-xfs-support-for-large-pages/20190823-191138 config: x86_64-randconfig-f001-201933 (attached as .config) compiler: gcc-7 (Debian 7.4.0-10) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from arch/x86/include/asm/pgalloc.h:7:0, from include/asm-generic/tlb.h:16, from arch/x86/include/asm/tlb.h:12, from arch/x86/include/asm/efi.h:8, from drivers/firmware/efi/libstub/tpm.c:12: include/linux/pagemap.h: In function 'file_offset_of_next_page': >> include/linux/pagemap.h:445:32: warning: implicit declaration of function 'compound_nr'; did you mean 'compound_order'? [-Wimplicit-function-declaration] return ((loff_t)page->index + compound_nr(page)) << PAGE_SHIFT; ^~~~~~~~~~~ compound_order include/linux/pagemap.h: In function 'i_blocks_per_page': include/linux/pagemap.h:659:9: warning: implicit declaration of function 'page_size'; did you mean 'page_zone'? [-Wimplicit-function-declaration] return page_size(page) >> inode->i_blkbits; ^~~~~~~~~ page_zone vim +445 include/linux/pagemap.h 435 436 /** 437 * file_offset_of_next_page - File offset of the next page. 438 * @page: Page cache page. 439 * 440 * Context: Any context. 441 * Return: The offset of the first byte after this page. 442 */ 443 static inline loff_t file_offset_of_next_page(struct page *page) 444 { > 445 return ((loff_t)page->index + compound_nr(page)) << PAGE_SHIFT; 446 } 447 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 31936 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] mm: Add file_offset_of_ helpers 2019-08-24 11:48 ` kbuild test robot @ 2019-08-24 15:28 ` Matthew Wilcox 0 siblings, 0 replies; 19+ messages in thread From: Matthew Wilcox @ 2019-08-24 15:28 UTC (permalink / raw) To: kbuild test robot; +Cc: kbuild-all, linux-fsdevel, hch, linux-xfs, linux-mm On Sat, Aug 24, 2019 at 07:48:24PM +0800, kbuild test robot wrote: > Hi Matthew, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on linus/master] > [cannot apply to v5.3-rc5 next-20190823] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] It depends on various patches which are in -next, although I didn't generate them against -next. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] mm: Add file_offset_of_ helpers 2019-08-21 0:30 ` [PATCH v2 2/5] mm: Add file_offset_of_ helpers Matthew Wilcox 2019-08-23 12:49 ` kbuild test robot 2019-08-24 11:48 ` kbuild test robot @ 2019-09-18 21:17 ` Darrick J. Wong 2019-09-18 23:49 ` Matthew Wilcox 2 siblings, 1 reply; 19+ messages in thread From: Darrick J. Wong @ 2019-09-18 21:17 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-fsdevel, hch, linux-xfs, linux-mm On Tue, Aug 20, 2019 at 05:30:36PM -0700, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > The page_offset function is badly named for people reading the functions > which call it. The natural meaning of a function with this name would > be 'offset within a page', not 'page offset in bytes within a file'. > Dave Chinner suggests file_offset_of_page() as a replacement function > name and I'm also adding file_offset_of_next_page() as a helper for the > large page work. Also add kernel-doc for these functions so they show > up in the kernel API book. > > page_offset() is retained as a compatibility define for now. No SOB? Looks fine to me, and I appreciate the much less confusing name. I was hoping for a page_offset conversion for fs/iomap/ (and not a treewide change because yuck), but I guess that can be done if and when this lands. --D > --- > include/linux/pagemap.h | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 2728f20fbc49..84f341109710 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -436,14 +436,33 @@ static inline pgoff_t page_to_pgoff(struct page *page) > return page_to_index(page); > } > > -/* > - * Return byte-offset into filesystem object for page. > +/** > + * file_offset_of_page - File offset of this page. > + * @page: Page cache page. > + * > + * Context: Any context. > + * Return: The offset of the first byte of this page. > */ > -static inline loff_t page_offset(struct page *page) > +static inline loff_t file_offset_of_page(struct page *page) > { > return ((loff_t)page->index) << PAGE_SHIFT; > } > > +/* Legacy; please convert callers */ > +#define page_offset(page) file_offset_of_page(page) > + > +/** > + * file_offset_of_next_page - File offset of the next page. > + * @page: Page cache page. > + * > + * Context: Any context. > + * Return: The offset of the first byte after this page. > + */ > +static inline loff_t file_offset_of_next_page(struct page *page) > +{ > + return ((loff_t)page->index + compound_nr(page)) << PAGE_SHIFT; > +} > + > static inline loff_t page_file_offset(struct page *page) > { > return ((loff_t)page_index(page)) << PAGE_SHIFT; > -- > 2.23.0.rc1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] mm: Add file_offset_of_ helpers 2019-09-18 21:17 ` Darrick J. Wong @ 2019-09-18 23:49 ` Matthew Wilcox 2019-09-19 0:04 ` Darrick J. Wong 0 siblings, 1 reply; 19+ messages in thread From: Matthew Wilcox @ 2019-09-18 23:49 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-fsdevel, hch, linux-xfs, linux-mm On Wed, Sep 18, 2019 at 02:17:55PM -0700, Darrick J. Wong wrote: > On Tue, Aug 20, 2019 at 05:30:36PM -0700, Matthew Wilcox wrote: > > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > > > The page_offset function is badly named for people reading the functions > > which call it. The natural meaning of a function with this name would > > be 'offset within a page', not 'page offset in bytes within a file'. > > Dave Chinner suggests file_offset_of_page() as a replacement function > > name and I'm also adding file_offset_of_next_page() as a helper for the > > large page work. Also add kernel-doc for these functions so they show > > up in the kernel API book. > > > > page_offset() is retained as a compatibility define for now. > > No SOB? > > Looks fine to me, and I appreciate the much less confusing name. I was > hoping for a page_offset conversion for fs/iomap/ (and not a treewide > change because yuck), but I guess that can be done if and when this > lands. Sure, I'll do that once everything else has landed. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/5] mm: Add file_offset_of_ helpers 2019-09-18 23:49 ` Matthew Wilcox @ 2019-09-19 0:04 ` Darrick J. Wong 0 siblings, 0 replies; 19+ messages in thread From: Darrick J. Wong @ 2019-09-19 0:04 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-fsdevel, hch, linux-xfs, linux-mm, Dave Chinner [add dave to cc] On Wed, Sep 18, 2019 at 04:49:24PM -0700, Matthew Wilcox wrote: > On Wed, Sep 18, 2019 at 02:17:55PM -0700, Darrick J. Wong wrote: > > On Tue, Aug 20, 2019 at 05:30:36PM -0700, Matthew Wilcox wrote: > > > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > > > > > The page_offset function is badly named for people reading the functions > > > which call it. The natural meaning of a function with this name would > > > be 'offset within a page', not 'page offset in bytes within a file'. > > > Dave Chinner suggests file_offset_of_page() as a replacement function > > > name and I'm also adding file_offset_of_next_page() as a helper for the > > > large page work. Also add kernel-doc for these functions so they show > > > up in the kernel API book. > > > > > > page_offset() is retained as a compatibility define for now. > > > > No SOB? > > > > Looks fine to me, and I appreciate the much less confusing name. I was > > hoping for a page_offset conversion for fs/iomap/ (and not a treewide > > change because yuck), but I guess that can be done if and when this > > lands. > > Sure, I'll do that once everything else has landed. You might also want to ask Dave Chinner what changes he's making to iomap to support blocksize > pagesize filesystems, since that's /definitely/ going to clash. :) --D ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 3/5] iomap: Support large pages 2019-08-21 0:30 [PATCH v2 0/5] iomap & xfs support for large pages Matthew Wilcox 2019-08-21 0:30 ` [PATCH v2 1/5] fs: Introduce i_blocks_per_page Matthew Wilcox 2019-08-21 0:30 ` [PATCH v2 2/5] mm: Add file_offset_of_ helpers Matthew Wilcox @ 2019-08-21 0:30 ` Matthew Wilcox 2019-08-23 12:48 ` kbuild test robot 2019-09-18 21:29 ` Darrick J. Wong 2019-08-21 0:30 ` [PATCH v2 4/5] xfs: " Matthew Wilcox 2019-08-21 0:30 ` [PATCH v2 5/5] xfs: Pass a page to xfs_finish_page_writeback Matthew Wilcox 4 siblings, 2 replies; 19+ messages in thread From: Matthew Wilcox @ 2019-08-21 0:30 UTC (permalink / raw) To: linux-fsdevel; +Cc: Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm From: "Matthew Wilcox (Oracle)" <willy@infradead.org> Change iomap_page from a statically sized uptodate bitmap to a dynamically allocated uptodate bitmap, allowing an arbitrarily large page. The only remaining places where iomap assumes an order-0 page are for files with inline data, where there's no sense in allocating a larger page. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/iomap/buffered-io.c | 119 ++++++++++++++++++++++++++--------------- include/linux/iomap.h | 2 +- include/linux/mm.h | 2 + 3 files changed, 80 insertions(+), 43 deletions(-) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 0e76a4b6d98a..15d844a88439 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -23,14 +23,14 @@ static struct iomap_page * iomap_page_create(struct inode *inode, struct page *page) { struct iomap_page *iop = to_iomap_page(page); + unsigned int n; if (iop || i_blocks_per_page(inode, page) <= 1) return iop; - iop = kmalloc(sizeof(*iop), GFP_NOFS | __GFP_NOFAIL); - atomic_set(&iop->read_count, 0); - atomic_set(&iop->write_count, 0); - bitmap_zero(iop->uptodate, PAGE_SIZE / SECTOR_SIZE); + n = BITS_TO_LONGS(i_blocks_per_page(inode, page)); + iop = kmalloc(struct_size(iop, uptodate, n), + GFP_NOFS | __GFP_NOFAIL | __GFP_ZERO); /* * migrate_page_move_mapping() assumes that pages with private data have @@ -61,15 +61,16 @@ iomap_page_release(struct page *page) * Calculate the range inside the page that we actually need to read. */ static void -iomap_adjust_read_range(struct inode *inode, struct iomap_page *iop, +iomap_adjust_read_range(struct inode *inode, struct page *page, loff_t *pos, loff_t length, unsigned *offp, unsigned *lenp) { + struct iomap_page *iop = to_iomap_page(page); loff_t orig_pos = *pos; loff_t isize = i_size_read(inode); unsigned block_bits = inode->i_blkbits; unsigned block_size = (1 << block_bits); - unsigned poff = offset_in_page(*pos); - unsigned plen = min_t(loff_t, PAGE_SIZE - poff, length); + unsigned poff = offset_in_this_page(page, *pos); + unsigned plen = min_t(loff_t, page_size(page) - poff, length); unsigned first = poff >> block_bits; unsigned last = (poff + plen - 1) >> block_bits; @@ -107,7 +108,8 @@ iomap_adjust_read_range(struct inode *inode, struct iomap_page *iop, * page cache for blocks that are entirely outside of i_size. */ if (orig_pos <= isize && orig_pos + length > isize) { - unsigned end = offset_in_page(isize - 1) >> block_bits; + unsigned end = offset_in_this_page(page, isize - 1) >> + block_bits; if (first <= end && last > end) plen -= (last - end) * block_size; @@ -121,19 +123,16 @@ static void iomap_set_range_uptodate(struct page *page, unsigned off, unsigned len) { struct iomap_page *iop = to_iomap_page(page); - struct inode *inode = page->mapping->host; - unsigned first = off >> inode->i_blkbits; - unsigned last = (off + len - 1) >> inode->i_blkbits; - unsigned int i; bool uptodate = true; if (iop) { - for (i = 0; i < i_blocks_per_page(inode, page); i++) { - if (i >= first && i <= last) - set_bit(i, iop->uptodate); - else if (!test_bit(i, iop->uptodate)) - uptodate = false; - } + struct inode *inode = page->mapping->host; + unsigned first = off >> inode->i_blkbits; + unsigned count = len >> inode->i_blkbits; + + bitmap_set(iop->uptodate, first, count); + if (!bitmap_full(iop->uptodate, i_blocks_per_page(inode, page))) + uptodate = false; } if (uptodate && !PageError(page)) @@ -194,6 +193,7 @@ iomap_read_inline_data(struct inode *inode, struct page *page, return; BUG_ON(page->index); + BUG_ON(PageCompound(page)); BUG_ON(size > PAGE_SIZE - offset_in_page(iomap->inline_data)); addr = kmap_atomic(page); @@ -203,6 +203,16 @@ iomap_read_inline_data(struct inode *inode, struct page *page, SetPageUptodate(page); } +/* + * Estimate the number of vectors we need based on the current page size; + * if we're wrong we'll end up doing an overly large allocation or needing + * to do a second allocation, neither of which is a big deal. + */ +static unsigned int iomap_nr_vecs(struct page *page, loff_t length) +{ + return (length + page_size(page) - 1) >> page_shift(page); +} + static loff_t iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, struct iomap *iomap) @@ -222,7 +232,7 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, } /* zero post-eof blocks as the page may be mapped */ - iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); + iomap_adjust_read_range(inode, page, &pos, length, &poff, &plen); if (plen == 0) goto done; @@ -258,7 +268,7 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, if (!ctx->bio || !is_contig || bio_full(ctx->bio, plen)) { gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL); - int nr_vecs = (length + PAGE_SIZE - 1) >> PAGE_SHIFT; + int nr_vecs = iomap_nr_vecs(page, length); if (ctx->bio) submit_bio(ctx->bio); @@ -293,9 +303,9 @@ iomap_readpage(struct page *page, const struct iomap_ops *ops) unsigned poff; loff_t ret; - for (poff = 0; poff < PAGE_SIZE; poff += ret) { - ret = iomap_apply(inode, page_offset(page) + poff, - PAGE_SIZE - poff, 0, ops, &ctx, + for (poff = 0; poff < page_size(page); poff += ret) { + ret = iomap_apply(inode, file_offset_of_page(page) + poff, + page_size(page) - poff, 0, ops, &ctx, iomap_readpage_actor); if (ret <= 0) { WARN_ON_ONCE(ret == 0); @@ -328,7 +338,7 @@ iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos, while (!list_empty(pages)) { struct page *page = lru_to_page(pages); - if (page_offset(page) >= (u64)pos + length) + if (file_offset_of_page(page) >= (u64)pos + length) break; list_del(&page->lru); @@ -342,7 +352,7 @@ iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos, * readpages call itself as every page gets checked again once * actually needed. */ - *done += PAGE_SIZE; + *done += page_size(page); put_page(page); } @@ -355,9 +365,14 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length, { struct iomap_readpage_ctx *ctx = data; loff_t done, ret; + size_t left = 0; + + if (ctx->cur_page) + left = page_size(ctx->cur_page) - + offset_in_this_page(ctx->cur_page, pos); for (done = 0; done < length; done += ret) { - if (ctx->cur_page && offset_in_page(pos + done) == 0) { + if (ctx->cur_page && left == 0) { if (!ctx->cur_page_in_bio) unlock_page(ctx->cur_page); put_page(ctx->cur_page); @@ -369,14 +384,27 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length, if (!ctx->cur_page) break; ctx->cur_page_in_bio = false; + left = page_size(ctx->cur_page); } ret = iomap_readpage_actor(inode, pos + done, length - done, ctx, iomap); + left -= ret; } return done; } +/* move to fs.h? */ +static inline struct page *readahead_first_page(struct list_head *head) +{ + return list_entry(head->prev, struct page, lru); +} + +static inline struct page *readahead_last_page(struct list_head *head) +{ + return list_entry(head->next, struct page, lru); +} + int iomap_readpages(struct address_space *mapping, struct list_head *pages, unsigned nr_pages, const struct iomap_ops *ops) @@ -385,9 +413,10 @@ iomap_readpages(struct address_space *mapping, struct list_head *pages, .pages = pages, .is_readahead = true, }; - loff_t pos = page_offset(list_entry(pages->prev, struct page, lru)); - loff_t last = page_offset(list_entry(pages->next, struct page, lru)); - loff_t length = last - pos + PAGE_SIZE, ret = 0; + loff_t pos = file_offset_of_page(readahead_first_page(pages)); + loff_t end = file_offset_of_next_page(readahead_last_page(pages)); + loff_t length = end - pos; + loff_t ret = 0; while (length > 0) { ret = iomap_apply(mapping->host, pos, length, 0, ops, @@ -410,7 +439,7 @@ iomap_readpages(struct address_space *mapping, struct list_head *pages, } /* - * Check that we didn't lose a page due to the arcance calling + * Check that we didn't lose a page due to the arcane calling * conventions.. */ WARN_ON_ONCE(!ret && !list_empty(ctx.pages)); @@ -435,7 +464,7 @@ iomap_is_partially_uptodate(struct page *page, unsigned long from, unsigned i; /* Limit range to one page */ - len = min_t(unsigned, PAGE_SIZE - from, count); + len = min_t(unsigned, page_size(page) - from, count); /* First and last blocks in range within page */ first = from >> inode->i_blkbits; @@ -474,7 +503,7 @@ iomap_invalidatepage(struct page *page, unsigned int offset, unsigned int len) * If we are invalidating the entire page, clear the dirty state from it * and release it to avoid unnecessary buildup of the LRU. */ - if (offset == 0 && len == PAGE_SIZE) { + if (offset == 0 && len == page_size(page)) { WARN_ON_ONCE(PageWriteback(page)); cancel_dirty_page(page); iomap_page_release(page); @@ -550,18 +579,20 @@ static int __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, struct page *page, struct iomap *iomap) { - struct iomap_page *iop = iomap_page_create(inode, page); loff_t block_size = i_blocksize(inode); loff_t block_start = pos & ~(block_size - 1); loff_t block_end = (pos + len + block_size - 1) & ~(block_size - 1); - unsigned from = offset_in_page(pos), to = from + len, poff, plen; + unsigned from = offset_in_this_page(page, pos); + unsigned to = from + len; + unsigned poff, plen; int status = 0; if (PageUptodate(page)) return 0; + iomap_page_create(inode, page); do { - iomap_adjust_read_range(inode, iop, &block_start, + iomap_adjust_read_range(inode, page, &block_start, block_end - block_start, &poff, &plen); if (plen == 0) break; @@ -673,7 +704,7 @@ __iomap_write_end(struct inode *inode, loff_t pos, unsigned len, */ if (unlikely(copied < len && !PageUptodate(page))) return 0; - iomap_set_range_uptodate(page, offset_in_page(pos), len); + iomap_set_range_uptodate(page, offset_in_this_page(page, pos), len); iomap_set_page_dirty(page); return copied; } @@ -685,6 +716,7 @@ iomap_write_end_inline(struct inode *inode, struct page *page, void *addr; WARN_ON_ONCE(!PageUptodate(page)); + BUG_ON(PageCompound(page)); BUG_ON(pos + copied > PAGE_SIZE - offset_in_page(iomap->inline_data)); addr = kmap_atomic(page); @@ -749,6 +781,10 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data, unsigned long bytes; /* Bytes to write to page */ size_t copied; /* Bytes copied from user */ + /* + * XXX: We don't know what size page we'll find in the + * page cache, so only copy up to a regular page boundary. + */ offset = offset_in_page(pos); bytes = min_t(unsigned long, PAGE_SIZE - offset, iov_iter_count(i)); @@ -1041,19 +1077,18 @@ vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf, const struct iomap_ops *ops) lock_page(page); size = i_size_read(inode); if ((page->mapping != inode->i_mapping) || - (page_offset(page) > size)) { + (file_offset_of_page(page) > size)) { /* We overload EFAULT to mean page got truncated */ ret = -EFAULT; goto out_unlock; } - /* page is wholly or partially inside EOF */ - if (((page->index + 1) << PAGE_SHIFT) > size) - length = offset_in_page(size); + offset = file_offset_of_page(page); + if (size - offset < page_size(page)) + length = offset_in_this_page(page, size); else - length = PAGE_SIZE; + length = page_size(page); - offset = page_offset(page); while (length > 0) { ret = iomap_apply(inode, offset, length, IOMAP_WRITE | IOMAP_FAULT, ops, page, diff --git a/include/linux/iomap.h b/include/linux/iomap.h index bc499ceae392..86be24a8259b 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -139,7 +139,7 @@ loff_t iomap_apply(struct inode *inode, loff_t pos, loff_t length, struct iomap_page { atomic_t read_count; atomic_t write_count; - DECLARE_BITMAP(uptodate, PAGE_SIZE / 512); + unsigned long uptodate[]; }; static inline struct iomap_page *to_iomap_page(struct page *page) diff --git a/include/linux/mm.h b/include/linux/mm.h index 726d7f046b49..6892cd712428 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1414,6 +1414,8 @@ static inline void clear_page_pfmemalloc(struct page *page) extern void pagefault_out_of_memory(void); #define offset_in_page(p) ((unsigned long)(p) & ~PAGE_MASK) +#define offset_in_this_page(page, p) \ + ((unsigned long)(p) & (page_size(page) - 1)) /* * Flags passed to show_mem() and show_free_areas() to suppress output in -- 2.23.0.rc1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/5] iomap: Support large pages 2019-08-21 0:30 ` [PATCH v2 3/5] iomap: Support large pages Matthew Wilcox @ 2019-08-23 12:48 ` kbuild test robot 2019-09-18 21:29 ` Darrick J. Wong 1 sibling, 0 replies; 19+ messages in thread From: kbuild test robot @ 2019-08-23 12:48 UTC (permalink / raw) To: Matthew Wilcox Cc: kbuild-all, linux-fsdevel, Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm [-- Attachment #1: Type: text/plain, Size: 2747 bytes --] Hi Matthew, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3-rc5 next-20190823] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Matthew-Wilcox/iomap-xfs-support-for-large-pages/20190823-191138 config: c6x-allyesconfig (attached as .config) compiler: c6x-elf-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=c6x If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from include/linux/blkdev.h:16:0, from include/linux/iomap.h:10, from fs/iomap/buffered-io.c:9: include/linux/pagemap.h: In function 'file_offset_of_next_page': include/linux/pagemap.h:445:32: error: implicit declaration of function 'compound_nr'; did you mean 'compound_order'? [-Werror=implicit-function-declaration] return ((loff_t)page->index + compound_nr(page)) << PAGE_SHIFT; ^~~~~~~~~~~ compound_order include/linux/pagemap.h: In function 'i_blocks_per_page': include/linux/pagemap.h:659:9: error: implicit declaration of function 'page_size'; did you mean 'page_zone'? [-Werror=implicit-function-declaration] return page_size(page) >> inode->i_blkbits; ^~~~~~~~~ page_zone fs/iomap/buffered-io.c: In function 'iomap_nr_vecs': >> fs/iomap/buffered-io.c:213:43: error: implicit declaration of function 'page_shift'; did you mean 'page_pgdat'? [-Werror=implicit-function-declaration] return (length + page_size(page) - 1) >> page_shift(page); ^~~~~~~~~~ page_pgdat cc1: some warnings being treated as errors vim +213 fs/iomap/buffered-io.c 205 206 /* 207 * Estimate the number of vectors we need based on the current page size; 208 * if we're wrong we'll end up doing an overly large allocation or needing 209 * to do a second allocation, neither of which is a big deal. 210 */ 211 static unsigned int iomap_nr_vecs(struct page *page, loff_t length) 212 { > 213 return (length + page_size(page) - 1) >> page_shift(page); 214 } 215 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 49703 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/5] iomap: Support large pages 2019-08-21 0:30 ` [PATCH v2 3/5] iomap: Support large pages Matthew Wilcox 2019-08-23 12:48 ` kbuild test robot @ 2019-09-18 21:29 ` Darrick J. Wong 1 sibling, 0 replies; 19+ messages in thread From: Darrick J. Wong @ 2019-09-18 21:29 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-fsdevel, hch, linux-xfs, linux-mm On Tue, Aug 20, 2019 at 05:30:37PM -0700, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > Change iomap_page from a statically sized uptodate bitmap to a dynamically > allocated uptodate bitmap, allowing an arbitrarily large page. > > The only remaining places where iomap assumes an order-0 page are for > files with inline data, where there's no sense in allocating a larger > page. I wonder, will anything bad happen if that occurs? (XFS doesn't have inline data for files so I have no idea...) > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/iomap/buffered-io.c | 119 ++++++++++++++++++++++++++--------------- > include/linux/iomap.h | 2 +- > include/linux/mm.h | 2 + > 3 files changed, 80 insertions(+), 43 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 0e76a4b6d98a..15d844a88439 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -23,14 +23,14 @@ static struct iomap_page * > iomap_page_create(struct inode *inode, struct page *page) > { > struct iomap_page *iop = to_iomap_page(page); > + unsigned int n; > > if (iop || i_blocks_per_page(inode, page) <= 1) > return iop; > > - iop = kmalloc(sizeof(*iop), GFP_NOFS | __GFP_NOFAIL); > - atomic_set(&iop->read_count, 0); > - atomic_set(&iop->write_count, 0); > - bitmap_zero(iop->uptodate, PAGE_SIZE / SECTOR_SIZE); > + n = BITS_TO_LONGS(i_blocks_per_page(inode, page)); > + iop = kmalloc(struct_size(iop, uptodate, n), > + GFP_NOFS | __GFP_NOFAIL | __GFP_ZERO); > > /* > * migrate_page_move_mapping() assumes that pages with private data have > @@ -61,15 +61,16 @@ iomap_page_release(struct page *page) > * Calculate the range inside the page that we actually need to read. > */ > static void > -iomap_adjust_read_range(struct inode *inode, struct iomap_page *iop, > +iomap_adjust_read_range(struct inode *inode, struct page *page, > loff_t *pos, loff_t length, unsigned *offp, unsigned *lenp) > { > + struct iomap_page *iop = to_iomap_page(page); > loff_t orig_pos = *pos; > loff_t isize = i_size_read(inode); > unsigned block_bits = inode->i_blkbits; > unsigned block_size = (1 << block_bits); > - unsigned poff = offset_in_page(*pos); > - unsigned plen = min_t(loff_t, PAGE_SIZE - poff, length); > + unsigned poff = offset_in_this_page(page, *pos); > + unsigned plen = min_t(loff_t, page_size(page) - poff, length); > unsigned first = poff >> block_bits; > unsigned last = (poff + plen - 1) >> block_bits; > > @@ -107,7 +108,8 @@ iomap_adjust_read_range(struct inode *inode, struct iomap_page *iop, > * page cache for blocks that are entirely outside of i_size. > */ > if (orig_pos <= isize && orig_pos + length > isize) { > - unsigned end = offset_in_page(isize - 1) >> block_bits; > + unsigned end = offset_in_this_page(page, isize - 1) >> > + block_bits; > > if (first <= end && last > end) > plen -= (last - end) * block_size; > @@ -121,19 +123,16 @@ static void > iomap_set_range_uptodate(struct page *page, unsigned off, unsigned len) > { > struct iomap_page *iop = to_iomap_page(page); > - struct inode *inode = page->mapping->host; > - unsigned first = off >> inode->i_blkbits; > - unsigned last = (off + len - 1) >> inode->i_blkbits; > - unsigned int i; > bool uptodate = true; > > if (iop) { > - for (i = 0; i < i_blocks_per_page(inode, page); i++) { > - if (i >= first && i <= last) > - set_bit(i, iop->uptodate); > - else if (!test_bit(i, iop->uptodate)) > - uptodate = false; > - } > + struct inode *inode = page->mapping->host; > + unsigned first = off >> inode->i_blkbits; > + unsigned count = len >> inode->i_blkbits; > + > + bitmap_set(iop->uptodate, first, count); > + if (!bitmap_full(iop->uptodate, i_blocks_per_page(inode, page))) > + uptodate = false; > } > > if (uptodate && !PageError(page)) > @@ -194,6 +193,7 @@ iomap_read_inline_data(struct inode *inode, struct page *page, > return; > > BUG_ON(page->index); > + BUG_ON(PageCompound(page)); > BUG_ON(size > PAGE_SIZE - offset_in_page(iomap->inline_data)); > > addr = kmap_atomic(page); > @@ -203,6 +203,16 @@ iomap_read_inline_data(struct inode *inode, struct page *page, > SetPageUptodate(page); > } > > +/* > + * Estimate the number of vectors we need based on the current page size; > + * if we're wrong we'll end up doing an overly large allocation or needing > + * to do a second allocation, neither of which is a big deal. > + */ > +static unsigned int iomap_nr_vecs(struct page *page, loff_t length) > +{ > + return (length + page_size(page) - 1) >> page_shift(page); > +} > + > static loff_t > iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > struct iomap *iomap) > @@ -222,7 +232,7 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > } > > /* zero post-eof blocks as the page may be mapped */ > - iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen); > + iomap_adjust_read_range(inode, page, &pos, length, &poff, &plen); > if (plen == 0) > goto done; > > @@ -258,7 +268,7 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > > if (!ctx->bio || !is_contig || bio_full(ctx->bio, plen)) { > gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL); > - int nr_vecs = (length + PAGE_SIZE - 1) >> PAGE_SHIFT; > + int nr_vecs = iomap_nr_vecs(page, length); > > if (ctx->bio) > submit_bio(ctx->bio); > @@ -293,9 +303,9 @@ iomap_readpage(struct page *page, const struct iomap_ops *ops) > unsigned poff; > loff_t ret; > > - for (poff = 0; poff < PAGE_SIZE; poff += ret) { > - ret = iomap_apply(inode, page_offset(page) + poff, > - PAGE_SIZE - poff, 0, ops, &ctx, > + for (poff = 0; poff < page_size(page); poff += ret) { > + ret = iomap_apply(inode, file_offset_of_page(page) + poff, > + page_size(page) - poff, 0, ops, &ctx, > iomap_readpage_actor); > if (ret <= 0) { > WARN_ON_ONCE(ret == 0); > @@ -328,7 +338,7 @@ iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos, > while (!list_empty(pages)) { > struct page *page = lru_to_page(pages); > > - if (page_offset(page) >= (u64)pos + length) > + if (file_offset_of_page(page) >= (u64)pos + length) > break; > > list_del(&page->lru); > @@ -342,7 +352,7 @@ iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos, > * readpages call itself as every page gets checked again once > * actually needed. > */ > - *done += PAGE_SIZE; > + *done += page_size(page); > put_page(page); > } > > @@ -355,9 +365,14 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length, > { > struct iomap_readpage_ctx *ctx = data; > loff_t done, ret; > + size_t left = 0; > + > + if (ctx->cur_page) > + left = page_size(ctx->cur_page) - > + offset_in_this_page(ctx->cur_page, pos); > > for (done = 0; done < length; done += ret) { > - if (ctx->cur_page && offset_in_page(pos + done) == 0) { > + if (ctx->cur_page && left == 0) { > if (!ctx->cur_page_in_bio) > unlock_page(ctx->cur_page); > put_page(ctx->cur_page); > @@ -369,14 +384,27 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length, > if (!ctx->cur_page) > break; > ctx->cur_page_in_bio = false; > + left = page_size(ctx->cur_page); > } > ret = iomap_readpage_actor(inode, pos + done, length - done, > ctx, iomap); > + left -= ret; > } > > return done; > } > > +/* move to fs.h? */ > +static inline struct page *readahead_first_page(struct list_head *head) > +{ > + return list_entry(head->prev, struct page, lru); > +} > + > +static inline struct page *readahead_last_page(struct list_head *head) > +{ > + return list_entry(head->next, struct page, lru); > +} > + > int > iomap_readpages(struct address_space *mapping, struct list_head *pages, > unsigned nr_pages, const struct iomap_ops *ops) > @@ -385,9 +413,10 @@ iomap_readpages(struct address_space *mapping, struct list_head *pages, > .pages = pages, > .is_readahead = true, > }; > - loff_t pos = page_offset(list_entry(pages->prev, struct page, lru)); > - loff_t last = page_offset(list_entry(pages->next, struct page, lru)); > - loff_t length = last - pos + PAGE_SIZE, ret = 0; > + loff_t pos = file_offset_of_page(readahead_first_page(pages)); > + loff_t end = file_offset_of_next_page(readahead_last_page(pages)); > + loff_t length = end - pos; > + loff_t ret = 0; > > while (length > 0) { > ret = iomap_apply(mapping->host, pos, length, 0, ops, > @@ -410,7 +439,7 @@ iomap_readpages(struct address_space *mapping, struct list_head *pages, > } > > /* > - * Check that we didn't lose a page due to the arcance calling > + * Check that we didn't lose a page due to the arcane calling > * conventions.. > */ > WARN_ON_ONCE(!ret && !list_empty(ctx.pages)); > @@ -435,7 +464,7 @@ iomap_is_partially_uptodate(struct page *page, unsigned long from, > unsigned i; > > /* Limit range to one page */ > - len = min_t(unsigned, PAGE_SIZE - from, count); > + len = min_t(unsigned, page_size(page) - from, count); > > /* First and last blocks in range within page */ > first = from >> inode->i_blkbits; > @@ -474,7 +503,7 @@ iomap_invalidatepage(struct page *page, unsigned int offset, unsigned int len) > * If we are invalidating the entire page, clear the dirty state from it > * and release it to avoid unnecessary buildup of the LRU. > */ > - if (offset == 0 && len == PAGE_SIZE) { > + if (offset == 0 && len == page_size(page)) { > WARN_ON_ONCE(PageWriteback(page)); > cancel_dirty_page(page); > iomap_page_release(page); > @@ -550,18 +579,20 @@ static int > __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, > struct page *page, struct iomap *iomap) > { > - struct iomap_page *iop = iomap_page_create(inode, page); > loff_t block_size = i_blocksize(inode); > loff_t block_start = pos & ~(block_size - 1); > loff_t block_end = (pos + len + block_size - 1) & ~(block_size - 1); > - unsigned from = offset_in_page(pos), to = from + len, poff, plen; > + unsigned from = offset_in_this_page(page, pos); > + unsigned to = from + len; > + unsigned poff, plen; > int status = 0; > > if (PageUptodate(page)) > return 0; > + iomap_page_create(inode, page); > > do { > - iomap_adjust_read_range(inode, iop, &block_start, > + iomap_adjust_read_range(inode, page, &block_start, > block_end - block_start, &poff, &plen); > if (plen == 0) > break; > @@ -673,7 +704,7 @@ __iomap_write_end(struct inode *inode, loff_t pos, unsigned len, > */ > if (unlikely(copied < len && !PageUptodate(page))) > return 0; > - iomap_set_range_uptodate(page, offset_in_page(pos), len); > + iomap_set_range_uptodate(page, offset_in_this_page(page, pos), len); > iomap_set_page_dirty(page); > return copied; > } > @@ -685,6 +716,7 @@ iomap_write_end_inline(struct inode *inode, struct page *page, > void *addr; > > WARN_ON_ONCE(!PageUptodate(page)); > + BUG_ON(PageCompound(page)); > BUG_ON(pos + copied > PAGE_SIZE - offset_in_page(iomap->inline_data)); > > addr = kmap_atomic(page); > @@ -749,6 +781,10 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > unsigned long bytes; /* Bytes to write to page */ > size_t copied; /* Bytes copied from user */ > > + /* > + * XXX: We don't know what size page we'll find in the > + * page cache, so only copy up to a regular page boundary. How might we fix this? > + */ > offset = offset_in_page(pos); > bytes = min_t(unsigned long, PAGE_SIZE - offset, > iov_iter_count(i)); > @@ -1041,19 +1077,18 @@ vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf, const struct iomap_ops *ops) > lock_page(page); > size = i_size_read(inode); > if ((page->mapping != inode->i_mapping) || > - (page_offset(page) > size)) { > + (file_offset_of_page(page) > size)) { > /* We overload EFAULT to mean page got truncated */ > ret = -EFAULT; > goto out_unlock; > } > > - /* page is wholly or partially inside EOF */ > - if (((page->index + 1) << PAGE_SHIFT) > size) > - length = offset_in_page(size); > + offset = file_offset_of_page(page); > + if (size - offset < page_size(page)) > + length = offset_in_this_page(page, size); > else > - length = PAGE_SIZE; > + length = page_size(page); > > - offset = page_offset(page); > while (length > 0) { > ret = iomap_apply(inode, offset, length, > IOMAP_WRITE | IOMAP_FAULT, ops, page, > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index bc499ceae392..86be24a8259b 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -139,7 +139,7 @@ loff_t iomap_apply(struct inode *inode, loff_t pos, loff_t length, > struct iomap_page { > atomic_t read_count; > atomic_t write_count; > - DECLARE_BITMAP(uptodate, PAGE_SIZE / 512); > + unsigned long uptodate[]; > }; > > static inline struct iomap_page *to_iomap_page(struct page *page) > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 726d7f046b49..6892cd712428 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -1414,6 +1414,8 @@ static inline void clear_page_pfmemalloc(struct page *page) > extern void pagefault_out_of_memory(void); > > #define offset_in_page(p) ((unsigned long)(p) & ~PAGE_MASK) > +#define offset_in_this_page(page, p) \ > + ((unsigned long)(p) & (page_size(page) - 1)) What's the difference between these two macros? I guess the macro with the longer name works for compound pages? Whereas the first one only works with order-0 pages? --D > > /* > * Flags passed to show_mem() and show_free_areas() to suppress output in > -- > 2.23.0.rc1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] xfs: Support large pages 2019-08-21 0:30 [PATCH v2 0/5] iomap & xfs support for large pages Matthew Wilcox ` (2 preceding siblings ...) 2019-08-21 0:30 ` [PATCH v2 3/5] iomap: Support large pages Matthew Wilcox @ 2019-08-21 0:30 ` Matthew Wilcox 2019-09-18 21:31 ` Darrick J. Wong 2019-08-21 0:30 ` [PATCH v2 5/5] xfs: Pass a page to xfs_finish_page_writeback Matthew Wilcox 4 siblings, 1 reply; 19+ messages in thread From: Matthew Wilcox @ 2019-08-21 0:30 UTC (permalink / raw) To: linux-fsdevel; +Cc: Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm From: "Matthew Wilcox (Oracle)" <willy@infradead.org> Mostly this is just checking the page size of each page instead of assuming PAGE_SIZE. Clean up the logic in writepage a little. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/xfs/xfs_aops.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c index 102cfd8a97d6..1a26e9ca626b 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -765,7 +765,7 @@ xfs_add_to_ioend( struct xfs_mount *mp = ip->i_mount; struct block_device *bdev = xfs_find_bdev_for_inode(inode); unsigned len = i_blocksize(inode); - unsigned poff = offset & (PAGE_SIZE - 1); + unsigned poff = offset & (page_size(page) - 1); bool merged, same_page = false; sector_t sector; @@ -843,7 +843,7 @@ xfs_aops_discard_page( if (error && !XFS_FORCED_SHUTDOWN(mp)) xfs_alert(mp, "page discard unable to remove delalloc mapping."); out_invalidate: - xfs_vm_invalidatepage(page, 0, PAGE_SIZE); + xfs_vm_invalidatepage(page, 0, page_size(page)); } /* @@ -984,8 +984,7 @@ xfs_do_writepage( struct xfs_writepage_ctx *wpc = data; struct inode *inode = page->mapping->host; loff_t offset; - uint64_t end_offset; - pgoff_t end_index; + uint64_t end_offset; trace_xfs_writepage(inode, page, 0, 0); @@ -1024,10 +1023,9 @@ xfs_do_writepage( * ---------------------------------^------------------| */ offset = i_size_read(inode); - end_index = offset >> PAGE_SHIFT; - if (page->index < end_index) - end_offset = (xfs_off_t)(page->index + 1) << PAGE_SHIFT; - else { + end_offset = file_offset_of_next_page(page); + + if (end_offset > offset) { /* * Check whether the page to write out is beyond or straddles * i_size or not. @@ -1039,7 +1037,8 @@ xfs_do_writepage( * | | Straddles | * ---------------------------------^-----------|--------| */ - unsigned offset_into_page = offset & (PAGE_SIZE - 1); + unsigned offset_into_page = offset_in_this_page(page, offset); + pgoff_t end_index = offset >> PAGE_SHIFT; /* * Skip the page if it is fully outside i_size, e.g. due to a @@ -1070,7 +1069,7 @@ xfs_do_writepage( * memory is zeroed when mapped, and writes to that region are * not written out to the file." */ - zero_user_segment(page, offset_into_page, PAGE_SIZE); + zero_user_segment(page, offset_into_page, page_size(page)); /* Adjust the end_offset to the end of file */ end_offset = offset; -- 2.23.0.rc1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/5] xfs: Support large pages 2019-08-21 0:30 ` [PATCH v2 4/5] xfs: " Matthew Wilcox @ 2019-09-18 21:31 ` Darrick J. Wong 0 siblings, 0 replies; 19+ messages in thread From: Darrick J. Wong @ 2019-09-18 21:31 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-fsdevel, hch, linux-xfs, linux-mm On Tue, Aug 20, 2019 at 05:30:38PM -0700, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > Mostly this is just checking the page size of each page instead of > assuming PAGE_SIZE. Clean up the logic in writepage a little. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Looks ok, let's see what happens when I get back to the "make xfs use iomap writeback" series... Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_aops.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 102cfd8a97d6..1a26e9ca626b 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -765,7 +765,7 @@ xfs_add_to_ioend( > struct xfs_mount *mp = ip->i_mount; > struct block_device *bdev = xfs_find_bdev_for_inode(inode); > unsigned len = i_blocksize(inode); > - unsigned poff = offset & (PAGE_SIZE - 1); > + unsigned poff = offset & (page_size(page) - 1); > bool merged, same_page = false; > sector_t sector; > > @@ -843,7 +843,7 @@ xfs_aops_discard_page( > if (error && !XFS_FORCED_SHUTDOWN(mp)) > xfs_alert(mp, "page discard unable to remove delalloc mapping."); > out_invalidate: > - xfs_vm_invalidatepage(page, 0, PAGE_SIZE); > + xfs_vm_invalidatepage(page, 0, page_size(page)); > } > > /* > @@ -984,8 +984,7 @@ xfs_do_writepage( > struct xfs_writepage_ctx *wpc = data; > struct inode *inode = page->mapping->host; > loff_t offset; > - uint64_t end_offset; > - pgoff_t end_index; > + uint64_t end_offset; > > trace_xfs_writepage(inode, page, 0, 0); > > @@ -1024,10 +1023,9 @@ xfs_do_writepage( > * ---------------------------------^------------------| > */ > offset = i_size_read(inode); > - end_index = offset >> PAGE_SHIFT; > - if (page->index < end_index) > - end_offset = (xfs_off_t)(page->index + 1) << PAGE_SHIFT; > - else { > + end_offset = file_offset_of_next_page(page); > + > + if (end_offset > offset) { > /* > * Check whether the page to write out is beyond or straddles > * i_size or not. > @@ -1039,7 +1037,8 @@ xfs_do_writepage( > * | | Straddles | > * ---------------------------------^-----------|--------| > */ > - unsigned offset_into_page = offset & (PAGE_SIZE - 1); > + unsigned offset_into_page = offset_in_this_page(page, offset); > + pgoff_t end_index = offset >> PAGE_SHIFT; > > /* > * Skip the page if it is fully outside i_size, e.g. due to a > @@ -1070,7 +1069,7 @@ xfs_do_writepage( > * memory is zeroed when mapped, and writes to that region are > * not written out to the file." > */ > - zero_user_segment(page, offset_into_page, PAGE_SIZE); > + zero_user_segment(page, offset_into_page, page_size(page)); > > /* Adjust the end_offset to the end of file */ > end_offset = offset; > -- > 2.23.0.rc1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 5/5] xfs: Pass a page to xfs_finish_page_writeback 2019-08-21 0:30 [PATCH v2 0/5] iomap & xfs support for large pages Matthew Wilcox ` (3 preceding siblings ...) 2019-08-21 0:30 ` [PATCH v2 4/5] xfs: " Matthew Wilcox @ 2019-08-21 0:30 ` Matthew Wilcox 2019-09-18 21:32 ` Darrick J. Wong 4 siblings, 1 reply; 19+ messages in thread From: Matthew Wilcox @ 2019-08-21 0:30 UTC (permalink / raw) To: linux-fsdevel; +Cc: Matthew Wilcox (Oracle), hch, linux-xfs, linux-mm From: "Matthew Wilcox (Oracle)" <willy@infradead.org> The only part of the bvec we were accessing was the bv_page, so just pass that instead of the whole bvec. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/xfs/xfs_aops.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c index 1a26e9ca626b..edcb4797fcc2 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -58,21 +58,21 @@ xfs_find_daxdev_for_inode( static void xfs_finish_page_writeback( struct inode *inode, - struct bio_vec *bvec, + struct page *page, int error) { - struct iomap_page *iop = to_iomap_page(bvec->bv_page); + struct iomap_page *iop = to_iomap_page(page); if (error) { - SetPageError(bvec->bv_page); + SetPageError(page); mapping_set_error(inode->i_mapping, -EIO); } - ASSERT(iop || i_blocks_per_page(inode, bvec->bv_page) <= 1); + ASSERT(iop || i_blocks_per_page(inode, page) <= 1); ASSERT(!iop || atomic_read(&iop->write_count) > 0); if (!iop || atomic_dec_and_test(&iop->write_count)) - end_page_writeback(bvec->bv_page); + end_page_writeback(page); } /* @@ -106,7 +106,7 @@ xfs_destroy_ioend( /* walk each page on bio, ending page IO on them */ bio_for_each_segment_all(bvec, bio, iter_all) - xfs_finish_page_writeback(inode, bvec, error); + xfs_finish_page_writeback(inode, bvec->bv_page, error); bio_put(bio); } -- 2.23.0.rc1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/5] xfs: Pass a page to xfs_finish_page_writeback 2019-08-21 0:30 ` [PATCH v2 5/5] xfs: Pass a page to xfs_finish_page_writeback Matthew Wilcox @ 2019-09-18 21:32 ` Darrick J. Wong 0 siblings, 0 replies; 19+ messages in thread From: Darrick J. Wong @ 2019-09-18 21:32 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-fsdevel, hch, linux-xfs, linux-mm On Tue, Aug 20, 2019 at 05:30:39PM -0700, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > The only part of the bvec we were accessing was the bv_page, so just > pass that instead of the whole bvec. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Seems fine but same caveats about tree churn as the previous patch. --D > --- > fs/xfs/xfs_aops.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 1a26e9ca626b..edcb4797fcc2 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -58,21 +58,21 @@ xfs_find_daxdev_for_inode( > static void > xfs_finish_page_writeback( > struct inode *inode, > - struct bio_vec *bvec, > + struct page *page, > int error) > { > - struct iomap_page *iop = to_iomap_page(bvec->bv_page); > + struct iomap_page *iop = to_iomap_page(page); > > if (error) { > - SetPageError(bvec->bv_page); > + SetPageError(page); > mapping_set_error(inode->i_mapping, -EIO); > } > > - ASSERT(iop || i_blocks_per_page(inode, bvec->bv_page) <= 1); > + ASSERT(iop || i_blocks_per_page(inode, page) <= 1); > ASSERT(!iop || atomic_read(&iop->write_count) > 0); > > if (!iop || atomic_dec_and_test(&iop->write_count)) > - end_page_writeback(bvec->bv_page); > + end_page_writeback(page); > } > > /* > @@ -106,7 +106,7 @@ xfs_destroy_ioend( > > /* walk each page on bio, ending page IO on them */ > bio_for_each_segment_all(bvec, bio, iter_all) > - xfs_finish_page_writeback(inode, bvec, error); > + xfs_finish_page_writeback(inode, bvec->bv_page, error); > bio_put(bio); > } > > -- > 2.23.0.rc1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-09-19 0:05 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-21 0:30 [PATCH v2 0/5] iomap & xfs support for large pages Matthew Wilcox 2019-08-21 0:30 ` [PATCH v2 1/5] fs: Introduce i_blocks_per_page Matthew Wilcox 2019-08-23 12:26 ` kbuild test robot 2019-09-18 21:14 ` Darrick J. Wong 2019-09-18 23:48 ` Matthew Wilcox 2019-08-21 0:30 ` [PATCH v2 2/5] mm: Add file_offset_of_ helpers Matthew Wilcox 2019-08-23 12:49 ` kbuild test robot 2019-08-24 11:48 ` kbuild test robot 2019-08-24 15:28 ` Matthew Wilcox 2019-09-18 21:17 ` Darrick J. Wong 2019-09-18 23:49 ` Matthew Wilcox 2019-09-19 0:04 ` Darrick J. Wong 2019-08-21 0:30 ` [PATCH v2 3/5] iomap: Support large pages Matthew Wilcox 2019-08-23 12:48 ` kbuild test robot 2019-09-18 21:29 ` Darrick J. Wong 2019-08-21 0:30 ` [PATCH v2 4/5] xfs: " Matthew Wilcox 2019-09-18 21:31 ` Darrick J. Wong 2019-08-21 0:30 ` [PATCH v2 5/5] xfs: Pass a page to xfs_finish_page_writeback Matthew Wilcox 2019-09-18 21:32 ` Darrick J. Wong
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).