ACK: [SRU][F][PATCH 1/1] ceph: fix inode number handling on arches with 32-bit ino_t

[Andrea Righi]

On Wed, Oct 14, 2020 at 09:12:01PM +0200, frank.heimes at canonical.com wrote:
> From: Tuan Hoang <tmhoang at linux.ibm.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1899582
> 
> Upstream commit : ebce3eb2f7ef9f6ef01a60874ebd232450107c9a
> 
> Tuan and Ulrich mentioned that they were hitting a problem on s390x,
> which has a 32-bit ino_t value, even though it's a 64-bit arch (for
> historical reasons).
> 
> I think the current handling of inode numbers in the ceph driver is
> wrong. It tries to use 32-bit inode numbers on 32-bit arches, but that's
> actually not a problem. 32-bit arches can deal with 64-bit inode numbers
> just fine when userland code is compiled with LFS support (the common
> case these days).
> 
> What we really want to do is just use 64-bit numbers everywhere, unless
> someone has mounted with the ino32 mount option. In that case, we want
> to ensure that we hash the inode number down to something that will fit
> in 32 bits before presenting the value to userland.
> 
> Add new helper functions that do this, and only do the conversion before
> presenting these values to userland in getattr and readdir.
> 
> The inode table hashvalue is changed to just cast the inode number to
> unsigned long, as low-order bits are the most likely to vary anyway.
> 
> While it's not strictly required, we do want to put something in
> inode->i_ino. Instead of basing it on BITS_PER_LONG, however, base it on
> the size of the ino_t type.
> 
> NOTE: This is a user-visible change on 32-bit arches:
> 
> 1/ inode numbers will be seen to have changed between kernel versions.
>    32-bit arches will see large inode numbers now instead of the hashed
>    ones they saw before.
> 
> 2/ any really old software not built with LFS support may start failing
>    stat() calls with -EOVERFLOW on inode numbers >2^32. Nothing much we
>    can do about these, but hopefully the intersection of people running
>    such code on ceph will be very small.
> 
> The workaround for both problems is to mount with "-o ino32".
> 
> [ idryomov: changelog tweak ]
> 
> URL: https://tracker.ceph.com/issues/46828
> Reported-by: Ulrich Weigand <Ulrich.Weigand at de.ibm.com>
> Reported-and-Tested-by: Tuan Hoang1 <Tuan.Hoang1 at ibm.com>
> Signed-off-by: Jeff Layton <jlayton at kernel.org>
> Reviewed-by: "Yan, Zheng" <zyan at redhat.com>
> Signed-off-by: Ilya Dryomov <idryomov at gmail.com>
> (backported from commit ebce3eb2f7ef9f6ef01a60874ebd232450107c9a)
> Signed-off-by: Frank Heimes <frank.heimes at canonical.com>

As already mentioned by Stefan, you could have mentioned some details
about the backport activity (in particular about the struct cap_wait
part that is not needed in focal). But the fix makes sense and the
backport looks good to me, thanks!

Acked-by: Andrea Righi <andrea.righi at canonical.com>