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.
