linux-kernel - Re: [PATCH v8 07/10] drivers: qcom: rpmh: cache sleep/wake state requests

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 May 2018 13:18:12 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Lina Iyer <ilina@...eaurora.org>
Cc:     Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>,
        linux-arm-msm@...r.kernel.org,
        "open list:ARM/QUALCOMM SUPPORT" <linux-soc@...r.kernel.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Evan Green <evgreen@...omium.org>,
        Matthias Kaehlcke <mka@...omium.org>, rplsssn@...eaurora.org
Subject: Re: [PATCH v8 07/10] drivers: qcom: rpmh: cache sleep/wake state requests

Hi,

On Wed, May 9, 2018 at 10:01 AM, Lina Iyer <ilina@...eaurora.org> wrote:
>  /**
>   * struct rpmh_request: the message to be sent to rpmh-rsc
>   *
> @@ -54,9 +71,15 @@ struct rpmh_request {
>   * struct rpmh_ctrlr: our representation of the controller
>   *
>   * @drv: the controller instance
> + * @cache: the list of cached requests
> + * @lock: synchronize access to the controller data

nit: this makes it sound like this lock will be grabbed for all calls
into rpmh-rsc.  In fact, it only protects access to the cache.
Ideally name it cache_lock and document that it's for protecting the
cache.


> +/**
> + * rpmh_flush: Flushes the buffered active and sleep sets to TCS
> + *
> + * @dev: The device making the request
> + *
> + * Return: -EBUSY if the controller is busy, probably waiting on a response
> + * to a RPMH request sent earlier.
> + *
> + * This function is generally called from the sleep code from the last CPU

"is generally" implies that sometimes it's not called from the sleep
code.  Change to "is always".  If "is generally" is more correct, you
can't run lockless right?


-Doug