[PATCH v5 0/2] EDAC/ghes: refactor memory error reporting to avoid code duplication

Shuai Xue posted 2 patches 4 years, 5 months ago
Only 0 patches received!
There is a newer version of this series
drivers/edac/Kconfig        |   1 +
drivers/edac/ghes_edac.c    | 196 +++++++-----------------------------
drivers/firmware/efi/cper.c |  66 ++++++++----
include/linux/cper.h        |   3 +
4 files changed, 86 insertions(+), 180 deletions(-)
[PATCH v5 0/2] EDAC/ghes: refactor memory error reporting to avoid code duplication
Posted by Shuai Xue 4 years, 5 months ago
ghes_edac_report_mem_error() in ghes_edac.c is a Long Method and have
Duplicated Code with cper_mem_err_location(), cper_dimm_err_location(), and
cper_mem_err_type_str() in drivers/firmware/efi/cper.c. In addition, the
cper_print_mem() in drivers/firmware/efi/cper.c only reports the error
status and misses its description.

This patch set is to refactor ghes_edac_report_mem_error with:

- Patch 01 is to wrap up error status decoding logics and reuse it in
    cper_print_mem().
- Patch 02 is to introduces cper_*(), into ghes_edac_report_mem_error(),
  this can avoid bunch of duplicate code lines;

Changes since v4:
- Fix alignment and format problem
- Link: https://lore.kernel.org/all/20220125024939.30635-1-xueshuai@linux.alibaba.com/
- Thanks Borislav Petkov for review comments.

Changes since v3:

- make cper_mem_err_status_str table a lot more compact
- Fix alignment and format problem
- Link: https://lore.kernel.org/all/20220124024759.19176-1-xueshuai@linux.alibaba.com/
- Thanks Borislav Petkov for review comments.

Changes since v2:

- delete the unified patch
- adjusted the order of patches
- Link: https://lore.kernel.org/all/20211210134019.28536-1-xueshuai@linux.alibaba.com/
- Thanks Borislav Petkov for review comments.

Changes since v1:

- add a new patch to unify ghes and cper before removing duplication.
- document the changes in patch description
- add EXPORT_SYMBOL_GPL()s for cper_*()
- document and the dependency and add UEFI_CPER dependency explicitly
- Link: https://lore.kernel.org/all/20211207031905.61906-2-xueshuai@linux.alibaba.com/
- Thanks Robert Richter for review comments.

Shuai Xue (2):
  efi/cper: add cper_mem_err_status_str to decode error description
  EDAC/ghes: use cper functions to avoid code duplication

 drivers/edac/Kconfig        |   1 +
 drivers/edac/ghes_edac.c    | 196 +++++++-----------------------------
 drivers/firmware/efi/cper.c |  66 ++++++++----
 include/linux/cper.h        |   3 +
 4 files changed, 86 insertions(+), 180 deletions(-)

-- 
2.20.1.12.g72788fdb

Re: [PATCH v5 0/2] EDAC/ghes: refactor memory error reporting to avoid code duplication
Posted by Borislav Petkov 4 years, 5 months ago
On Wed, Jan 26, 2022 at 04:17:00PM +0800, Shuai Xue wrote:
> ghes_edac_report_mem_error() in ghes_edac.c is a Long Method and have
> Duplicated Code with cper_mem_err_location(), cper_dimm_err_location(), and
> cper_mem_err_type_str() in drivers/firmware/efi/cper.c. In addition, the
> cper_print_mem() in drivers/firmware/efi/cper.c only reports the error
> status and misses its description.

Dude, what about

	wait for a week or until the patchset has been fully reviewed

don't you understand?!

Please let me know what about the review process is not clear to you so
that we can document it better.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v5 0/2] EDAC/ghes: refactor memory error reporting to avoid code duplication
Posted by Shuai Xue 4 years, 5 months ago
Hi, Borislav,

在 2022/1/26 PM4:20, Borislav Petkov 写道:
> On Wed, Jan 26, 2022 at 04:17:00PM +0800, Shuai Xue wrote:
>> ghes_edac_report_mem_error() in ghes_edac.c is a Long Method and have
>> Duplicated Code with cper_mem_err_location(), cper_dimm_err_location(), and
>> cper_mem_err_type_str() in drivers/firmware/efi/cper.c. In addition, the
>> cper_print_mem() in drivers/firmware/efi/cper.c only reports the error
>> status and misses its description.
> 
> Dude, what about
> 
> 	wait for a week or until the patchset has been fully reviewed
> 
> don't you understand?!
> 
> Please let me know what about the review process is not clear to you so
> that we can document it better.

Emmm, when I received your replied email, I thought you had fully reviewed them. So
I work to address your comments and reply as soon as possible. Sorry, I misunderstood.

Of course, I can wait. As I said before, take your time.

By the way, I have a question about review process: after waiting for a period
of time, how can I tell whether you have no comments or are still in review process?

Best Regards,
Shuai
Re: [PATCH v5 0/2] EDAC/ghes: refactor memory error reporting to avoid code duplication
Posted by Boris Petkov 4 years, 5 months ago
On January 26, 2022 9:26:01 AM UTC, Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>By the way, I have a question about review process: after waiting for a period
>of time, how can I tell whether you have no comments or are still in review process?
>

A good sign for when review is done is to wait to see replies to every patch.

BUT, there are other people on CC too so they would need to get a chance to have a look too.

Regardless, you wait for a week and then you incorporate all review comments and resend - not before.

This constant spamming with the patchset is not productive. You're not the only one who sends patches and wants review - you should consider that there are others who would need to get reviewed too.

-- 
Sent from a small device: formatting sux and brevity is inevitable. 
Re: [PATCH v5 0/2] EDAC/ghes: refactor memory error reporting to avoid code duplication
Posted by Shuai Xue 4 years, 5 months ago
Hi, Borislav,

在 2022/1/26 PM6:16, Boris Petkov 写道:
> On January 26, 2022 9:26:01 AM UTC, Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>> By the way, I have a question about review process: after waiting for a period
>> of time, how can I tell whether you have no comments or are still in review process?
>>
> 
> A good sign for when review is done is to wait to see replies to every patch.
> 
> BUT, there are other people on CC too so they would need to get a chance to have a look too.
> 
> Regardless, you wait for a week and then you incorporate all review comments and resend - not before.
> 
> This constant spamming with the patchset is not productive. You're not the only one who sends patches and wants review - you should consider that there are others who would need to get reviewed too.

Got it. Thank you very much.

Best Regards,
Shuai