linux-kernel - Re: [PATCH] thp: reduce khugepaged freezing latency

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20111109181925.GN5075@redhat.com>
Date:	Wed, 9 Nov 2011 19:19:25 +0100
From:	Andrea Arcangeli <aarcange@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	"Rafael J. Wysocki" <rjw@...e.com>, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org, Jiri Slaby <jirislaby@...il.com>,
	linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] thp: reduce khugepaged freezing latency

On Wed, Nov 09, 2011 at 10:09:00AM -0800, Tejun Heo wrote:
> I'm confused.  You're doing add_wait_queue() before
> schedule_timeout_interruptible().  prepare_to_wait() is essentially
> add_wait_queue() + set_current_state().  What am I missing?  ie. why
> not do the following?

Ah the reason of the waitqueue is the sysfs store, to get out of there
if somebody decreases the wait time from 1min to 10sec or
similar. It's not really needed for other things, in theory it could
be a separate waitqueue just for sysfs but probably not worth it.

> 	prepare_to_wait(INTERRUPTIBLE);
> 	try_to_freeze();
> 	schedule_timeout();
> 	try_to_freeze();
> 	finish_wait();
> 
> or even simpler,
> 
> 	wait_event_freezable_timeout(wq, false, timeout);
> 
> In terms of overhead, there is no appreciable difference from
> 
> 	add_wait_queue();
> 	schedule_timeout_interruptible();
> 	remove_wait_queue()
> 
> Or is the logic there scheduled to change?

I have no "event" to wait other than the wakeup itself, this in the
end is the only reason it isn't already using
wait_event_freezable_timeout. Of course I can pass "false" as the
event.

> Hmmm... I don't know.  I really hope all freezable tasks stick to
> higher level interface.  It's way too easy to get things wrong and eat
> either freezing or actual wakeup condition.

Well you've just to tell me if I have to pass "false" and if
add_wait_queue+schedule_timeout_interruptible is obsoleted. If it's
not obsoleted the patch I posted should already be ok. It also will be
useful if others need to wait for a long time (> the freezer max wait)
without a waitqueue which I don't think is necessarily impossible. It
wasn't the case here just because I need to promptly react to the
sysfs writes (or setting the wait time to 1 day would then require 1
day before sysfs new value becomes meaningful, well unless somebody
doess killall khugepaged.. :)
--
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/