Re: [PATCH 06/12] PM / OPP: Add 'struct kref' to struct dev_pm_opp
On 01/13, Viresh Kumar wrote: > On 13-01-17, 00:52, Stephen Boyd wrote: > > What still doesn't make sense is how an individual OPP could go > > away without the table that the OPP lives in also going away. > > dev_pm_opp_remove() is one such option, which can remove OPPs > individually. Over that, while remove tables we remove all the OPPs > one by one. So that really does happen. > > > If > > an OPP is going away while a driver has a reference to it, then > > the driver using that OPP should probably not be using it. > > That is being protected with this patch now and the drivers can use > them freely. > > > TL;DR > > letting drivers use OPP pointers outside of the OPP core feels > > racy. > > Hmm, we don't update the OPP a lot after creating it today. But that's > a different problem to solve, if we really see a race there. > Ok. We still have work to do to fix the race between drivers using dev_pm_opp pointers and other drivers updating the data those pointers point to like voltage, enable/disable, etc. This isn't making anything worse than it already is though, so: Reviewed-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 06/12] PM / OPP: Add 'struct kref' to struct dev_pm_opp
On 13-01-17, 00:52, Stephen Boyd wrote: > What still doesn't make sense is how an individual OPP could go > away without the table that the OPP lives in also going away. dev_pm_opp_remove() is one such option, which can remove OPPs individually. Over that, while remove tables we remove all the OPPs one by one. So that really does happen. > If > an OPP is going away while a driver has a reference to it, then > the driver using that OPP should probably not be using it. That is being protected with this patch now and the drivers can use them freely. > TL;DR > letting drivers use OPP pointers outside of the OPP core feels > racy. Hmm, we don't update the OPP a lot after creating it today. But that's a different problem to solve, if we really see a race there. -- viresh
Re: [PATCH 06/12] PM / OPP: Add 'struct kref' to struct dev_pm_opp
On 01/10, Viresh Kumar wrote: > On 09-01-17, 15:44, Stephen Boyd wrote: > > On 12/07, Viresh Kumar wrote: > > > Add kref to struct dev_pm_opp for easier accounting of the OPPs. > > > > > > Note that the OPPs are freed under the opp_table->lock mutex only. > > > > I'm lost. Why add another level of krefs? > > Heh. The earlier krefs were for the OPP table itself, so that it gets freed > once > there are no more users of it. > > The kref introduced now is for individual OPPs, so that they don't disappear > while being used and gets freed once all are done. > > Also note that the OPP table will get freed only after all the OPPs are freed, > plus there are no more users left, like platform code which might have set > suppoerted-hw property. > What still doesn't make sense is how an individual OPP could go away without the table that the OPP lives in also going away. If an OPP is going away while a driver has a reference to it, then the driver using that OPP should probably not be using it. TL;DR letting drivers use OPP pointers outside of the OPP core feels racy. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 06/12] PM / OPP: Add 'struct kref' to struct dev_pm_opp
On 09-01-17, 15:44, Stephen Boyd wrote: > On 12/07, Viresh Kumar wrote: > > Add kref to struct dev_pm_opp for easier accounting of the OPPs. > > > > Note that the OPPs are freed under the opp_table->lock mutex only. > > I'm lost. Why add another level of krefs? Heh. The earlier krefs were for the OPP table itself, so that it gets freed once there are no more users of it. The kref introduced now is for individual OPPs, so that they don't disappear while being used and gets freed once all are done. Also note that the OPP table will get freed only after all the OPPs are freed, plus there are no more users left, like platform code which might have set suppoerted-hw property. -- viresh
Re: [PATCH 06/12] PM / OPP: Add 'struct kref' to struct dev_pm_opp
On 12/07, Viresh Kumar wrote: > Add kref to struct dev_pm_opp for easier accounting of the OPPs. > > Note that the OPPs are freed under the opp_table->lock mutex only. I'm lost. Why add another level of krefs? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
