* [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
* [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
* [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
* [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
* [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 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 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 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 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 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 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
* 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
* 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
* 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
* 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
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).