On Tue, Nov 04, 2025 at 10:10:06AM -0800, Eric Biggers wrote: > On Tue, Nov 04, 2025 at 03:12:53AM -0800, Christoph Hellwig wrote: > > On Mon, Nov 03, 2025 at 08:48:29AM -0800, Eric Biggers wrote: > > > > *inode_ret = inode; > > > > - *lblk_num_ret = ((u64)folio->index << (PAGE_SHIFT - > > > > inode->i_blkbits)) + > > > > + *lblk_num_ret = (((u64)folio->index << PAGE_SHIFT) >> > > > > inode->i_blkbits) + > > > > This should be using folio_pos() instead of open coding the arithmetics. > > Well, folio_pos() doesn't work with sizes greater than S64_MAX, and it > uses multiplication rather than a shift.
What do you mean with "sizes greater than S64_MAX"? folio_pos works on a folio and is the MM designated helper to get the file offset from a folio, where a file offset is a loff_t, aka s64. And as answered to the previous mail, the compiler turns that multiplication into a shift. > Anyway, the trivial version avoids having to consider any of this... I see it the other way around - folio_pos is the defined way to get the index into the inode (block device inode here) in the abstract way.
