Re: fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

Re: fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

[Christoph Hellwig]

On Mon, Feb 17, 2025 at 01:53:08PM +0800, kernel test robot wrote:
> New smatch warnings:
> fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

Looks like this is an issue in the riscv virt_to_page implementation
which also shows up in various other places.  Any chance this could get
fixed in the riscv code?

> 
> Old smatch warnings:
> fs/xfs/xfs_linux.h:283 kmem_to_page() warn: unsigned '_x' is never less than zero.
> fs/xfs/xfs_buf.c:761 xfs_buf_get_map() error: we previously assumed 'bp' could be null (see line 743)
> arch/riscv/include/asm/atomic.h:218 arch_atomic_fetch_add_unless() warn: inconsistent indenting
> 
> vim +/_x +1534 fs/xfs/xfs_buf.c
> 
>   1518	
>   1519	static void
>   1520	xfs_buf_submit_bio(
>   1521		struct xfs_buf		*bp)
>   1522	{
>   1523		unsigned int		size = BBTOB(bp->b_length);
>   1524		unsigned int		map = 0, p;
>   1525		struct blk_plug		plug;
>   1526		struct bio		*bio;
>   1527	
>   1528		bio = bio_alloc(bp->b_target->bt_bdev, bp->b_page_count,
>   1529				xfs_buf_bio_op(bp), GFP_NOIO);
>   1530		bio->bi_private = bp;
>   1531		bio->bi_end_io = xfs_buf_bio_end_io;
>   1532	
>   1533		if (bp->b_flags & _XBF_KMEM) {
> > 1534			__bio_add_page(bio, virt_to_page(bp->b_addr), size,
>   1535					bp->b_offset);
>   1536		} else {
>   1537			for (p = 0; p < bp->b_page_count; p++)
>   1538				__bio_add_page(bio, bp->b_pages[p], PAGE_SIZE, 0);
>   1539			bio->bi_iter.bi_size = size; /* limit to the actual size used */
>   1540	
>   1541			if (xfs_buf_is_vmapped(bp))
>   1542				flush_kernel_vmap_range(bp->b_addr,
>   1543						xfs_buf_vmap_len(bp));
>   1544		}
>   1545	
>   1546		/*
>   1547		 * If there is more than one map segment, split out a new bio for each
>   1548		 * map except of the last one.  The last map is handled by the
>   1549		 * remainder of the original bio outside the loop.
>   1550		 */
>   1551		blk_start_plug(&plug);
>   1552		for (map = 0; map < bp->b_map_count - 1; map++) {
>   1553			struct bio	*split;
>   1554	
>   1555			split = bio_split(bio, bp->b_maps[map].bm_len, GFP_NOFS,
>   1556					&fs_bio_set);
>   1557			split->bi_iter.bi_sector = bp->b_maps[map].bm_bn;
>   1558			bio_chain(split, bio);
>   1559			submit_bio(split);
>   1560		}
>   1561		bio->bi_iter.bi_sector = bp->b_maps[map].bm_bn;
>   1562		submit_bio(bio);
>   1563		blk_finish_plug(&plug);
>   1564	}
>   1565	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
---end quoted text---

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

[Alexandre Ghiti]

Hi Christoph,

On 17/02/2025 10:24, Christoph Hellwig wrote:
> On Mon, Feb 17, 2025 at 01:53:08PM +0800, kernel test robot wrote:
>> New smatch warnings:
>> fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.
> Looks like this is an issue in the riscv virt_to_page implementation
> which also shows up in various other places.  Any chance this could get
> fixed in the riscv code?


To me, the only test that could give rise to this warning is the last 
part of:

#define is_linear_mapping(x) \
         ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < 
PAGE_OFFSET + KERN_VIRT_SIZE))

But given that the config is a 32-bit config, it should not be evaluated 
at all.

Could that be a false-positive and then an issue in smatch?

Thanks,

Alex


>
>> Old smatch warnings:
>> fs/xfs/xfs_linux.h:283 kmem_to_page() warn: unsigned '_x' is never less than zero.
>> fs/xfs/xfs_buf.c:761 xfs_buf_get_map() error: we previously assumed 'bp' could be null (see line 743)
>> arch/riscv/include/asm/atomic.h:218 arch_atomic_fetch_add_unless() warn: inconsistent indenting
>>
>> vim +/_x +1534 fs/xfs/xfs_buf.c
>>
>>    1518	
>>    1519	static void
>>    1520	xfs_buf_submit_bio(
>>    1521		struct xfs_buf		*bp)
>>    1522	{
>>    1523		unsigned int		size = BBTOB(bp->b_length);
>>    1524		unsigned int		map = 0, p;
>>    1525		struct blk_plug		plug;
>>    1526		struct bio		*bio;
>>    1527	
>>    1528		bio = bio_alloc(bp->b_target->bt_bdev, bp->b_page_count,
>>    1529				xfs_buf_bio_op(bp), GFP_NOIO);
>>    1530		bio->bi_private = bp;
>>    1531		bio->bi_end_io = xfs_buf_bio_end_io;
>>    1532	
>>    1533		if (bp->b_flags & _XBF_KMEM) {
>>> 1534			__bio_add_page(bio, virt_to_page(bp->b_addr), size,
>>    1535					bp->b_offset);
>>    1536		} else {
>>    1537			for (p = 0; p < bp->b_page_count; p++)
>>    1538				__bio_add_page(bio, bp->b_pages[p], PAGE_SIZE, 0);
>>    1539			bio->bi_iter.bi_size = size; /* limit to the actual size used */
>>    1540	
>>    1541			if (xfs_buf_is_vmapped(bp))
>>    1542				flush_kernel_vmap_range(bp->b_addr,
>>    1543						xfs_buf_vmap_len(bp));
>>    1544		}
>>    1545	
>>    1546		/*
>>    1547		 * If there is more than one map segment, split out a new bio for each
>>    1548		 * map except of the last one.  The last map is handled by the
>>    1549		 * remainder of the original bio outside the loop.
>>    1550		 */
>>    1551		blk_start_plug(&plug);
>>    1552		for (map = 0; map < bp->b_map_count - 1; map++) {
>>    1553			struct bio	*split;
>>    1554	
>>    1555			split = bio_split(bio, bp->b_maps[map].bm_len, GFP_NOFS,
>>    1556					&fs_bio_set);
>>    1557			split->bi_iter.bi_sector = bp->b_maps[map].bm_bn;
>>    1558			bio_chain(split, bio);
>>    1559			submit_bio(split);
>>    1560		}
>>    1561		bio->bi_iter.bi_sector = bp->b_maps[map].bm_bn;
>>    1562		submit_bio(bio);
>>    1563		blk_finish_plug(&plug);
>>    1564	}
>>    1565	
>>
>> -- 
>> 0-DAY CI Kernel Test Service
>> https://github.com/intel/lkp-tests/wiki
> ---end quoted text---
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

[David Laight]

On Mon, 17 Feb 2025 13:06:49 +0100
Alexandre Ghiti <alex@ghiti.fr> wrote:

> Hi Christoph,
> 
> On 17/02/2025 10:24, Christoph Hellwig wrote:
> > On Mon, Feb 17, 2025 at 01:53:08PM +0800, kernel test robot wrote:  
> >> New smatch warnings:
> >> fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.  
> > Looks like this is an issue in the riscv virt_to_page implementation
> > which also shows up in various other places.  Any chance this could get
> > fixed in the riscv code?  
> 
> 
> To me, the only test that could give rise to this warning is the last 
> part of:
> 
> #define is_linear_mapping(x) \
>          ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < 
> PAGE_OFFSET + KERN_VIRT_SIZE))
> 
> But given that the config is a 32-bit config, it should not be evaluated 
> at all.
> 
> Could that be a false-positive and then an issue in smatch?

Why is smatch even looking.
The equivalent check in gcc has been moved to -W2 because of all false positives.

	David

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

[Philip Li]

+ Dan

Hi Dan,

On Mon, Feb 17, 2025 at 01:12:03PM +0000, David Laight wrote:
> On Mon, 17 Feb 2025 13:06:49 +0100
> Alexandre Ghiti <alex@ghiti.fr> wrote:
> 
> > Hi Christoph,
> > 
> > On 17/02/2025 10:24, Christoph Hellwig wrote:
> > > On Mon, Feb 17, 2025 at 01:53:08PM +0800, kernel test robot wrote:  
> > >> New smatch warnings:
> > >> fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.  
> > > Looks like this is an issue in the riscv virt_to_page implementation
> > > which also shows up in various other places.  Any chance this could get
> > > fixed in the riscv code?  
> > 
> > 
> > To me, the only test that could give rise to this warning is the last 
> > part of:
> > 
> > #define is_linear_mapping(x) \
> >          ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < 
> > PAGE_OFFSET + KERN_VIRT_SIZE))
> > 
> > But given that the config is a 32-bit config, it should not be evaluated 
> > at all.
> > 
> > Could that be a false-positive and then an issue in smatch?
> 
> Why is smatch even looking.
> The equivalent check in gcc has been moved to -W2 because of all false positives.

Can you help check this one?

Thanks

> 
> 	David
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

[Dan Carpenter]

On Tue, Feb 18, 2025 at 08:39:21AM +0800, Philip Li wrote:
> + Dan
> 
> Hi Dan,
> 
> On Mon, Feb 17, 2025 at 01:12:03PM +0000, David Laight wrote:
> > On Mon, 17 Feb 2025 13:06:49 +0100
> > Alexandre Ghiti <alex@ghiti.fr> wrote:
> > 
> > > Hi Christoph,
> > > 
> > > On 17/02/2025 10:24, Christoph Hellwig wrote:
> > > > On Mon, Feb 17, 2025 at 01:53:08PM +0800, kernel test robot wrote:  
> > > >> New smatch warnings:
> > > >> fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.  
> > > > Looks like this is an issue in the riscv virt_to_page implementation
> > > > which also shows up in various other places.  Any chance this could get
> > > > fixed in the riscv code?  
> > > 
> > > 
> > > To me, the only test that could give rise to this warning is the last 
> > > part of:
> > > 
> > > #define is_linear_mapping(x) \
> > >          ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < 
> > > PAGE_OFFSET + KERN_VIRT_SIZE))
> > > 
> > > But given that the config is a 32-bit config, it should not be evaluated 
> > > at all.
> > > 
> > > Could that be a false-positive and then an issue in smatch?
> > 
> > Why is smatch even looking.
> > The equivalent check in gcc has been moved to -W2 because of all false positives.

The Smatch check is a bit more sophisticated than the GCC check...
I think if you removed the (!IS_ENABLED(CONFIG_64BIT) condition then
Smatch wouldn't trigger a warning here.  How would I duplicate this
warning?  The "ARCH=riscv make.cross" command does a 64bit build.

Screw it, I can just silence this warning based on that it's a kernel
build and the variable is called "_x".

regards,
dan carpenter


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

[Alexandre Ghiti]

Hi Dan,

On 18/02/2025 15:21, Dan Carpenter wrote:
> On Tue, Feb 18, 2025 at 08:39:21AM +0800, Philip Li wrote:
>> + Dan
>>
>> Hi Dan,
>>
>> On Mon, Feb 17, 2025 at 01:12:03PM +0000, David Laight wrote:
>>> On Mon, 17 Feb 2025 13:06:49 +0100
>>> Alexandre Ghiti <alex@ghiti.fr> wrote:
>>>
>>>> Hi Christoph,
>>>>
>>>> On 17/02/2025 10:24, Christoph Hellwig wrote:
>>>>> On Mon, Feb 17, 2025 at 01:53:08PM +0800, kernel test robot wrote:
>>>>>> New smatch warnings:
>>>>>> fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.
>>>>> Looks like this is an issue in the riscv virt_to_page implementation
>>>>> which also shows up in various other places.  Any chance this could get
>>>>> fixed in the riscv code?
>>>>
>>>> To me, the only test that could give rise to this warning is the last
>>>> part of:
>>>>
>>>> #define is_linear_mapping(x) \
>>>>           ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) <
>>>> PAGE_OFFSET + KERN_VIRT_SIZE))
>>>>
>>>> But given that the config is a 32-bit config, it should not be evaluated
>>>> at all.
>>>>
>>>> Could that be a false-positive and then an issue in smatch?
>>> Why is smatch even looking.
>>> The equivalent check in gcc has been moved to -W2 because of all false positives.
> The Smatch check is a bit more sophisticated than the GCC check...
> I think if you removed the (!IS_ENABLED(CONFIG_64BIT) condition then
> Smatch wouldn't trigger a warning here.  How would I duplicate this
> warning?  The "ARCH=riscv make.cross" command does a 64bit build.


riscv uses the same toolchain for both 32 and 64-bit, if you look at the 
faulty config, you can see that CONFIG_32BIT is enabled so it actually 
is a 32-bit build.

Thanks for jumping in,

Alex


>
> Screw it, I can just silence this warning based on that it's a kernel
> build and the variable is called "_x".
>
> regards,
> dan carpenter
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.

[David Laight]

On Mon, 17 Feb 2025 13:06:49 +0100
Alexandre Ghiti <alex@ghiti.fr> wrote:

> Hi Christoph,
> 
> On 17/02/2025 10:24, Christoph Hellwig wrote:
> > On Mon, Feb 17, 2025 at 01:53:08PM +0800, kernel test robot wrote:  
> >> New smatch warnings:
> >> fs/xfs/xfs_buf.c:1534 xfs_buf_submit_bio() warn: unsigned '_x' is never less than zero.  
> > Looks like this is an issue in the riscv virt_to_page implementation
> > which also shows up in various other places.  Any chance this could get
> > fixed in the riscv code?  
> 
> 
> To me, the only test that could give rise to this warning is the last 
> part of:
> 
> #define is_linear_mapping(x) \
>          ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < PAGE_OFFSET + KERN_VIRT_SIZE))
> 
> But given that the config is a 32-bit config, it should not be evaluated 
> at all.
> 
> Could that be a false-positive and then an issue in smatch?

It's not managing to try to build when:
#define PAGE_OFFSET		_AC(CONFIG_PAGE_OFFSET, UL)
is zero?

I think that is supposed to be (near) the user-kernel boundary.
So probably 0x80000000 or 0xc0000000.
(or 0xe0000000 for some old sparc32)

	David

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv