Hello, Andrea. On Wed, Nov 09, 2011 at 07:19:25PM +0100, Andrea Arcangeli wrote: > 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. Oh I see. > 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. I think, for this specific case, wait_event_freezable_timeout() w/ false is the simplest thing to do. > > 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.. :) I agree that there can be use cases where freezable interruptible sleep is useful. Thanks to the the inherently racy nature of schedule_interruptible_timeout() w.r.t. non-persistent interruptible wakeups (ie. everything other than signal), race conditions introduced by try_to_freeze() should be okay The biggest problem I have with schedule_timeout_freezable() is that it doesn't advertise that it's racy - ie. it doesn't have sleep condition in the function name. Its wait counterpart wait_event_freezable() isn't racy thanks to the explicit wait condition and doesn't have such problem. Maybe my concern is just paraonia and people wouldn't assume it's schedule_timeout() with magic freezer support. Or we can name it schedule_timeout_interruptible_freezable() (urgh........). I don't know. My instinct tells me to strongly recommend use of wait_event_freezable_timeout() and run away. :) Thanks. -- tejun -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>