* ERST: how to avoid a dynamic memory allocation in panic case
@ 2013-01-22 23:30 Seiji Aguchi
2013-01-23 0:39 ` Huang Ying
0 siblings, 1 reply; 3+ messages in thread
From: Seiji Aguchi @ 2013-01-22 23:30 UTC (permalink / raw)
To: Huang, Ying (ying.huang@intel.com), rjw
Cc: linux-kernel, linux-acpi, H. Peter Anvin (hpa@zytor.com),
robert.moore, trenn, myron.stowe
[Issue]
Current erst driver is kicked in panic case.
On the other hand, a dynamic memory allocation seems to run in it
with a following code path.
erst_writer -> erst_write -> __erst_write_to_storage -> many of apei_exec_run ->
ctx->ins_table[entry->instruction].run()
which are functions defined in erst_ins_type table i.e:
apei_exec_read_register -> apei_read -> acpi_os_read_memory64 -> acpi_os_ioremap -> ioremap_cache
which possibly involve IOMMU allocator.
I may cause a failure of an erst driver if the panic happens in an interrupt context.
[Idea]
If we can remove ioremap_cache() from acpi_os_read_memory() as follows,
It is easy to fix this issue.
But I'm not sure if it is feasible because I can't see the reason of tying acpi_os_ioremap() from a git log...
Any comment?
<snip>
@@ -918,10 +918,7 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
if (!virt_addr) {
rcu_read_unlock();
- virt_addr = acpi_os_ioremap(phys_addr, size);
- if (!virt_addr)
- return AE_BAD_ADDRESS;
- unmap = true;
+ return AE_BAD_ADDRESS;
}
if (!value)
<snip>
Seiji
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: ERST: how to avoid a dynamic memory allocation in panic case
2013-01-22 23:30 ERST: how to avoid a dynamic memory allocation in panic case Seiji Aguchi
@ 2013-01-23 0:39 ` Huang Ying
2013-01-23 0:44 ` Seiji Aguchi
0 siblings, 1 reply; 3+ messages in thread
From: Huang Ying @ 2013-01-23 0:39 UTC (permalink / raw)
To: Seiji Aguchi
Cc: rjw, linux-kernel, linux-acpi, H. Peter Anvin (hpa@zytor.com),
robert.moore, trenn, myron.stowe
On Tue, 2013-01-22 at 23:30 +0000, Seiji Aguchi wrote:
> [Issue]
>
> Current erst driver is kicked in panic case.
> On the other hand, a dynamic memory allocation seems to run in it
> with a following code path.
>
> erst_writer -> erst_write -> __erst_write_to_storage -> many of apei_exec_run ->
> ctx->ins_table[entry->instruction].run()
>
> which are functions defined in erst_ins_type table i.e:
>
> apei_exec_read_register -> apei_read -> acpi_os_read_memory64 -> acpi_os_ioremap -> ioremap_cache
>
> which possibly involve IOMMU allocator.
>
> I may cause a failure of an erst driver if the panic happens in an interrupt context.
>
> [Idea]
>
> If we can remove ioremap_cache() from acpi_os_read_memory() as follows,
> It is easy to fix this issue.
> But I'm not sure if it is feasible because I can't see the reason of tying acpi_os_ioremap() from a git log...
>
> Any comment?
>
> <snip>
> @@ -918,10 +918,7 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
> virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
> if (!virt_addr) {
> rcu_read_unlock();
> - virt_addr = acpi_os_ioremap(phys_addr, size);
> - if (!virt_addr)
> - return AE_BAD_ADDRESS;
> - unmap = true;
> + return AE_BAD_ADDRESS;
No. We can not do that. Because some users rely on acpi_os_read_memory
to do ioremap for them.
The correct fixing should be pre-map the io-memory that may be accessed
in erst code patch with acpi_map().
Best Regards,
Huang Ying
> }
>
> if (!value)
> <snip>
>
> Seiji
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: ERST: how to avoid a dynamic memory allocation in panic case
2013-01-23 0:39 ` Huang Ying
@ 2013-01-23 0:44 ` Seiji Aguchi
0 siblings, 0 replies; 3+ messages in thread
From: Seiji Aguchi @ 2013-01-23 0:44 UTC (permalink / raw)
To: Huang Ying
Cc: rjw, linux-kernel, linux-acpi, H. Peter Anvin (hpa@zytor.com),
robert.moore, trenn, myron.stowe
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 835 bytes --]
> > <snip>
> > @@ -918,10 +918,7 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
> > virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
> > if (!virt_addr) {
> > rcu_read_unlock();
> > - virt_addr = acpi_os_ioremap(phys_addr, size);
> > - if (!virt_addr)
> > - return AE_BAD_ADDRESS;
> > - unmap = true;
> > + return AE_BAD_ADDRESS;
>
> No. We can not do that. Because some users rely on acpi_os_read_memory to do ioremap for them.
Thank you for giving me the information.
> The correct fixing should be pre-map the io-memory that may be accessed in erst code patch with acpi_map().
I will take a look at the code.
Seiji
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-01-23 0:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-22 23:30 ERST: how to avoid a dynamic memory allocation in panic case Seiji Aguchi
2013-01-23 0:39 ` Huang Ying
2013-01-23 0:44 ` Seiji Aguchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).