On Fri, Feb 10, 2017 at 08:55:23AM +0100, Peter Zijlstra wrote: > On Fri, Feb 10, 2017 at 01:09:31PM +0900, Byungchul Park wrote: > > +#define for_each_wake_list(task, node) \ > > + for ((task) = llist_entry((node), struct task_struct, wake_entry); \ > > + node; (node) = llist_next(node), \ > > + (task) = llist_entry((node), struct task_struct, wake_entry)) > > + > > How about you make that llist_for_each(pos, member) or similar and > replace all while (foo) { foo = llist_next(foo); } instances? > > Because most llist_next() users have the same pattern.
I did what you recommand, like the following. Would it be better if I split the patch into several ones? Or keep it one patch? -----8<----- diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c index 864e673..7d5286b 100644 --- a/drivers/md/bcache/closure.c +++ b/drivers/md/bcache/closure.c @@ -70,21 +70,10 @@ void __closure_wake_up(struct closure_waitlist *wait_list) 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(cl, reverse, list) { closure_set_waiting(cl, 0); closure_sub(cl, CLOSURE_WAITING + 1); } diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index 36c13e4..c82243a 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -359,11 +359,9 @@ static int release_stripe_list(struct r5conf *conf, head = llist_del_all(&conf->released_stripes); head = llist_reverse_order(head); - while (head) { + llist_for_each_entry(sh, 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); diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index 253310c..280b912 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -501,9 +501,7 @@ static void vhost_scsi_evt_work(struct vhost_work *work) 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(evt, 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__, diff --git a/fs/file_table.c b/fs/file_table.c index 6d982b5..8800153 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -232,11 +232,10 @@ 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; - for (; node; node = next) { - next = llist_next(node); - __fput(llist_entry(node, struct file, f_u.fu_llist)); - } + llist_for_each_entry(f, node, f_u.fu_llist) + __fput(f); } static void ____fput(struct callback_head *work) @@ -310,7 +309,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 +328,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); -} +} diff --git a/fs/namespace.c b/fs/namespace.c index b5b1259..0caf954 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1083,11 +1083,10 @@ 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; - for (; node; node = next) { - next = llist_next(node); - cleanup_mnt(llist_entry(node, struct mount, mnt_llist)); - } + llist_for_each_entry(m, node, mnt_llist) + cleanup_mnt(m); } static DECLARE_DELAYED_WORK(delayed_mntput_work, delayed_mntput); @@ -1615,7 +1614,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 +2158,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); diff --git a/include/linux/llist.h b/include/linux/llist.h index fd4ca0b..418f566 100644 --- a/include/linux/llist.h +++ b/include/linux/llist.h @@ -160,11 +160,6 @@ static inline bool llist_empty(const struct llist_head *head) return ACCESS_ONCE(head->first) == NULL; } -static inline struct llist_node *llist_next(struct llist_node *node) -{ - return node->next; -} - extern bool llist_add_batch(struct llist_node *new_first, struct llist_node *new_last, struct llist_head *head); 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. diff --git a/kernel/sched/core.c b/kernel/sched/core.c index d01f9d0..417060b 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -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(p, 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); diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 3ca82d4..829de9e 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -50,11 +50,9 @@ 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); - } + + llist_for_each(llnode, llnode) + __vunmap((void *)llnode, 1); } /*** Page table manipulation functions ***/