| Message ID | 20200515131656.12890-15-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Large pages in the page cache | expand |
On Fri, May 15, 2020 at 06:16:34AM -0700, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > Pass the struct page instead of the iomap_page so we can determine the > size of the page. Use offset_in_thp() instead of offset_in_page() and use > thp_size() instead of PAGE_SIZE. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/iomap/buffered-io.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 4a79061073eb..423ffc9d4a97 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -83,15 +83,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_thp(page, *pos); > + unsigned plen = min_t(loff_t, thp_size(page) - poff, length); > unsigned first = poff >> block_bits; > unsigned last = (poff + plen - 1) >> block_bits; > > @@ -129,7 +130,7 @@ 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_thp(page, isize - 1) >> block_bits; > > if (first <= end && last > end) > plen -= (last - end) * block_size; > @@ -256,7 +257,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; > > @@ -571,7 +572,6 @@ static int > __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, > struct page *page, struct iomap *srcmap) > { > - 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); > @@ -580,9 +580,10 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, > > if (PageUptodate(page)) > return 0; > + iomap_page_create(inode, page); What problem does this fix? i.e. if we can get here with an uninitialised page, why isn't this a separate bug fix. I don't see anything in this patch that actually changes behaviour, and there's nothing in the commit description to tell me why this is here, so... ??? Cheers, Dave.
On Fri, May 22, 2020 at 08:24:38AM +1000, Dave Chinner wrote: > > @@ -571,7 +572,6 @@ static int > > __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, > > struct page *page, struct iomap *srcmap) > > { > > - 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); > > @@ -580,9 +580,10 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, > > > > if (PageUptodate(page)) > > return 0; > > + iomap_page_create(inode, page); > > What problem does this fix? i.e. if we can get here with an > uninitialised page, why isn't this a separate bug fix. I don't see > anything in this patch that actually changes behaviour, and there's > nothing in the commit description to tell me why this is here, > so... ??? I'm not fixing anything ... just moving the call to iomap_page_create() from the opening stanza to down here because we no longer need a struct iomap_page pointer in this function.
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 4a79061073eb..423ffc9d4a97 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -83,15 +83,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_thp(page, *pos); + unsigned plen = min_t(loff_t, thp_size(page) - poff, length); unsigned first = poff >> block_bits; unsigned last = (poff + plen - 1) >> block_bits; @@ -129,7 +130,7 @@ 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_thp(page, isize - 1) >> block_bits; if (first <= end && last > end) plen -= (last - end) * block_size; @@ -256,7 +257,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; @@ -571,7 +572,6 @@ static int __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, struct page *page, struct iomap *srcmap) { - 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); @@ -580,9 +580,10 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, 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;