All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-fsdevel@vger.kernel.org, hch@lst.de,
	linux-xfs@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH v2 2/5] mm: Add file_offset_of_ helpers
Date: Wed, 18 Sep 2019 14:17:55 -0700	[thread overview]
Message-ID: <20190918211755.GC2229799@magnolia> (raw)
In-Reply-To: <20190821003039.12555-3-willy@infradead.org>

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
> 

  parent reply	other threads:[~2019-09-18 21:20 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190918211755.GC2229799@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.