* [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition
@ 2015-04-06 9:04 Alexander Kuleshov
2015-04-06 19:35 ` Andreas Mohr
2015-04-07 8:35 ` Ingo Molnar
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Kuleshov @ 2015-04-06 9:04 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin
Cc: linux-kernel, Alexander Kuleshov
arch/x86/boot/boot.h defines a couple functions as die and etc..., with
'noreturn' attribute. Let's use __noreturn macro instead of directly
__attribute__ declaration from the <linux/compiler.h>.
We no need to include <linux/compiler.h> to the arch/x86/boot/boot.h,
because boot.h already includes "bitops.h" which already includes
<linux/compiler.h>.
Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
arch/x86/boot/boot.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index bd49ec6..3351528 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -305,7 +305,7 @@ void console_init(void);
void query_edd(void);
/* header.S */
-void __attribute__((noreturn)) die(void);
+void __noreturn die(void);
/* mca.c */
int query_mca(void);
@@ -314,11 +314,10 @@ int query_mca(void);
int detect_memory(void);
/* pm.c */
-void __attribute__((noreturn)) go_to_protected_mode(void);
+void __noreturn go_to_protected_mode(void);
/* pmjump.S */
-void __attribute__((noreturn))
- protected_mode_jump(u32 entrypoint, u32 bootparams);
+void __noreturn protected_mode_jump(u32 entrypoint, u32 bootparams);
/* printf.c */
int sprintf(char *buf, const char *fmt, ...);
--
2.3.3.611.g09038fc.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition
2015-04-06 9:04 [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition Alexander Kuleshov
@ 2015-04-06 19:35 ` Andreas Mohr
2015-04-07 7:32 ` Alexander Kuleshov
2015-04-07 8:35 ` Ingo Molnar
1 sibling, 1 reply; 6+ messages in thread
From: Andreas Mohr @ 2015-04-06 19:35 UTC (permalink / raw)
To: Alexander Kuleshov
Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
Alexander Kuleshov
Hi,
> We no need to include <linux/compiler.h> to the arch/x86/boot/boot.h,
> because boot.h already includes "bitops.h" which already includes
> <linux/compiler.h>.
"Depends", I'd say (no pun, and intended :).
This currently implicitly working constellation
might break both for:
- boot.h no longer requiring bitops.h
- bitops.h no longer including linux/compiler.h
IMHO explicitly #include:ing parts
which certain code does actively depend on after all
is both a "code honesty"
and a documentation issue ("Hmm, which interfaces does this implementation
depend on? Ah, yeah...")
Food for thought,
Andreas Mohr
P.S.: thanks for your contribution!
--
GNU/Linux. It's not the software that's free, it's you.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition
2015-04-06 19:35 ` Andreas Mohr
@ 2015-04-07 7:32 ` Alexander Kuleshov
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Kuleshov @ 2015-04-07 7:32 UTC (permalink / raw)
To: Andreas Mohr; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, LKML
Hello Andreas,
>
> IMHO explicitly #include:ing parts
> which certain code does actively depend on after all
> is both a "code honesty"
> and a documentation issue ("Hmm, which interfaces does this implementation
> depend on? Ah, yeah...")
>
Yes, before the patch i was in hesitancy, how to be with 'includes'.
To add linux/compiler.h
to the boot.h or do not do it as it already in the bitops.h. That's
why I have added note
about in to the commit message, for getting feedback about this.
Thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition
2015-04-06 9:04 [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition Alexander Kuleshov
2015-04-06 19:35 ` Andreas Mohr
@ 2015-04-07 8:35 ` Ingo Molnar
2015-04-07 8:56 ` Borislav Petkov
1 sibling, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2015-04-07 8:35 UTC (permalink / raw)
To: Alexander Kuleshov
Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
Borislav Petkov
* Alexander Kuleshov <kuleshovmail@gmail.com> wrote:
> arch/x86/boot/boot.h defines a couple functions as die and etc..., with
> 'noreturn' attribute. Let's use __noreturn macro instead of directly
> __attribute__ declaration from the <linux/compiler.h>.
>
> We no need to include <linux/compiler.h> to the arch/x86/boot/boot.h,
> because boot.h already includes "bitops.h" which already includes
> <linux/compiler.h>.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
> arch/x86/boot/boot.h | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index bd49ec6..3351528 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -305,7 +305,7 @@ void console_init(void);
> void query_edd(void);
>
> /* header.S */
> -void __attribute__((noreturn)) die(void);
> +void __noreturn die(void);
>
> /* mca.c */
> int query_mca(void);
> @@ -314,11 +314,10 @@ int query_mca(void);
> int detect_memory(void);
>
> /* pm.c */
> -void __attribute__((noreturn)) go_to_protected_mode(void);
> +void __noreturn go_to_protected_mode(void);
>
> /* pmjump.S */
> -void __attribute__((noreturn))
> - protected_mode_jump(u32 entrypoint, u32 bootparams);
> +void __noreturn protected_mode_jump(u32 entrypoint, u32 bootparams);
>
> /* printf.c */
> int sprintf(char *buf, const char *fmt, ...);
Please don't bother producing and sending me such trivial patches
unless they:
- fix a real bug (in which case they are not trivial patches anymore)
- or are part of a larger (non-trivial!) series that does some real,
substantial work on this code that tries to:
- fix existing code
- speed up existing code
- or expand upon existing code with new code
The reason I'm not applying your patch is that trivial patches with no
substance following them up have more costs than benefits:
- they lead to pointless churn:
- they take up Git space for no good reason
- they slow down bisection of real changes
- they take up (valuable!) reviewer bandwidth
- they take up maintainer bandwidth
there's literally a million pointless cleanup patches that could be
done on the kernel, and we don't want to add a million commits to the
kernel tree.
This applies for this patch but also for other future patches you
might intend to send for code that I (co-)maintain.
My advice to you is to try to raise beyond newbie patches and write
something more substantial that helps Linux:
- take a look at the many bugs on bugzilla.kernel.org and try to
analyze, reproduce or fix them
- go read kernel code, understand it and try to find real bugs.
- go test the latest kernels and find bugs in it. The fresher the
code, the more likely it is that it has bugs.
- go read kernel code and try to expand upon it
Fortunately it's not hard to contribute to the kernel: there's
literally an infinite amount of work to be done on the kernel, and I
welcome productive contributions - but churning out trivial patches
with no substantial patches following them up is not productive and in
fact they are harmful once you are not a totally fresh newbie kernel
developer anymore...
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition
2015-04-07 8:35 ` Ingo Molnar
@ 2015-04-07 8:56 ` Borislav Petkov
2015-04-07 9:17 ` Alexander Kuleshov
0 siblings, 1 reply; 6+ messages in thread
From: Borislav Petkov @ 2015-04-07 8:56 UTC (permalink / raw)
To: Ingo Molnar
Cc: Alexander Kuleshov, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel
On Tue, Apr 07, 2015 at 10:35:27AM +0200, Ingo Molnar wrote:
> My advice to you is to try to raise beyond newbie patches and write
> something more substantial that helps Linux:
Yeah, I tried:
https://lkml.kernel.org/r/20150228134832.GB11038@pd.tnic
https://lkml.kernel.org/r/20150305181755.GL3915@pd.tnic
Let's see whether he listens to you. My suggestions and genuine desire
to help were a pure waste of time.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition
2015-04-07 8:56 ` Borislav Petkov
@ 2015-04-07 9:17 ` Alexander Kuleshov
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Kuleshov @ 2015-04-07 9:17 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, LKML
Hello Ingo and Borislav,
2015-04-07 14:56 GMT+06:00 Borislav Petkov <bp@alien8.de>:
> On Tue, Apr 07, 2015 at 10:35:27AM +0200, Ingo Molnar wrote:
>> My advice to you is to try to raise beyond newbie patches and write
>> something more substantial that helps Linux:
>
> Yeah, I tried:
>
> https://lkml.kernel.org/r/20150228134832.GB11038@pd.tnic
> https://lkml.kernel.org/r/20150305181755.GL3915@pd.tnic
That's true.
>
> My suggestions and genuine desire to help were a pure waste of time.
No, it weren't. It was my last trivial patch and until I'll start to
do that - useful.
I really don't want to waste time of community on trifles.
Thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-07 9:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-06 9:04 [PATCH] x86/boot: use __noreturn instead of directly __attribute__ definition Alexander Kuleshov
2015-04-06 19:35 ` Andreas Mohr
2015-04-07 7:32 ` Alexander Kuleshov
2015-04-07 8:35 ` Ingo Molnar
2015-04-07 8:56 ` Borislav Petkov
2015-04-07 9:17 ` Alexander Kuleshov
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.