* [PATCH] acpi: Fix hed module initialization order when it is built-in @ 2024-11-15 3:50 Xiaofei Tan 2024-12-10 17:59 ` Rafael J. Wysocki 2024-12-11 16:22 ` Mauro Carvalho Chehab 0 siblings, 2 replies; 7+ messages in thread From: Xiaofei Tan @ 2024-11-15 3:50 UTC (permalink / raw) To: rafael, lenb Cc: linux-acpi, linux-kernel, jonathan.cameron, M.Chehab, roberto.sassu, shiju.jose, prime.zeng, linuxarm, Xiaofei Tan, Jonathan Cameron When the module hed is built-in, the init order is determined by Makefile order. That order violates expectations. Because the module hed init is behind evged. RAS records can't be handled in the special time window that evged has initialized while hed not. If the number of such RAS records is more than the APEI HEST error source number, the HEST resources could be occupied all, and then could affect subsequent RAS error reporting. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com> --- drivers/acpi/Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 61ca4afe83dc..54f60b7922ad 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -15,6 +15,13 @@ endif obj-$(CONFIG_ACPI) += tables.o +# +# The hed.o needs to be in front of evged.o to avoid the problem that +# RAS errors cannot be handled in the special time window of startup +# phase that evged has initialized while hed not. +# +obj-$(CONFIG_ACPI_HED) += hed.o + # # ACPI Core Subsystem (Interpreter) # @@ -95,7 +102,6 @@ obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o obj-$(CONFIG_ACPI_BATTERY) += battery.o obj-$(CONFIG_ACPI_SBS) += sbshc.o obj-$(CONFIG_ACPI_SBS) += sbs.o -obj-$(CONFIG_ACPI_HED) += hed.o obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o obj-$(CONFIG_ACPI_BGRT) += bgrt.o obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o -- 2.33.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Fix hed module initialization order when it is built-in 2024-11-15 3:50 [PATCH] acpi: Fix hed module initialization order when it is built-in Xiaofei Tan @ 2024-12-10 17:59 ` Rafael J. Wysocki 2024-12-23 9:31 ` Xiaofei Tan 2024-12-11 16:22 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2024-12-10 17:59 UTC (permalink / raw) To: Xiaofei Tan Cc: rafael, lenb, linux-acpi, linux-kernel, jonathan.cameron, M.Chehab, roberto.sassu, shiju.jose, prime.zeng, linuxarm On Fri, Nov 15, 2024 at 4:56 AM Xiaofei Tan <tanxiaofei@huawei.com> wrote: > > When the module hed is built-in, the init order is determined by > Makefile order. Are you sure? > That order violates expectations. Because the module > hed init is behind evged. RAS records can't be handled in the > special time window that evged has initialized while hed not. > If the number of such RAS records is more than the APEI HEST error > source number, the HEST resources could be occupied all, and then > could affect subsequent RAS error reporting. Well, the problem is real, but does the change really prevent it from happening or does it just increase the likelihood of success? In the latter case, and generally speaking too, it would be better to add explicit synchronization between evged and hed. > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com> > --- > drivers/acpi/Makefile | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index 61ca4afe83dc..54f60b7922ad 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -15,6 +15,13 @@ endif > > obj-$(CONFIG_ACPI) += tables.o > > +# > +# The hed.o needs to be in front of evged.o to avoid the problem that > +# RAS errors cannot be handled in the special time window of startup > +# phase that evged has initialized while hed not. > +# > +obj-$(CONFIG_ACPI_HED) += hed.o > + > # > # ACPI Core Subsystem (Interpreter) > # > @@ -95,7 +102,6 @@ obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o > obj-$(CONFIG_ACPI_BATTERY) += battery.o > obj-$(CONFIG_ACPI_SBS) += sbshc.o > obj-$(CONFIG_ACPI_SBS) += sbs.o > -obj-$(CONFIG_ACPI_HED) += hed.o > obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o > obj-$(CONFIG_ACPI_BGRT) += bgrt.o > obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o > -- > 2.33.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Fix hed module initialization order when it is built-in 2024-12-10 17:59 ` Rafael J. Wysocki @ 2024-12-23 9:31 ` Xiaofei Tan 2024-12-23 19:33 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Xiaofei Tan @ 2024-12-23 9:31 UTC (permalink / raw) To: Rafael J. Wysocki Cc: lenb, linux-acpi, linux-kernel, jonathan.cameron, M.Chehab, roberto.sassu, shiju.jose, prime.zeng, linuxarm, mchehab+huawei Hi Rafael, 在 2024/12/11 1:59, Rafael J. Wysocki 写道: > On Fri, Nov 15, 2024 at 4:56 AM Xiaofei Tan <tanxiaofei@huawei.com> wrote: >> When the module hed is built-in, the init order is determined by >> Makefile order. > Are you sure? yes >> That order violates expectations. Because the module >> hed init is behind evged. RAS records can't be handled in the >> special time window that evged has initialized while hed not. >> If the number of such RAS records is more than the APEI HEST error >> source number, the HEST resources could be occupied all, and then >> could affect subsequent RAS error reporting. > Well, the problem is real, but does the change really prevent it from > happening or does it just increase the likelihood of success? It can be completely solved if the driver used as built-in way. If build HED as a module, it not solved. > > In the latter case, and generally speaking too, it would be better to > add explicit synchronization between evged and hed. > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> >> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com> >> --- >> drivers/acpi/Makefile | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile >> index 61ca4afe83dc..54f60b7922ad 100644 >> --- a/drivers/acpi/Makefile >> +++ b/drivers/acpi/Makefile >> @@ -15,6 +15,13 @@ endif >> >> obj-$(CONFIG_ACPI) += tables.o >> >> +# >> +# The hed.o needs to be in front of evged.o to avoid the problem that >> +# RAS errors cannot be handled in the special time window of startup >> +# phase that evged has initialized while hed not. >> +# >> +obj-$(CONFIG_ACPI_HED) += hed.o >> + >> # >> # ACPI Core Subsystem (Interpreter) >> # >> @@ -95,7 +102,6 @@ obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o >> obj-$(CONFIG_ACPI_BATTERY) += battery.o >> obj-$(CONFIG_ACPI_SBS) += sbshc.o >> obj-$(CONFIG_ACPI_SBS) += sbs.o >> -obj-$(CONFIG_ACPI_HED) += hed.o >> obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o >> obj-$(CONFIG_ACPI_BGRT) += bgrt.o >> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o >> -- >> 2.33.0 >> > . ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Fix hed module initialization order when it is built-in 2024-12-23 9:31 ` Xiaofei Tan @ 2024-12-23 19:33 ` Jonathan Cameron 2024-12-28 10:23 ` Xiaofei Tan 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2024-12-23 19:33 UTC (permalink / raw) To: Xiaofei Tan Cc: Rafael J. Wysocki, lenb, linux-acpi, linux-kernel, M.Chehab, roberto.sassu, shiju.jose, prime.zeng, linuxarm, mchehab+huawei On Mon, 23 Dec 2024 17:31:08 +0800 Xiaofei Tan <tanxiaofei@huawei.com> wrote: > Hi Rafael, > > 在 2024/12/11 1:59, Rafael J. Wysocki 写道: > > On Fri, Nov 15, 2024 at 4:56 AM Xiaofei Tan <tanxiaofei@huawei.com> wrote: > >> When the module hed is built-in, the init order is determined by > >> Makefile order. > > Are you sure? > > yes We had a similar fix in CXL recently (which is why I suggested this approach internally when tanxiaofei mentioned the problem). https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/cxl?id=6575b268157f37929948a8d1f3bafb3d7c055bc1 The related discussion for the CXL patch was the first time I'd come across solution to load order for built in cases. > > >> That order violates expectations. Because the module > >> hed init is behind evged. RAS records can't be handled in the > >> special time window that evged has initialized while hed not. > >> If the number of such RAS records is more than the APEI HEST error > >> source number, the HEST resources could be occupied all, and then > >> could affect subsequent RAS error reporting. > > Well, the problem is real, but does the change really prevent it from > > happening or does it just increase the likelihood of success? > > It can be completely solved if the driver used as built-in way. If build HED as a > module, it not solved. Can we enforce that condition not happening with appropriate Kconfig? It's annoying to restrict build options, but if needed to make it work then better than not working! Jonathan > > > > > In the latter case, and generally speaking too, it would be better to > > add explicit synchronization between evged and hed. > > > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > >> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com> > >> --- > >> drivers/acpi/Makefile | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > >> index 61ca4afe83dc..54f60b7922ad 100644 > >> --- a/drivers/acpi/Makefile > >> +++ b/drivers/acpi/Makefile > >> @@ -15,6 +15,13 @@ endif > >> > >> obj-$(CONFIG_ACPI) += tables.o > >> > >> +# > >> +# The hed.o needs to be in front of evged.o to avoid the problem that > >> +# RAS errors cannot be handled in the special time window of startup > >> +# phase that evged has initialized while hed not. > >> +# > >> +obj-$(CONFIG_ACPI_HED) += hed.o > >> + > >> # > >> # ACPI Core Subsystem (Interpreter) > >> # > >> @@ -95,7 +102,6 @@ obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o > >> obj-$(CONFIG_ACPI_BATTERY) += battery.o > >> obj-$(CONFIG_ACPI_SBS) += sbshc.o > >> obj-$(CONFIG_ACPI_SBS) += sbs.o > >> -obj-$(CONFIG_ACPI_HED) += hed.o > >> obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o > >> obj-$(CONFIG_ACPI_BGRT) += bgrt.o > >> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o > >> -- > >> 2.33.0 > >> > > . ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Fix hed module initialization order when it is built-in 2024-12-23 19:33 ` Jonathan Cameron @ 2024-12-28 10:23 ` Xiaofei Tan 0 siblings, 0 replies; 7+ messages in thread From: Xiaofei Tan @ 2024-12-28 10:23 UTC (permalink / raw) To: Jonathan Cameron Cc: Rafael J. Wysocki, lenb, linux-acpi, linux-kernel, M.Chehab, roberto.sassu, shiju.jose, prime.zeng, linuxarm, mchehab+huawei Hi Jonathan, 在 2024/12/24 3:33, Jonathan Cameron 写道: > On Mon, 23 Dec 2024 17:31:08 +0800 > Xiaofei Tan <tanxiaofei@huawei.com> wrote: > >> Hi Rafael, >> >> 在 2024/12/11 1:59, Rafael J. Wysocki 写道: >>> On Fri, Nov 15, 2024 at 4:56 AM Xiaofei Tan <tanxiaofei@huawei.com> wrote: >>>> When the module hed is built-in, the init order is determined by >>>> Makefile order. >>> Are you sure? >> yes > We had a similar fix in CXL recently (which is why I suggested this approach > internally when tanxiaofei mentioned the problem). > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/cxl?id=6575b268157f37929948a8d1f3bafb3d7c055bc1 > > The related discussion for the CXL patch was the first time I'd come across solution > to load order for built in cases. > Yes :) >>>> That order violates expectations. Because the module >>>> hed init is behind evged. RAS records can't be handled in the >>>> special time window that evged has initialized while hed not. >>>> If the number of such RAS records is more than the APEI HEST error >>>> source number, the HEST resources could be occupied all, and then >>>> could affect subsequent RAS error reporting. >>> Well, the problem is real, but does the change really prevent it from >>> happening or does it just increase the likelihood of success? >> It can be completely solved if the driver used as built-in way. If build HED as a >> module, it not solved. > Can we enforce that condition not happening with appropriate Kconfig? > It's annoying to restrict build options, but if needed to make it work > then better than not working! Agree, i will change ACPI_HED from tristate to bool if there are no other comments, thanks. > > Jonathan > > >>> In the latter case, and generally speaking too, it would be better to >>> add explicit synchronization between evged and hed. >>> >>>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> >>>> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com> >>>> --- >>>> drivers/acpi/Makefile | 8 +++++++- >>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile >>>> index 61ca4afe83dc..54f60b7922ad 100644 >>>> --- a/drivers/acpi/Makefile >>>> +++ b/drivers/acpi/Makefile >>>> @@ -15,6 +15,13 @@ endif >>>> >>>> obj-$(CONFIG_ACPI) += tables.o >>>> >>>> +# >>>> +# The hed.o needs to be in front of evged.o to avoid the problem that >>>> +# RAS errors cannot be handled in the special time window of startup >>>> +# phase that evged has initialized while hed not. >>>> +# >>>> +obj-$(CONFIG_ACPI_HED) += hed.o >>>> + >>>> # >>>> # ACPI Core Subsystem (Interpreter) >>>> # >>>> @@ -95,7 +102,6 @@ obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o >>>> obj-$(CONFIG_ACPI_BATTERY) += battery.o >>>> obj-$(CONFIG_ACPI_SBS) += sbshc.o >>>> obj-$(CONFIG_ACPI_SBS) += sbs.o >>>> -obj-$(CONFIG_ACPI_HED) += hed.o >>>> obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o >>>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o >>>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o >>>> -- >>>> 2.33.0 >>>> >>> . > . ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Fix hed module initialization order when it is built-in 2024-11-15 3:50 [PATCH] acpi: Fix hed module initialization order when it is built-in Xiaofei Tan 2024-12-10 17:59 ` Rafael J. Wysocki @ 2024-12-11 16:22 ` Mauro Carvalho Chehab 2024-12-23 9:44 ` Xiaofei Tan 1 sibling, 1 reply; 7+ messages in thread From: Mauro Carvalho Chehab @ 2024-12-11 16:22 UTC (permalink / raw) To: Xiaofei Tan Cc: rafael, lenb, linux-acpi, linux-kernel, jonathan.cameron, M.Chehab, roberto.sassu, shiju.jose, prime.zeng, linuxarm Em Fri, 15 Nov 2024 11:50:14 +0800 Xiaofei Tan <tanxiaofei@huawei.com> escreveu: Please always copy my @kernel.org address for upstream work. > When the module hed is built-in, the init order is determined by > Makefile order. That order violates expectations. Because the module > hed init is behind evged. RAS records can't be handled in the > special time window that evged has initialized while hed not. > If the number of such RAS records is more than the APEI HEST error > source number, the HEST resources could be occupied all, and then > could affect subsequent RAS error reporting. IMO, it is a lot better to use a late init call. Please see: include/linux/init.h This would be done by, for instance, using late_initcall(). Now, what we have is: acpi-y += evged.o obj-$(CONFIG_ACPI_HED) += hed.o Where ACPI_HED being a tri-state. It sounds to me, that even, with your patch, if you build HED as a module, you'll still have a problem. Shouldn't be ACPI_HED be changed from tristate to bool? Regards, Mauro > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com> > --- > drivers/acpi/Makefile | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index 61ca4afe83dc..54f60b7922ad 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -15,6 +15,13 @@ endif > > obj-$(CONFIG_ACPI) += tables.o > > +# > +# The hed.o needs to be in front of evged.o to avoid the problem that > +# RAS errors cannot be handled in the special time window of startup > +# phase that evged has initialized while hed not. > +# > +obj-$(CONFIG_ACPI_HED) += hed.o > + > # > # ACPI Core Subsystem (Interpreter) > # > @@ -95,7 +102,6 @@ obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o > obj-$(CONFIG_ACPI_BATTERY) += battery.o > obj-$(CONFIG_ACPI_SBS) += sbshc.o > obj-$(CONFIG_ACPI_SBS) += sbs.o > -obj-$(CONFIG_ACPI_HED) += hed.o > obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o > obj-$(CONFIG_ACPI_BGRT) += bgrt.o > obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Fix hed module initialization order when it is built-in 2024-12-11 16:22 ` Mauro Carvalho Chehab @ 2024-12-23 9:44 ` Xiaofei Tan 0 siblings, 0 replies; 7+ messages in thread From: Xiaofei Tan @ 2024-12-23 9:44 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rafael J. Wysocki Cc: rafael, lenb, linux-acpi, linux-kernel, jonathan.cameron, M.Chehab, roberto.sassu, shiju.jose, prime.zeng, linuxarm Hi Mauro, 在 2024/12/12 0:22, Mauro Carvalho Chehab 写道: > Em Fri, 15 Nov 2024 11:50:14 +0800 > Xiaofei Tan <tanxiaofei@huawei.com> escreveu: > > Please always copy my @kernel.org address for upstream work. OK >> When the module hed is built-in, the init order is determined by >> Makefile order. That order violates expectations. Because the module >> hed init is behind evged. RAS records can't be handled in the >> special time window that evged has initialized while hed not. >> If the number of such RAS records is more than the APEI HEST error >> source number, the HEST resources could be occupied all, and then >> could affect subsequent RAS error reporting. > IMO, it is a lot better to use a late init call. Please see: > include/linux/init.h > > This would be done by, for instance, using late_initcall(). > > Now, what we have is: > > acpi-y += evged.o > obj-$(CONFIG_ACPI_HED) += hed.o > > Where ACPI_HED being a tri-state. > > It sounds to me, that even, with your patch, if you build > HED as a module, you'll still have a problem. Yes, and it is also affected by loading sequence of HED and GHES. Anyway, the risk remains. > > Shouldn't be ACPI_HED be changed from tristate to bool? agree, @Rafael Hi Rafael, Please help check if we can do this change, thanks. > > Regards, > Mauro > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> >> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com> >> --- >> drivers/acpi/Makefile | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile >> index 61ca4afe83dc..54f60b7922ad 100644 >> --- a/drivers/acpi/Makefile >> +++ b/drivers/acpi/Makefile >> @@ -15,6 +15,13 @@ endif >> >> obj-$(CONFIG_ACPI) += tables.o >> >> +# >> +# The hed.o needs to be in front of evged.o to avoid the problem that >> +# RAS errors cannot be handled in the special time window of startup >> +# phase that evged has initialized while hed not. >> +# >> +obj-$(CONFIG_ACPI_HED) += hed.o >> + >> # >> # ACPI Core Subsystem (Interpreter) >> # >> @@ -95,7 +102,6 @@ obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o >> obj-$(CONFIG_ACPI_BATTERY) += battery.o >> obj-$(CONFIG_ACPI_SBS) += sbshc.o >> obj-$(CONFIG_ACPI_SBS) += sbs.o >> -obj-$(CONFIG_ACPI_HED) += hed.o >> obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o >> obj-$(CONFIG_ACPI_BGRT) += bgrt.o >> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o > . ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-12-28 10:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-15 3:50 [PATCH] acpi: Fix hed module initialization order when it is built-in Xiaofei Tan 2024-12-10 17:59 ` Rafael J. Wysocki 2024-12-23 9:31 ` Xiaofei Tan 2024-12-23 19:33 ` Jonathan Cameron 2024-12-28 10:23 ` Xiaofei Tan 2024-12-11 16:22 ` Mauro Carvalho Chehab 2024-12-23 9:44 ` Xiaofei Tan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox