lkml.org 
[lkml]   [2016]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH 11/19] cpufreq: assert policy->rwsem is held in __cpufreq_governor
    On 02/01/2016 02:22 AM, Rafael J. Wysocki wrote:
    > On Mon, Feb 1, 2016 at 7:09 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
    >> On 30-01-16, 12:49, Rafael J. Wysocki wrote:
    >>> On Friday, January 29, 2016 04:33:39 PM Saravana Kannan wrote:
    >>>> AFAIR, the ABBA issue was between the sysfs lock and the policy lock.
    >>
    >> Yeah, to be precise here it is:
    >>
    >> CPU0 (sysfs read) CPU1 (exit governor)
    >>
    >> sysfs-read set_policy()-> lock policy->rwsem
    >> sysfs-active lock Remove sysfs files
    >> lock policy->rwsem sysfs-active lock
    >> Actual read
    >>
    >>>> The fix for that issue should not be dropping the lock around
    >>>> POLICY_EXIT.
    >>>
    >>> Right. Dropping the lock is a mistake (which I overlooked, sadly).
    >>
    >> I joined the party at around time of 3.10, and we had this problem and
    >> hacky solution then as well. We tried to get rid of it multiple times,
    >> but sadly failed.
    >
    > I kind of like your idea of accessing governor attributes without
    > holding the policy rwsem.

    I'm not sure whose idea you are referring to. Viresh's (I don't think I
    saw his proposal) or mine.

    > I looked at that code and it seems doable to me. The problem to solve
    > there would be to ensure that the dbs_data pointer is valid when
    > show/store runs for those attributes.
    >
    > The fact that we make the distinction between global and policy
    > governors in there doesn't really help, but it looks like getting rid
    > of that bit wouldn't be too much effort. Let me take a deeper look at
    > that.
    >

    Anyway, to explain my suggestion better, I'm proposing to make it so
    that we don't have a need for the AB BA locking. The only reason the
    governor needs to even grab the sysfs lock is to add/remove the sysfs
    attribute files.

    That can be easily achieved if the policy struct has some "gov_attrs"
    field(s) that each governor populates. Then the framework just has to
    create them after POLICY_INIT is processed by the governor and remove
    them before POILICY_EXIT is sent to the governor.

    That way, we also avoid having to worry about the gov attributes
    accessed by the show/store disappearing while the files are being
    accessed. Since we remove those files before we even ask the gov to
    clean up, that situation can never happen.

    The current problem is that there is no good place for the governor to
    populate this "gov_attrs" field(s). Maybe the governor register might be
    one place for it to provide the data to the framework and the framework
    can later fill it up itself when switching governors.

    Thanks,
    Saravana

    --
    Qualcomm Innovation Center, Inc.
    The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
    a Linux Foundation Collaborative Project

    \
     
     \ /
      Last update: 2016-02-01 21:41    [W:2.689 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site