kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chegu Vinod <chegu_vinod@hp.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: prarit@redhat.com, LKML <linux-kernel@vger.kernel.org>,
	Gleb Natapov <gleb@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, KVM <kvm@vger.kernel.org>
Subject: Re: kvm_intel: Could not allocate 42 bytes percpu data
Date: Mon, 01 Jul 2013 18:13:21 -0700	[thread overview]
Message-ID: <51D22931.1080008@hp.com> (raw)
In-Reply-To: <87ehbisstv.fsf@rustcorp.com.au>

On 6/30/2013 11:22 PM, Rusty Russell wrote:
> Chegu Vinod <chegu_vinod@hp.com> writes:
>> Hello,
>>
>> Lots (~700+) of the following messages are showing up in the dmesg of a
>> 3.10-rc1 based kernel (Host OS is running on a large socket count box
>> with HT-on).
>>
>> [   82.270682] PERCPU: allocation failed, size=42 align=16, alloc from
>> reserved chunk failed
>> [   82.272633] kvm_intel: Could not allocate 42 bytes percpu data
> Woah, weird....
>
> Oh.  Shit.  Um, this is embarrassing.
>
> Thanks,
> Rusty.


Thanks for your response!

> ===
> module: do percpu allocation after uniqueness check.  No, really!
>
> v3.8-rc1-5-g1fb9341 was supposed to stop parallel kvm loads exhausting
> percpu memory on large machines:
>
>      Now we have a new state MODULE_STATE_UNFORMED, we can insert the
>      module into the list (and thus guarantee its uniqueness) before we
>      allocate the per-cpu region.
>
> In my defence, it didn't actually say the patch did this.  Just that
> we "can".
>
> This patch actually *does* it.
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> Tested-by: Noone it seems.

Your following "updated" fix seems to be working fine on the larger 
socket count machine with HT-on.

Thx
Vinod
>
> diff --git a/kernel/module.c b/kernel/module.c
> index cab4bce..fa53db8 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2927,7 +2927,6 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>   {
>   	/* Module within temporary copy. */
>   	struct module *mod;
> -	Elf_Shdr *pcpusec;
>   	int err;
>   
>   	mod = setup_load_info(info, flags);
> @@ -2942,17 +2941,10 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>   	err = module_frob_arch_sections(info->hdr, info->sechdrs,
>   					info->secstrings, mod);
>   	if (err < 0)
> -		goto out;
> +		return ERR_PTR(err);
>   
> -	pcpusec = &info->sechdrs[info->index.pcpu];
> -	if (pcpusec->sh_size) {
> -		/* We have a special allocation for this section. */
> -		err = percpu_modalloc(mod,
> -				      pcpusec->sh_size, pcpusec->sh_addralign);
> -		if (err)
> -			goto out;
> -		pcpusec->sh_flags &= ~(unsigned long)SHF_ALLOC;
> -	}
> +	/* We will do a special allocation for per-cpu sections later. */
> +	info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC;
>   
>   	/* Determine total sizes, and put offsets in sh_entsize.  For now
>   	   this is done generically; there doesn't appear to be any
> @@ -2963,17 +2955,22 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>   	/* Allocate and move to the final place */
>   	err = move_module(mod, info);
>   	if (err)
> -		goto free_percpu;
> +		return ERR_PTR(err);
>   
>   	/* Module has been copied to its final place now: return it. */
>   	mod = (void *)info->sechdrs[info->index.mod].sh_addr;
>   	kmemleak_load_module(mod, info);
>   	return mod;
> +}
>   
> -free_percpu:
> -	percpu_modfree(mod);
> -out:
> -	return ERR_PTR(err);
> +static int alloc_module_percpu(struct module *mod, struct load_info *info)
> +{
> +	Elf_Shdr *pcpusec = &info->sechdrs[info->index.pcpu];
> +	if (!pcpusec->sh_size)
> +		return 0;
> +
> +	/* We have a special allocation for this section. */
> +	return percpu_modalloc(mod, pcpusec->sh_size, pcpusec->sh_addralign);
>   }
>   
>   /* mod is no longer valid after this! */
> @@ -3237,6 +3234,11 @@ static int load_module(struct load_info *info, const char __user *uargs,
>   	}
>   #endif
>   
> +	/* To avoid stressing percpu allocator, do this once we're unique. */
> +	err = alloc_module_percpu(mod, info);
> +	if (err)
> +		goto unlink_mod;
> +
>   	/* Now module is in final location, initialize linked lists, etc. */
>   	err = module_unload_init(mod);
>   	if (err)
> .
>

  reply	other threads:[~2013-07-02  1:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-24 19:01 kvm_intel: Could not allocate 42 bytes percpu data Chegu Vinod
2013-06-24 22:52 ` Prarit Bhargava
2013-06-27  0:53   ` Marcelo Tosatti
2013-07-01  6:22 ` Rusty Russell
2013-07-02  1:13   ` Chegu Vinod [this message]
2013-07-02  5:49     ` Rusty Russell
2013-07-02 16:34       ` Chegu Vinod
2013-07-03  0:37         ` Rusty Russell

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=51D22931.1080008@hp.com \
    --to=chegu_vinod@hp.com \
    --cc=gleb@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=prarit@redhat.com \
    --cc=rusty@rustcorp.com.au \
    /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;
as well as URLs for NNTP newsgroup(s).