Re: [GIT PULL] Rename page_offset() to page_pos()

From: Matthew Wilcox
Date: Sat Apr 11 2020 - 17:48:27 EST


On Sat, Apr 11, 2020 at 01:57:56PM -0700, Linus Torvalds wrote:
> So honestly, i the confusion is that we have "pgoff_t", which is the
> offset of the page counted in _pages_, then my reaction is that
>
> (a) I think the truly confusing name is "pgoff_t" (and any
> "page_offset" variable of that type). Calling that "pgindex_t" and
> "page_index" would be a real clarification.

I think you're right. I have a patch series queued for 5.8 which
renames a lot of 'pgoff_t offset' to 'pgoff_t index'. I wouldn't mind
at all renaming pgoff_t to pgindex_t. If you're amenable, pgidx_t would
be shorter.

> (b) if we really do want to rename page_offset() because of confusion
> with the page index "offset", then the logical thing would be to
> clarify that it's a byte offset, not the page index.

I wasn't entirely forthcoming ... I actually want to introduce a new

#define page_offset(page, x) ((unsigned long)(x) & (page_size(page) - 1))

to simplify handling huge pages. So I always want to see offset be a
byte count. offset_in_page() is already taken, and I have no idea what
else to call the function to get the offset of this address within a
particular page.

> If we'd want a _descriptive_ name, then "byte_offset_of_page()" would
> probably be that. That's hard to mis-understand.
>
> Yes that's also more of a mouthful, and it still has the "two
> different names for the same thing" issue wrt
> stable/old/rebased/whatever patches.

That was one of the options we discussed, along with file_offset_of_page().

> Which is why I'd much rather change "pgoff_t" to "pgindex_t" and
> related "page_offset" variables to "page_index" variables.

There's only about 20 of those out of the 938 pgoff_t users. But there's
over a hundred called 'pgoff'. I need to get smarter about using
Coccinelle; I'm sure it can do this.