Re: [RFC PATCH v2 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()

[Ming Lei <ming.lei@canonical.com>]

On Mon, Oct 22, 2012 at 10:33 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> Tail recursion should be implemented as a loop, not as an explicit
> recursion.  That is, the function should be:
>
> void pm_runtime_set_memalloc_noio(struct device *dev, bool enable)
> {
>         do {
>                 dev->power.memalloc_noio_resume = enable;
>
>                 if (!enable) {
>                         /*
>                          * Don't clear the parent's flag if any of the
>                          * parent's children have their flag set.
>                          */
>                         if (device_for_each_child(dev->parent, NULL,
>                                           dev_memalloc_noio))
>                                 return;
>                 }
>                 dev = dev->parent;
>         } while (dev);
> }

OK, will take the non-recursion implementation for saving kernel
stack space.

>
> except that you need to add locking, for two reasons:
>
>         There's a race.  What happens if another child sets the flag
>         between the time device_for_each_child() runs and the next loop
>         iteration?

Yes, I know the race, and not adding a lock because the function
is mostly called in .probe() or .remove() callback and its parent's device
lock is held to avoid this race.

Considered that it may be called in async probe() (scsi disk), one lock
is needed, the simplest way is to add a global lock. Any suggestion?

>
>         Even without a race, access to bitfields is not SMP-safe
>         without locking.

You mean one ancestor device might not be in active when
one of its descendants is being probed or removed?


Thanks,
--
Ming Lei

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>