linux-kernel - Re: [PULL] cpumask: finally make them variable size w/ CPUMASK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4FB1B37A.5050501@gmail.com>
Date:	Mon, 14 May 2012 21:38:02 -0400
From:	KOSAKI Motohiro <kosaki.motohiro@...il.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
CC:	KOSAKI Motohiro <kosaki.motohiro@...il.com>,
	Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
	LKML <linux-kernel@...r.kernel.org>, anton@...ba.org,
	Arnd Bergmann <arnd@...db.de>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Mike Travis <travis@....com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PULL] cpumask: finally make them variable size w/ CPUMASK_OFFSTACK.

>> This code still slow than original. when calling reclaim path, new allocation is almost always
>> fail. then, your code almost always invoke all cpu batch invalidation. i.e. many ipi.
>
> I don't know this code.  Does that happen often?Do we really need to
> optimize the out-of-memory path?

we don't need optimize out-of-memory path. but it's not out-of-memory path. our reclaim code
has two steps 1) purge small file cache (try_to_free_pages) 2) get new page (get_page_from_freelist).
but if you have smp box, it's racy. To success 1) doesn't guarantee to success 2). then, drain_all_pages()
is called frequently than you expected.



> But I should have used on_each_cpu_cond() helper which does this for us
> (except it falls back to individial IPIs) which would make this code
> neater.

Ah, yes. that definitely makes sense.


>>>> 2) When CONFIG_CPUMASK_OFFSTACK=n and NR_CPUS is relatively large, cpumask on stack may
>>>> cause stack overflow. because of, alloc_pages() can be called from
>>>> very deep call stack.
>>>
>>> You can't have large NR_CPUS without CONFIG_CPUMASK_OFFSTACK=y,
>>> otherwise you'll get many other stack overflows, too.
>>
>> Original code put cpumask bss instead stack then. :-)
>
> Yes, and this is what it looks like if we convert it directly, but I
> still don't want to encourage people to do this :(
>
> Cheers,
> Rusty.
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1179,7 +1179,7 @@ void drain_all_pages(void)
>   	 * Allocate in the BSS so we wont require allocation in
>   	 * direct reclaim path for CONFIG_CPUMASK_OFFSTACK=y
>   	 */
> -	static cpumask_t cpus_with_pcps;
> +	static DECLARE_BITMAP(cpus_with_pcps, NR_CPUS);
>
>   	/*
>   	 * We don't care about racing with CPU hotplug event
> @@ -1197,11 +1197,12 @@ void drain_all_pages(void)
>   			}
>   		}
>   		if (has_pcps)
> -			cpumask_set_cpu(cpu,&cpus_with_pcps);
> +			cpumask_set_cpu(cpu, to_cpumask(cpus_with_pcps));
>   		else
> -			cpumask_clear_cpu(cpu,&cpus_with_pcps);
> +			cpumask_clear_cpu(cpu, to_cpumask(cpus_with_pcps));
>   	}
> -	on_each_cpu_mask(&cpus_with_pcps, drain_local_pages, NULL, 1);
> +	on_each_cpu_mask(to_cpumask(cpus_with_pcps),
> +			 drain_local_pages, NULL, 1);
>   }

Looks good to me. thanks.

   Acked-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/