| Message ID | 20201014030357.21898-3-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Transparent Huge Page support for XFS | expand |
On Wed, Oct 14, 2020 at 04:03:45AM +0100, Matthew Wilcox (Oracle) wrote: > If the page is compound, check the last index in the page and return > the appropriate size. Change the return type to ssize_t in case we ever > support pages larger than 2GB. "But what about 16G pages?" :P > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > include/linux/pagemap.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 7dc6bf713d27..5083a0efafa8 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -983,22 +983,22 @@ static inline unsigned long dir_pages(struct inode *inode) > * @page: the page to check > * @inode: the inode to check the page against > * > - * Returns the number of bytes in the page up to EOF, > + * Return: The number of bytes in the page up to EOF, > * or -EFAULT if the page was truncated. > */ > -static inline int page_mkwrite_check_truncate(struct page *page, > +static inline ssize_t page_mkwrite_check_truncate(struct page *page, > struct inode *inode) > { > loff_t size = i_size_read(inode); > pgoff_t index = size >> PAGE_SHIFT; > - int offset = offset_in_page(size); > + unsigned long offset = offset_in_thp(page, size); > > if (page->mapping != inode->i_mapping) > return -EFAULT; > > /* page is wholly inside EOF */ > - if (page->index < index) > - return PAGE_SIZE; > + if (page->index + thp_nr_pages(page) - 1 < index) Just curious, is this expression common enough to create a helper for that too? #define thp_last_index(page) ((page)->index + thp_nr_pages(page) - 1) ? (Maybe make that a static inline, I used a macro for brevity) Otherwise looks fine to me... --D > + return thp_size(page); > /* page is wholly past EOF */ > if (page->index > index || !offset) > return -EFAULT; > -- > 2.28.0 >
On Wed, Oct 14, 2020 at 09:17:04AM -0700, Darrick J. Wong wrote: > On Wed, Oct 14, 2020 at 04:03:45AM +0100, Matthew Wilcox (Oracle) wrote: > > If the page is compound, check the last index in the page and return > > the appropriate size. Change the return type to ssize_t in case we ever > > support pages larger than 2GB. > > "But what about 16G pages?" :P They're not practical with today's I/O limits; a x4 PCIe link running at gen4 speeds will take 2 seconds to do 16GB of I/O. Assuming doubling of PCIe speeds every four years, and a reasonable latency of 0.1 seconds, we're about fifteen years away from that being reasonable. I'm sure this code will have bitrotted by then and whoever tries to add support for them will have work to do ... > > /* page is wholly inside EOF */ > > - if (page->index < index) > > - return PAGE_SIZE; > > + if (page->index + thp_nr_pages(page) - 1 < index) > > Just curious, is this expression common enough to create a helper for > that too? > > #define thp_last_index(page) ((page)->index + thp_nr_pages(page) - 1) ? > > (Maybe make that a static inline, I used a macro for brevity) I had that in an earlier version and there were two callers, so I took it out again because it was more effort to explain what it did than it was to open-code it.
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 7dc6bf713d27..5083a0efafa8 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -983,22 +983,22 @@ static inline unsigned long dir_pages(struct inode *inode) * @page: the page to check * @inode: the inode to check the page against * - * Returns the number of bytes in the page up to EOF, + * Return: The number of bytes in the page up to EOF, * or -EFAULT if the page was truncated. */ -static inline int page_mkwrite_check_truncate(struct page *page, +static inline ssize_t page_mkwrite_check_truncate(struct page *page, struct inode *inode) { loff_t size = i_size_read(inode); pgoff_t index = size >> PAGE_SHIFT; - int offset = offset_in_page(size); + unsigned long offset = offset_in_thp(page, size); if (page->mapping != inode->i_mapping) return -EFAULT; /* page is wholly inside EOF */ - if (page->index < index) - return PAGE_SIZE; + if (page->index + thp_nr_pages(page) - 1 < index) + return thp_size(page); /* page is wholly past EOF */ if (page->index > index || !offset) return -EFAULT;
If the page is compound, check the last index in the page and return the appropriate size. Change the return type to ssize_t in case we ever support pages larger than 2GB. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/pagemap.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)