Hi, Borislav, Thank you for your comments. 在 2021/12/31 AM1:57, Borislav Petkov 写道: > 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. The ghes_edac log message is printed only when a validation bit is set, e.g.: if (mem_err->validation_bits & CPER_MEM_VALID_NODE) p += sprintf(p, "node:%d ", mem_err->node); Not all bits are populated by BIOS in my platform, I manually enable all validation bits during test so that we can see log message and differences of all fields more clearly. + mem_err->validation_bits = 0xfffffffffffffff; >> 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. I see. Thank you. >> 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. Got it. I will send next version latter. Merry Christmas and happy New Year. Best Regards, Shuai
