Re: [PATCH] thp: reduce khugepaged freezing latency

From: Andrea Arcangeli
Date: Wed Nov 09 2011 - 13:19:45 EST


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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/