[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
On 04/11/2012 01:17 PM, Turquette, Mike wrote:
On Wed, Apr 11, 2012 at 12:57 PM, Saravana Kannan <skannan@xxxxxxxxxxxxxx> wrote:On 04/11/2012 10:59 AM, Turquette, Mike wrote:On Thu, Apr 5, 2012 at 6:30 PM, Saravana Kannan<skannan@xxxxxxxxxxxxxx> My only concern with this series is that platform clock code will access struct clk_hw members without the appropriate lock held (namely prepare_lock). I'm a bit worried that there might be a case where some clock code access clk_hw->name when clk_hw has somehow been destroyed/altered (e.g. if clk_put every finally gets implemented...). Besides clk_put are there any real instances where this might happen?This problem is true for any struct/field marked as __init_data. So, I don't think this is a real problem. If someone is stupid enough to mark their data as __init_data and access them later, then there is not much we can do. Also, I believe the compiler throws out some warning when you try to access __init_data from non-init code.I'm referring to platform clock code accessing clk_hw->name or clk_hw->parent_names AFTER the core code has copied the initialization data. So this isn't an __initdata problem as much as it is a design issue with exposing some members of struct clk_hw. I think that this is basically OK since the benefits of not having to use so many helpers functions in platform clock code is obvious and if a platform gets bitten by this then they will learn the hard way to honor the locking semantics outlined in Documentation/clk.txt
That's what I meant too. The platform code and the static init data will most likely be written by the same dev. So, hopefully they know what they are doing.
I'll be pushing my fixes branch out to the list soon. Do you want to rebase this change on top of it taking into account the __initdata bits?I thought it might be easier for you to base your changes on top of my patch. But I can try to rebase mine on top of your changes. Hopefully your fixes aren't crazy big/complex.I don't think the fixes are a big deal, especially in the core code.This is a busy week for me at work. I will try to send a patch in a day or two.OK. I might take a swing at it myself if I have time.
I still need to make the changes to add the copying part. So, don't bother with taking a swing at it. If you want to help out, help with rebasing it on top of your fixes would be appreciated. But based on what you said, it should be hard.
TLDR: Just wait for me to send out the patches. :) -Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html