linux-kernel - Re: [PATCH 1/1] edac x38: new MC driver module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 29 Nov 2008 11:52:29 +0100
From:	Sam Ravnborg <sam@...nborg.org>
To:	Hitoshi Mitake <h.mitake@...il.com>
Cc:	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	"Luck, Tony" <tony.luck@...el.com>,
	Russell King <rmk+lkml@....linux.org.uk>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Doug Thompson <norsk5@...oo.com>, dougthompson@...ssion.com,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH 1/1] edac x38: new MC driver module

> 
> But this is old way. ARCH_HAS_READQ and ARCH_HAS_WRITEQ are new ways
> to determine existence of readq/writeq. Drivers which use readq/writeq should
> depend on these values in their Kconfig file.

If we look at arch/x86/Kconfig we see:
### Arch settings
config X86
        def_bool y
        select HAVE_AOUT if X86_32
        select HAVE_UNSTABLE_SCHED_CLOCK
        select HAVE_IDE
        select HAVE_OPROFILE
        select HAVE_IOREMAP_PROT
        select HAVE_KPROBES
        select ARCH_WANT_OPTIONAL_GPIOLIB
	...

So the normal syntax here is "HAVE_XXX_XXX" - not ARCH_HAS_XXX_XXX

If you update your patch please use this syntax,
and locate the select under X86 - not under the 32/64 entries.

But I do not see why adding these in the first place.

See following advice from Linus:
http://marc.info/?l=linux-arch&m=121710129310710&w=2

===> Quote:
I really think that whoever started that 'HAVE_ARCH_x'/'ARCH_HAS_x' mess 
with totally random symbols that have NOTHING WHAT-SO-EVER to do with the 
actual symbols in question (so they do _not_ show up in grep'ing for some 
use) should be shot. 

We should never _ever_ use that model. And we use it way too much.

We should generally strive for the simpler and much more obvious

	/* Generic definition */
	#ifndef symbol
	int symbol(..)
	...
	#endif

and then architecture code can do

	#define symbol(x) ...

or if they want to do a function, and you _really_ don't like the '__weak' 
part (or you want to make it an inline function and don't want the clash 
with the real declaration), then you can just do

	static inline int symbol(x)
	{
		...
	}
	#define symbol symbol

and again it all works fine WITHOUT having to introduce some idiotic new 
and unrelated element called ARCH_HAS_SYMBOL.

<====

	Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/