Re: [PATCH 2/2] PM / Domains: Avoid infinite loops in attach/detach code

On Tuesday, June 23, 2015 03:45:43 PM Geert Uytterhoeven wrote:

Hi Rafael,

On Tue, Jun 23, 2015 at 3:38 PM, Rafael J. Wysocki [off-list ref] wrote:

quoted

quoted

quoted

quoted

@@ -2218,10 +2227,13 @@ int genpd_dev_pm_attach(struct device *dev)

        dev_dbg(dev, "adding to PM domain %s\n", pd->name);

-       while (1) {
+       for (i = 0; i < GENPD_RETRIES; i++) {
                ret = pm_genpd_add_device(pd, dev);
                if (ret != -EAGAIN)
                        break;
+
+               if (i > GENPD_RETRIES / 2)
+                       udelay(GENPD_DELAY_US);

In this execution path, we retry when getting -EAGAIN while believing
the reason to the error are only *temporary* as we are soon waiting
for all devices in the genpd to be system PM resumed. At least that's
my understanding to why we want to deal with -EAGAIN here, but I might
be wrong.

In this regards, I wonder whether it could be better to re-try only a
few times but with a far longer interval time than a couple us. What
do you think?

That's indeed viable. I have no idea for how long this temporary state can
extend.

A usual approach to this kind of thing is to use exponential fallback
where you increase the delay twice with respect to the previous one
every time.

Right, but when do you give up?

Well, I guess you know what a reasonable timeout should be?

Note that udelay() is a busy loop. Should it fall back to msleep() after
a while?

If we can't fall back to msleep() at one point, you may as well simply poll
periodically as you did originally.

Thanks,
Rafael