Prev: pci: Update pci_set_vga_state to call arch functions
Next: scripts/kallsyms build warning fix
From: Frederic Weisbecker on 25 Jan 2010 21:20
On Tue, Jan 26, 2010 at 11:13:59AM +0900, Tejun Heo wrote:
> Hello,
>
> On 01/26/2010 11:01 AM, Frederic Weisbecker wrote:
> > So, may be it considers you are applying the address space overriding
> > to the pointer to the type and not to the type itself.
> >
> > Consider:
> >
> > int __percpu i;
> >
> > What you do above *might* be considered as if SHIFT_PERCPU_PTR
> > returns something of a type:
> >
> > int * __percpu i;
> >
> > So the pointer is in the normal address space, but its content is in
> > __percpu address space.
> >
> > What if you do this:
> >
> >
> > #define SHIFT_PERCPU_PTR(__p, __offset) ({ \
> > __verify_pcpu_ptr((__p)); \
> > RELOC_HIDE((__p), (__offset)); \
> > })
> >
> > #define per_cpu(var, cpu) \
> > (typeof(var) __kernel __force)(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
>
> arch/x86/kernel/cpu/common.c:1149:20: warning: cast to non-scalar
> arch/x86/kernel/cpu/common.c:1149:20: error: strange non-value function or array
> CC arch/x86/kernel/cpu/common.o
> arch/x86/kernel/cpu/common.c: In function 'cpu_init':
> arch/x86/kernel/cpu/common.c:1149: error: cast specifies array type
>
> Can't cast that way. :-(


What about this? It doesn't use direct cast to scalar but should
create a pointer type to kernel space datas:


#define kernel_space_t(var) \
(typeof(var) __kernel __force)

#define SHIFT_PERCPU_PTR(__p, __offset) ({ \
__verify_pcpu_ptr((__p)); \
RELOC_HIDE((typeof(*(kernel_space_t(var)) __kernel __force *)(__p), (__offset)); \
})

#define per_cpu(var, cpu) \
(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 25 Jan 2010 21:30
On Tue, Jan 26, 2010 at 03:18:48AM +0100, Frederic Weisbecker wrote:
> On Tue, Jan 26, 2010 at 11:13:59AM +0900, Tejun Heo wrote:
> > Hello,
> >
> > On 01/26/2010 11:01 AM, Frederic Weisbecker wrote:
> > > So, may be it considers you are applying the address space overriding
> > > to the pointer to the type and not to the type itself.
> > >
> > > Consider:
> > >
> > > int __percpu i;
> > >
> > > What you do above *might* be considered as if SHIFT_PERCPU_PTR
> > > returns something of a type:
> > >
> > > int * __percpu i;
> > >
> > > So the pointer is in the normal address space, but its content is in
> > > __percpu address space.
> > >
> > > What if you do this:
> > >
> > >
> > > #define SHIFT_PERCPU_PTR(__p, __offset) ({ \
> > > __verify_pcpu_ptr((__p)); \
> > > RELOC_HIDE((__p), (__offset)); \
> > > })
> > >
> > > #define per_cpu(var, cpu) \
> > > (typeof(var) __kernel __force)(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> >
> > arch/x86/kernel/cpu/common.c:1149:20: warning: cast to non-scalar
> > arch/x86/kernel/cpu/common.c:1149:20: error: strange non-value function or array
> > CC arch/x86/kernel/cpu/common.o
> > arch/x86/kernel/cpu/common.c: In function 'cpu_init':
> > arch/x86/kernel/cpu/common.c:1149: error: cast specifies array type
> >
> > Can't cast that way. :-(
>
>
> What about this? It doesn't use direct cast to scalar but should
> create a pointer type to kernel space datas:
>
>
> #define kernel_space_t(var) \
> (typeof(var) __kernel __force)


Should be typeof(*var)



>
> #define SHIFT_PERCPU_PTR(__p, __offset) ({ \
> __verify_pcpu_ptr((__p)); \
> RELOC_HIDE((typeof(*(kernel_space_t(var)) __kernel __force *)(__p), (__offset)); \
> })
>
> #define per_cpu(var, cpu) \
> (*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Al Viro on 25 Jan 2010 21:40
On Tue, Jan 26, 2010 at 11:16:42AM +0900, Tejun Heo wrote:

> # define RELOC_HIDE(ptr, off) \
> ({ unsigned long __ptr; \
> __ptr = (unsigned long) (ptr); \
> (typeof(ptr)) (__ptr + (off)); })
>
> #define __verify_pcpu_ptr(ptr) do { \
> const void __percpu *__vpp_verify = (typeof(ptr))NULL; \
> (void)__vpp_verify; \
> } while (0)
>
> /* Weird cast keeps both GCC and sparse happy. */
> #define SHIFT_PERCPU_PTR(__p, __offset) ({ \
> __verify_pcpu_ptr((__p)); \
> RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)); \
> })
>
> #define per_cpu(var, cpu) \
> (*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))

Eh... You are leaving that noderef in place in case of array. And _that_
is not an address space, so casts to AS 0 won't do you any good.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 25 Jan 2010 21:40
On Tue, Jan 26, 2010 at 11:34:40AM +0900, Tejun Heo wrote:
> On 01/26/2010 11:22 AM, Frederic Weisbecker wrote:
> >> What about this? It doesn't use direct cast to scalar but should
> >> create a pointer type to kernel space datas:
> >>
> >>
> >> #define kernel_space_t(var) \
> >> (typeof(var) __kernel __force)
> >
> >
> > Should be typeof(*var)
> >
> >>
> >> #define SHIFT_PERCPU_PTR(__p, __offset) ({ \
> >> __verify_pcpu_ptr((__p)); \
> >> RELOC_HIDE((typeof(*(kernel_space_t(var)) __kernel __force *)(__p), (__offset)); \
> >> })
>
> Can you balance ()'s here too?


#define kernel_space_t(var) \
(typeof(*(var)) __kernel __force)


/* Weird cast keeps both GCC and sparse happy. */
#define SHIFT_PERCPU_PTR(__p, __offset) ({ \
__verify_pcpu_ptr((__p)); \
RELOC_HIDE((typeof(*(kernel_space_t(__p))) __kernel __force *)(__p), (__offset)); \
})

#define per_cpu(var, cpu) \
(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)));


No guarantee that will build. I should pull your tree and install
sparse (yeah, shame on me, I've never installed it).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Al Viro on 25 Jan 2010 22:00
On Tue, Jan 26, 2010 at 11:43:56AM +0900, Tejun Heo wrote:

> > Eh... You are leaving that noderef in place in case of array. And _that_
> > is not an address space, so casts to AS 0 won't do you any good.
>
> Any ideas on how to fix it?

BTW, before we go any further, which warnings are you getting from sparse
and which version of sparse are you using?

noderef is one thing; address_space mess is a different story. The version
I have here steps into the former, but not the latter; what are you seeing?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: pci: Update pci_set_vga_state to call arch functions
Next: scripts/kallsyms build warning fix