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]

Message-ID: <2c6fc480-7894-2a69-b1ff-fc3cbc96a9a1@codeaurora.org>
Date:   Wed, 23 May 2018 17:51:48 +0530
From:   Raju P L S S S N <rplsssn@...eaurora.org>
To:     Doug Anderson <dianders@...omium.org>,
        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>,
        msivasub@...eaurora.org, mkshah@...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>
Subject: Re: [PATCH v8 07/10] drivers: qcom: rpmh: cache sleep/wake state
 requests

Hi,

On 5/12/2018 1:48 AM, Doug Anderson wrote:
> 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
> 

Yes. Will change in next patch.

Thanks,
Raju