[PATCHv2 01/13] list: Use argument hlist_add_after names from rcu variant

[PATCHv2 01/13] list: Use argument hlist_add_after names from rcu variant

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

The argument names of the hlist_add_after are poorly chosen because they look
the same as the ones from hlist_add_before but have to be used completely
different. This easily confuses the reader. The creator of the 
hlist_add_after_rcu function has made a lot better choice.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
---
v2:
Splitted into two patches

 include/linux/list.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/list.h b/include/linux/list.h
index ef95941..624ec7f 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -654,15 +654,15 @@ static inline void hlist_add_before(struct hlist_node *n,
 	*(n->pprev) = n;
 }
 
-static inline void hlist_add_after(struct hlist_node *n,
-					struct hlist_node *next)
+static inline void hlist_add_after(struct hlist_node *prev,
+				   struct hlist_node *n)
 {
-	next->next = n->next;
-	n->next = next;
-	next->pprev = &n->next;
+	n->next = prev->next;
+	prev->next = n;
+	n->pprev = &prev->next;
 
-	if(next->next)
-		next->next->pprev  = &next->next;
+	if (n->next)
+		n->next->pprev  = &n->next;
 }
 
 /* after that we'll appear to be on some hlist and hlist_del will work */
-- 
2.0.0

[PATCHv2 02/13] list: Fix order of arguments for hlist_add_after(_rcu)

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

All other add functions for lists have the new item as first argument and the
position where it is added as second argument. This was changed for no good
reason in this function and makes using it unnecessary confusing.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: Linux NICS <linux.nics@intel.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: dri-devel@lists.freedesktop.org
Cc: e1000-devel@lists.sourceforge.net
Cc: netdev@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: linux-fsdevel@vger.kernel.org
Cc: b.a.t.m.a.n@lists.open-mesh.org
Cc: bridge@lists.linux-foundation.org
---
Patch based on "Add linux-next specific files for 20140606"

v2:
Splitted into two patches
reduced number of Cc

 drivers/gpu/drm/drm_hashtab.c                    | 2 +-
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c   | 2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 2 +-
 drivers/staging/lustre/lustre/libcfs/hash.c      | 4 ++--
 fs/namespace.c                                   | 2 +-
 fs/notify/inode_mark.c                           | 2 +-
 fs/notify/vfsmount_mark.c                        | 2 +-
 include/linux/list.h                             | 4 ++--
 include/linux/rculist.h                          | 6 +++---
 net/batman-adv/fragmentation.c                   | 2 +-
 net/bridge/br_multicast.c                        | 2 +-
 net/ipv4/fib_trie.c                              | 2 +-
 net/ipv6/addrlabel.c                             | 2 +-
 net/xfrm/xfrm_policy.c                           | 4 ++--
 14 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_hashtab.c b/drivers/gpu/drm/drm_hashtab.c
index 7e4bae7..4077a35 100644
--- a/drivers/gpu/drm/drm_hashtab.c
+++ b/drivers/gpu/drm/drm_hashtab.c
@@ -125,7 +125,7 @@ int drm_ht_insert_item(struct drm_open_hash *ht, struct drm_hash_item *item)
 		parent = &entry->head;
 	}
 	if (parent) {
-		hlist_add_after_rcu(parent, &item->head);
+		hlist_add_after_rcu(&item->head, parent);
 	} else {
 		hlist_add_head_rcu(&item->head, h_list);
 	}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 1bb470b..db0a7e1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1437,7 +1437,7 @@ static int i40e_update_ethtool_fdir_entry(struct i40e_vsi *vsi,
 
 	/* add filter to the list */
 	if (parent)
-		hlist_add_after(&parent->fdir_node, &input->fdir_node);
+		hlist_add_after(&input->fdir_node, &parent->fdir_node);
 	else
 		hlist_add_head(&input->fdir_node,
 			       &pf->fdir_filter_list);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 23e4e6a..23f4ff3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2518,7 +2518,7 @@ static int ixgbe_update_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
 
 	/* add filter to the list */
 	if (parent)
-		hlist_add_after(&parent->fdir_node, &input->fdir_node);
+		hlist_add_after(&input->fdir_node, &parent->fdir_node);
 	else
 		hlist_add_head(&input->fdir_node,
 			       &adapter->fdir_filter_list);
diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
index 6d2b455..35835e5 100644
--- a/drivers/staging/lustre/lustre/libcfs/hash.c
+++ b/drivers/staging/lustre/lustre/libcfs/hash.c
@@ -351,7 +351,7 @@ cfs_hash_dh_hnode_add(struct cfs_hash *hs, struct cfs_hash_bd *bd,
 					    cfs_hash_dhead_t, dh_head);
 
 	if (dh->dh_tail != NULL) /* not empty */
-		hlist_add_after(dh->dh_tail, hnode);
+		hlist_add_after(hnode, dh->dh_tail);
 	else /* empty list */
 		hlist_add_head(hnode, &dh->dh_head);
 	dh->dh_tail = hnode;
@@ -406,7 +406,7 @@ cfs_hash_dd_hnode_add(struct cfs_hash *hs, struct cfs_hash_bd *bd,
 						cfs_hash_dhead_dep_t, dd_head);
 
 	if (dh->dd_tail != NULL) /* not empty */
-		hlist_add_after(dh->dd_tail, hnode);
+		hlist_add_after(hnode, dh->dd_tail);
 	else /* empty list */
 		hlist_add_head(hnode, &dh->dd_head);
 	dh->dd_tail = hnode;
diff --git a/fs/namespace.c b/fs/namespace.c
index b10db3d..14b751f 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -845,7 +845,7 @@ static void commit_tree(struct mount *mnt, struct mount *shadows)
 	list_splice(&head, n->list.prev);
 
 	if (shadows)
-		hlist_add_after_rcu(&shadows->mnt_hash, &mnt->mnt_hash);
+		hlist_add_after_rcu(&mnt->mnt_hash, &shadows->mnt_hash);
 	else
 		hlist_add_head_rcu(&mnt->mnt_hash,
 				m_hash(&parent->mnt, mnt->mnt_mountpoint));
diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
index 74825be..b5fc34b 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -232,7 +232,7 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark,
 
 	BUG_ON(last == NULL);
 	/* mark should be the last entry.  last is the current last entry */
-	hlist_add_after_rcu(&last->i.i_list, &mark->i.i_list);
+	hlist_add_after_rcu(&mark->i.i_list, &last->i.i_list);
 out:
 	fsnotify_recalc_inode_mask_locked(inode);
 	spin_unlock(&inode->i_lock);
diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c
index 68ca5a8..b283776 100644
--- a/fs/notify/vfsmount_mark.c
+++ b/fs/notify/vfsmount_mark.c
@@ -191,7 +191,7 @@ int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark,
 
 	BUG_ON(last == NULL);
 	/* mark should be the last entry.  last is the current last entry */
-	hlist_add_after_rcu(&last->m.m_list, &mark->m.m_list);
+	hlist_add_after_rcu(&mark->m.m_list, &last->m.m_list);
 out:
 	fsnotify_recalc_vfsmount_mask_locked(mnt);
 	spin_unlock(&mnt->mnt_root->d_lock);
diff --git a/include/linux/list.h b/include/linux/list.h
index 624ec7f..ab43d01 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -654,8 +654,8 @@ static inline void hlist_add_before(struct hlist_node *n,
 	*(n->pprev) = n;
 }
 
-static inline void hlist_add_after(struct hlist_node *prev,
-				   struct hlist_node *n)
+static inline void hlist_add_after(struct hlist_node *n,
+				   struct hlist_node *prev)
 {
 	n->next = prev->next;
 	prev->next = n;
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index 8183b46..648773f 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -433,8 +433,8 @@ static inline void hlist_add_before_rcu(struct hlist_node *n,
 
 /**
  * hlist_add_after_rcu
- * @prev: the existing element to add the new element after.
  * @n: the new element to add to the hash list.
+ * @prev: the existing element to add the new element after.
  *
  * Description:
  * Adds the specified element to the specified hlist
@@ -449,8 +449,8 @@ static inline void hlist_add_before_rcu(struct hlist_node *n,
  * hlist_for_each_entry_rcu(), used to prevent memory-consistency
  * problems on Alpha CPUs.
  */
-static inline void hlist_add_after_rcu(struct hlist_node *prev,
-				       struct hlist_node *n)
+static inline void hlist_add_after_rcu(struct hlist_node *n,
+				       struct hlist_node *prev)
 {
 	n->next = prev->next;
 	n->pprev = &prev->next;
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c
index f14e54a..61ff139 100644
--- a/net/batman-adv/fragmentation.c
+++ b/net/batman-adv/fragmentation.c
@@ -184,7 +184,7 @@ static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node,
 
 	/* Reached the end of the list, so insert after 'frag_entry_curr'. */
 	if (likely(frag_entry_curr)) {
-		hlist_add_after(&frag_entry_curr->list, &frag_entry_new->list);
+		hlist_add_after(&frag_entry_new->list, &frag_entry_curr->list);
 		chain->size += skb->len - hdr_size;
 		chain->timestamp = jiffies;
 		ret = true;
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 7b757b5..986a9c4 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1093,7 +1093,7 @@ static void br_multicast_add_router(struct net_bridge *br,
 	}
 
 	if (slot)
-		hlist_add_after_rcu(slot, &port->rlist);
+		hlist_add_after_rcu(&port->rlist, slot);
 	else
 		hlist_add_head_rcu(&port->rlist, &br->router_list);
 }
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 5afeb5a..853940a 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -940,7 +940,7 @@ static void insert_leaf_info(struct hlist_head *head, struct leaf_info *new)
 			last = li;
 		}
 		if (last)
-			hlist_add_after_rcu(&last->hlist, &new->hlist);
+			hlist_add_after_rcu(&new->hlist, &last->hlist);
 		else
 			hlist_add_before_rcu(&new->hlist, &li->hlist);
 	}
diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
index 731e1e1..f512d5d 100644
--- a/net/ipv6/addrlabel.c
+++ b/net/ipv6/addrlabel.c
@@ -277,7 +277,7 @@ static int __ip6addrlbl_add(struct ip6addrlbl_entry *newp, int replace)
 		last = p;
 	}
 	if (last)
-		hlist_add_after_rcu(&last->list, &newp->list);
+		hlist_add_after_rcu(&newp->list, &last->list);
 	else
 		hlist_add_head_rcu(&newp->list, &ip6addrlbl_table.head);
 out:
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index a8ef510..b6d4464 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -389,7 +389,7 @@ redo:
 			if (h != h0)
 				continue;
 			hlist_del(&pol->bydst);
-			hlist_add_after(entry0, &pol->bydst);
+			hlist_add_after(&pol->bydst, entry0);
 		}
 		entry0 = &pol->bydst;
 	}
@@ -654,7 +654,7 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
 			break;
 	}
 	if (newpos)
-		hlist_add_after(newpos, &policy->bydst);
+		hlist_add_after(&policy->bydst, newpos);
 	else
 		hlist_add_head(&policy->bydst, chain);
 	xfrm_pol_hold(policy);
-- 
2.0.0

[PATCH 03/13] list: Add list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. hlist already have
functions to do the same which makes the code a lot more readable. It would be
a lot easier to understand when all places use a self explaining function name
instead of misusing other functions.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: Dipankar Sarma <dipankar@in.ibm.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ken Helias <kenhelias@firemail.de>
Cc: Dave Jones <davej@redhat.com>
---
 include/linux/list.h    | 22 ++++++++++++++++++++++
 include/linux/rculist.h | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/include/linux/list.h b/include/linux/list.h
index ab43d01..5ea0eca 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -76,6 +76,28 @@ static inline void list_add_tail(struct list_head *new, struct list_head *head)
 	__list_add(new, head->prev, head);
 }
 
+/**
+ * list_add_after
+ * @n: new entry to be added
+ * @prev: the existing element to add the new element after.
+ *
+ * Description:
+ * Adds the specified element to the specified list
+ * after the specified node.
+ */
+#define list_add_after(n, prev) list_add(n, prev)
+
+/**
+ * list_add_before
+ * @n: new entry to be added
+ * @next:  the existing element to add the new element before.
+ *
+ * Description:
+ * Adds the specified element to the specified list
+ * before the specified node.
+ */
+#define list_add_before(n, next) list_add_tail(n, next)
+
 /*
  * Delete a list entry by making the prev/next entries
  * point to each other.
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index 648773f..eec274d 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -103,6 +103,44 @@ static inline void list_add_tail_rcu(struct list_head *new,
 }
 
 /**
+ * list_add_after_rcu
+ * @n: new entry to be added
+ * @prev: the existing element to add the new element after.
+ *
+ * Description:
+ * Adds the specified element to the specified list
+ * after the specified node while permitting racing traversals.
+ *
+ * The caller must take whatever precautions are necessary
+ * (such as holding appropriate locks) to avoid racing
+ * with another list-mutation primitive, such as list_add_rcu()
+ * or list_del_rcu(), running on this same list.
+ * However, it is perfectly legal to run concurrently with
+ * the _rcu list-traversal primitives, such as
+ * list_for_each_entry_rcu().
+ */
+#define list_add_after_rcu(n, next) list_add_rcu(n, next)
+
+/**
+ * list_add_before_rcu
+ * @n: new entry to be added
+ * @next:  the existing element to add the new element before.
+ *
+ * Description:
+ * Adds the specified element to the specified list
+ * before the specified node while permitting racing traversals.
+ *
+ * The caller must take whatever precautions are necessary
+ * (such as holding appropriate locks) to avoid racing
+ * with another list-mutation primitive, such as list_add_tail_rcu()
+ * or list_del_rcu(), running on this same list.
+ * However, it is perfectly legal to run concurrently with
+ * the _rcu list-traversal primitives, such as
+ * list_for_each_entry_rcu().
+ */
+#define list_add_before_rcu(n, next) list_add_tail_rcu(n, next)
+
+/**
  * list_del_rcu - deletes entry from list without re-initialization
  * @entry: the element to delete from the list.
  *
-- 
2.0.0

[PATCH 04/13] metag: dma: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: linux-metag@vger.kernel.org
---
 arch/metag/kernel/dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
index c700d62..8b384d8 100644
--- a/arch/metag/kernel/dma.c
+++ b/arch/metag/kernel/dma.c
@@ -138,7 +138,7 @@ found:
 	/*
 	 * Insert this entry _before_ the one we found.
 	 */
-	list_add_tail(&new->vm_list, &c->vm_list);
+	list_add_before(&new->vm_list, &c->vm_list);
 	new->vm_start = addr;
 	new->vm_end = addr + size;
 	new->vm_active = 1;
-- 
2.0.0

[PATCH 05/13] powerpc: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/lib/rheap.c          | 2 +-
 arch/powerpc/mm/dma-noncoherent.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/rheap.c b/arch/powerpc/lib/rheap.c
index a1060a8..d8c5f16 100644
--- a/arch/powerpc/lib/rheap.c
+++ b/arch/powerpc/lib/rheap.c
@@ -486,7 +486,7 @@ unsigned long rh_alloc_align(rh_info_t * info, int size, int alignment, const ch
 			spblk->start = blk->start;
 			spblk->size = sp_size;
 			/* add before the blk */
-			list_add(&spblk->list, blk->list.prev);
+			list_add_before(&spblk->list, &blk->list);
 		}
 		newblk = get_slot(info);
 		newblk->start = start;
diff --git a/arch/powerpc/mm/dma-noncoherent.c b/arch/powerpc/mm/dma-noncoherent.c
index d85e86a..222ae97 100644
--- a/arch/powerpc/mm/dma-noncoherent.c
+++ b/arch/powerpc/mm/dma-noncoherent.c
@@ -120,7 +120,7 @@ ppc_vm_region_alloc(struct ppc_vm_region *head, size_t size, gfp_t gfp)
 	/*
 	 * Insert this entry _before_ the one we found.
 	 */
-	list_add_tail(&new->vm_list, &c->vm_list);
+	list_add_before(&new->vm_list, &c->vm_list);
 	new->vm_start = addr;
 	new->vm_end = addr + size;
 
-- 
2.0.0

[PATCH 06/13] EDAC: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: linux-edac@vger.kernel.org
---
 drivers/edac/edac_device.c | 2 +-
 drivers/edac/edac_mc.c     | 2 +-
 drivers/edac/edac_pci.c    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 592af5f..b9216b2 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -318,7 +318,7 @@ static int add_edac_dev_to_global_list(struct edac_device_ctl_info *edac_dev)
 		}
 	}
 
-	list_add_tail_rcu(&edac_dev->link, insert_before);
+	list_add_before_rcu(&edac_dev->link, insert_before);
 	return 0;
 
 fail0:
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 2c694b5..ef4d653 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -653,7 +653,7 @@ static int add_mc_to_global_list(struct mem_ctl_info *mci)
 		}
 	}
 
-	list_add_tail_rcu(&mci->link, insert_before);
+	list_add_before_rcu(&mci->link, insert_before);
 	atomic_inc(&edac_handlers);
 	return 0;
 
diff --git a/drivers/edac/edac_pci.c b/drivers/edac/edac_pci.c
index 2cf44b4d..df6c69b 100644
--- a/drivers/edac/edac_pci.c
+++ b/drivers/edac/edac_pci.c
@@ -144,7 +144,7 @@ static int add_edac_pci_to_global_list(struct edac_pci_ctl_info *pci)
 		}
 	}
 
-	list_add_tail_rcu(&pci->link, insert_before);
+	list_add_before_rcu(&pci->link, insert_before);
 	return 0;
 
 fail0:
-- 
2.0.0

[PATCH 07/13] PCI: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: linux-pci@vger.kernel.org
---
 drivers/pci/setup-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index fd9b545..77ceaab 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -167,7 +167,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
 			}
 		}
 		/* Insert it just before n*/
-		list_add_tail(&tmp->list, n);
+		list_add_before(&tmp->list, n);
 	}
 }
 
-- 
2.0.0

[PATCH 08/13] mac80211: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: linux-wireless@vger.kernel.org
---
 net/mac80211/cfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index d7513a5..110c42b 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2822,7 +2822,7 @@ static int ieee80211_start_roc_work(struct ieee80211_local *local,
 
 				if (new_dur > 0) {
 					/* add right after tmp */
-					list_add(&roc->list, &tmp->list);
+					list_add_after(&roc->list, &tmp->list);
 				} else {
 					list_add_tail(&roc->list,
 						      &tmp->dependents);
-- 
2.0.0

[PATCH 09/13] jfs: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: jfs-discussion@lists.sourceforge.net
---
 fs/jfs/jfs_dmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
index 2d514c7..d08c6d6 100644
--- a/fs/jfs/jfs_dmap.c
+++ b/fs/jfs/jfs_dmap.c
@@ -560,7 +560,7 @@ dbUpdatePMap(struct inode *ipbmap,
 
 			/* insert bp after tblock in logsync list */
 			log->count++;
-			list_add(&mp->synclist, &tblk->synclist);
+			list_add_after(&mp->synclist, &tblk->synclist);
 
 			mp->clsn = tblk->clsn;
 		}
-- 
2.0.0

[PATCH 10/13] xhci: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2b8d9a2..4f4b702 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2471,7 +2471,7 @@ static void xhci_add_ep_to_interval_table(struct xhci_hcd *xhci,
 		if (ep_bw->max_packet_size >=
 				smaller_ep->bw_info.max_packet_size) {
 			/* Add the new ep before the smaller endpoint */
-			list_add_tail(&virt_ep->bw_endpoint_list,
+			list_add_before(&virt_ep->bw_endpoint_list,
 					&smaller_ep->bw_endpoint_list);
 			return;
 		}
-- 
2.0.0

[PATCH 11/13] iscsi-target: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: linux-scsi@vger.kernel.org
Cc: target-devel@vger.kernel.org
---
 drivers/target/iscsi/iscsi_target_erl1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_erl1.c b/drivers/target/iscsi/iscsi_target_erl1.c
index cda4d80..1926d0b 100644
--- a/drivers/target/iscsi/iscsi_target_erl1.c
+++ b/drivers/target/iscsi/iscsi_target_erl1.c
@@ -835,8 +835,8 @@ static int iscsit_attach_ooo_cmdsn(
 					continue;
 
 				/* Insert before this entry */
-				list_add(&ooo_cmdsn->ooo_list,
-					ooo_tmp->ooo_list.prev);
+				list_add_before(&ooo_cmdsn->ooo_list,
+						&ooo_tmp->ooo_list);
 				break;
 			}
 		}
-- 
2.0.0

[PATCH 12/13] s390: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: linux-s390@vger.kernel.org
---
 drivers/s390/block/dasd_3990_erp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/block/dasd_3990_erp.c b/drivers/s390/block/dasd_3990_erp.c
index d261347..2189e45 100644
--- a/drivers/s390/block/dasd_3990_erp.c
+++ b/drivers/s390/block/dasd_3990_erp.c
@@ -2761,7 +2761,7 @@ dasd_3990_erp_action(struct dasd_ccw_req * cqr)
 	if (list_empty(&erp->blocklist)) {
 		cqr->status = DASD_CQR_IN_ERP;
 		/* add erp request before the cqr */
-		list_add_tail(&erp->blocklist, &cqr->blocklist);
+		list_add_before(&erp->blocklist, &cqr->blocklist);
 	}
 
 
-- 
2.0.0

[PATCH 13/13] staging: tidspbridge: Use list_add_(before|after) macros

[Ken Helias]

From: Ken Helias <kenhelias@firemail.de>

Many places in the code uses list_add_tail/list_add to insert an entry
before/after another entry. This confuses the reader because these are usually
used to add an item to a list_head and not an entry. Better use the self
explaining function name.

Signed-off-by: Ken Helias <kenhelias@firemail.de>
Cc: devel@driverdev.osuosl.org
---
 drivers/staging/tidspbridge/rmgr/rmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/tidspbridge/rmgr/rmm.c b/drivers/staging/tidspbridge/rmgr/rmm.c
index 52187bd..7e1ba84 100644
--- a/drivers/staging/tidspbridge/rmgr/rmm.c
+++ b/drivers/staging/tidspbridge/rmgr/rmm.c
@@ -139,7 +139,7 @@ int rmm_alloc(struct rmm_target_obj *target, u32 segid, u32 size,
 						&target->ovly_list);
 			else
 				/* Put new section just before sect */
-				list_add_tail(&new_sect->list_elem,
+				list_add_before(&new_sect->list_elem,
 						&sect->list_elem);
 		}
 	}
-- 
2.0.0

Re: [PATCH 03/13] list: Add list_add_(before|after) macros

[Hugh Dickins]

On Fri, 6 Jun 2014, Ken Helias wrote:
> From: Ken Helias <kenhelias@firemail.de>
> 
> Many places in the code uses list_add_tail/list_add to insert an entry
> before/after another entry. This confuses the reader because these are usually
> used to add an item to a list_head and not an entry. hlist already have
> functions to do the same which makes the code a lot more readable. It would be
> a lot easier to understand when all places use a self explaining function name
> instead of misusing other functions.

I'll express my opinion: perhaps it's widely shared, perhaps it is not.

I think a patch like this (and the subsequent conversions) is unhelpful
churn.

The Linux struct list_head is designed and intended to be used for both
head and entry, and introducing aliases to make unnecessary distinctions
just hinders development instead of helping it.

If the reader is confused at first, I hope the reader will soon learn,
without needing to rely on aliases such as these.

Hugh

> 
> Signed-off-by: Ken Helias <kenhelias@firemail.de>
> Cc: Dipankar Sarma <dipankar@in.ibm.com>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Ken Helias <kenhelias@firemail.de>
> Cc: Dave Jones <davej@redhat.com>
> ---
>  include/linux/list.h    | 22 ++++++++++++++++++++++
>  include/linux/rculist.h | 38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/include/linux/list.h b/include/linux/list.h
> index ab43d01..5ea0eca 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -76,6 +76,28 @@ static inline void list_add_tail(struct list_head *new, struct list_head *head)
>  	__list_add(new, head->prev, head);
>  }
>  
> +/**
> + * list_add_after
> + * @n: new entry to be added
> + * @prev: the existing element to add the new element after.
> + *
> + * Description:
> + * Adds the specified element to the specified list
> + * after the specified node.
> + */
> +#define list_add_after(n, prev) list_add(n, prev)
> +
> +/**
> + * list_add_before
> + * @n: new entry to be added
> + * @next:  the existing element to add the new element before.
> + *
> + * Description:
> + * Adds the specified element to the specified list
> + * before the specified node.
> + */
> +#define list_add_before(n, next) list_add_tail(n, next)
> +
>  /*
>   * Delete a list entry by making the prev/next entries
>   * point to each other.
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index 648773f..eec274d 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -103,6 +103,44 @@ static inline void list_add_tail_rcu(struct list_head *new,
>  }
>  
>  /**
> + * list_add_after_rcu
> + * @n: new entry to be added
> + * @prev: the existing element to add the new element after.
> + *
> + * Description:
> + * Adds the specified element to the specified list
> + * after the specified node while permitting racing traversals.
> + *
> + * The caller must take whatever precautions are necessary
> + * (such as holding appropriate locks) to avoid racing
> + * with another list-mutation primitive, such as list_add_rcu()
> + * or list_del_rcu(), running on this same list.
> + * However, it is perfectly legal to run concurrently with
> + * the _rcu list-traversal primitives, such as
> + * list_for_each_entry_rcu().
> + */
> +#define list_add_after_rcu(n, next) list_add_rcu(n, next)
> +
> +/**
> + * list_add_before_rcu
> + * @n: new entry to be added
> + * @next:  the existing element to add the new element before.
> + *
> + * Description:
> + * Adds the specified element to the specified list
> + * before the specified node while permitting racing traversals.
> + *
> + * The caller must take whatever precautions are necessary
> + * (such as holding appropriate locks) to avoid racing
> + * with another list-mutation primitive, such as list_add_tail_rcu()
> + * or list_del_rcu(), running on this same list.
> + * However, it is perfectly legal to run concurrently with
> + * the _rcu list-traversal primitives, such as
> + * list_for_each_entry_rcu().
> + */
> +#define list_add_before_rcu(n, next) list_add_tail_rcu(n, next)
> +
> +/**
>   * list_del_rcu - deletes entry from list without re-initialization
>   * @entry: the element to delete from the list.
>   *
> -- 
> 2.0.0

Re: [PATCH 03/13] list: Add list_add_(before|after) macros

[Christoph Hellwig]

On Fri, Jun 06, 2014 at 12:51:35PM -0700, Hugh Dickins wrote:
> I'll express my opinion: perhaps it's widely shared, perhaps it is not.
> 
> I think a patch like this (and the subsequent conversions) is unhelpful
> churn.
> 
> The Linux struct list_head is designed and intended to be used for both
> head and entry, and introducing aliases to make unnecessary distinctions
> just hinders development instead of helping it.
> 
> If the reader is confused at first, I hope the reader will soon learn,
> without needing to rely on aliases such as these.

I fully concur with Hugh - the list_head is symmetric for heads vs
entires, and lots of code depends on that.

In addition to that multiple functions that do exactly the same thing
generall are a bad idea.