public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Daniel J Blueman <daniel.blueman@gmail.com>,
	Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, H Peter Anvin <hpa@zytor.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ioapic: fix potential resume deadlock
Date: Tue, 10 May 2011 09:35:51 +0200	[thread overview]
Message-ID: <20110510073551.GH11595@elte.hu> (raw)
In-Reply-To: <1304908814-23369-1-git-send-email-daniel.blueman@gmail.com>


* Daniel J Blueman <daniel.blueman@gmail.com> wrote:

> Fix a potential deadlock when resuming; here the calling function
> has disabled interrupts, so we cannot sleep.
> 
> Signed-off-by: Daniel J Blueman <daniel.blueman@gmail.com>
> 
> ---
>  arch/x86/kernel/apic/io_apic.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 45fd33d..df63620 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -621,14 +621,14 @@ struct IO_APIC_route_entry **alloc_ioapic_entries(void)
>  	struct IO_APIC_route_entry **ioapic_entries;
>  
>  	ioapic_entries = kzalloc(sizeof(*ioapic_entries) * nr_ioapics,
> -				GFP_KERNEL);
> +				GFP_ATOMIC);
>  	if (!ioapic_entries)
>  		return 0;
>  
>  	for (apic = 0; apic < nr_ioapics; apic++) {
>  		ioapic_entries[apic] =
>  			kzalloc(sizeof(struct IO_APIC_route_entry) *
> -				nr_ioapic_registers[apic], GFP_KERNEL);
> +				nr_ioapic_registers[apic], GFP_ATOMIC);
>  		if (!ioapic_entries[apic])
>  			goto nomem;
>  	}

Hm, there must be some other bug here.

ioapic entries should be allocated on system bootup and should never really be 
deallocated.

The bug might be in the idea to call to enable_IR_x2apic() on resume - why are 
ioapic entries reallocated there? That call in default_setup_apic_routing() was 
introduced some time ago in:

  fa47f7e52874: x86, x2apic: Simplify apic init in SMP and UP builds

and that it results in reallocation in the suspend patch is distinctly wrong.

I suspect the reason why this has not triggered for others is the relative 
rarity of affected systems?

Suresh?

Thanks,

	Ingo

  parent reply	other threads:[~2011-05-10  7:36 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-09  2:40 [PATCH] ioapic: fix potential resume deadlock Daniel J Blueman
2011-05-10  5:32 ` Daniel J Blueman
2011-05-10  7:35 ` Ingo Molnar [this message]
2011-05-10 10:53   ` Daniel J Blueman
2011-05-10 23:58     ` Suresh Siddha
2011-05-11 16:15       ` Daniel J Blueman
2011-05-13 17:48         ` Suresh Siddha
2011-05-14 14:39           ` Daniel J Blueman
2011-05-16 11:32             ` Ingo Molnar
2011-05-16 17:04               ` Suresh Siddha
2011-05-16 11:34           ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110510073551.GH11595@elte.hu \
    --to=mingo@elte.hu \
    --cc=daniel.blueman@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=suresh.b.siddha@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox