lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20161124142747.GL4082@atomide.com>
Date:   Thu, 24 Nov 2016 06:27:47 -0800
From:   Tony Lindgren <tony@...mide.com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     Brian Norris <briannorris@...omium.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Brian Norris <computersforpeace@...il.com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] PM / wakeirq: report wakeup events in dedicated wake-IRQs

* Rafael J. Wysocki <rafael@...nel.org> [161123 14:37]:
> On Fri, Nov 18, 2016 at 9:18 PM, Tony Lindgren <tony@...mide.com> wrote:
> > Hi,
> >
> > * Rafael J. Wysocki <rafael@...nel.org> [161111 16:35]:
> >> However, my understanding is that the current code actually works for
> >> runtime PM just fine.
> >
> > Hmm well I just noticed that for drivers not using autosuspend it can be
> > flakey, see the patch below. That probably explains some mysteries people
> > are seeing with wakeirqs.
> >
> > Do you have any better ideas for setting wirq->active on the first
> > rpm_suspend()?
> 
> You could change dedicated_irq into status and have three values for
> it: INVALID, ALLOCATED and IN_USE.
> 
> dev_pm_set_dedicated_wake_irq() would make it ALLOCATED and
> dev_pm_check_wake_irq() would change it into IN_USE.  In turn,
> dev_pm_enable/disable_wake_irq() would only touch it if it is IN_USE
> and dev_pm_clear_wake_irq() would free it if it is not INVALID.

OK sounds good to me.

> > +static inline void dev_pm_check_wake_irq(struct device *dev)
> > +{
> > +       struct wake_irq *wirq = dev->power.wakeirq;
> > +
> > +       if (!wirq)
> > +               return;
> > +
> > +       if (unlikely(!wirq->active)) {
> > +               wirq->active = true;
> > +               wmb();  /* ensure dev_pm_enable_wake_irq() sees active */
> 
> smp_wmb()?
> 
> Also, do we have a corresponding barrier on the reader side?

Will check thanks.

> I wonder if it would make sense to fold dev_pm_check_wake_irq() into
> dev_pm_enable_wake_irq()?

I was thinking that too but then bus or device itself won't be
able to manage the wakeirq if needed.

Let me check if we could easily add something to initialize things
like dev_pm_manage_wake_irq() and dev_pm_dont_manage_wake_irq().
That means we need to update the drivers using it, but then we don't
need to add extra checks to the idle path and can let bus or
drivers mange the wakeirq if necessary.

Regards,

Tony

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ