From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 17 Feb 2016 21:01:36 +0100 From: Jan Kara To: Jeff Layton Cc: Andrew Morton , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, "Paul E. McKenney" , Jan Kara , Eric Paris , Eryu Guan Subject: Re: [PATCH 2/2] fsnotify: turn fsnotify reaper thread into a workqueue job Message-ID: <20160217200136.GB14140@quack.suse.cz> References: <1455495323-29605-1-git-send-email-jeff.layton@primarydata.com> <1455495323-29605-2-git-send-email-jeff.layton@primarydata.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1455495323-29605-2-git-send-email-jeff.layton@primarydata.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: 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 > Cc: Eric Paris > Cc: Andrew Morton > Cc: Eryu Guan > Signed-off-by: Jeff Layton The patch looks correct to me. You can add: Reviewed-by: Jan Kara 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 > #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 SUSE Labs, CR