* Re: [PATCH] sched: Enhance readability of iterating wake_list
2017-02-10 7:55 ` Peter Zijlstra
2017-02-10 9:55 ` Byungchul Park
@ 2017-02-10 10:49 ` Byungchul Park
1 sibling, 0 replies; 4+ messages in thread
From: Byungchul Park @ 2017-02-10 10:49 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: mingo, linux-kernel
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 ***/
^ permalink raw reply [flat|nested] 4+ messages in thread