Re: [PATCH] thp: reduce khugepaged freezing latency

From: Tejun Heo
Date: Wed Nov 09 2011 - 11:59:32 EST


Hello,

On Wed, Nov 09, 2011 at 05:52:01PM +0100, Andrea Arcangeli wrote:
> > schedule_timeout_interruptible(
> > msecs_to_jiffies(
> > khugepaged_alloc_sleep_millisecs));
> > try_to_freeze();
> > remove_wait_queue(&khugepaged_wait, &wait);
> > }
>
> I thought about that but isn't there a race condition if TIF_FREEZE is
> set just in the point I marked above? I thought the
> set_freezable_with_signal by forcing the task runnable would fix it.
>
> How exactly wait_event_freezable_timeout() would avoid the same race
> as above? I mean the freezer won't have visibility on the
> khugepaged_wait waitqueue head so it surely cannot wake it up. And if
> the freezing() check happens before TIF_FREEZE get set but before
> schedule() is called, we're still screwed even if I use
> wait_event_freezable_timeout()... Or is the signal_pending check
> fixing that? But without set_freezable_with_signal() we don't set
> TIF_SIGPENDING... so it's not immediately care how this whole logic is
> race free. If you use stop_machine that could avoid the races though,
> but it doesn't look like the freezer uses that.

Freezer depends on the usual "set_current_state(INTERRUPTIBLE); check
freezing; schedule(); check freezing" construct and sends
INTERRUPTIBLE wake up after setting freezing state. The
synchronization hasn't been completely clear but recently been cleaned
up, so as long as freezing condition is tested after INTERRUPTIBLE is
set before going to sleep, the event won't go missing.

Maybe we need a helper here, which would be named horribly -
schedule_timeout_interruptible_freezable(). (cc'ing Oleg) Oleg, maybe
we need schedule_timeout(@sleep_type) too?

Thanks.

--
tejun
--
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/