Re: [PATCH v2 2/2] mm/filemap: fix mapping_seek_hole_data on THP & 32-bit

From: Hugh Dickins
Date: Fri Apr 23 2021 - 13:23:10 EST


On Thu, 22 Apr 2021, Andrew Morton wrote:
> On Thu, 22 Apr 2021 13:48:57 -0700 (PDT) Hugh Dickins <hughd@xxxxxxxxxx> wrote:
>
> > Andrew, I'd have just sent a -fix.patch to remove the unnecessary u64s,
> > but need to reword the commit message: so please replace yesterday's
> > mm-filemap-fix-mapping_seek_hole_data-on-thp-32-bit.patch
> > by this one - thanks.
>
> Actually, I routinely update the base patch's changelog when queueing a -fix.

And thank you for that, but if there's time, I think we would still
prefer the final commit message to include corrections where Matthew
enlightened me (that "sign-extension" claim came from my confusion):

-u64 casts added to stop unfortunate sign-extension when shifting (and
-let's use shifts throughout, rather than mixed with * and /).
-
-Use round_up() when advancing pos, to stop assuming that pos was already
-THP-aligned when advancing it by THP-size. (But I believe this use of
-round_up() assumes that any THP must be THP-aligned: true while tmpfs
-enforces that alignment, and is the only fs with FS_THP_SUPPORT; but might
-need to be generalized in the future? If I try to generalize it right
-now, I'm sure to get it wrong!)
+u64 cast to stop losing bits when converting unsigned long to loff_t
+(and let's use shifts throughout, rather than mixed with * and /).
+
+Use round_up() when advancing pos, to stop assuming that pos was
+already THP-aligned when advancing it by THP-size. (This use of
+round_up() assumes that any THP has THP-aligned index: true at present
+and true going forward, but could be recoded to avoid the assumption.)

Thanks,
Hugh