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: <CAJKOXPcENx5_uHQTzEnUg6nJgpeqqR8+Ra7JxFaB-Jk_b9vF2g@mail.gmail.com>
Date:	Wed, 1 Jul 2015 12:51:47 +0900
From:	Krzysztof Kozlowski <k.kozlowski@...sung.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. 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/