On Sun 14-02-16 19:15:23, Jeff Layton wrote:
> We don't require a dedicated thread for fsnotify cleanup. Switch it over
> to a workqueue job instead that runs on the system_unbound_wq.
>
> In the interest of not thrashing the queued job too often when there are
> a lot of marks being removed, we delay the reaper job slightly when
> queueing it, to allow several to gather on the list.
>
> Cc: Jan Kara <jack@xxxxxxxx>
> Cc: Eric Paris <eparis@xxxxxxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Eryu Guan <guaneryu@xxxxxxxxx>
> Signed-off-by: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
The patch looks correct to me. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/notify/mark.c | 49 ++++++++++++++++++-------------------------------
> 1 file changed, 18 insertions(+), 31 deletions(-)
>
> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index fc0df4442f7b..7115c5d7d373 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -91,10 +91,14 @@
> #include <linux/fsnotify_backend.h>
> #include "fsnotify.h"
>
> +#define FSNOTIFY_REAPER_DELAY (1) /* 1 jiffy */
> +
> struct srcu_struct fsnotify_mark_srcu;
> static DEFINE_SPINLOCK(destroy_lock);
> static LIST_HEAD(destroy_list);
> -static DECLARE_WAIT_QUEUE_HEAD(destroy_waitq);
> +
> +static void fsnotify_mark_destroy(struct work_struct *work);
> +static DECLARE_DELAYED_WORK(reaper_work, fsnotify_mark_destroy);
>
> void fsnotify_get_mark(struct fsnotify_mark *mark)
> {
> @@ -189,7 +193,8 @@ void fsnotify_free_mark(struct fsnotify_mark *mark)
> spin_lock(&destroy_lock);
> list_add(&mark->g_list, &destroy_list);
> spin_unlock(&destroy_lock);
> - wake_up(&destroy_waitq);
> + queue_delayed_work(system_unbound_wq, &reaper_work,
> + FSNOTIFY_REAPER_DELAY);
>
> /*
> * Some groups like to know that marks are being freed. This is a
> @@ -388,7 +393,8 @@ err:
> spin_lock(&destroy_lock);
> list_add(&mark->g_list, &destroy_list);
> spin_unlock(&destroy_lock);
> - wake_up(&destroy_waitq);
> + queue_delayed_work(system_unbound_wq, &reaper_work,
> + FSNOTIFY_REAPER_DELAY);
>
> return ret;
> }
> @@ -493,39 +499,20 @@ void fsnotify_init_mark(struct fsnotify_mark *mark,
> mark->free_mark = free_mark;
> }
>
> -static int fsnotify_mark_destroy(void *ignored)
> +static void fsnotify_mark_destroy(struct work_struct *work)
> {
> struct fsnotify_mark *mark, *next;
> struct list_head private_destroy_list;
>
> - for (;;) {
> - spin_lock(&destroy_lock);
> - /* exchange the list head */
> - list_replace_init(&destroy_list, &private_destroy_list);
> - spin_unlock(&destroy_lock);
> -
> - synchronize_srcu(&fsnotify_mark_srcu);
> + spin_lock(&destroy_lock);
> + /* exchange the list head */
> + list_replace_init(&destroy_list, &private_destroy_list);
> + spin_unlock(&destroy_lock);
>
> - list_for_each_entry_safe(mark, next, &private_destroy_list, g_list) {
> - list_del_init(&mark->g_list);
> - fsnotify_put_mark(mark);
> - }
> + synchronize_srcu(&fsnotify_mark_srcu);
>
> - wait_event_interruptible(destroy_waitq, !list_empty(&destroy_list));
> + list_for_each_entry_safe(mark, next, &private_destroy_list, g_list) {
> + list_del_init(&mark->g_list);
> + fsnotify_put_mark(mark);
> }
> -
> - return 0;
> -}
> -
> -static int __init fsnotify_mark_init(void)
> -{
> - struct task_struct *thread;
> -
> - thread = kthread_run(fsnotify_mark_destroy, NULL,
> - "fsnotify_mark");
> - if (IS_ERR(thread))
> - panic("unable to start fsnotify mark destruction thread.");
> -
> - return 0;
> }
> -device_initcall(fsnotify_mark_init);
> --
> 2.5.0
>
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
