lkml.org 
[lkml]   [2011]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3] PM/Memory-hotplug: Avoid task freezing failures
    Hello, Srivatsa.

    On Thu, Nov 17, 2011 at 02:00:50PM +0530, Srivatsa S. Bhat wrote:
    > @@ -380,7 +382,40 @@ static inline void unlock_system_sleep(void) {}
    >
    > static inline void lock_system_sleep(void)
    > {
    > - mutex_lock(&pm_mutex);
    > + /*
    > + * "To sleep, or not to sleep, that is the question!"
    > + *
    > + * We should not use mutex_lock() here because, in case we fail to
    > + * acquire the lock, it would put us to sleep in TASK_UNINTERRUPTIBLE
    > + * state, which would lead to task freezing failures. As a
    > + * consequence, hibernation would fail (even though it had acquired
    > + * the 'pm_mutex' lock).
    > + * Using mutex_lock_interruptible() in a loop is not a good idea,
    > + * because we could end up treating non-freezing signals badly.
    > + * So we use mutex_trylock() in a loop instead.
    > + *
    > + * Also, we add try_to_freeze() to the loop, to co-operate with the
    > + * freezer, to avoid task freezing failures due to busy-looping.
    > + *
    > + * But then, since it is not guaranteed that we will get frozen
    > + * rightaway, we could keep spinning for some time, breaking the
    > + * expectation that we go to sleep when we fail to acquire the lock.
    > + * So we add an msleep() to the loop, to dampen the spin (but we are
    > + * careful enough not to sleep for too long at a stretch, lest the
    > + * freezer whine and give up again!).
    > + *
    > + * Now that we no longer busy-loop, try_to_freeze() becomes all the
    > + * more important, due to a subtle reason: if we don't cooperate with
    > + * the freezer at this point, we could end up in a situation very
    > + * similar to mutex_lock() due to the usage of msleep() (which sleeps
    > + * uninterruptibly).
    > + *
    > + * Phew! What a delicate balance!
    > + */
    > + while (!mutex_trylock(&pm_mutex)) {
    > + try_to_freeze();
    > + msleep(10);
    > + }

    I tried to think about a better way to do it but couldn't, so I
    suppose this is what we should go with for now. That said, I think
    the comment is a bit too umm.... verbose. What we want here is
    freezable but !interruptible mutex_lock() and while I do appreciate
    the detailed comment, I think it makes it look a lot more complex than
    it actually is. Other than that,

    Acked-by: Tejun Heo <tj@kernel.org>

    Thank you very much.

    --
    tejun


    \
     
     \ /
      Last update: 2011-11-19 19:35    [W:0.026 / U:0.296 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site