From: Benjamin Herrenschmidt <benh@ozlabs.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
linuxppc-dev@ozlabs.org, takata@linux-m32r.org,
linux-m32r@ml.linux-m32r.org,
Linux-Arch <linux-arch@vger.kernel.org>
Subject: Re: [PATCH 1/2] Add thread_info_cache_init() to all archs
Date: Mon, 14 Apr 2008 10:38:26 +1000 [thread overview]
Message-ID: <1208133506.6958.82.camel@pasglop> (raw)
In-Reply-To: <20080413171953.bde5e9ac.akpm@linux-foundation.org>
> > +#ifndef thread_info_cache_init
> > +#define thread_info_cache_init do { } while(0)
> > +#endif
>
> This trick does cause a bit of a problem: it is undefined which arch header
> file is to provide the alternative definition of thread_info_cache_init.
I this case it's well defined: thread_info.h. Maybe I should add a
comment ?
> So we can (and have) ended up in the situation where the override appears
> in different files on different architectures and various screwups ensue.
Yup.
> So I'd suggest that we have a bigfatcomment telling implementors which file
> the override should be implemented in. And make sure that this arch file is
> directly included from within sched.h.
Will do.
> I have a suspicion that we can still get in a mess if .c files include the
> per-arch file and don't include sched.h, but I forget where this happened
> and why it broke stuff.
In this case, there's only one call site and will only every be one, so
that shouldn't be a problem. I don't see init/main.c not including
sched.h
> Sigh. A nice, coded-in-C implementation within each and every architecture
> remains the best implementation, and all the little tricks-to-save-typing
> have failure modes.
Well, I started doing it in all arch, and people around here told me
that was not a good idea , that it would be trouble if the prototype
ever had to change (adding an arg, etc... though very unlikely to happen
in that case, granted).
> otoh, if only one .c file will ever call this function then I think that
> all problems are solved by
>
> a) moving the above ifdeffery into the .c file
> b) adding a comment explaining which arch file must provide the override
> c) directly including that file from within the .c file.
I can definitely do that. I have no problem either way. I can add to all
archs too, it's just that whatever way I choose, some people won't be
happy with it :-)
Anyway, I'll move the ifdeferry to init/main.c then.
Cheers,
Ben.
WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@ozlabs.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Linux-Arch <linux-arch@vger.kernel.org>,
linux-m32r@ml.linux-m32r.org, takata@linux-m32r.org,
linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
Paul Mackerras <paulus@samba.org>
Subject: Re: [PATCH 1/2] Add thread_info_cache_init() to all archs
Date: Mon, 14 Apr 2008 10:38:26 +1000 [thread overview]
Message-ID: <1208133506.6958.82.camel@pasglop> (raw)
In-Reply-To: <20080413171953.bde5e9ac.akpm@linux-foundation.org>
> > +#ifndef thread_info_cache_init
> > +#define thread_info_cache_init do { } while(0)
> > +#endif
>
> This trick does cause a bit of a problem: it is undefined which arch header
> file is to provide the alternative definition of thread_info_cache_init.
I this case it's well defined: thread_info.h. Maybe I should add a
comment ?
> So we can (and have) ended up in the situation where the override appears
> in different files on different architectures and various screwups ensue.
Yup.
> So I'd suggest that we have a bigfatcomment telling implementors which file
> the override should be implemented in. And make sure that this arch file is
> directly included from within sched.h.
Will do.
> I have a suspicion that we can still get in a mess if .c files include the
> per-arch file and don't include sched.h, but I forget where this happened
> and why it broke stuff.
In this case, there's only one call site and will only every be one, so
that shouldn't be a problem. I don't see init/main.c not including
sched.h
> Sigh. A nice, coded-in-C implementation within each and every architecture
> remains the best implementation, and all the little tricks-to-save-typing
> have failure modes.
Well, I started doing it in all arch, and people around here told me
that was not a good idea , that it would be trouble if the prototype
ever had to change (adding an arg, etc... though very unlikely to happen
in that case, granted).
> otoh, if only one .c file will ever call this function then I think that
> all problems are solved by
>
> a) moving the above ifdeffery into the .c file
> b) adding a comment explaining which arch file must provide the override
> c) directly including that file from within the .c file.
I can definitely do that. I have no problem either way. I can add to all
archs too, it's just that whatever way I choose, some people won't be
happy with it :-)
Anyway, I'll move the ifdeferry to init/main.c then.
Cheers,
Ben.
next prev parent reply other threads:[~2008-04-14 0:38 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-04-10 3:22 [PATCH 1/2] Add thread_info_cache_init() to all archs Benjamin Herrenschmidt
2008-04-10 3:22 ` Benjamin Herrenschmidt
2008-04-10 3:22 ` Benjamin Herrenschmidt
2008-04-10 3:22 ` Benjamin Herrenschmidt
[not found] ` <20080410032354.90CB1DDF0F-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
2008-04-10 21:46 ` Benjamin Herrenschmidt
2008-04-10 21:46 ` Benjamin Herrenschmidt
2008-04-14 0:19 ` Andrew Morton
2008-04-14 0:19 ` Andrew Morton
2008-04-14 0:19 ` Andrew Morton
2008-04-14 0:19 ` Andrew Morton
2008-04-14 0:38 ` Benjamin Herrenschmidt [this message]
2008-04-14 0:38 ` Benjamin Herrenschmidt
2008-04-14 2:13 ` Andrew Morton
2008-04-14 2:13 ` Andrew Morton
2008-04-14 2:13 ` Andrew Morton
[not found] ` <20080413191338.9776ebd0.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2008-04-18 3:58 ` Benjamin Herrenschmidt
2008-04-18 3:58 ` Benjamin Herrenschmidt
2008-04-18 3:58 ` Benjamin Herrenschmidt
2008-04-18 4:19 ` Andrew Morton
2008-04-18 4:19 ` Andrew Morton
2008-04-18 4:19 ` Andrew Morton
[not found] ` <20080417211905.8ff769fa.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2008-04-18 4:38 ` Michael Ellerman
2008-04-18 4:38 ` Michael Ellerman
2008-04-18 6:44 ` Benjamin Herrenschmidt
2008-04-18 6:44 ` Benjamin Herrenschmidt
2008-04-18 6:44 ` Benjamin Herrenschmidt
2008-05-21 17:56 ` Benjamin Herrenschmidt
2008-05-21 17:56 ` Benjamin Herrenschmidt
2008-05-21 18:41 ` Andrew Morton
2008-05-21 18:41 ` Andrew Morton
2008-05-21 19:06 ` Sam Ravnborg
2008-05-21 19:06 ` Sam Ravnborg
2008-05-21 20:38 ` Benjamin Herrenschmidt
2008-05-21 20:38 ` Benjamin Herrenschmidt
2008-05-21 19:44 ` Benjamin Herrenschmidt
2008-05-21 19:44 ` Benjamin Herrenschmidt
2008-05-21 20:52 ` Andrew Morton
2008-05-21 20:52 ` Andrew Morton
2008-04-18 4:21 ` Kyle McMartin
2008-04-18 4:21 ` Kyle McMartin
-- strict thread matches above, loose matches on Subject: below --
2008-04-10 23:36 Benjamin Herrenschmidt
2008-04-10 23:36 ` Benjamin Herrenschmidt
2008-04-10 23:36 ` Benjamin Herrenschmidt
2008-04-10 23:36 ` Benjamin Herrenschmidt
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=1208133506.6958.82.camel@pasglop \
--to=benh@ozlabs.org \
--cc=akpm@linux-foundation.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m32r@ml.linux-m32r.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=paulus@samba.org \
--cc=takata@linux-m32r.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.