From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Alex G." Subject: Re: [RFC PATCH v2 2/4] acpi: apei: Split GHES handlers outside of ghes_do_proc Date: Thu, 19 Apr 2018 09:19:03 -0500 Message-ID: <5f26275c-5896-c552-69c4-78e5aaaa6558@gmail.com> References: <20180416215903.7318-1-mr.nuke.me@gmail.com> <20180416215903.7318-3-mr.nuke.me@gmail.com> <20180418175201.GI4795@pd.tnic> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180418175201.GI4795@pd.tnic> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Borislav Petkov Cc: linux-acpi@vger.kernel.org, linux-edac@vger.kernel.org, rjw@rjwysocki.net, lenb@kernel.org, tony.luck@intel.com, tbaicar@codeaurora.org, will.deacon@arm.com, james.morse@arm.com, shiju.jose@huawei.com, zjzhang@codeaurora.org, gengdongjiu@huawei.com, linux-kernel@vger.kernel.org, alex_gagniuc@dellteam.com, austin_bolen@dell.com, shyam_iyer@dell.com, devel@acpica.org, mchehab@kernel.org, robert.moore@intel.com, erik.schmauss@intel.com List-Id: linux-acpi@vger.kernel.org On 04/18/2018 12:52 PM, Borislav Petkov wrote: > On Mon, Apr 16, 2018 at 04:59:01PM -0500, Alexandru Gagniuc wrote: >> static void ghes_do_proc(struct ghes *ghes, >> const struct acpi_hest_generic_status *estatus) >> { >> int sev, sec_sev; >> struct acpi_hest_generic_data *gdata; >> + const struct ghes_handler *handler; >> guid_t *sec_type; >> guid_t *fru_id = &NULL_UUID_LE; >> char *fru_text = ""; >> @@ -478,21 +537,10 @@ static void ghes_do_proc(struct ghes *ghes, >> if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT) >> fru_text = gdata->fru_text; >> >> - if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM)) { >> - struct cper_sec_mem_err *mem_err = acpi_hest_get_payload(gdata); >> - >> - ghes_edac_report_mem_error(sev, mem_err); >> - >> - arch_apei_report_mem_error(sev, mem_err); >> - ghes_handle_memory_failure(gdata, sev); >> - } >> - else if (guid_equal(sec_type, &CPER_SEC_PCIE)) { >> - ghes_handle_aer(gdata); >> - } >> - else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) { >> - struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata); >> >> - log_arm_hw_error(err); >> + handler = get_handler(sec_type); > > I don't like this - it was better and more readable before because I can > follow which handler gets called. This change makes is less readable. I agree with the readability argument in the current situation of three handlers. I guess I was thinking ahead and generalizing for an arbitrary number of handlers. On the other side, you lose readability as soon as you get a few more handlers and the function becomes too long. And more importantly, you lose generality: it's not obvious that there's ghes_edac_report_mem_error() which too wide a context. Alex From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [RFC,v2,2/4] acpi: apei: Split GHES handlers outside of ghes_do_proc From: Alexandru Gagniuc Message-Id: <5f26275c-5896-c552-69c4-78e5aaaa6558@gmail.com> Date: Thu, 19 Apr 2018 09:19:03 -0500 To: Borislav Petkov Cc: linux-acpi@vger.kernel.org, linux-edac@vger.kernel.org, rjw@rjwysocki.net, lenb@kernel.org, tony.luck@intel.com, tbaicar@codeaurora.org, will.deacon@arm.com, james.morse@arm.com, shiju.jose@huawei.com, zjzhang@codeaurora.org, gengdongjiu@huawei.com, linux-kernel@vger.kernel.org, alex_gagniuc@dellteam.com, austin_bolen@dell.com, shyam_iyer@dell.com, devel@acpica.org, mchehab@kernel.org, robert.moore@intel.com, erik.schmauss@intel.com List-ID: T24gMDQvMTgvMjAxOCAxMjo1MiBQTSwgQm9yaXNsYXYgUGV0a292IHdyb3RlOgo+IE9uIE1vbiwg QXByIDE2LCAyMDE4IGF0IDA0OjU5OjAxUE0gLTA1MDAsIEFsZXhhbmRydSBHYWduaXVjIHdyb3Rl Ogo+PiAgc3RhdGljIHZvaWQgZ2hlc19kb19wcm9jKHN0cnVjdCBnaGVzICpnaGVzLAo+PiAgCQkJ IGNvbnN0IHN0cnVjdCBhY3BpX2hlc3RfZ2VuZXJpY19zdGF0dXMgKmVzdGF0dXMpCj4+ICB7Cj4+ ICAJaW50IHNldiwgc2VjX3NldjsKPj4gIAlzdHJ1Y3QgYWNwaV9oZXN0X2dlbmVyaWNfZGF0YSAq Z2RhdGE7Cj4+ICsJY29uc3Qgc3RydWN0IGdoZXNfaGFuZGxlciAqaGFuZGxlcjsKPj4gIAlndWlk X3QgKnNlY190eXBlOwo+PiAgCWd1aWRfdCAqZnJ1X2lkID0gJk5VTExfVVVJRF9MRTsKPj4gIAlj aGFyICpmcnVfdGV4dCA9ICIiOwo+PiBAQCAtNDc4LDIxICs1MzcsMTAgQEAgc3RhdGljIHZvaWQg Z2hlc19kb19wcm9jKHN0cnVjdCBnaGVzICpnaGVzLAo+PiAgCQlpZiAoZ2RhdGEtPnZhbGlkYXRp b25fYml0cyAmIENQRVJfU0VDX1ZBTElEX0ZSVV9URVhUKQo+PiAgCQkJZnJ1X3RleHQgPSBnZGF0 YS0+ZnJ1X3RleHQ7Cj4+ICAKPj4gLQkJaWYgKGd1aWRfZXF1YWwoc2VjX3R5cGUsICZDUEVSX1NF Q19QTEFURk9STV9NRU0pKSB7Cj4+IC0JCQlzdHJ1Y3QgY3Blcl9zZWNfbWVtX2VyciAqbWVtX2Vy ciA9IGFjcGlfaGVzdF9nZXRfcGF5bG9hZChnZGF0YSk7Cj4+IC0KPj4gLQkJCWdoZXNfZWRhY19y ZXBvcnRfbWVtX2Vycm9yKHNldiwgbWVtX2Vycik7Cj4+IC0KPj4gLQkJCWFyY2hfYXBlaV9yZXBv cnRfbWVtX2Vycm9yKHNldiwgbWVtX2Vycik7Cj4+IC0JCQlnaGVzX2hhbmRsZV9tZW1vcnlfZmFp bHVyZShnZGF0YSwgc2V2KTsKPj4gLQkJfQo+PiAtCQllbHNlIGlmIChndWlkX2VxdWFsKHNlY190 eXBlLCAmQ1BFUl9TRUNfUENJRSkpIHsKPj4gLQkJCWdoZXNfaGFuZGxlX2FlcihnZGF0YSk7Cj4+ IC0JCX0KPj4gLQkJZWxzZSBpZiAoZ3VpZF9lcXVhbChzZWNfdHlwZSwgJkNQRVJfU0VDX1BST0Nf QVJNKSkgewo+PiAtCQkJc3RydWN0IGNwZXJfc2VjX3Byb2NfYXJtICplcnIgPSBhY3BpX2hlc3Rf Z2V0X3BheWxvYWQoZ2RhdGEpOwo+PiAgCj4+IC0JCQlsb2dfYXJtX2h3X2Vycm9yKGVycik7Cj4+ ICsJCWhhbmRsZXIgPSBnZXRfaGFuZGxlcihzZWNfdHlwZSk7Cj4gCj4gSSBkb24ndCBsaWtlIHRo aXMgLSBpdCB3YXMgYmV0dGVyIGFuZCBtb3JlIHJlYWRhYmxlIGJlZm9yZSBiZWNhdXNlIEkgY2Fu Cj4gZm9sbG93IHdoaWNoIGhhbmRsZXIgZ2V0cyBjYWxsZWQuIFRoaXMgY2hhbmdlIG1ha2VzIGlz IGxlc3MgcmVhZGFibGUuCgpJIGFncmVlIHdpdGggdGhlIHJlYWRhYmlsaXR5IGFyZ3VtZW50IGlu IHRoZSBjdXJyZW50IHNpdHVhdGlvbiBvZiB0aHJlZQpoYW5kbGVycy4gSSBndWVzcyBJIHdhcyB0 aGlua2luZyBhaGVhZCBhbmQgZ2VuZXJhbGl6aW5nIGZvciBhbiBhcmJpdHJhcnkKbnVtYmVyIG9m IGhhbmRsZXJzLgoKT24gdGhlIG90aGVyIHNpZGUsIHlvdSBsb3NlIHJlYWRhYmlsaXR5IGFzIHNv b24gYXMgeW91IGdldCBhIGZldyBtb3JlCmhhbmRsZXJzIGFuZCB0aGUgZnVuY3Rpb24gYmVjb21l cyB0b28gbG9uZy4gQW5kIG1vcmUgaW1wb3J0YW50bHksIHlvdQpsb3NlIGdlbmVyYWxpdHk6IGl0 J3Mgbm90IG9idmlvdXMgdGhhdCB0aGVyZSdzCmdoZXNfZWRhY19yZXBvcnRfbWVtX2Vycm9yKCkg d2hpY2ggdG9vIHdpZGUgYSBjb250ZXh0LgoKQWxleAotLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0 aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LWVkYWMiIGluCnRoZSBi b2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnCk1vcmUgbWFqb3Jk b21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbAo=