linux-kernel - Re: [PATCH v2 1/3] ghes_edac: unify memory error report format with cper

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:   Thu, 30 Dec 2021 18:57:14 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Shuai Xue <xueshuai@...ux.alibaba.com>
Cc:     mchehab@...nel.org, tony.luck@...el.com, james.morse@....com,
        rric@...nel.org, ardb@...nel.org, linux-edac@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-efi@...r.kernel.org,
        zhangliguang@...ux.alibaba.com, zhuo.song@...ux.alibaba.com
Subject: Re: [PATCH v2 1/3] ghes_edac: unify memory error report format with
 cper

On Wed, Dec 29, 2021 at 11:22:11AM +0800, Shuai Xue wrote:
> Yep, these fields are unpopulated by BIOS, I manually enable all Validation
> Bits for debug so that we see the difference more clearly. I will declare it
> in next version.

Declare what? I can't parse your statement.

> Well, the purpose is not to improve but unify.

The most importang goal with kernel code is improvement and less bugs.
Unification is second. We should not jump through hoops and unify at
every price just because there's a duplicated function somewhere.
Remember that when doing your changes.

> Well, Robert suggested me add a unification patch[1] so that we could review
> the changes more clearly. I think it makes sense.

Not really. I can imagine why Robert suggested that but this strategy is
causing unnecessary churn. What one usually does in such cases is:

1. Add changes to the target functionality - the one in cper.c - by
explaining *why* those changes are needed.

2. Switch ghes_edac.c to that functionality and remove the redundant one
there.

Simple and clean diffstat and easy review.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette