From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: Christopher Lameter <cl@linux.com>
Cc: Dennis Zhou <dennis@kernel.org>,
Ben Dooks <ben.dooks@codethink.co.uk>,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
Tejun Heo <tj@kernel.org>, Nicholas Piggin <npiggin@gmail.com>,
Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH] fix __percpu annotation in asm-generic
Date: Sat, 30 Nov 2019 01:00:37 +0100 [thread overview]
Message-ID: <20191130000037.zsendu5pk7p75xqf@ltop.local> (raw)
In-Reply-To: <alpine.DEB.2.21.1911291808530.1365@www.lameter.com>
On Fri, Nov 29, 2019 at 06:11:59PM +0000, Christopher Lameter wrote:
> On Wed, 27 Nov 2019, Luc Van Oostenryck wrote:
>
> > 1) it would strip any address space, not just __percpu, so:
> > it would need to be combined with __verify_pcpu_ptr() or,
> > * a better name should be used,
>
> typeof_cast_kernel() to express the fact that it creates a kernel pointer
> and ignored the attributes??
typeof_strip_address_space() would, I think, express this better.
It's not obvious at all to me that 'kernel' in 'typeof_cast_kernel()'
relates to the (default) kernel address space.
Maybe it's just me. I don't know.
> > * it should be defined in a generic header, any idea where?
>
> include/linux/compiler-types.h
Yes, OK.
> > 2) while I find the current solution:
> > typeof(T) __kernel __force *ptr = ...;
>
> It would be
>
> typeof_cast_kernel(&T) *xx = xxx
>
> or so?
No, it would not. __percpu, and more generally, the address space
is a property of the object, not of its address.
For example, let's say T is a __percpu object:
int __percpu obj;
then '&T' is just a 'normal'/__kernel pointer to it:
int __percpu *;
There is nothing to strip (it would be if the __percpu
would be 'on the other side of the *': int * __percpu).
It's exactly the same as with 'const': a 'const char *'
is not const, only a pointer to const.
The situation with raw_cpu_generic_add_return() is:
- pcp is a lvalue of of a __percpu object of type T, so:
typeof(pcp) := T __percpu
- pcp's address is given to raw_cpu_ptr(), so
typeof(&pcp) := T __percpu *
- raw_cpu_ptr() return the corresponding __kernel pointer
(adjusted for the current percu offset), so:
typeof(raw_cpu_ptr(&pcp)) := T *
- so, the macro needs to declare a variable __p of type T*
hence:
typeof(pcp) __kernel __force *__p;
or, with this new macro:
typeof_cast_kernel(pcp) *__p;
Maybe a better solution would be to directly play at pointer
level and thus have something like this:
typeof_<some good name>(&pcp) __p = raw_cpu_ptr(&pcp);
or even:
__kernel_pointer(&pcp) __p = raw_cpu_ptr(&pcp);
I dunno.
Note: at implementation level, it complicates things slightly
to want this 'strip_percpu' macro to behaves like typeof()
because it means that it can take in argument either an
expression or a type. And if it's a type, you can't do a
simple cast on it, you need to declare an intermediate
variable, hence the horrible:
typeof(({ typeof(T) __kernel __force __fakename; __fakename; }))
Note: it would be much much nicer to do all these type generic
macros with '__auto_type' (only supported in GCC 4.9 IIUC
and supported in sparse but it shouldn't be very hard to do)..
-- Luc
next prev parent reply other threads:[~2019-11-30 0:00 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-26 20:06 [PATCH] fix __percpu annotation in asm-generic Luc Van Oostenryck
2019-11-27 15:55 ` Christopher Lameter
2019-11-27 17:53 ` Dennis Zhou
2019-11-27 22:54 ` Luc Van Oostenryck
2019-11-29 18:11 ` Christopher Lameter
2019-11-30 0:00 ` Luc Van Oostenryck [this message]
2019-12-02 19:07 ` Dennis Zhou
2019-12-02 19:42 ` Christopher Lameter
2019-12-03 3:01 ` Luc Van Oostenryck
2020-03-24 4:13 ` Jason A. Donenfeld
2020-03-24 6:44 ` Luc Van Oostenryck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191130000037.zsendu5pk7p75xqf@ltop.local \
--to=luc.vanoostenryck@gmail.com \
--cc=arnd@arndb.de \
--cc=ben.dooks@codethink.co.uk \
--cc=cl@linux.com \
--cc=dennis@kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=npiggin@gmail.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).