linux-kernel - Re: [PATCH 3/3] PM / QoS: Add function dev_pm_qos_read

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: <CAORVsuV4ccJvJg-zCrhVXJKmHkEYLLQh032=BMh7qH27bTK5uQ@mail.gmail.com>
Date:	Fri, 30 Sep 2011 10:08:36 +0200
From:	Jean Pihet <jean.pihet@...oldbits.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Linux PM mailing list <linux-pm@...ts.linux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Linux-sh list <linux-sh@...r.kernel.org>,
	Magnus Damm <magnus.damm@...il.com>,
	Kevin Hilman <khilman@...com>, Ming Lei <tom.leiming@...il.com>
Subject: Re: [PATCH 3/3] PM / QoS: Add function dev_pm_qos_read_value() (v2)

Hi Rafael,

On Thu, Sep 29, 2011 at 10:33 PM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> Hi,
>
> On Thursday, September 29, 2011, Jean Pihet wrote:
>> Hi Rafael,
>>
>> I have a few minor remarks, inlined below.
>
> In the meantime it turned out that the patch caused a build failure
> for CONFIG_PM_SLEEP unset (your original patches won't really work
> in this case too), so I posted a patch to fix it.  However, since
> you had remarks anyway, I've simply folded the fix into the next
> version of the $subject patch, which is appended.
OK to me.

I have 2 remarks though:
- some drivers are using the power_state field in the suspend/resume
operation, IIUC without conflict. Can you confirm?
- the power_state field is scheduled for removal, cf.
Documentation/feature-removal-schedule.txt. That does not look
correct.

...
>> > +static void __dev_pm_qos_constraints_init(struct device *dev)
>> I would change the name or add a comment stating that this is an
>> internal function which reinits the constraints field under the lock.
>
> This function was used only once, so it wasn't really necessary.  I simply
> removed it.
>
> Thanks,
> Rafael
>
> ---
> From: Rafael J. Wysocki <rjw@...k.pl>
> Subject: PM / QoS: Add function dev_pm_qos_read_value() (v3)
>
> To read the current PM QoS value for a given device we need to
> make sure that the device's power.constraints object won't be
> removed while we're doing that.  For this reason, put the
> operation under dev->power.lock and acquire the lock
> around the initialization and removal of power.constraints.
>
> Moreover, since we're using the value of power.constraints to
> determine whether or not the object is present, the
> power.constraints_state field isn't necessary any more and may be
> removed.  However, dev_pm_qos_add_request() needs to check if the
> device is being removed from the system before allocating a new
> PM QoS constraints object for it, so make it use the
> power.power_state field of struct device for this purpose.
>
> Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>

Regards,
Jean
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/