[Drbd-dev] [PATCH 00/17] lru_cache: checkpatch clean ups

[Drbd-dev] [PATCH 00/17] lru_cache: checkpatch clean ups

[Tobin C. Harding]

Checkpatch clean ups for lib/lru_cache.c

This patch set is intended to test the waters as to whether DRBD wants/accepts checkpatch clean
ups. I know that many maintainers consider such patches as code churn.

I am taking a look at the DRBD code after bumping into Brian at OSSNA in LA.

If you do want/like checkpatch fixes I am happy to do so for drivers/block/drbd/* while I look
around the code base.

I don't know which tree you guys are working off. This series applies on top of Linus' development
tree, commit:

a8c964eacb21288b2dbfa9d80cee5968a3b8fb21

thanks,
Tobin.

Tobin C. Harding (17):
  lru_cache: remove FSF address from licence
  lru_cache: fix licence comment format
  lru_cache: move trailing */ to a separate line
  lru_cache: remove quoted string across lines
  lru_cache: use 'unsigned int' instead of 'unsigned'
  lru_cache: use kcalloc instead of kzalloc with multiply
  lru_cache: clean macro definition
  lru_cache: add blank line after declarations
  lru_cache: remove space after function name
  lru_cache: fix function argument alignment
  lru_cache: remove multiple blank lines
  lru_cache: remove space before tabs
  lru_cache: move constant to the right side of test
  lru_cache: use braces on all arms of statement
  lru_cache: remove unnecessary space before arguments
  lru_cache: add spaces around '|'
  lru_cache: move EXPORT_SYMBOL macro to follow function

 lib/lru_cache.c | 166 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 86 insertions(+), 80 deletions(-)

-- 
2.7.4

[Drbd-dev] [PATCH 01/17] lru_cache: remove FSF address from licence

[Tobin C. Harding]

checkpatch emits warning 'Do not include the paragraph about writing to the Free Software
Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past,
and may do so again. Linux already includes a copy of the GPL.'

Remove paragraph containing FSF address from licence.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 28ba40b..44d7ab0 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -16,11 +16,6 @@
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with drbd; see the file COPYING.  If not, write to
-   the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
-
  */
 
 #include <linux/module.h>
-- 
2.7.4

[Drbd-dev] [PATCH 02/17] lru_cache: fix licence comment format

[Tobin C. Harding]

checkpatch emits WARNING: Block comments use * on subsequent lines

Add '*' to each subsequent line of licence comment.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 44d7ab0..273af4b 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -1,21 +1,21 @@
 /*
-   lru_cache.c
-
-   This file is part of DRBD by Philipp Reisner and Lars Ellenberg.
-
-   Copyright (C) 2003-2008, LINBIT Information Technologies GmbH.
-   Copyright (C) 2003-2008, Philipp Reisner <philipp.reisner@linbit.com>.
-   Copyright (C) 2003-2008, Lars Ellenberg <lars.ellenberg@linbit.com>.
-
-   drbd is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 2, or (at your option)
-   any later version.
-
-   drbd is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
+ *   lru_cache.c
+ *
+ *   This file is part of DRBD by Philipp Reisner and Lars Ellenberg.
+ *
+ *   Copyright (C) 2003-2008, LINBIT Information Technologies GmbH.
+ *   Copyright (C) 2003-2008, Philipp Reisner <philipp.reisner@linbit.com>.
+ *   Copyright (C) 2003-2008, Lars Ellenberg <lars.ellenberg@linbit.com>.
+ *
+ *   drbd is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2, or (at your option)
+ *   any later version.
+ *
+ *   drbd is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
  */
 
 #include <linux/module.h>
-- 
2.7.4

[Drbd-dev] [PATCH 03/17] lru_cache: move trailing */ to a separate line

[Tobin C. Harding]

checkpatch emits WARNING: Block comments use a trailing */ on a
separate line.

Move trailing */ to a separate line.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 273af4b..898feb9 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -30,8 +30,9 @@ MODULE_AUTHOR("Philipp Reisner <phil@linbit.com>, "
 MODULE_DESCRIPTION("lru_cache - Track sets of hot objects");
 MODULE_LICENSE("GPL");
 
-/* this is developers aid only.
- * it catches concurrent access (lack of locking on the users part) */
+/* This is developers aid only.
+ * it catches concurrent access (lack of locking on the users part)
+ */
 #define PARANOIA_ENTRY() do {		\
 	BUG_ON(!lc);			\
 	BUG_ON(!lc->nr_elements);	\
@@ -69,7 +70,8 @@ int lc_try_lock(struct lru_cache *lc)
 	return 0 == val;
 #if 0
 	/* Alternative approach, spin in case someone enters or leaves a
-	 * PARANOIA_ENTRY()/RETURN() section. */
+	 * PARANOIA_ENTRY()/RETURN() section.
+	 */
 	unsigned long old, new, val;
 	do {
 		old = lc->flags & LC_PARANOIA;
@@ -107,7 +109,8 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
 		return NULL;
 
 	/* e_count too big; would probably fail the allocation below anyways.
-	 * for typical use cases, e_count should be few thousand at most. */
+	 * for typical use cases, e_count should be few thousand at most.
+	 */
 	if (e_count > LC_MAX_ACTIVE)
 		return NULL;
 
@@ -263,7 +266,8 @@ static struct lc_element *__lc_find(struct lru_cache *lc, unsigned int enr,
 	hlist_for_each_entry(e, lc_hash_slot(lc, enr), colision) {
 		/* "about to be changed" elements, pending transaction commit,
 		 * are hashed by their "new number". "Normal" elements have
-		 * lc_number == lc_new_number. */
+		 * lc_number == lc_new_number.
+		 */
 		if (e->lc_new_number != enr)
 			continue;
 		if (e->lc_new_number == e->lc_number || include_changing)
@@ -379,7 +383,8 @@ static struct lc_element *__lc_get(struct lru_cache *lc, unsigned int enr, unsig
 	/* if lc_new_number != lc_number,
 	 * this enr is currently being pulled in already,
 	 * and will be available once the pending transaction
-	 * has been committed. */
+	 * has been committed.
+	 */
 	if (e) {
 		if (e->lc_new_number != e->lc_number) {
 			/* It has been found above, but on the "to_be_changed"
@@ -389,7 +394,8 @@ static struct lc_element *__lc_get(struct lru_cache *lc, unsigned int enr, unsig
 			if (!(flags & LC_GET_MAY_USE_UNCOMMITTED))
 				RETURN(NULL);
 			/* ... unless the caller is aware of the implications,
-			 * probably preparing a cumulative transaction. */
+			 * probably preparing a cumulative transaction.
+			 */
 			++e->refcnt;
 			++lc->hits;
 			RETURN(e);
@@ -408,7 +414,8 @@ static struct lc_element *__lc_get(struct lru_cache *lc, unsigned int enr, unsig
 		RETURN(NULL);
 
 	/* To avoid races with lc_try_lock(), first, mark us dirty
-	 * (using test_and_set_bit, as it implies memory barriers), ... */
+	 * (using test_and_set_bit, as it implies memory barriers), ...
+	 */
 	test_and_set_bit(__LC_DIRTY, &lc->flags);
 
 	/* ... only then check if it is locked anyways. If lc_unlock clears
@@ -429,7 +436,8 @@ static struct lc_element *__lc_get(struct lru_cache *lc, unsigned int enr, unsig
 
 	/* It was not present in the active set.  We are going to recycle an
 	 * unused (or even "free") element, but we won't accumulate more than
-	 * max_pending_changes changes.  */
+	 * max_pending_changes changes.
+	 */
 	if (lc->pending_changes >= lc->max_pending_changes)
 		RETURN(NULL);
 
-- 
2.7.4

[Drbd-dev] [PATCH 04/17] lru_cache: remove quoted string across lines

[Tobin C. Harding]

checkpatch emits WARNING: quoted string split across lines. This is
caused by the MODULE_AUTHOR string for multiple authors. We can call
the MODULE_AUTHOR macro multiple times instead.

Remove quoted string across lines. Replace with second call to
MODULE_AUTHOR macro.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 898feb9..0d3c3bb 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -25,8 +25,8 @@
 #include <linux/seq_file.h> /* for seq_printf */
 #include <linux/lru_cache.h>
 
-MODULE_AUTHOR("Philipp Reisner <phil@linbit.com>, "
-	      "Lars Ellenberg <lars@linbit.com>");
+MODULE_AUTHOR("Philipp Reisner <phil@linbit.com>");
+MODULE_AUTHOR("Lars Ellenberg <lars@linbit.com>");
 MODULE_DESCRIPTION("lru_cache - Track sets of hot objects");
 MODULE_LICENSE("GPL");
 
-- 
2.7.4

[Drbd-dev] [PATCH 05/17] lru_cache: use 'unsigned int' instead of 'unsigned'

[Tobin C. Harding]

checkpatch emits WARNING: Prefer 'unsigned int' to bare use of
'unsigned'.

Use 'unsigned int' instead of 'unsigned'.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 0d3c3bb..0bdb858 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -47,7 +47,7 @@ MODULE_LICENSE("GPL");
 #define PARANOIA_LC_ELEMENT(lc, e) do {	\
 	struct lru_cache *lc_ = (lc);	\
 	struct lc_element *e_ = (e);	\
-	unsigned i = e_->lc_index;	\
+	unsigned int i = e_->lc_index;	\
 	BUG_ON(i >= lc_->nr_elements);	\
 	BUG_ON(lc_->lc_element[i] != e_); } while (0)
 
@@ -94,15 +94,15 @@ int lc_try_lock(struct lru_cache *lc)
  * or NULL on (allocation) failure.
  */
 struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
-		unsigned max_pending_changes,
-		unsigned e_count, size_t e_size, size_t e_off)
+		unsigned int max_pending_changes,
+		unsigned int e_count, size_t e_size, size_t e_off)
 {
 	struct hlist_head *slot = NULL;
 	struct lc_element **element = NULL;
 	struct lru_cache *lc;
 	struct lc_element *e;
-	unsigned cache_obj_size = kmem_cache_size(cache);
-	unsigned i;
+	unsigned int cache_obj_size = kmem_cache_size(cache);
+	unsigned int i;
 
 	WARN_ON(cache_obj_size < e_size);
 	if (cache_obj_size < e_size)
@@ -167,7 +167,7 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
 	return NULL;
 }
 
-static void lc_free_by_index(struct lru_cache *lc, unsigned i)
+static void lc_free_by_index(struct lru_cache *lc, unsigned int i)
 {
 	void *p = lc->lc_element[i];
 	WARN_ON(!p);
@@ -183,7 +183,7 @@ static void lc_free_by_index(struct lru_cache *lc, unsigned i)
  */
 void lc_destroy(struct lru_cache *lc)
 {
-	unsigned i;
+	unsigned int i;
 	if (!lc)
 		return;
 	for (i = 0; i < lc->nr_elements; i++)
@@ -202,7 +202,7 @@ void lc_destroy(struct lru_cache *lc)
  */
 void lc_reset(struct lru_cache *lc)
 {
-	unsigned i;
+	unsigned int i;
 
 	INIT_LIST_HEAD(&lc->in_use);
 	INIT_LIST_HEAD(&lc->lru);
@@ -329,7 +329,7 @@ void lc_del(struct lru_cache *lc, struct lc_element *e)
 	RETURN();
 }
 
-static struct lc_element *lc_prepare_for_change(struct lru_cache *lc, unsigned new_number)
+static struct lc_element *lc_prepare_for_change(struct lru_cache *lc, unsigned int new_number)
 {
 	struct list_head *n;
 	struct lc_element *e;
@@ -591,7 +591,7 @@ unsigned int lc_put(struct lru_cache *lc, struct lc_element *e)
  * @lc: the lru cache to operate on
  * @i: the index of the element to return
  */
-struct lc_element *lc_element_by_index(struct lru_cache *lc, unsigned i)
+struct lc_element *lc_element_by_index(struct lru_cache *lc, unsigned int i)
 {
 	BUG_ON(i >= lc->nr_elements);
 	BUG_ON(lc->lc_element[i] == NULL);
-- 
2.7.4

[Drbd-dev] [PATCH 06/17] lru_cache: use kcalloc instead of kzalloc with multiply

[Tobin C. Harding]

checkpatch emits WARNING: Prefer kcalloc over kzalloc with multiply.

Use kcalloc instead of kzalloc with multiply.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 0bdb858..18043b9 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -117,7 +117,7 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
 	slot = kcalloc(e_count, sizeof(struct hlist_head), GFP_KERNEL);
 	if (!slot)
 		goto out_fail;
-	element = kzalloc(e_count * sizeof(struct lc_element *), GFP_KERNEL);
+	element = kcalloc(e_count, sizeof(struct lc_element *), GFP_KERNEL);
 	if (!element)
 		goto out_fail;
 
-- 
2.7.4

[Drbd-dev] [PATCH 07/17] lru_cache: clean macro definition

[Tobin C. Harding]

checkpatch emits ERROR: trailing statements should be on next line.
Also macro definition is slightly cluttered. If we lay the definition
out more cleanly the code is easier to read.

Clean macro definition, clearing checkpatch error.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 18043b9..c15df31 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -44,13 +44,14 @@ MODULE_LICENSE("GPL");
 	return x ; } while (0)
 
 /* BUG() if e is not one of the elements tracked by lc */
-#define PARANOIA_LC_ELEMENT(lc, e) do {	\
-	struct lru_cache *lc_ = (lc);	\
-	struct lc_element *e_ = (e);	\
-	unsigned int i = e_->lc_index;	\
-	BUG_ON(i >= lc_->nr_elements);	\
-	BUG_ON(lc_->lc_element[i] != e_); } while (0)
-
+#define PARANOIA_LC_ELEMENT(lc, e)			\
+	do {						\
+		struct lru_cache *lc_ = (lc);		\
+		struct lc_element *e_ = (e);		\
+		unsigned int i = e_->lc_index;		\
+		BUG_ON(i >= lc_->nr_elements);		\
+		BUG_ON(lc_->lc_element[i] != e_);	\
+	} while (0)
 
 /* We need to atomically
  *  - try to grab the lock (set LC_LOCKED)
-- 
2.7.4

[Drbd-dev] [PATCH 08/17] lru_cache: add blank line after declarations

[Tobin C. Harding]

checkpatch emits WARNING: Missing a blank line after declarations.
Clearing this warning for variables declared within code blocks
that are not function definitions is of questionable benefit. Let's
just clear the function level declarations.

Add blank line after declarations.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index c15df31..34af31e 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -64,6 +64,7 @@ MODULE_LICENSE("GPL");
 int lc_try_lock(struct lru_cache *lc)
 {
 	unsigned long val;
+
 	do {
 		val = cmpxchg(&lc->flags, 0, LC_LOCKED);
 	} while (unlikely (val == LC_PARANOIA));
@@ -74,6 +75,7 @@ int lc_try_lock(struct lru_cache *lc)
 	 * PARANOIA_ENTRY()/RETURN() section.
 	 */
 	unsigned long old, new, val;
+
 	do {
 		old = lc->flags & LC_PARANOIA;
 		new = old | LC_LOCKED;
@@ -171,6 +173,7 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
 static void lc_free_by_index(struct lru_cache *lc, unsigned int i)
 {
 	void *p = lc->lc_element[i];
+
 	WARN_ON(!p);
 	if (p) {
 		p -= lc->element_off;
@@ -185,6 +188,7 @@ static void lc_free_by_index(struct lru_cache *lc, unsigned int i)
 void lc_destroy(struct lru_cache *lc)
 {
 	unsigned int i;
+
 	if (!lc)
 		return;
 	for (i = 0; i < lc->nr_elements; i++)
-- 
2.7.4

[Drbd-dev] [PATCH 09/17] lru_cache: remove space after function name

[Tobin C. Harding]

checkpatch emits WARNING: space prohibited between function name and
open parenthesis '('.

Remove space between function name and open parenthesis.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 34af31e..8a5ef8c 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -67,7 +67,7 @@ int lc_try_lock(struct lru_cache *lc)
 
 	do {
 		val = cmpxchg(&lc->flags, 0, LC_LOCKED);
-	} while (unlikely (val == LC_PARANOIA));
+	} while (unlikely(val == LC_PARANOIA));
 	/* Spin until no-one is inside a PARANOIA_ENTRY()/RETURN() section. */
 	return 0 == val;
 #if 0
@@ -80,7 +80,7 @@ int lc_try_lock(struct lru_cache *lc)
 		old = lc->flags & LC_PARANOIA;
 		new = old | LC_LOCKED;
 		val = cmpxchg(&lc->flags, old, new);
-	} while (unlikely (val == (old ^ LC_PARANOIA)));
+	} while (unlikely(val == (old ^ LC_PARANOIA)));
 	return old == val;
 #endif
 }
-- 
2.7.4

[Drbd-dev] [PATCH 10/17] lru_cache: fix function argument alignment

[Tobin C. Harding]

checkpatch emits CHECK: Alignment should match open parenthesis

Fix alignment to match open parenthesis.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 8a5ef8c..dc34bbc 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -97,8 +97,8 @@ int lc_try_lock(struct lru_cache *lc)
  * or NULL on (allocation) failure.
  */
 struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
-		unsigned int max_pending_changes,
-		unsigned int e_count, size_t e_size, size_t e_off)
+			    unsigned int max_pending_changes,
+			    unsigned int e_count, size_t e_size, size_t e_off)
 {
 	struct hlist_head *slot = NULL;
 	struct lc_element **element = NULL;
@@ -262,7 +262,7 @@ static struct hlist_head *lc_hash_slot(struct lru_cache *lc, unsigned int enr)
 
 
 static struct lc_element *__lc_find(struct lru_cache *lc, unsigned int enr,
-		bool include_changing)
+				    bool include_changing)
 {
 	struct lc_element *e;
 
@@ -656,7 +656,7 @@ void lc_set(struct lru_cache *lc, unsigned int enr, int index)
  * Note: a leading space ' ' and trailing newline '\n' is implied.
  */
 void lc_seq_dump_details(struct seq_file *seq, struct lru_cache *lc, char *utext,
-	     void (*detail) (struct seq_file *, struct lc_element *))
+			 void (*detail) (struct seq_file *, struct lc_element *))
 {
 	unsigned int nr_elements = lc->nr_elements;
 	struct lc_element *e;
@@ -667,10 +667,10 @@ void lc_seq_dump_details(struct seq_file *seq, struct lru_cache *lc, char *utext
 		e = lc_element_by_index(lc, i);
 		if (e->lc_number != e->lc_new_number)
 			seq_printf(seq, "\t%5d: %6d %8d %6d ",
-				i, e->lc_number, e->lc_new_number, e->refcnt);
+				   i, e->lc_number, e->lc_new_number, e->refcnt);
 		else
 			seq_printf(seq, "\t%5d: %6d %-8s %6d ",
-				i, e->lc_number, "-\"-", e->refcnt);
+				   i, e->lc_number, "-\"-", e->refcnt);
 		if (detail)
 			detail(seq, e);
 		seq_putc(seq, '\n');
-- 
2.7.4

[Drbd-dev] [PATCH 11/17] lru_cache: remove multiple blank lines

[Tobin C. Harding]

checkpatch emits CHECK: Please don't use multiple blank lines.

Remove multiple blank lines.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index dc34bbc..b9cc261 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -260,7 +260,6 @@ static struct hlist_head *lc_hash_slot(struct lru_cache *lc, unsigned int enr)
 	return  lc->lc_slot + (enr % lc->nr_elements);
 }
 
-
 static struct lc_element *__lc_find(struct lru_cache *lc, unsigned int enr,
 				    bool include_changing)
 {
@@ -566,7 +565,6 @@ void lc_committed(struct lru_cache *lc)
 	RETURN();
 }
 
-
 /**
  * lc_put - give up refcnt of @e
  * @lc: the lru cache to operate on
-- 
2.7.4

[Drbd-dev] [PATCH 12/17] lru_cache: remove space before tabs

[Tobin C. Harding]

checkpatch emits WARNING: please, no space before tabs

Remove space before tabs.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index b9cc261..9eed675 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -511,7 +511,7 @@ struct lc_element *lc_get(struct lru_cache *lc, unsigned int enr)
  * plus:
  *
  * pointer to an element already on the "to_be_changed" list.
- * 	In this case, the cache was already marked %LC_DIRTY.
+ * In this case, the cache was already marked %LC_DIRTY.
  *
  * Caller needs to make sure that the pending transaction is completed,
  * before proceeding to actually use this element.
-- 
2.7.4

[Drbd-dev] [PATCH 13/17] lru_cache: move constant to the right side of test

[Tobin C. Harding]

checkpatch emits WARNING: Comparisons should place the constant on the
right side of the test.

Move constant to the right hand side of the test.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 9eed675..a581b33 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -69,7 +69,7 @@ int lc_try_lock(struct lru_cache *lc)
 		val = cmpxchg(&lc->flags, 0, LC_LOCKED);
 	} while (unlikely(val == LC_PARANOIA));
 	/* Spin until no-one is inside a PARANOIA_ENTRY()/RETURN() section. */
-	return 0 == val;
+	return val == 0;
 #if 0
 	/* Alternative approach, spin in case someone enters or leaves a
 	 * PARANOIA_ENTRY()/RETURN() section.
-- 
2.7.4

[Drbd-dev] [PATCH 14/17] lru_cache: use braces on all arms of statement

[Tobin C. Harding]

checkpatch emits CHECK: braces {} should be used on all arms of this
statement. Also unbalanced braces warning is emitted for this same
snippet.

Use braces on all arms of statement, clearing two checkpatch warnings.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index a581b33..012c6b8 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -635,9 +635,9 @@ void lc_set(struct lru_cache *lc, unsigned int enr, int index)
 
 	e->lc_number = e->lc_new_number = enr;
 	hlist_del_init(&e->colision);
-	if (enr == LC_FREE)
+	if (enr == LC_FREE) {
 		lh = &lc->free;
-	else {
+	} else {
 		hlist_add_head(&e->colision, lc_hash_slot(lc, enr));
 		lh = &lc->lru;
 	}
-- 
2.7.4

[Drbd-dev] [PATCH 15/17] lru_cache: remove unnecessary space before arguments

[Tobin C. Harding]

checkpatch emits WARNING: Unnecessary space before function pointer
arguments.

Remove unnecessary space before function arguments.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 012c6b8..09fdb7c 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -654,7 +654,7 @@ void lc_set(struct lru_cache *lc, unsigned int enr, int index)
  * Note: a leading space ' ' and trailing newline '\n' is implied.
  */
 void lc_seq_dump_details(struct seq_file *seq, struct lru_cache *lc, char *utext,
-			 void (*detail) (struct seq_file *, struct lc_element *))
+			 void (*detail)(struct seq_file *, struct lc_element *))
 {
 	unsigned int nr_elements = lc->nr_elements;
 	struct lc_element *e;
-- 
2.7.4

[Drbd-dev] [PATCH 16/17] lru_cache: add spaces around '|'

[Tobin C. Harding]

checkpatch emits CHECK: spaces preferred around that '|' (ctx:VxV)

Add spaces around '|'.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 09fdb7c..5acfbe0 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -518,7 +518,8 @@ struct lc_element *lc_get(struct lru_cache *lc, unsigned int enr)
  */
 struct lc_element *lc_get_cumulative(struct lru_cache *lc, unsigned int enr)
 {
-	return __lc_get(lc, enr, LC_GET_MAY_CHANGE|LC_GET_MAY_USE_UNCOMMITTED);
+	return __lc_get(lc, enr,
+			LC_GET_MAY_CHANGE | LC_GET_MAY_USE_UNCOMMITTED);
 }
 
 /**
-- 
2.7.4

[Drbd-dev] [PATCH 17/17] lru_cache: move EXPORT_SYMBOL macro to follow function

[Tobin C. Harding]

checkpatch emits WARNING: EXPORT_SYMBOL(foo); should immediately follow
its function/variable.

Move EXPORT_SYMBOL macro call to immediately follow function definition.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/lru_cache.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 5acfbe0..9d20e24 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -84,6 +84,7 @@ int lc_try_lock(struct lru_cache *lc)
 	return old == val;
 #endif
 }
+EXPORT_SYMBOL(lc_try_lock);
 
 /**
  * lc_create - prepares to track objects in an active set
@@ -169,6 +170,7 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
 	kfree(slot);
 	return NULL;
 }
+EXPORT_SYMBOL(lc_create);
 
 static void lc_free_by_index(struct lru_cache *lc, unsigned int i)
 {
@@ -197,6 +199,7 @@ void lc_destroy(struct lru_cache *lc)
 	kfree(lc->lc_slot);
 	kfree(lc);
 }
+EXPORT_SYMBOL(lc_destroy);
 
 /**
  * lc_reset - does a full reset for @lc and the hash table slots.
@@ -235,6 +238,7 @@ void lc_reset(struct lru_cache *lc)
 		list_add(&e->list, &lc->free);
 	}
 }
+EXPORT_SYMBOL(lc_reset);
 
 /**
  * lc_seq_printf_stats - print stats about @lc into @seq
@@ -254,6 +258,7 @@ void lc_seq_printf_stats(struct seq_file *seq, struct lru_cache *lc)
 		   lc->name, lc->used, lc->nr_elements,
 		   lc->hits, lc->misses, lc->starving, lc->locked, lc->changed);
 }
+EXPORT_SYMBOL(lc_seq_printf_stats);
 
 static struct hlist_head *lc_hash_slot(struct lru_cache *lc, unsigned int enr)
 {
@@ -296,6 +301,7 @@ struct lc_element *lc_find(struct lru_cache *lc, unsigned int enr)
 {
 	return __lc_find(lc, enr, 0);
 }
+EXPORT_SYMBOL(lc_find);
 
 /**
  * lc_is_used - find element by label
@@ -312,6 +318,7 @@ bool lc_is_used(struct lru_cache *lc, unsigned int enr)
 	struct lc_element *e = __lc_find(lc, enr, 1);
 	return e && e->refcnt;
 }
+EXPORT_SYMBOL(lc_is_used);
 
 /**
  * lc_del - removes an element from the cache
@@ -332,6 +339,7 @@ void lc_del(struct lru_cache *lc, struct lc_element *e)
 	list_move(&e->list, &lc->free);
 	RETURN();
 }
+EXPORT_SYMBOL(lc_del);
 
 static struct lc_element *lc_prepare_for_change(struct lru_cache *lc, unsigned int new_number)
 {
@@ -500,6 +508,7 @@ struct lc_element *lc_get(struct lru_cache *lc, unsigned int enr)
 {
 	return __lc_get(lc, enr, LC_GET_MAY_CHANGE);
 }
+EXPORT_SYMBOL(lc_get);
 
 /**
  * lc_get_cumulative - like lc_get; also finds to-be-changed elements
@@ -521,6 +530,7 @@ struct lc_element *lc_get_cumulative(struct lru_cache *lc, unsigned int enr)
 	return __lc_get(lc, enr,
 			LC_GET_MAY_CHANGE | LC_GET_MAY_USE_UNCOMMITTED);
 }
+EXPORT_SYMBOL(lc_get_cumulative);
 
 /**
  * lc_try_get - get element by label, if present; do not change the active set
@@ -542,6 +552,7 @@ struct lc_element *lc_try_get(struct lru_cache *lc, unsigned int enr)
 {
 	return __lc_get(lc, enr, 0);
 }
+EXPORT_SYMBOL(lc_try_get);
 
 /**
  * lc_committed - tell @lc that pending changes have been recorded
@@ -565,6 +576,7 @@ void lc_committed(struct lru_cache *lc)
 	lc->pending_changes = 0;
 	RETURN();
 }
+EXPORT_SYMBOL(lc_committed);
 
 /**
  * lc_put - give up refcnt of @e
@@ -589,6 +601,7 @@ unsigned int lc_put(struct lru_cache *lc, struct lc_element *e)
 	}
 	RETURN(e->refcnt);
 }
+EXPORT_SYMBOL(lc_put);
 
 /**
  * lc_element_by_index
@@ -602,6 +615,7 @@ struct lc_element *lc_element_by_index(struct lru_cache *lc, unsigned int i)
 	BUG_ON(lc->lc_element[i]->lc_index != i);
 	return lc->lc_element[i];
 }
+EXPORT_SYMBOL(lc_element_by_index);
 
 /**
  * lc_index_of
@@ -613,6 +627,7 @@ unsigned int lc_index_of(struct lru_cache *lc, struct lc_element *e)
 	PARANOIA_LC_ELEMENT(lc, e);
 	return e->lc_index;
 }
+EXPORT_SYMBOL(lc_index_of);
 
 /**
  * lc_set - associate index with label
@@ -644,6 +659,7 @@ void lc_set(struct lru_cache *lc, unsigned int enr, int index)
 	}
 	list_move(&e->list, lh);
 }
+EXPORT_SYMBOL(lc_set);
 
 /**
  * lc_dump - Dump a complete LRU cache to seq in textual form.
@@ -675,21 +691,4 @@ void lc_seq_dump_details(struct seq_file *seq, struct lru_cache *lc, char *utext
 		seq_putc(seq, '\n');
 	}
 }
-
-EXPORT_SYMBOL(lc_create);
-EXPORT_SYMBOL(lc_reset);
-EXPORT_SYMBOL(lc_destroy);
-EXPORT_SYMBOL(lc_set);
-EXPORT_SYMBOL(lc_del);
-EXPORT_SYMBOL(lc_try_get);
-EXPORT_SYMBOL(lc_find);
-EXPORT_SYMBOL(lc_get);
-EXPORT_SYMBOL(lc_put);
-EXPORT_SYMBOL(lc_committed);
-EXPORT_SYMBOL(lc_element_by_index);
-EXPORT_SYMBOL(lc_index_of);
-EXPORT_SYMBOL(lc_seq_printf_stats);
 EXPORT_SYMBOL(lc_seq_dump_details);
-EXPORT_SYMBOL(lc_try_lock);
-EXPORT_SYMBOL(lc_is_used);
-EXPORT_SYMBOL(lc_get_cumulative);
-- 
2.7.4

Re: [Drbd-dev] [PATCH 00/17] lru_cache: checkpatch clean ups

[Lars Ellenberg]

On Mon, Oct 02, 2017 at 09:33:59AM +1100, Tobin C. Harding wrote:
> Checkpatch clean ups for lib/lru_cache.c
> 
> This patch set is intended to test the waters as to whether DRBD
> wants/accepts checkpatch clean ups. I know that many maintainers
> consider such patches as code churn.

I'd consider them such.
Probably nothing wrong with those patches, really,
though I did not look very closely.

I'd maybe do this kind of cleanup IF I was to touch the code anyways.

> I am taking a look at the DRBD code after bumping into Brian at OSSNA in LA.
> 
> If you do want/like checkpatch fixes I am happy to do so for drivers/block/drbd/* while I look
> around the code base.
> 
> I don't know which tree you guys are working off.
> This series applies on top of Linus' development
> tree, commit:
> 
> a8c964eacb21288b2dbfa9d80cee5968a3b8fb21

Our "upstream" is out-of-tree still,
and tries to be compatible with a wide range of kernel.org tree versions.
We recently move the public master to github.com/LINBIT/drbd-8.4/
and it looks in some parts quite different from the in kernel code,
for compatilibity reasons.

But for this kind of cleanup stuff,
kernel.org master is the right reference, yes.

As I said, I don't care much for "no-op" commits, and even if I'd take
them, I'd fold them all into one "fixes for checkpatch complaints".

But don't let that keep you from "looking around the code base",
maybe you still spot things that actually need fixing
or can otherwise be improved.  It's not that we don't like contributors,
it's just that contributing "white-space only changes" to C code won't
earn you much points in my book ;-)

Thanks,

    Lars

Re: [Drbd-dev] [PATCH 00/17] lru_cache: checkpatch clean ups

[Tobin C. Harding]

On Mon, Oct 02, 2017 at 03:13:36PM +0200, Lars Ellenberg wrote:
> On Mon, Oct 02, 2017 at 09:33:59AM +1100, Tobin C. Harding wrote:
> > Checkpatch clean ups for lib/lru_cache.c
> > 
> > This patch set is intended to test the waters as to whether DRBD
> > wants/accepts checkpatch clean ups. I know that many maintainers
> > consider such patches as code churn.
> 
> I'd consider them such.
> Probably nothing wrong with those patches, really,
> though I did not look very closely.
> 
> I'd maybe do this kind of cleanup IF I was to touch the code anyways.

Point noted, thank you.

> > I am taking a look at the DRBD code after bumping into Brian at OSSNA in LA.
> > 
> > If you do want/like checkpatch fixes I am happy to do so for drivers/block/drbd/* while I look
> > around the code base.
> > 
> > I don't know which tree you guys are working off.
> > This series applies on top of Linus' development
> > tree, commit:
> > 
> > a8c964eacb21288b2dbfa9d80cee5968a3b8fb21
> 
> Our "upstream" is out-of-tree still,
> and tries to be compatible with a wide range of kernel.org tree versions.
> We recently move the public master to github.com/LINBIT/drbd-8.4/
> and it looks in some parts quite different from the in kernel code,
> for compatilibity reasons.

cloned.

> But for this kind of cleanup stuff,
> kernel.org master is the right reference, yes.
> 
> As I said, I don't care much for "no-op" commits, and even if I'd take
> them, I'd fold them all into one "fixes for checkpatch complaints".
> 
> But don't let that keep you from "looking around the code base",
> maybe you still spot things that actually need fixing
> or can otherwise be improved.  It's not that we don't like contributors,
> it's just that contributing "white-space only changes" to C code won't
> earn you much points in my book ;-)

Thanks for your comments. Let's consider this set dropped.

thanks,
Tobin