[PATCH v3 0/9] Don't reinvent the wheel but use existing llist API

[PATCH v3 0/9] Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Change from v2
- replace for_each(wake_list) with the safe version in scheduler.
- fix a trivial comment in llist.h

Change from v1
- split one patch to several ones, one for each subsystem.
- replace for_each with the safe version where it's necessary.

Byungchul Park (9):
  llist: Provide a safe version for llist_for_each
  bcache: Don't reinvent the wheel but use existing llist API
  raid5: Don't reinvent the wheel but use existing llist API
  vhost/scsi: Don't reinvent the wheel but use existing llist API
  fput: Don't reinvent the wheel but use existing llist API
  namespace.c: Don't reinvent the wheel but use existing llist API
  irq_work: Don't reinvent the wheel but use existing llist API
  sched: Don't reinvent the wheel but use existing llist API
  mm: Don't reinvent the wheel but use existing llist API

 drivers/md/bcache/closure.c | 17 +++--------------
 drivers/md/raid5.c          |  6 ++----
 drivers/vhost/scsi.c        | 11 +++--------
 fs/file_table.c             | 12 +++++-------
 fs/namespace.c              | 12 +++++-------
 include/linux/llist.h       | 19 +++++++++++++++++++
 kernel/irq_work.c           |  6 +-----
 kernel/sched/core.c         | 13 ++-----------
 mm/vmalloc.c                | 10 ++++------
 9 files changed, 44 insertions(+), 62 deletions(-)

-- 
1.9.1

*** BLURB HERE ***

Byungchul Park (9):
  llist: Provide a safe version for llist_for_each
  bcache: Don't reinvent the wheel but use existing llist API
  raid5: Don't reinvent the wheel but use existing llist API
  vhost/scsi: Don't reinvent the wheel but use existing llist API
  fput: Don't reinvent the wheel but use existing llist API
  namespace.c: Don't reinvent the wheel but use existing llist API
  irq_work: Don't reinvent the wheel but use existing llist API
  sched: Don't reinvent the wheel but use existing llist API
  mm: Don't reinvent the wheel but use existing llist API

 drivers/md/bcache/closure.c | 17 +++--------------
 drivers/md/raid5.c          |  6 ++----
 drivers/vhost/scsi.c        | 11 +++--------
 fs/file_table.c             | 12 +++++-------
 fs/namespace.c              | 12 +++++-------
 include/linux/llist.h       | 19 +++++++++++++++++++
 kernel/irq_work.c           |  6 +-----
 kernel/sched/core.c         | 15 +++------------
 mm/vmalloc.c                | 10 ++++------
 9 files changed, 45 insertions(+), 63 deletions(-)

-- 
1.9.1

[PATCH v3 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

Sometimes we have to dereference next field of llist node before entering
loop becasue the node might be deleted or the next field might be
modified within the loop. So this adds the safe version of llist_for_each,
that is, llist_for_each_safe.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 include/linux/llist.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/llist.h b/include/linux/llist.h
index fd4ca0b..b90c9f2 100644
--- a/include/linux/llist.h
+++ b/include/linux/llist.h
@@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head *list)
 	for ((pos) = (node); pos; (pos) = (pos)->next)
 
 /**
+ * llist_for_each_safe - iterate over some deleted entries of a lock-less list
+ *			 safe against removal of list entry
+ * @pos:	the &struct llist_node to use as a loop cursor
+ * @n:		another &struct llist_node to use as temporary storage
+ * @node:	the first entry of deleted list entries
+ *
+ * In general, some entries of the lock-less list can be traversed
+ * safely only after being deleted from list, so start with an entry
+ * instead of list head.
+ *
+ * If being used on entries deleted from lock-less list directly, the
+ * traverse order is from the newest to the oldest added entry.  If
+ * you want to traverse from the oldest to the newest, you must
+ * reverse the order by yourself before traversing.
+ */
+#define llist_for_each_safe(pos, n, node)			\
+	for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
+
+/**
  * llist_for_each_entry - iterate over some deleted entries of lock-less list of given type
  * @pos:	the type * to use as a loop cursor.
  * @node:	the fist entry of deleted list entries.
-- 
1.9.1

[PATCH v3 2/9] bcache: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Although llist provides proper APIs, they are not used. Make them used.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 drivers/md/bcache/closure.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
index 864e673..1841d03 100644
--- a/drivers/md/bcache/closure.c
+++ b/drivers/md/bcache/closure.c
@@ -64,27 +64,16 @@ void closure_put(struct closure *cl)
 void __closure_wake_up(struct closure_waitlist *wait_list)
 {
 	struct llist_node *list;
-	struct closure *cl;
+	struct closure *cl, *t;
 	struct llist_node *reverse = NULL;
 
 	list = llist_del_all(&wait_list->list);
 
 	/* We first reverse the list to preserve FIFO ordering and fairness */
-
-	while (list) {
-		struct llist_node *t = list;
-		list = llist_next(list);
-
-		t->next = reverse;
-		reverse = t;
-	}
+	reverse = llist_reverse_order(list);
 
 	/* Then do the wakeups */
-
-	while (reverse) {
-		cl = container_of(reverse, struct closure, list);
-		reverse = llist_next(reverse);
-
+	llist_for_each_entry_safe(cl, t, reverse, list) {
 		closure_set_waiting(cl, 0);
 		closure_sub(cl, CLOSURE_WAITING + 1);
 	}
-- 
1.9.1

[PATCH v3 3/9] raid5: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Although llist provides proper APIs, they are not used. Make them used.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 drivers/md/raid5.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 36c13e4..22a0326 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -353,17 +353,15 @@ static void release_inactive_stripe_list(struct r5conf *conf,
 static int release_stripe_list(struct r5conf *conf,
 			       struct list_head *temp_inactive_list)
 {
-	struct stripe_head *sh;
+	struct stripe_head *sh, *t;
 	int count = 0;
 	struct llist_node *head;
 
 	head = llist_del_all(&conf->released_stripes);
 	head = llist_reverse_order(head);
-	while (head) {
+	llist_for_each_entry_safe(sh, t, head, release_list) {
 		int hash;
 
-		sh = llist_entry(head, struct stripe_head, release_list);
-		head = llist_next(head);
 		/* sh could be readded after STRIPE_ON_RELEASE_LIST is cleard */
 		smp_mb();
 		clear_bit(STRIPE_ON_RELEASE_LIST, &sh->state);
-- 
1.9.1

[PATCH v3 4/9] vhost/scsi: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Although llist provides proper APIs, they are not used. Make them used.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 drivers/vhost/scsi.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 253310c..a4cb966 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -496,14 +496,12 @@ static void vhost_scsi_evt_work(struct vhost_work *work)
 	struct vhost_scsi *vs = container_of(work, struct vhost_scsi,
 					vs_event_work);
 	struct vhost_virtqueue *vq = &vs->vqs[VHOST_SCSI_VQ_EVT].vq;
-	struct vhost_scsi_evt *evt;
+	struct vhost_scsi_evt *evt, *t;
 	struct llist_node *llnode;
 
 	mutex_lock(&vq->mutex);
 	llnode = llist_del_all(&vs->vs_event_list);
-	while (llnode) {
-		evt = llist_entry(llnode, struct vhost_scsi_evt, list);
-		llnode = llist_next(llnode);
+	llist_for_each_entry_safe(evt, t, llnode, list) {
 		vhost_scsi_do_evt_work(vs, evt);
 		vhost_scsi_free_evt(vs, evt);
 	}
@@ -529,10 +527,7 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work)
 
 	bitmap_zero(signal, VHOST_SCSI_MAX_VQ);
 	llnode = llist_del_all(&vs->vs_completion_list);
-	while (llnode) {
-		cmd = llist_entry(llnode, struct vhost_scsi_cmd,
-				     tvc_completion_list);
-		llnode = llist_next(llnode);
+	llist_for_each_entry(cmd, llnode, tvc_completion_list) {
 		se_cmd = &cmd->tvc_se_cmd;
 
 		pr_debug("%s tv_cmd %p resid %u status %#02x\n", __func__,
-- 
1.9.1

[PATCH v3 5/9] fput: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Although llist provides proper APIs, they are not used. Make them used.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 fs/file_table.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index 6d982b5..3209da2 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -231,12 +231,10 @@ static void __fput(struct file *file)
 static void delayed_fput(struct work_struct *unused)
 {
 	struct llist_node *node = llist_del_all(&delayed_fput_list);
-	struct llist_node *next;
+	struct file *f, *t;
 
-	for (; node; node = next) {
-		next = llist_next(node);
-		__fput(llist_entry(node, struct file, f_u.fu_llist));
-	}
+	llist_for_each_entry_safe(f, t, node, f_u.fu_llist)
+		__fput(f);
 }
 
 static void ____fput(struct callback_head *work)
@@ -310,7 +308,7 @@ void put_filp(struct file *file)
 }
 
 void __init files_init(void)
-{ 
+{
 	filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0,
 			SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
 	percpu_counter_init(&nr_files, 0, GFP_KERNEL);
@@ -329,4 +327,4 @@ void __init files_maxfiles_init(void)
 	n = ((totalram_pages - memreserve) * (PAGE_SIZE / 1024)) / 10;
 
 	files_stat.max_files = max_t(unsigned long, n, NR_FILE);
-} 
+}
-- 
1.9.1

[PATCH v3 6/9] namespace.c: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Although llist provides proper APIs, they are not used. Make them used.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 fs/namespace.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index b5b1259..5cb2229 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1082,12 +1082,10 @@ static void __cleanup_mnt(struct rcu_head *head)
 static void delayed_mntput(struct work_struct *unused)
 {
 	struct llist_node *node = llist_del_all(&delayed_mntput_list);
-	struct llist_node *next;
+	struct mount *m, *t;
 
-	for (; node; node = next) {
-		next = llist_next(node);
-		cleanup_mnt(llist_entry(node, struct mount, mnt_llist));
-	}
+	llist_for_each_entry_safe(m, t, node, mnt_llist)
+		cleanup_mnt(m);
 }
 static DECLARE_DELAYED_WORK(delayed_mntput_work, delayed_mntput);
 
@@ -1615,7 +1613,7 @@ void __detach_mounts(struct dentry *dentry)
 	namespace_unlock();
 }
 
-/* 
+/*
  * Is the caller allowed to modify his namespace?
  */
 static inline bool may_mount(void)
@@ -2159,7 +2157,7 @@ static int do_loopback(struct path *path, const char *old_name,
 
 	err = -EINVAL;
 	if (mnt_ns_loop(old_path.dentry))
-		goto out; 
+		goto out;
 
 	mp = lock_mount(path);
 	err = PTR_ERR(mp);
-- 
1.9.1

[PATCH v3 7/9] irq_work: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Although llist provides proper APIs, they are not used. Make them used.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 kernel/irq_work.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index bcf107c..e2ebe8c 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -138,11 +138,7 @@ static void irq_work_run_list(struct llist_head *list)
 		return;
 
 	llnode = llist_del_all(list);
-	while (llnode != NULL) {
-		work = llist_entry(llnode, struct irq_work, llnode);
-
-		llnode = llist_next(llnode);
-
+	llist_for_each_entry(work, llnode, llnode) {
 		/*
 		 * Clear the PENDING bit, after this point the @work
 		 * can be re-used.
-- 
1.9.1

[PATCH v3 8/9] sched: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Although llist provides proper APIs, they are not used. Make them used.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 kernel/sched/core.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d01f9d0..8938125 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1773,7 +1773,7 @@ void sched_ttwu_pending(void)
 {
 	struct rq *rq = this_rq();
 	struct llist_node *llist = llist_del_all(&rq->wake_list);
-	struct task_struct *p;
+	struct task_struct *p, *t;
 	unsigned long flags;
 	struct rq_flags rf;
 
@@ -1783,17 +1783,8 @@ void sched_ttwu_pending(void)
 	raw_spin_lock_irqsave(&rq->lock, flags);
 	rq_pin_lock(rq, &rf);
 
-	while (llist) {
-		int wake_flags = 0;
-
-		p = llist_entry(llist, struct task_struct, wake_entry);
-		llist = llist_next(llist);
-
-		if (p->sched_remote_wakeup)
-			wake_flags = WF_MIGRATED;
-
-		ttwu_do_activate(rq, p, wake_flags, &rf);
-	}
+	llist_for_each_entry_safe(p, t, llist, wake_entry)
+		ttwu_do_activate(rq, p, p->sched_remote_wakeup ? WF_MIGRATED : 0, &rf);
 
 	rq_unpin_lock(rq, &rf);
 	raw_spin_unlock_irqrestore(&rq->lock, flags);
-- 
1.9.1

[PATCH v3 9/9] mm: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

Although llist provides proper APIs, they are not used. Make them used.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 mm/vmalloc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 3ca82d4..8c0eb45 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -49,12 +49,10 @@ struct vfree_deferred {
 static void free_work(struct work_struct *w)
 {
 	struct vfree_deferred *p = container_of(w, struct vfree_deferred, wq);
-	struct llist_node *llnode = llist_del_all(&p->list);
-	while (llnode) {
-		void *p = llnode;
-		llnode = llist_next(llnode);
-		__vunmap(p, 1);
-	}
+	struct llist_node *t, *llnode;
+
+	llist_for_each_safe(llnode, t, llist_del_all(&p->list))
+		__vunmap((void *)llnode, 1);
 }
 
 /*** Page table manipulation functions ***/
-- 
1.9.1

Re: [PATCH v3 1/9] llist: Provide a safe version for llist_for_each

[Huang, Ying]

Byungchul Park <byungchul.park@lge.com> writes:

> Sometimes we have to dereference next field of llist node before entering
> loop becasue the node might be deleted or the next field might be
> modified within the loop. So this adds the safe version of llist_for_each,
> that is, llist_for_each_safe.
>
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>

Reviewed-by: "Huang, Ying" <ying.huang@intel.com>

Best Regards,
Huang, Ying

> ---
>  include/linux/llist.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/llist.h b/include/linux/llist.h
> index fd4ca0b..b90c9f2 100644
> --- a/include/linux/llist.h
> +++ b/include/linux/llist.h
> @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head *list)
>  	for ((pos) = (node); pos; (pos) = (pos)->next)
>  
>  /**
> + * llist_for_each_safe - iterate over some deleted entries of a lock-less list
> + *			 safe against removal of list entry
> + * @pos:	the &struct llist_node to use as a loop cursor
> + * @n:		another &struct llist_node to use as temporary storage
> + * @node:	the first entry of deleted list entries
> + *
> + * In general, some entries of the lock-less list can be traversed
> + * safely only after being deleted from list, so start with an entry
> + * instead of list head.
> + *
> + * If being used on entries deleted from lock-less list directly, the
> + * traverse order is from the newest to the oldest added entry.  If
> + * you want to traverse from the oldest to the newest, you must
> + * reverse the order by yourself before traversing.
> + */
> +#define llist_for_each_safe(pos, n, node)			\
> +	for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
> +
> +/**
>   * llist_for_each_entry - iterate over some deleted entries of lock-less list of given type
>   * @pos:	the type * to use as a loop cursor.
>   * @node:	the fist entry of deleted list entries.

Re: [PATCH v3 5/9] fput: Don't reinvent the wheel but use existing llist API

[Oleg Nesterov]

On 02/14, Byungchul Park wrote:
>
> Although llist provides proper APIs, they are not used. Make them used.

Reviewed-by: Oleg Nesterov <oleg@redhat.com>

> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  fs/file_table.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 6d982b5..3209da2 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -231,12 +231,10 @@ static void __fput(struct file *file)
>  static void delayed_fput(struct work_struct *unused)
>  {
>  	struct llist_node *node = llist_del_all(&delayed_fput_list);
> -	struct llist_node *next;
> +	struct file *f, *t;
>  
> -	for (; node; node = next) {
> -		next = llist_next(node);
> -		__fput(llist_entry(node, struct file, f_u.fu_llist));
> -	}
> +	llist_for_each_entry_safe(f, t, node, f_u.fu_llist)
> +		__fput(f);
>  }
>  
>  static void ____fput(struct callback_head *work)
> @@ -310,7 +308,7 @@ void put_filp(struct file *file)
>  }
>  
>  void __init files_init(void)
> -{ 
> +{
>  	filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0,
>  			SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
>  	percpu_counter_init(&nr_files, 0, GFP_KERNEL);
> @@ -329,4 +327,4 @@ void __init files_maxfiles_init(void)
>  	n = ((totalram_pages - memreserve) * (PAGE_SIZE / 1024)) / 10;
>  
>  	files_stat.max_files = max_t(unsigned long, n, NR_FILE);
> -} 
> +}
> -- 
> 1.9.1
> 

Re: [PATCH v3 3/9] raid5: Don't reinvent the wheel but use existing llist API

[Shaohua Li]

On Tue, Feb 14, 2017 at 04:26:24PM +0900, Byungchul Park wrote:
> Although llist provides proper APIs, they are not used. Make them used.

This one has no dependence of previous patches, so I added to md tree.

Thanks,
Shaohua 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  drivers/md/raid5.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 36c13e4..22a0326 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -353,17 +353,15 @@ static void release_inactive_stripe_list(struct r5conf *conf,
>  static int release_stripe_list(struct r5conf *conf,
>  			       struct list_head *temp_inactive_list)
>  {
> -	struct stripe_head *sh;
> +	struct stripe_head *sh, *t;
>  	int count = 0;
>  	struct llist_node *head;
>  
>  	head = llist_del_all(&conf->released_stripes);
>  	head = llist_reverse_order(head);
> -	while (head) {
> +	llist_for_each_entry_safe(sh, t, head, release_list) {
>  		int hash;
>  
> -		sh = llist_entry(head, struct stripe_head, release_list);
> -		head = llist_next(head);
>  		/* sh could be readded after STRIPE_ON_RELEASE_LIST is cleard */
>  		smp_mb();
>  		clear_bit(STRIPE_ON_RELEASE_LIST, &sh->state);
> -- 
> 1.9.1
> 

Re: [PATCH v3 4/9] vhost/scsi: Don't reinvent the wheel but use existing llist API

[Nicholas A. Bellinger]

Hi Byungchul,

On Tue, 2017-02-14 at 16:26 +0900, Byungchul Park wrote:
> Although llist provides proper APIs, they are not used. Make them used.
> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  drivers/vhost/scsi.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)

Acked-by: Nicholas Bellinger <nab@linux-iscsi.org>

Re: [PATCH v3 8/9] sched: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

On Tue, Feb 14, 2017 at 04:26:29PM +0900, Byungchul Park wrote:
> Although llist provides proper APIs, they are not used. Make them used.

Hello,

What do you think?

> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  kernel/sched/core.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index d01f9d0..8938125 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1773,7 +1773,7 @@ void sched_ttwu_pending(void)
>  {
>  	struct rq *rq = this_rq();
>  	struct llist_node *llist = llist_del_all(&rq->wake_list);
> -	struct task_struct *p;
> +	struct task_struct *p, *t;
>  	unsigned long flags;
>  	struct rq_flags rf;
>  
> @@ -1783,17 +1783,8 @@ void sched_ttwu_pending(void)
>  	raw_spin_lock_irqsave(&rq->lock, flags);
>  	rq_pin_lock(rq, &rf);
>  
> -	while (llist) {
> -		int wake_flags = 0;
> -
> -		p = llist_entry(llist, struct task_struct, wake_entry);
> -		llist = llist_next(llist);
> -
> -		if (p->sched_remote_wakeup)
> -			wake_flags = WF_MIGRATED;
> -
> -		ttwu_do_activate(rq, p, wake_flags, &rf);
> -	}
> +	llist_for_each_entry_safe(p, t, llist, wake_entry)
> +		ttwu_do_activate(rq, p, p->sched_remote_wakeup ? WF_MIGRATED : 0, &rf);
>  
>  	rq_unpin_lock(rq, &rf);
>  	raw_spin_unlock_irqrestore(&rq->lock, flags);
> -- 
> 1.9.1

Re: [PATCH v3 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

On Tue, Feb 14, 2017 at 03:45:44PM +0800, Huang, Ying wrote:
> Byungchul Park <byungchul.park@lge.com> writes:
> 
> > Sometimes we have to dereference next field of llist node before entering
> > loop becasue the node might be deleted or the next field might be
> > modified within the loop. So this adds the safe version of llist_for_each,
> > that is, llist_for_each_safe.
> >
> > Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> 
> Reviewed-by: "Huang, Ying" <ying.huang@intel.com>

Hello, Ingo and Peterz

Does this have some problems? My 9th patch in this thread should be
considered with this together. So could you check this?

> 
> Best Regards,
> Huang, Ying
> 
> > ---
> >  include/linux/llist.h | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/include/linux/llist.h b/include/linux/llist.h
> > index fd4ca0b..b90c9f2 100644
> > --- a/include/linux/llist.h
> > +++ b/include/linux/llist.h
> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head *list)
> >  	for ((pos) = (node); pos; (pos) = (pos)->next)
> >  
> >  /**
> > + * llist_for_each_safe - iterate over some deleted entries of a lock-less list
> > + *			 safe against removal of list entry
> > + * @pos:	the &struct llist_node to use as a loop cursor
> > + * @n:		another &struct llist_node to use as temporary storage
> > + * @node:	the first entry of deleted list entries
> > + *
> > + * In general, some entries of the lock-less list can be traversed
> > + * safely only after being deleted from list, so start with an entry
> > + * instead of list head.
> > + *
> > + * If being used on entries deleted from lock-less list directly, the
> > + * traverse order is from the newest to the oldest added entry.  If
> > + * you want to traverse from the oldest to the newest, you must
> > + * reverse the order by yourself before traversing.
> > + */
> > +#define llist_for_each_safe(pos, n, node)			\
> > +	for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
> > +
> > +/**
> >   * llist_for_each_entry - iterate over some deleted entries of lock-less list of given type
> >   * @pos:	the type * to use as a loop cursor.
> >   * @node:	the fist entry of deleted list entries.

Re: [PATCH v3 5/9] fput: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

On Tue, Feb 14, 2017 at 06:44:44PM +0100, Oleg Nesterov wrote:
> On 02/14, Byungchul Park wrote:
> >
> > Although llist provides proper APIs, they are not used. Make them used.
> 
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>

Hello, Al Viro

Sorry for my mistake at the previous spin. I should have been more
careful. Do you think this still has some problems?

> 
> > Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> > ---
> >  fs/file_table.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/file_table.c b/fs/file_table.c
> > index 6d982b5..3209da2 100644
> > --- a/fs/file_table.c
> > +++ b/fs/file_table.c
> > @@ -231,12 +231,10 @@ static void __fput(struct file *file)
> >  static void delayed_fput(struct work_struct *unused)
> >  {
> >  	struct llist_node *node = llist_del_all(&delayed_fput_list);
> > -	struct llist_node *next;
> > +	struct file *f, *t;
> >  
> > -	for (; node; node = next) {
> > -		next = llist_next(node);
> > -		__fput(llist_entry(node, struct file, f_u.fu_llist));
> > -	}
> > +	llist_for_each_entry_safe(f, t, node, f_u.fu_llist)
> > +		__fput(f);
> >  }
> >  
> >  static void ____fput(struct callback_head *work)
> > @@ -310,7 +308,7 @@ void put_filp(struct file *file)
> >  }
> >  
> >  void __init files_init(void)
> > -{ 
> > +{
> >  	filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0,
> >  			SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
> >  	percpu_counter_init(&nr_files, 0, GFP_KERNEL);
> > @@ -329,4 +327,4 @@ void __init files_maxfiles_init(void)
> >  	n = ((totalram_pages - memreserve) * (PAGE_SIZE / 1024)) / 10;
> >  
> >  	files_stat.max_files = max_t(unsigned long, n, NR_FILE);
> > -} 
> > +}
> > -- 
> > 1.9.1
> > 

Re: [PATCH v3 2/9] bcache: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

On Tue, Feb 14, 2017 at 04:26:23PM +0900, Byungchul Park wrote:
> Although llist provides proper APIs, they are not used. Make them used.

+ koverstreet@google.com
+ jkosina@suse.cz

I am not sure who I should cc for this subsystem..
Could you check this patch?

Thanks,
Byungchul

> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  drivers/md/bcache/closure.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
> index 864e673..1841d03 100644
> --- a/drivers/md/bcache/closure.c
> +++ b/drivers/md/bcache/closure.c
> @@ -64,27 +64,16 @@ void closure_put(struct closure *cl)
>  void __closure_wake_up(struct closure_waitlist *wait_list)
>  {
>  	struct llist_node *list;
> -	struct closure *cl;
> +	struct closure *cl, *t;
>  	struct llist_node *reverse = NULL;
>  
>  	list = llist_del_all(&wait_list->list);
>  
>  	/* We first reverse the list to preserve FIFO ordering and fairness */
> -
> -	while (list) {
> -		struct llist_node *t = list;
> -		list = llist_next(list);
> -
> -		t->next = reverse;
> -		reverse = t;
> -	}
> +	reverse = llist_reverse_order(list);
>  
>  	/* Then do the wakeups */
> -
> -	while (reverse) {
> -		cl = container_of(reverse, struct closure, list);
> -		reverse = llist_next(reverse);
> -
> +	llist_for_each_entry_safe(cl, t, reverse, list) {
>  		closure_set_waiting(cl, 0);
>  		closure_sub(cl, CLOSURE_WAITING + 1);
>  	}
> -- 
> 1.9.1

Re: [PATCH v3 2/9] bcache: Don't reinvent the wheel but use existing llist API

[Jiri Kosina]

On Fri, 3 Mar 2017, Byungchul Park wrote:

> On Tue, Feb 14, 2017 at 04:26:23PM +0900, Byungchul Park wrote:
> > Although llist provides proper APIs, they are not used. Make them used.
> 
> + koverstreet@google.com
> + jkosina@suse.cz
> 
> I am not sure who I should cc for this subsystem..
> Could you check this patch?

I'd suggest to send it directly to Jens Axboe.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v3 2/9] bcache: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

On Sun, Mar 05, 2017 at 08:59:48PM +0100, Jiri Kosina wrote:
> On Fri, 3 Mar 2017, Byungchul Park wrote:
> 
> > On Tue, Feb 14, 2017 at 04:26:23PM +0900, Byungchul Park wrote:
> > > Although llist provides proper APIs, they are not used. Make them used.
> > 
> > + koverstreet@google.com
> > + jkosina@suse.cz
> > 
> > I am not sure who I should cc for this subsystem..
> > Could you check this patch?
> 
> I'd suggest to send it directly to Jens Axboe.

I'll try it. Thank you very much.

> 
> -- 
> Jiri Kosina
> SUSE Labs

Re: [PATCH v3 2/9] bcache: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

On Tue, Feb 14, 2017 at 04:26:23PM +0900, Byungchul Park wrote:
> Although llist provides proper APIs, they are not used. Make them used.

+ TO : Jens Axboe

What do you think about this patch?

Thanks,
Byungchul

> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  drivers/md/bcache/closure.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
> index 864e673..1841d03 100644
> --- a/drivers/md/bcache/closure.c
> +++ b/drivers/md/bcache/closure.c
> @@ -64,27 +64,16 @@ void closure_put(struct closure *cl)
>  void __closure_wake_up(struct closure_waitlist *wait_list)
>  {
>  	struct llist_node *list;
> -	struct closure *cl;
> +	struct closure *cl, *t;
>  	struct llist_node *reverse = NULL;
>  
>  	list = llist_del_all(&wait_list->list);
>  
>  	/* We first reverse the list to preserve FIFO ordering and fairness */
> -
> -	while (list) {
> -		struct llist_node *t = list;
> -		list = llist_next(list);
> -
> -		t->next = reverse;
> -		reverse = t;
> -	}
> +	reverse = llist_reverse_order(list);
>  
>  	/* Then do the wakeups */
> -
> -	while (reverse) {
> -		cl = container_of(reverse, struct closure, list);
> -		reverse = llist_next(reverse);
> -
> +	llist_for_each_entry_safe(cl, t, reverse, list) {
>  		closure_set_waiting(cl, 0);
>  		closure_sub(cl, CLOSURE_WAITING + 1);
>  	}
> -- 
> 1.9.1

Re: [PATCH v3 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

On Tue, Feb 14, 2017 at 03:45:44PM +0800, Huang, Ying wrote:
> Byungchul Park <byungchul.park@lge.com> writes:
> 
> > Sometimes we have to dereference next field of llist node before entering
> > loop becasue the node might be deleted or the next field might be
> > modified within the loop. So this adds the safe version of llist_for_each,
> > that is, llist_for_each_safe.
> >
> > Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> 
> Reviewed-by: "Huang, Ying" <ying.huang@intel.com>

Hello,

Could you check this out?

> 
> Best Regards,
> Huang, Ying
> 
> > ---
> >  include/linux/llist.h | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/include/linux/llist.h b/include/linux/llist.h
> > index fd4ca0b..b90c9f2 100644
> > --- a/include/linux/llist.h
> > +++ b/include/linux/llist.h
> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head *list)
> >  	for ((pos) = (node); pos; (pos) = (pos)->next)
> >  
> >  /**
> > + * llist_for_each_safe - iterate over some deleted entries of a lock-less list
> > + *			 safe against removal of list entry
> > + * @pos:	the &struct llist_node to use as a loop cursor
> > + * @n:		another &struct llist_node to use as temporary storage
> > + * @node:	the first entry of deleted list entries
> > + *
> > + * In general, some entries of the lock-less list can be traversed
> > + * safely only after being deleted from list, so start with an entry
> > + * instead of list head.
> > + *
> > + * If being used on entries deleted from lock-less list directly, the
> > + * traverse order is from the newest to the oldest added entry.  If
> > + * you want to traverse from the oldest to the newest, you must
> > + * reverse the order by yourself before traversing.
> > + */
> > +#define llist_for_each_safe(pos, n, node)			\
> > +	for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
> > +
> > +/**
> >   * llist_for_each_entry - iterate over some deleted entries of lock-less list of given type
> >   * @pos:	the type * to use as a loop cursor.
> >   * @node:	the fist entry of deleted list entries.

Re: [PATCH v3 2/9] bcache: Don't reinvent the wheel but use existing llist API

[Byungchul Park]

On Tue, Feb 14, 2017 at 04:26:23PM +0900, Byungchul Park wrote:
> Although llist provides proper APIs, they are not used. Make them used.

+cc axboe@fb.com

> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  drivers/md/bcache/closure.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
> index 864e673..1841d03 100644
> --- a/drivers/md/bcache/closure.c
> +++ b/drivers/md/bcache/closure.c
> @@ -64,27 +64,16 @@ void closure_put(struct closure *cl)
>  void __closure_wake_up(struct closure_waitlist *wait_list)
>  {
>  	struct llist_node *list;
> -	struct closure *cl;
> +	struct closure *cl, *t;
>  	struct llist_node *reverse = NULL;
>  
>  	list = llist_del_all(&wait_list->list);
>  
>  	/* We first reverse the list to preserve FIFO ordering and fairness */
> -
> -	while (list) {
> -		struct llist_node *t = list;
> -		list = llist_next(list);
> -
> -		t->next = reverse;
> -		reverse = t;
> -	}
> +	reverse = llist_reverse_order(list);
>  
>  	/* Then do the wakeups */
> -
> -	while (reverse) {
> -		cl = container_of(reverse, struct closure, list);
> -		reverse = llist_next(reverse);
> -
> +	llist_for_each_entry_safe(cl, t, reverse, list) {
>  		closure_set_waiting(cl, 0);
>  		closure_sub(cl, CLOSURE_WAITING + 1);
>  	}
> -- 
> 1.9.1

Re: [PATCH v3 0/9] Don't reinvent the wheel but use existing llist API

[Byungchul Park]

On Tue, Feb 14, 2017 at 04:26:21PM +0900, Byungchul Park wrote:

Hello,

I did what you suggested. Is there anything I shoud do more?
If so, please let know.

Thank you,
Byungchul

> Change from v2
> - replace for_each(wake_list) with the safe version in scheduler.
> - fix a trivial comment in llist.h
> 
> Change from v1
> - split one patch to several ones, one for each subsystem.
> - replace for_each with the safe version where it's necessary.
> 
> Byungchul Park (9):
>   llist: Provide a safe version for llist_for_each
>   bcache: Don't reinvent the wheel but use existing llist API
>   raid5: Don't reinvent the wheel but use existing llist API
>   vhost/scsi: Don't reinvent the wheel but use existing llist API
>   fput: Don't reinvent the wheel but use existing llist API
>   namespace.c: Don't reinvent the wheel but use existing llist API
>   irq_work: Don't reinvent the wheel but use existing llist API
>   sched: Don't reinvent the wheel but use existing llist API
>   mm: Don't reinvent the wheel but use existing llist API
> 
>  drivers/md/bcache/closure.c | 17 +++--------------
>  drivers/md/raid5.c          |  6 ++----
>  drivers/vhost/scsi.c        | 11 +++--------
>  fs/file_table.c             | 12 +++++-------
>  fs/namespace.c              | 12 +++++-------
>  include/linux/llist.h       | 19 +++++++++++++++++++
>  kernel/irq_work.c           |  6 +-----
>  kernel/sched/core.c         | 13 ++-----------
>  mm/vmalloc.c                | 10 ++++------
>  9 files changed, 44 insertions(+), 62 deletions(-)
> 
> -- 
> 1.9.1
> 
> *** BLURB HERE ***
> 
> Byungchul Park (9):
>   llist: Provide a safe version for llist_for_each
>   bcache: Don't reinvent the wheel but use existing llist API
>   raid5: Don't reinvent the wheel but use existing llist API
>   vhost/scsi: Don't reinvent the wheel but use existing llist API
>   fput: Don't reinvent the wheel but use existing llist API
>   namespace.c: Don't reinvent the wheel but use existing llist API
>   irq_work: Don't reinvent the wheel but use existing llist API
>   sched: Don't reinvent the wheel but use existing llist API
>   mm: Don't reinvent the wheel but use existing llist API
> 
>  drivers/md/bcache/closure.c | 17 +++--------------
>  drivers/md/raid5.c          |  6 ++----
>  drivers/vhost/scsi.c        | 11 +++--------
>  fs/file_table.c             | 12 +++++-------
>  fs/namespace.c              | 12 +++++-------
>  include/linux/llist.h       | 19 +++++++++++++++++++
>  kernel/irq_work.c           |  6 +-----
>  kernel/sched/core.c         | 15 +++------------
>  mm/vmalloc.c                | 10 ++++------
>  9 files changed, 45 insertions(+), 63 deletions(-)
> 
> -- 
> 1.9.1