Re: mm/gup.c:778:15: sparse: sparse: cast to non-scalar

[Peter Xu <peterx@redhat.com>]

On Fri, Jan 24, 2025 at 10:01:40AM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   e0b1f59142746f74476a03040f745329c8355a7e
> commit: e6fd5564c07c3c749ff3d1b2aa35540b4047e395 mm/gup: cache p4d in follow_p4d_mask()
> date:   9 months ago
> config: alpha-randconfig-r112-20250124 (https://download.01.org/0day-ci/archive/20250124/202501240922.a4YzQD7u-lkp@intel.com/config)
> compiler: alpha-linux-gcc (GCC) 14.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20250124/202501240922.a4YzQD7u-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202501240922.a4YzQD7u-lkp@intel.com/
> 
> sparse warnings: (new ones prefixed by >>)
> >> mm/gup.c:778:15: sparse: sparse: cast to non-scalar
> >> mm/gup.c:778:15: sparse: sparse: cast from non-scalar
>    mm/gup.c: note: in included file (through include/linux/mm.h):
>    include/linux/pgtable.h:315:16: sparse: sparse: cast to non-scalar
>    include/linux/pgtable.h:315:16: sparse: sparse: cast from non-scalar
>    include/linux/pgtable.h:315:16: sparse: sparse: cast to non-scalar
>    include/linux/pgtable.h:315:16: sparse: sparse: cast from non-scalar
>    mm/gup.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/mm.h):
>    include/linux/page-flags.h:241:46: sparse: sparse: self-comparison always evaluates to false
>    mm/gup.c:682:9: sparse: sparse: context imbalance in 'follow_page_pte' - unexpected unlock
>    mm/gup.c: note: in included file (through include/linux/mm.h):
>    include/linux/pgtable.h:322:16: sparse: sparse: cast to non-scalar
>    include/linux/pgtable.h:322:16: sparse: sparse: cast from non-scalar
>    include/linux/pgtable.h:315:16: sparse: sparse: cast to non-scalar
>    include/linux/pgtable.h:315:16: sparse: sparse: cast from non-scalar
>    mm/gup.c:911:18: sparse: sparse: context imbalance in 'get_gate_page' - unexpected unlock
> 
> vim +778 mm/gup.c
> 
>    769	
>    770	static struct page *follow_p4d_mask(struct vm_area_struct *vma,
>    771					    unsigned long address, pgd_t *pgdp,
>    772					    unsigned int flags,
>    773					    struct follow_page_context *ctx)
>    774	{
>    775		p4d_t *p4dp, p4d;
>    776	
>    777		p4dp = p4d_offset(pgdp, address);
>  > 778		p4d = READ_ONCE(*p4dp);
>    779		if (p4d_none(p4d))
>    780			return no_page_table(vma, flags);
>    781		BUILD_BUG_ON(p4d_huge(p4d));
>    782		if (unlikely(p4d_bad(p4d)))
>    783			return no_page_table(vma, flags);
>    784	
>    785		return follow_pud_mask(vma, address, p4dp, flags, ctx);
>    786	}
>    787	

There's one more similar sparse warning here, both reported today:

https://lore.kernel.org/all/202501241212.q9AAshQc-lkp@intel.com/

Firstly, I reproduced this sparse warning locally. And yes it yells about
that line, and even one more similar case of READ_ONCE().

The question is I thought READ_ONCE() should be ok to be used on things
like p4d_t, even if they can be structs / non-scalar types.  At least from
sparse POV I cannot trigger similar sparse on non-alpha.  So looks like
Alpha has something special.

Then I noticed that Alpha indeed has a custom definition of __READ_ONCE()
which is probably needed due to the mb():

#define __READ_ONCE(x)							\
({									\
	__unqual_scalar_typeof(x) __x =					\
		(*(volatile typeof(__x) *)(&(x)));			\
	mb();								\
	(typeof(x))__x;							\
})

I didn't obviously see yet on how that could affect sparse, if READ_ONCE()
works for non-scalar somehow on most of the rest archs.

I tried to switch to the default __READ_ONCE for alpha build (it misses
mb() so it isn't correct, but trying to figure out whether it'll trigger
similar sparse warning), then the warning is gone.

Then I found that this change can also work for alpha:

diff --git a/arch/alpha/include/asm/rwonce.h b/arch/alpha/include/asm/rwonce.h
index 35542bcf92b3..0d61183ff764 100644
--- a/arch/alpha/include/asm/rwonce.h
+++ b/arch/alpha/include/asm/rwonce.h
@@ -25,7 +25,7 @@
        __unqual_scalar_typeof(x) __x =                                 \
                (*(volatile typeof(__x) *)(&(x)));                      \
        mb();                                                           \
-       (typeof(x))__x;                                                 \
+       __x;                                                    \
 })

I had a feeling that sparse somehow thinks the "typeof(x)" isn't the same
struct v.s. "__unqual_scalar_typeof(x)" above, even if IIUC the macro was
only trying to tear down type qualifiers.  Especially in Alpha's case IIUC
p4d_t is a struct (in my case, STRICT_MM_TYPECHECKS=n):

        typedef struct { pgd_t pgd; } p4d_t;
        typedef unsigned long pgd_t;

So I suppose __unqual_scalar_typeof() should do nothing on a non-scalar
type.

So even if above oneliner seems to fix this specific sparse warning, I
don't really know how it worked.

I copied Will (who introduced both the alpha's __READ_ONCE, and the unqual
macro for generic code) in case he has some thoughts..

Thanks,

-- 
Peter Xu