lkml.org 
[lkml]   [2012]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH 8/8] PM / Sleep: Add user space interface for manipulating wakeup sources
    On Tue, 24 Apr 2012 23:27:17 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:

    > From: Rafael J. Wysocki <rjw@sisk.pl>
    > Subject: PM / Sleep: Add user space interface for manipulating wakeup sources, v2
    >
    > Android allows user space to manipulate wakelocks using two
    > sysfs file located in /sys/power/, wake_lock and wake_unlock.
    > Writing a wakelock name and optionally a timeout to the wake_lock
    > file causes the wakelock whose name was written to be acquired (it
    > is created before is necessary), optionally with the given timeout.
    > Writing the name of a wakelock to wake_unlock causes that wakelock
    > to be released.
    >
    > Implement an analogous interface for user space using wakeup sources.
    > Add the /sys/power/wake_lock and /sys/power/wake_unlock files
    > allowing user space to create, activate and deactivate wakeup
    > sources, such that writing a name and optionally a timeout to
    > wake_lock causes the wakeup source of that name to be activated,
    > optionally with the given timeout. If that wakeup source doesn't
    > exist, it will be created and then activated. Writing a name to
    > wake_unlock causes the wakeup source of that name, if there is one,
    > to be deactivated. Wakeup sources created with the help of
    > wake_lock that haven't been used for more than 5 minutes are garbage
    > collected and destroyed. Moreover, there can be only WL_NUMBER_LIMIT
    > wakeup sources created with the help of wake_lock present at a time.
    >
    > The data type used to track wakeup sources created by user space is
    > called "struct wakelock" to indicate the origins of this feature.
    >
    > This version of the patch includes an rbtree manipulation fix from John Stultz.

    Looks good. Just a couple of minor suggestions.


    > +ssize_t pm_show_wakelocks(char *buf, bool show_active)
    > +{
    > + struct rb_node *node;
    > + struct wakelock *wl;
    > + char *str = buf;
    > + char *end = buf + PAGE_SIZE;
    > +
    > + mutex_lock(&wakelocks_lock);
    > +
    > + for (node = rb_first(&wakelocks_tree); node; node = rb_next(node)) {
    > + bool active;
    > +
    > + wl = rb_entry(node, struct wakelock, node);
    > + spin_lock_irq(&wl->ws.lock);
    > + active = wl->ws.active;
    > + spin_unlock_irq(&wl->ws.lock);

    I don't think the spin_lock is needed. We are just reading one value and it
    is either 0 or not. So there is no possibility for any inconsistency.
    if (wl->ws.active == show_active)
    ?

    > + if (active == show_active)
    > + str += scnprintf(str, end - str, "%s ", wl->name);

    Arg. Extra space on the end of the line!! :-)

    I would suggest the entries be terminated by '\n' rather than separate by
    space.
    one-item-per-line is much more common in Unix in general. 'grep' allows
    you to find things more easily etc.
    while read a
    do echo $a > wake_unlock
    done < wake_lock



    > + }
    > + str += scnprintf(str, end - str, "\n");
    > +
    > + mutex_unlock(&wakelocks_lock);
    > + return (str - buf);
    > +}
    > +


    Thanks,
    NeilBrown
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2012-04-26 08:35    [W:4.276 / U:0.568 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site