Messages in this thread | | | Date | Mon, 18 Jun 2007 12:58:34 +0530 | From | "Satyam Sharma" <> | Subject | Re: [PATCH] mm: More __meminit annotations. |
| |
On 6/18/07, Satyam Sharma <satyam.sharma@gmail.com> wrote: > Hi, > > On 6/18/07, Yasunori Goto <y-goto@jp.fujitsu.com> wrote: > > > On Mon, Jun 18, 2007 at 02:49:24PM +0900, Yasunori Goto wrote: > > > > > -static inline unsigned long zone_absent_pages_in_node(int nid, > > > > > +static inline unsigned long __meminit zone_absent_pages_in_node(int nid, > > > > > unsigned long zone_type, > > > > > unsigned long *zholes_size) > > > > > { > > > > > > > > I thought __meminit is not effective for these static functions, > > > > because they are inlined function. So, it depends on caller's > > > > defenition. Is it wrong? > > > > > > > Ah, that's possible, I hadn't considered that. It seems to be a bit more > > > obvious what the intention is if it's annotated, especially as this is > > > the convention that's used by the rest of mm/page_alloc.c. A bit more > > > consistent, if nothing more. > > > > I'm not sure which is intended. I found some functions define both > > __init and inline in kernel tree. And probably, some functions don't > > do it. So, it seems there is no convention. > > > > I'm Okay if you prefer both defined. :-) > > Marking inline functions as __init (or __meminit etc) is quite insane, > IMHO. Note that all callers of the said inline function will also have to > be __init anyway (else modpost will barf)
Actually, modpost will _not_ complain precisely _because_ kernel uses always_inline so a separate body for the function will never be emitted at all. But all callers of said inline function will *still* need to be in __init anyway, else if the said inline function itself calls some __init function (which is likely) and the caller of the said inline function is not __init *then* modpost will complain.
> so the said function will > have all callsites in .init.text anyway, and hence would be inlined > in the same section as the caller (i.e. .init.text). [Note that kernel > uses always_inline.] > > The annotation may still be a readability aid (which is subjective so > one can't really comment upon), but asking gcc to put into a separate > specified section, a function whose body would not be emitted by gcc > separately at all, doesn't really make much sense syntactically _or_ > semantically -- gcc might not warn, of course, perhaps it's one of those > little things it takes care of by itself silently without complaining (like > taking pointers to inline functions).
All this is valid, still. Perhaps sparse warns / can be made to warn about such cases (which may not be bugs, but weird C, at least)? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |