linux-kernel - Re: [PATCHv5] EDAC core changes in order to properly report errors from all types of memory controllers

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:	Tue, 6 Mar 2012 13:16:16 +0100
From:	Borislav Petkov <bp@...64.org>
To:	Mauro Carvalho Chehab <mchehab@...hat.com>
Cc:	Borislav Petkov <bp@...64.org>, Tony Luck <tony.luck@...el.com>,
	Ingo Molnar <mingo@...e.hu>,
	EDAC devel <linux-edac@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv5] EDAC core changes in order to properly report errors
 from all types of memory controllers

On Tue, Mar 06, 2012 at 08:31:36AM -0300, Mauro Carvalho Chehab wrote:

[..]

> Breaking it into smaller patchsets is not trivial, as the analysis of the
> individual changesets only makes sense in the light of the big change on
> the edac core structures.

I don't believe that and LKML contains countless examples of how patches
can be split into simple hunks containing only one logical change.
Simply put yourself in the reviewer's place and try to imagine what kind
of patch format you'd like to see.

[..]

> On a csrow-based MC, dual-rank memories would be mapped as two separate 
> (csrow, channel) addresses. Each will have half of the DIMM size on each
> address. The above "emulation" creates two (csrow, channel) addreses for every
> FB-DIMM, filling just one, but only if the memory is dual-rank.
> 
> For a FB-DIMM controller, the number of ranks is just a detail associated with
> a given DIMM slot, as the memory is selected by slot, and not by rank.
> 
> So, the logic is completely broken for single-rank memories and half-broken for 
> double-rank ones.

I'm still wondering whether FBDIMM-based drivers should get their own
EDAC infrastructure and own nomenclature instead of fitting them in the
existing scheme...

> 
> This is an example of a patch that should not be fold.
> 
> After this patch, the memories on the i5000 machine I'm testing are properly
> reported, being single-rank or dual-rank.

How about instead of verbosely explaining in a mail why you're doing
what you're doing, you add that explanation to your patches so that even
the unlightened one can understand what you're doing? I think that will
benefit all.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
--
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/