linux-kernel - Re: [PATCH] PM / Sleep: Use workqueue for user space wakeup sources garbage collector

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: <CAJKOXPfjeDj42422ZpsacJjEMVS77DwpKze5tz1XbCzVd0OoMQ@mail.gmail.com>
Date:	Wed, 1 Jul 2015 12:54:23 +0900
From:	Krzysztof Kozłowski <k.kozlowski.k@...il.com>
To:	SungEun Kim <cleaneye.kim@....com>
Cc:	rjw@...ysocki.net, pavel@....cz, len.brown@...el.com,
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PM / Sleep: Use workqueue for user space wakeup sources
 garbage collector

2015-07-01 11:48 GMT+09:00 SungEun Kim <cleaneye.kim@....com>:
> From: "cleaeye.kim" <cleaneye.kim@....com>
>
> The synchronous synchronize_rcu in wakeup_source_remove makes user process
> which writes to /sys/kernel/wake_unlock blocked sometimes.
>
> For example, when android eventhub tries to release wakelock,
> this blocking process can occur, and eventhub can't get input event
> for a while.
>
> Using workqueue instead of direct function call at pm_wake_unlock
> can prevent this unnecessary delay of an user space process.
>
> Signed-off-by: cleaeye.kim <cleaneye.kim@....com>

Hi,

You send this patch for third time, without changelog and any
versioning. Please put proper vesion indicating what have changed. Do
not resend continuously (unless you encountered some SMTP problems?).

The signed-off and from fields look incorrect (no real name?). What is
more important I have doubts that it even compiles (see below).

Could you follow the Documentation/SubmittingPatches?


> ---
>  kernel/power/wakelock.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
> index 019069c..ea10baa 100644
> --- a/kernel/power/wakelock.c
> +++ b/kernel/power/wakelock.c
> @@ -17,6 +17,7 @@
>  #include <linux/list.h>
>  #include <linux/rbtree.h>
>  #include <linux/slab.h>
> +#include <linux/workqueue.h>
>
>  #include "power.h"
>
> @@ -96,7 +97,7 @@ static inline void wakelocks_lru_most_recent(struct wakelock *wl)
>         list_move(&wl->lru, &wakelocks_lru_list);
>  }
>
> -static void wakelocks_gc(void)
> +static void wakelocks_gc(struct work_struct *work)
>  {
>         struct wakelock *wl, *aux;
>         ktime_t now;
> @@ -105,6 +106,7 @@ static void wakelocks_gc(void)
>                 return;
>
>         now = ktime_get();
> +       mutex_lock(&wakelocks_lock);
>         list_for_each_entry_safe_reverse(wl, aux, &wakelocks_lru_list, lru) {
>                 u64 idle_time_ns;
>                 bool active;
> @@ -126,12 +128,15 @@ static void wakelocks_gc(void)
>                         decrement_wakelocks_number();
>                 }
>         }
> +       mutex_unlock(&wakelocks_lock);
>         wakelocks_gc_count = 0;
>  }
> +
> +static DECLARE_WORK(wakelock_work, wakelocks_gc);
>  #else /* !CONFIG_PM_WAKELOCKS_GC */
>  static inline void wakelocks_lru_add(struct wakelock *wl) {}
>  static inline void wakelocks_lru_most_recent(struct wakelock *wl) {}
> -static inline void wakelocks_gc(void) {}
> +static void wakelocks_gc(struct worksturct) {}

worksturct? Does it compile?

Best regards,
Krzysztof
--
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/