Re: [PATCH 7/9] fsnotify: use existed call_srcu()
[Ping] Hi, Eric Paris Could you review this patch? Thanks, Lai On 03/16/2013 12:50 AM, Lai Jiangshan wrote: > fsnotify implements its own call_srcu() by: > dedicated thread + synchronize_srcu() > > But srcu provides call_srcu() now, so we should convert them to use > existed call_srcu() and remove the thread. > > Signed-off-by: Lai Jiangshan > Cc: Eric Paris > --- > fs/notify/mark.c | 59 ++--- > include/linux/fsnotify_backend.h |2 +- > 2 files changed, 11 insertions(+), 50 deletions(-) > > diff --git a/fs/notify/mark.c b/fs/notify/mark.c > index aeededc..af5f0e1 100644 > --- a/fs/notify/mark.c > +++ b/fs/notify/mark.c > @@ -98,9 +98,6 @@ > #include "fsnotify.h" > > DEFINE_SRCU(fsnotify_mark_srcu); > -static DEFINE_SPINLOCK(destroy_lock); > -static LIST_HEAD(destroy_list); > -static DECLARE_WAIT_QUEUE_HEAD(destroy_waitq); > > void fsnotify_get_mark(struct fsnotify_mark *mark) > { > @@ -116,6 +113,14 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) > } > } > > +static void fsnotify_destroy_mark_rcu(struct rcu_head *rcu) > +{ > + struct fsnotify_mark *mark; > + > + mark = container_of(rcu, struct fsnotify_mark, rcu); > + fsnotify_put_mark(mark); > +} > + > /* > * Any time a mark is getting freed we end up here. > * The caller had better be holding a reference to this mark so we don't > actually > @@ -155,10 +160,7 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark > *mark, > /* release lock temporarily */ > mutex_unlock(>mark_mutex); > > - spin_lock(_lock); > - list_add(>destroy_list, _list); > - spin_unlock(_lock); > - wake_up(_waitq); > + call_srcu(_mark_srcu, >rcu, fsnotify_destroy_mark_rcu); > /* >* We don't necessarily have a ref on mark from caller so the above > destroy >* may have actually freed it, unless this group provides a > 'freeing_mark' > @@ -273,11 +275,7 @@ err: > atomic_dec(>num_marks); > > spin_unlock(>lock); > - > - spin_lock(_lock); > - list_add(>destroy_list, _list); > - spin_unlock(_lock); > - wake_up(_waitq); > + call_srcu(_mark_srcu, >rcu, fsnotify_destroy_mark_rcu); > > return ret; > } > @@ -342,40 +340,3 @@ void fsnotify_init_mark(struct fsnotify_mark *mark, > atomic_set(>refcnt, 1); > mark->free_mark = free_mark; > } > - > -static int fsnotify_mark_destroy(void *ignored) > -{ > - struct fsnotify_mark *mark, *next; > - LIST_HEAD(private_destroy_list); > - > - for (;;) { > - spin_lock(_lock); > - /* exchange the list head */ > - list_replace_init(_list, _destroy_list); > - spin_unlock(_lock); > - > - synchronize_srcu(_mark_srcu); > - > - list_for_each_entry_safe(mark, next, _destroy_list, > destroy_list) { > - list_del_init(>destroy_list); > - fsnotify_put_mark(mark); > - } > - > - wait_event_interruptible(destroy_waitq, > !list_empty(_list)); > - } > - > - 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); > diff --git a/include/linux/fsnotify_backend.h > b/include/linux/fsnotify_backend.h > index d5b0910..3d435eb 100644 > --- a/include/linux/fsnotify_backend.h > +++ b/include/linux/fsnotify_backend.h > @@ -296,7 +296,7 @@ struct fsnotify_mark { > #define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x08 > #define FSNOTIFY_MARK_FLAG_ALIVE 0x10 > unsigned int flags; /* vfsmount or inode mark? */ > - struct list_head destroy_list; > + struct rcu_head rcu; > void (*free_mark)(struct fsnotify_mark *mark); /* called on final > put+free */ > }; > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/9] fsnotify: use existed call_srcu()
[Ping] Hi, Eric Paris Could you review this patch? Thanks, Lai On 03/16/2013 12:50 AM, Lai Jiangshan wrote: fsnotify implements its own call_srcu() by: dedicated thread + synchronize_srcu() But srcu provides call_srcu() now, so we should convert them to use existed call_srcu() and remove the thread. Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com Cc: Eric Paris epa...@parisplace.org --- fs/notify/mark.c | 59 ++--- include/linux/fsnotify_backend.h |2 +- 2 files changed, 11 insertions(+), 50 deletions(-) diff --git a/fs/notify/mark.c b/fs/notify/mark.c index aeededc..af5f0e1 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -98,9 +98,6 @@ #include fsnotify.h DEFINE_SRCU(fsnotify_mark_srcu); -static DEFINE_SPINLOCK(destroy_lock); -static LIST_HEAD(destroy_list); -static DECLARE_WAIT_QUEUE_HEAD(destroy_waitq); void fsnotify_get_mark(struct fsnotify_mark *mark) { @@ -116,6 +113,14 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) } } +static void fsnotify_destroy_mark_rcu(struct rcu_head *rcu) +{ + struct fsnotify_mark *mark; + + mark = container_of(rcu, struct fsnotify_mark, rcu); + fsnotify_put_mark(mark); +} + /* * Any time a mark is getting freed we end up here. * The caller had better be holding a reference to this mark so we don't actually @@ -155,10 +160,7 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, /* release lock temporarily */ mutex_unlock(group-mark_mutex); - spin_lock(destroy_lock); - list_add(mark-destroy_list, destroy_list); - spin_unlock(destroy_lock); - wake_up(destroy_waitq); + call_srcu(fsnotify_mark_srcu, mark-rcu, fsnotify_destroy_mark_rcu); /* * We don't necessarily have a ref on mark from caller so the above destroy * may have actually freed it, unless this group provides a 'freeing_mark' @@ -273,11 +275,7 @@ err: atomic_dec(group-num_marks); spin_unlock(mark-lock); - - spin_lock(destroy_lock); - list_add(mark-destroy_list, destroy_list); - spin_unlock(destroy_lock); - wake_up(destroy_waitq); + call_srcu(fsnotify_mark_srcu, mark-rcu, fsnotify_destroy_mark_rcu); return ret; } @@ -342,40 +340,3 @@ void fsnotify_init_mark(struct fsnotify_mark *mark, atomic_set(mark-refcnt, 1); mark-free_mark = free_mark; } - -static int fsnotify_mark_destroy(void *ignored) -{ - struct fsnotify_mark *mark, *next; - 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); - - list_for_each_entry_safe(mark, next, private_destroy_list, destroy_list) { - list_del_init(mark-destroy_list); - fsnotify_put_mark(mark); - } - - wait_event_interruptible(destroy_waitq, !list_empty(destroy_list)); - } - - 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); diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index d5b0910..3d435eb 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -296,7 +296,7 @@ struct fsnotify_mark { #define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x08 #define FSNOTIFY_MARK_FLAG_ALIVE 0x10 unsigned int flags; /* vfsmount or inode mark? */ - struct list_head destroy_list; + struct rcu_head rcu; void (*free_mark)(struct fsnotify_mark *mark); /* called on final put+free */ }; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/9] fsnotify: use existed call_srcu()
fsnotify implements its own call_srcu() by: dedicated thread + synchronize_srcu() But srcu provides call_srcu() now, so we should convert them to use existed call_srcu() and remove the thread. Signed-off-by: Lai Jiangshan Cc: Eric Paris --- fs/notify/mark.c | 59 ++--- include/linux/fsnotify_backend.h |2 +- 2 files changed, 11 insertions(+), 50 deletions(-) diff --git a/fs/notify/mark.c b/fs/notify/mark.c index aeededc..af5f0e1 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -98,9 +98,6 @@ #include "fsnotify.h" DEFINE_SRCU(fsnotify_mark_srcu); -static DEFINE_SPINLOCK(destroy_lock); -static LIST_HEAD(destroy_list); -static DECLARE_WAIT_QUEUE_HEAD(destroy_waitq); void fsnotify_get_mark(struct fsnotify_mark *mark) { @@ -116,6 +113,14 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) } } +static void fsnotify_destroy_mark_rcu(struct rcu_head *rcu) +{ + struct fsnotify_mark *mark; + + mark = container_of(rcu, struct fsnotify_mark, rcu); + fsnotify_put_mark(mark); +} + /* * Any time a mark is getting freed we end up here. * The caller had better be holding a reference to this mark so we don't actually @@ -155,10 +160,7 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, /* release lock temporarily */ mutex_unlock(>mark_mutex); - spin_lock(_lock); - list_add(>destroy_list, _list); - spin_unlock(_lock); - wake_up(_waitq); + call_srcu(_mark_srcu, >rcu, fsnotify_destroy_mark_rcu); /* * We don't necessarily have a ref on mark from caller so the above destroy * may have actually freed it, unless this group provides a 'freeing_mark' @@ -273,11 +275,7 @@ err: atomic_dec(>num_marks); spin_unlock(>lock); - - spin_lock(_lock); - list_add(>destroy_list, _list); - spin_unlock(_lock); - wake_up(_waitq); + call_srcu(_mark_srcu, >rcu, fsnotify_destroy_mark_rcu); return ret; } @@ -342,40 +340,3 @@ void fsnotify_init_mark(struct fsnotify_mark *mark, atomic_set(>refcnt, 1); mark->free_mark = free_mark; } - -static int fsnotify_mark_destroy(void *ignored) -{ - struct fsnotify_mark *mark, *next; - LIST_HEAD(private_destroy_list); - - for (;;) { - spin_lock(_lock); - /* exchange the list head */ - list_replace_init(_list, _destroy_list); - spin_unlock(_lock); - - synchronize_srcu(_mark_srcu); - - list_for_each_entry_safe(mark, next, _destroy_list, destroy_list) { - list_del_init(>destroy_list); - fsnotify_put_mark(mark); - } - - wait_event_interruptible(destroy_waitq, !list_empty(_list)); - } - - 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); diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index d5b0910..3d435eb 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -296,7 +296,7 @@ struct fsnotify_mark { #define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x08 #define FSNOTIFY_MARK_FLAG_ALIVE 0x10 unsigned int flags; /* vfsmount or inode mark? */ - struct list_head destroy_list; + struct rcu_head rcu; void (*free_mark)(struct fsnotify_mark *mark); /* called on final put+free */ }; -- 1.7.4.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/9] fsnotify: use existed call_srcu()
fsnotify implements its own call_srcu() by: dedicated thread + synchronize_srcu() But srcu provides call_srcu() now, so we should convert them to use existed call_srcu() and remove the thread. Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com Cc: Eric Paris epa...@parisplace.org --- fs/notify/mark.c | 59 ++--- include/linux/fsnotify_backend.h |2 +- 2 files changed, 11 insertions(+), 50 deletions(-) diff --git a/fs/notify/mark.c b/fs/notify/mark.c index aeededc..af5f0e1 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -98,9 +98,6 @@ #include fsnotify.h DEFINE_SRCU(fsnotify_mark_srcu); -static DEFINE_SPINLOCK(destroy_lock); -static LIST_HEAD(destroy_list); -static DECLARE_WAIT_QUEUE_HEAD(destroy_waitq); void fsnotify_get_mark(struct fsnotify_mark *mark) { @@ -116,6 +113,14 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) } } +static void fsnotify_destroy_mark_rcu(struct rcu_head *rcu) +{ + struct fsnotify_mark *mark; + + mark = container_of(rcu, struct fsnotify_mark, rcu); + fsnotify_put_mark(mark); +} + /* * Any time a mark is getting freed we end up here. * The caller had better be holding a reference to this mark so we don't actually @@ -155,10 +160,7 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, /* release lock temporarily */ mutex_unlock(group-mark_mutex); - spin_lock(destroy_lock); - list_add(mark-destroy_list, destroy_list); - spin_unlock(destroy_lock); - wake_up(destroy_waitq); + call_srcu(fsnotify_mark_srcu, mark-rcu, fsnotify_destroy_mark_rcu); /* * We don't necessarily have a ref on mark from caller so the above destroy * may have actually freed it, unless this group provides a 'freeing_mark' @@ -273,11 +275,7 @@ err: atomic_dec(group-num_marks); spin_unlock(mark-lock); - - spin_lock(destroy_lock); - list_add(mark-destroy_list, destroy_list); - spin_unlock(destroy_lock); - wake_up(destroy_waitq); + call_srcu(fsnotify_mark_srcu, mark-rcu, fsnotify_destroy_mark_rcu); return ret; } @@ -342,40 +340,3 @@ void fsnotify_init_mark(struct fsnotify_mark *mark, atomic_set(mark-refcnt, 1); mark-free_mark = free_mark; } - -static int fsnotify_mark_destroy(void *ignored) -{ - struct fsnotify_mark *mark, *next; - 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); - - list_for_each_entry_safe(mark, next, private_destroy_list, destroy_list) { - list_del_init(mark-destroy_list); - fsnotify_put_mark(mark); - } - - wait_event_interruptible(destroy_waitq, !list_empty(destroy_list)); - } - - 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); diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index d5b0910..3d435eb 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -296,7 +296,7 @@ struct fsnotify_mark { #define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x08 #define FSNOTIFY_MARK_FLAG_ALIVE 0x10 unsigned int flags; /* vfsmount or inode mark? */ - struct list_head destroy_list; + struct rcu_head rcu; void (*free_mark)(struct fsnotify_mark *mark); /* called on final put+free */ }; -- 1.7.4.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/