linux-kernel - Re: [PATCH 00/13] Convert EDAC internal strutures to support 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]

Message-ID: <20120402135959.GC24238@aftab>
Date:	Mon, 2 Apr 2012 15:59:59 +0200
From:	Borislav Petkov <bp@...64.org>
To:	Mauro Carvalho Chehab <mchehab@...hat.com>
Cc:	Linux Edac Mailing List <linux-edac@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Aristeu Rozanski Filho <arozansk@...hat.com>
Subject: Re: [PATCH 00/13] Convert EDAC internal strutures to support all
 types of Memory Controllers

On Thu, Mar 29, 2012 at 01:45:33PM -0300, Mauro Carvalho Chehab wrote:
> This is the 12th and final rebase of this patch series.
> 
> It is the first patchset for the EDAC rewrite. On this patchset,
> there are all the internal changes at the EDAC core, needed
> to properly represent memories at modern memory controllers that
> aren't oriented per rank/channel.
> 
> It is needed in order to fix a long-term bug at the EDAC drivers
> for the Intel memory controllers deployed since 2005 (well, in fact,
> there is one Rambus that it is older, but also suffers from the same
> syndrome), including the drivers for the recent Intel Nehalem and
> Sandy Bridge architectures.
> 
> The new EDAC architecture supports both per rank/channel memory
> controllers and per-DIMM ones.
> 
> On this changeset, there are no changes at the sysfs nodes. Just 
> like before this changeset, non-per-rank memory controllers 
> will expose memories as "virtual csrows/virtual channels[1].
> 
> [1] It sounds better to say "virtual" than to admit that all
> EDAC Intel drivers since 2005 need to lie about their age to
> the EDAC core, in order for the Kernel to accept them ;)
> 
> Mauro Carvalho Chehab (13):
>   edac: Create a dimm struct and move the labels into it
>   edac: move dimm properties to struct memset_info
>   edac: Don't initialize csrow's first_page & friends when not needed
>   edac: move nr_pages to dimm struct
>   edac: Fix core support for MC's that see DIMMS instead of ranks

I was wondering why 6/13 doesn't apply cleanly but there's the patch
above, 5/13 missing in the submission. It looks like vger has eaten it
at least for the linux-edac mailing list - the patch is still on lkml
though.

And what a patch it is: almost 5000 lines.

Please split it!

And don't tell me it cannot be done: each patch needs to do one thing
and one thing only. From looking at this monster, here's one possible
way to split it:

* add all changes to include/linux/edac.h
* a bunch of changes to edac_mc.c like edac_align_ptr etc
* changes to edac_mc_alloc
* add edac_mc_handle_error
* switch old edac_mc_handle* stuff to edac_mc_handle_error
...<a bunch more>

This way the code will be much easier to review.

-- 
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/