[PATCH] fuse: change fuse_wr_pages() to avoid signedness error from min()

[PATCH] fuse: change fuse_wr_pages() to avoid signedness error from min()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

On 32bit builds the 'number of pages required' calculation is signed
and min() complains because max_pages is unsigned.
Change the calcualtion that determines the number of pages by adding the
'offset in page' to 'len' rather than subtracting the end and start pages.
Although the 64bit value is still signed, the compiler knows it isn't
negative so min() doesn't complain.
The generated code is also slightly better.

Forcing the calculation to 32 bits (eg len + (size_t)(pos & ...))
generates much better code and is probably safe because len should
be limited to 'INT_MAX - PAGE_SIZE).

Fixes: 0f5bb0cfb0b4 ("fs: use min() or umin() instead of min_t()")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202512160948.O7QqxHj2-lkp@intel.com/
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 fs/fuse/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 4f71eb5a9bac..98edb6a2255d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1323,7 +1323,7 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
 static inline unsigned int fuse_wr_pages(loff_t pos, size_t len,
 				     unsigned int max_pages)
 {
-	return min(((pos + len - 1) >> PAGE_SHIFT) - (pos >> PAGE_SHIFT) + 1,
+	return min(((len + (pos & (PAGE_SIZE - 1)) - 1) >> PAGE_SHIFT) + 1,
 		   max_pages);
 }
 
-- 
2.39.5

Re: [PATCH] fuse: change fuse_wr_pages() to avoid signedness error from min()

[Joanne Koong]

On Wed, Dec 17, 2025 at 12:22 AM <david.laight.linux@gmail.com> wrote:
>
> From: David Laight <david.laight.linux@gmail.com>
>
> On 32bit builds the 'number of pages required' calculation is signed
> and min() complains because max_pages is unsigned.
> Change the calcualtion that determines the number of pages by adding the
> 'offset in page' to 'len' rather than subtracting the end and start pages.
> Although the 64bit value is still signed, the compiler knows it isn't
> negative so min() doesn't complain.
> The generated code is also slightly better.
>
> Forcing the calculation to 32 bits (eg len + (size_t)(pos & ...))
> generates much better code and is probably safe because len should
> be limited to 'INT_MAX - PAGE_SIZE).
>
> Fixes: 0f5bb0cfb0b4 ("fs: use min() or umin() instead of min_t()")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202512160948.O7QqxHj2-lkp@intel.com/
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>  fs/fuse/file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 4f71eb5a9bac..98edb6a2255d 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1323,7 +1323,7 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
>  static inline unsigned int fuse_wr_pages(loff_t pos, size_t len,
>                                      unsigned int max_pages)
>  {
> -       return min(((pos + len - 1) >> PAGE_SHIFT) - (pos >> PAGE_SHIFT) + 1,
> +       return min(((len + (pos & (PAGE_SIZE - 1)) - 1) >> PAGE_SHIFT) + 1,
>                    max_pages);

I find this logic a bit confusing to read still, what about something like:

unsigned int nr_pages = DIV_ROUND_UP(offset_in_page(pos) + len, PAGE_SIZE);
return min(nr_pages, max_pages);

instead? I think the compiler will automatically optimize the
DIV_ROUND_UP to use a bit shift.

Thanks,
Joanne
>  }
>
> --
> 2.39.5
>
>

Re: [PATCH] fuse: change fuse_wr_pages() to avoid signedness error from min()

[David Laight]

On Fri, 19 Dec 2025 11:24:23 +0800
Joanne Koong <joannelkoong@gmail.com> wrote:

> On Wed, Dec 17, 2025 at 12:22 AM <david.laight.linux@gmail.com> wrote:
> >
> > From: David Laight <david.laight.linux@gmail.com>
> >
> > On 32bit builds the 'number of pages required' calculation is signed
> > and min() complains because max_pages is unsigned.
> > Change the calcualtion that determines the number of pages by adding the
> > 'offset in page' to 'len' rather than subtracting the end and start pages.
> > Although the 64bit value is still signed, the compiler knows it isn't
> > negative so min() doesn't complain.
> > The generated code is also slightly better.
> >
> > Forcing the calculation to 32 bits (eg len + (size_t)(pos & ...))
> > generates much better code and is probably safe because len should
> > be limited to 'INT_MAX - PAGE_SIZE).
> >
> > Fixes: 0f5bb0cfb0b4 ("fs: use min() or umin() instead of min_t()")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202512160948.O7QqxHj2-lkp@intel.com/
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> >  fs/fuse/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 4f71eb5a9bac..98edb6a2255d 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -1323,7 +1323,7 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
> >  static inline unsigned int fuse_wr_pages(loff_t pos, size_t len,
> >                                      unsigned int max_pages)
> >  {
> > -       return min(((pos + len - 1) >> PAGE_SHIFT) - (pos >> PAGE_SHIFT) + 1,
> > +       return min(((len + (pos & (PAGE_SIZE - 1)) - 1) >> PAGE_SHIFT) + 1,
> >                    max_pages);  
> 
> I find this logic a bit confusing to read still, what about something like:
> 
> unsigned int nr_pages = DIV_ROUND_UP(offset_in_page(pos) + len, PAGE_SIZE);
> return min(nr_pages, max_pages);

You can just do:
	return min(DIV_ROUND_UP(offset_in_page(pos) + len, PAGE_SIZE), max_pages);

or splitting the long line:
	len += offset_in_page(pos);
	return min(DIV_ROUND_UP(len, PAGE_SIZE), max_pages);

Using offset_in_page() and DIV_ROUND_UP adds the 'hidden' requirement that
	'len <= MAX_ULONG - 2 * PAGE_SIZE'.
(Should be true - read/write (etc) are bounded to MAX_INT - PAGE_SIZE.)

> instead? I think the compiler will automatically optimize the
> DIV_ROUND_UP to use a bit shift.

Provided it is an unsigned divide - and the LHS is unsigned.

DIV_ROUNDUP(a, b) is '(a + b - 1)/b' which can overflow for large 'a'.
The other option is '(a - 1)/b + 1' which is valid for non-zero 'a'.

	David

> 
> Thanks,
> Joanne
> >  }
> >
> > --
> > 2.39.5
> >
> >