[PATCH 00/13] Assorted lustre clean-ups

[PATCH 00/13] Assorted lustre clean-ups

[NeilBrown]

This series includes a resend of some "list_entry" cleanup
patches which now how better commit comments, and also
includes some simplifications, removing unnecessary macros
and code.

Thanks,
NeilBrown

---

NeilBrown (12):
      staging: lustre: use list_last_entry to simplify fld_cache_shrink
      staging: lustre: ldlm: use list_for_each_entry in ldlm_extent_shift_kms()
      staging: lustre: ldlm: use list_first_entry in ldlm_lockd.c
      staging: lustre: ldlm: minor list_entry improvements in ldlm_request.c
      staging: lustre: ldlm: use list_for_each_entry in ldlm_resource.c
      staging: lustre: lov: use list_for_each_entry in lov_obd.c
      staging: lustre: libcfs: simplify memory allocation.
      staging: lustre: libcfs: remove unused rounding functions.
      staging: lustre: libcfs: discard MKSTR() macro
      staging: lustre: libcfs: discard MAX_NUMERIC_VALUE
      staging: lustre: libcfs: discard KLASSERT()
      staging: lustre: libcfs: discard LASSERT_CHECKED


 .../lustre/include/linux/libcfs/libcfs_private.h   |   91 +++-----------------
 drivers/staging/lustre/lustre/fld/fld_cache.c      |   13 +--
 drivers/staging/lustre/lustre/ldlm/ldlm_extent.c   |    4 -
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c    |   10 +-
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |   12 +--
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |   20 ++--
 drivers/staging/lustre/lustre/lov/lov_obd.c        |    6 -
 drivers/staging/lustre/lustre/obdclass/cl_page.c   |    4 -
 .../staging/lustre/lustre/obdclass/obd_config.c    |    2 
 9 files changed, 41 insertions(+), 121 deletions(-)

--
Signature

[PATCH 04/12] staging: lustre: ldlm: minor list_entry improvements in ldlm_request.c

[NeilBrown]

Small clarify improvements, and one local variable avoided.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
index a9a1551cc9bc..a1b909004de8 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
@@ -1653,7 +1653,7 @@ int ldlm_cli_cancel_list(struct list_head *cancels, int count,
 	 */
 	while (count > 0) {
 		LASSERT(!list_empty(cancels));
-		lock = list_entry(cancels->next, struct ldlm_lock, l_bl_ast);
+		lock = list_first_entry(cancels, struct ldlm_lock, l_bl_ast);
 		LASSERT(lock->l_conn_export);
 
 		if (exp_connect_cancelset(lock->l_conn_export)) {
@@ -1777,7 +1777,7 @@ EXPORT_SYMBOL(ldlm_cli_cancel_unused);
 static int ldlm_resource_foreach(struct ldlm_resource *res,
 				 ldlm_iterator_t iter, void *closure)
 {
-	struct list_head *tmp, *next;
+	struct ldlm_lock *tmp;
 	struct ldlm_lock *lock;
 	int rc = LDLM_ITER_CONTINUE;
 
@@ -1785,18 +1785,14 @@ static int ldlm_resource_foreach(struct ldlm_resource *res,
 		return LDLM_ITER_CONTINUE;
 
 	lock_res(res);
-	list_for_each_safe(tmp, next, &res->lr_granted) {
-		lock = list_entry(tmp, struct ldlm_lock, l_res_link);
-
+	list_for_each_entry_safe(lock, tmp, &res->lr_granted, l_res_link) {
 		if (iter(lock, closure) == LDLM_ITER_STOP) {
 			rc = LDLM_ITER_STOP;
 			goto out;
 		}
 	}
 
-	list_for_each_safe(tmp, next, &res->lr_waiting) {
-		lock = list_entry(tmp, struct ldlm_lock, l_res_link);
-
+	list_for_each_entry_safe(lock, tmp, &res->lr_waiting, l_res_link) {
 		if (iter(lock, closure) == LDLM_ITER_STOP) {
 			rc = LDLM_ITER_STOP;
 			goto out;

[PATCH 06/12] staging: lustre: lov: use list_for_each_entry in lov_obd.c

[NeilBrown]

Using the *_entry macro simplifies the code slightly.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/lov/lov_obd.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
index 7ce01026a409..ec70c12e5b40 100644
--- a/drivers/staging/lustre/lustre/lov/lov_obd.c
+++ b/drivers/staging/lustre/lustre/lov/lov_obd.c
@@ -828,11 +828,9 @@ int lov_setup(struct obd_device *obd, struct lustre_cfg *lcfg)
 static int lov_cleanup(struct obd_device *obd)
 {
 	struct lov_obd *lov = &obd->u.lov;
-	struct list_head *pos, *tmp;
-	struct pool_desc *pool;
+	struct pool_desc *pool, *tmp;
 
-	list_for_each_safe(pos, tmp, &lov->lov_pool_list) {
-		pool = list_entry(pos, struct pool_desc, pool_list);
+	list_for_each_entry_safe(pool, tmp, &lov->lov_pool_list, pool_list) {
 		/* free pool structs */
 		CDEBUG(D_INFO, "delete pool %p\n", pool);
 		/* In the function below, .hs_keycmp resolves to

[PATCH 01/12] staging: lustre: use list_last_entry to simplify fld_cache_shrink

[NeilBrown]

Using list_empty() and list_last_entry() makes the code clearer,
and allows a local variable to be discarded.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/fld/fld_cache.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/lustre/lustre/fld/fld_cache.c b/drivers/staging/lustre/lustre/fld/fld_cache.c
index 7d6a7106c0a5..ecf8b9e1ed5c 100644
--- a/drivers/staging/lustre/lustre/fld/fld_cache.c
+++ b/drivers/staging/lustre/lustre/fld/fld_cache.c
@@ -213,19 +213,18 @@ static inline void fld_cache_entry_add(struct fld_cache *cache,
  */
 static int fld_cache_shrink(struct fld_cache *cache)
 {
-	struct fld_cache_entry *flde;
-	struct list_head *curr;
 	int num = 0;
 
 	if (cache->fci_cache_count < cache->fci_cache_size)
 		return 0;
 
-	curr = cache->fci_lru.prev;
-
 	while (cache->fci_cache_count + cache->fci_threshold >
-	       cache->fci_cache_size && curr != &cache->fci_lru) {
-		flde = list_entry(curr, struct fld_cache_entry, fce_lru);
-		curr = curr->prev;
+	       cache->fci_cache_size &&
+	       !list_empty(&cache->fci_lru)) {
+		struct fld_cache_entry *flde =
+			list_last_entry(&cache->fci_lru,
+					struct fld_cache_entry, fce_lru);
+
 		fld_cache_entry_delete(cache, flde);
 		num++;
 	}

[PATCH 02/12] staging: lustre: ldlm: use list_for_each_entry in ldlm_extent_shift_kms()

[NeilBrown]

Using list_for_each_entry() means we don't need 'tmp'.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_extent.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c b/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
index fac9d19d50b6..11b11b5f3216 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
@@ -64,7 +64,6 @@
 __u64 ldlm_extent_shift_kms(struct ldlm_lock *lock, __u64 old_kms)
 {
 	struct ldlm_resource *res = lock->l_resource;
-	struct list_head *tmp;
 	struct ldlm_lock *lck;
 	__u64 kms = 0;
 
@@ -74,8 +73,7 @@ __u64 ldlm_extent_shift_kms(struct ldlm_lock *lock, __u64 old_kms)
 	 */
 	ldlm_set_kms_ignore(lock);
 
-	list_for_each(tmp, &res->lr_granted) {
-		lck = list_entry(tmp, struct ldlm_lock, l_res_link);
+	list_for_each_entry(lck, &res->lr_granted, l_res_link) {
 
 		if (ldlm_is_kms_ignore(lck))
 			continue;

[PATCH 05/12] staging: lustre: ldlm: use list_for_each_entry in ldlm_resource.c

[NeilBrown]

Having a stand-alone "list_entry()" call is often a sign
that something like "list_for_each_entry()" would
make the code clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |   20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
index 2689ffdf10e3..9958533cc227 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
@@ -752,24 +752,22 @@ extern struct ldlm_lock *ldlm_lock_get(struct ldlm_lock *lock);
 static void cleanup_resource(struct ldlm_resource *res, struct list_head *q,
 			     __u64 flags)
 {
-	struct list_head *tmp;
 	int rc = 0;
 	bool local_only = !!(flags & LDLM_FL_LOCAL_ONLY);
 
 	do {
-		struct ldlm_lock *lock = NULL;
+		struct ldlm_lock *lock = NULL, *tmp;
 		struct lustre_handle lockh;
 
 		/* First, we look for non-cleaned-yet lock
 		 * all cleaned locks are marked by CLEANED flag.
 		 */
 		lock_res(res);
-		list_for_each(tmp, q) {
-			lock = list_entry(tmp, struct ldlm_lock, l_res_link);
-			if (ldlm_is_cleaned(lock)) {
-				lock = NULL;
+		list_for_each_entry(tmp, q, l_res_link) {
+			if (ldlm_is_cleaned(tmp))
 				continue;
-			}
+
+			lock = tmp;
 			LDLM_LOCK_GET(lock);
 			ldlm_set_cleaned(lock);
 			break;
@@ -1283,19 +1281,15 @@ void ldlm_res2desc(struct ldlm_resource *res, struct ldlm_resource_desc *desc)
  */
 void ldlm_dump_all_namespaces(enum ldlm_side client, int level)
 {
-	struct list_head *tmp;
+	struct ldlm_namespace *ns;
 
 	if (!((libcfs_debug | D_ERROR) & level))
 		return;
 
 	mutex_lock(ldlm_namespace_lock(client));
 
-	list_for_each(tmp, ldlm_namespace_list(client)) {
-		struct ldlm_namespace *ns;
-
-		ns = list_entry(tmp, struct ldlm_namespace, ns_list_chain);
+	list_for_each_entry(ns, ldlm_namespace_list(client), ns_list_chain)
 		ldlm_namespace_dump(level, ns);
-	}
 
 	mutex_unlock(ldlm_namespace_lock(client));
 }

[PATCH 03/12] staging: lustre: ldlm: use list_first_entry in ldlm_lockd.c

[NeilBrown]

This is only a small simplification, but it makes the code
a little clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
index ada50b69c5f6..5f6e7c933b81 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
@@ -696,13 +696,13 @@ static int ldlm_bl_get_work(struct ldlm_bl_pool *blp,
 	/* process a request from the blp_list at least every blp_num_threads */
 	if (!list_empty(&blp->blp_list) &&
 	    (list_empty(&blp->blp_prio_list) || num_bl == 0))
-		blwi = list_entry(blp->blp_list.next,
-				  struct ldlm_bl_work_item, blwi_entry);
+		blwi = list_first_entry(&blp->blp_list,
+					struct ldlm_bl_work_item, blwi_entry);
 	else
 		if (!list_empty(&blp->blp_prio_list))
-			blwi = list_entry(blp->blp_prio_list.next,
-					  struct ldlm_bl_work_item,
-					  blwi_entry);
+			blwi = list_first_entry(&blp->blp_prio_list,
+						struct ldlm_bl_work_item,
+						blwi_entry);
 
 	if (blwi) {
 		if (++num_bl >= num_th)

[PATCH 08/12] staging: lustre: libcfs: remove unused rounding functions.

[NeilBrown]

These are all unused except cfs_size_round().
So discard the others, and use the kernel-standard round_up()
function to implement cfs_size_round().

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/include/linux/libcfs/libcfs_private.h   |   29 +-------------------
 1 file changed, 1 insertion(+), 28 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index c874f9d15c72..dee5f650197f 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -243,40 +243,13 @@ do {							    \
 
 #define MKSTR(ptr) ((ptr)) ? (ptr) : ""
 
-static inline size_t cfs_size_round4(int val)
-{
-	return (val + 3) & (~0x3);
-}
-
 #ifndef HAVE_CFS_SIZE_ROUND
 static inline size_t cfs_size_round(int val)
 {
-	return (val + 7) & (~0x7);
+	return round_up(val, 8);
 }
 
 #define HAVE_CFS_SIZE_ROUND
 #endif
 
-static inline size_t cfs_size_round16(int val)
-{
-	return (val + 0xf) & (~0xf);
-}
-
-static inline size_t cfs_size_round32(int val)
-{
-	return (val + 0x1f) & (~0x1f);
-}
-
-static inline size_t cfs_size_round0(int val)
-{
-	if (!val)
-		return 0;
-	return (val + 1 + 7) & (~0x7);
-}
-
-static inline size_t cfs_round_strlen(char *fset)
-{
-	return cfs_size_round((int)strlen(fset) + 1);
-}
-
 #endif

[PATCH 07/12] staging: lustre: libcfs: simplify memory allocation.

[NeilBrown]

1/ Use kvmalloc() instead of kmalloc or vmalloc
2/ Discard the _GFP() interfaces that are never used.
  We only ever do GFP_NOFS and GFP_ATOMIC allocations,
  so support each of those explicitly.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/include/linux/libcfs/libcfs_private.h   |   42 ++++++--------------
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index 2f4ff595fac9..c874f9d15c72 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -84,14 +84,6 @@ do {								    \
 	lbug_with_loc(&msgdata);					\
 } while (0)
 
-#ifndef LIBCFS_VMALLOC_SIZE
-#define LIBCFS_VMALLOC_SIZE	(2 << PAGE_SHIFT) /* 2 pages */
-#endif
-
-#define LIBCFS_ALLOC_PRE(size, mask)					\
-	LASSERT(!in_interrupt() || ((size) <= LIBCFS_VMALLOC_SIZE &&	\
-				    !gfpflags_allow_blocking(mask)))
-
 #define LIBCFS_ALLOC_POST(ptr, size)					    \
 do {									    \
 	if (unlikely(!(ptr))) {						    \
@@ -103,46 +95,36 @@ do {									    \
 } while (0)
 
 /**
- * allocate memory with GFP flags @mask
+ * default allocator
  */
-#define LIBCFS_ALLOC_GFP(ptr, size, mask)				    \
+#define LIBCFS_ALLOC(ptr, size)						    \
 do {									    \
-	LIBCFS_ALLOC_PRE((size), (mask));				    \
-	(ptr) = (size) <= LIBCFS_VMALLOC_SIZE ?				    \
-		kmalloc((size), (mask)) : vmalloc(size);	    \
+	LASSERT(!in_interrupt());					    \
+	(ptr) = kvmalloc((size), GFP_NOFS);				    \
 	LIBCFS_ALLOC_POST((ptr), (size));				    \
 } while (0)
 
-/**
- * default allocator
- */
-#define LIBCFS_ALLOC(ptr, size) \
-	LIBCFS_ALLOC_GFP(ptr, size, GFP_NOFS)
-
 /**
  * non-sleeping allocator
  */
-#define LIBCFS_ALLOC_ATOMIC(ptr, size) \
-	LIBCFS_ALLOC_GFP(ptr, size, GFP_ATOMIC)
+#define LIBCFS_ALLOC_ATOMIC(ptr, size)					\
+do {									\
+	(ptr) = kmalloc((size), GFP_ATOMIC);				\
+	LIBCFS_ALLOC_POST(ptr, size);					\
+} while (0)
 
 /**
  * allocate memory for specified CPU partition
  *   \a cptab != NULL, \a cpt is CPU partition id of \a cptab
  *   \a cptab == NULL, \a cpt is HW NUMA node id
  */
-#define LIBCFS_CPT_ALLOC_GFP(ptr, cptab, cpt, size, mask)		    \
+#define LIBCFS_CPT_ALLOC(ptr, cptab, cpt, size)				    \
 do {									    \
-	LIBCFS_ALLOC_PRE((size), (mask));				    \
-	(ptr) = (size) <= LIBCFS_VMALLOC_SIZE ?				    \
-		kmalloc_node((size), (mask), cfs_cpt_spread_node(cptab, cpt)) :\
-		vmalloc_node(size, cfs_cpt_spread_node(cptab, cpt));	    \
+	LASSERT(!in_interrupt());					    \
+	(ptr) = kvmalloc_node((size), GFP_NOFS, cfs_cpt_spread_node(cptab, cpt)); \
 	LIBCFS_ALLOC_POST((ptr), (size));				    \
 } while (0)
 
-/** default numa allocator */
-#define LIBCFS_CPT_ALLOC(ptr, cptab, cpt, size)				    \
-	LIBCFS_CPT_ALLOC_GFP(ptr, cptab, cpt, size, GFP_NOFS)
-
 #define LIBCFS_FREE(ptr, size)					  \
 do {								    \
 	if (unlikely(!(ptr))) {						\

Re: [PATCH 07/12] staging: lustre: libcfs: simplify memory allocation.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 3989 bytes --]

On Wed, Dec 13 2017, NeilBrown wrote:

> 1/ Use kvmalloc() instead of kmalloc or vmalloc
> 2/ Discard the _GFP() interfaces that are never used.
>   We only ever do GFP_NOFS and GFP_ATOMIC allocations,
>   so support each of those explicitly.

Hi,
 I just remembered that posting this patch was, maybe, a little premature.
vmalloc() doesn't support GFP_NOFS, so kvmalloc() warns about an attempt
to use GFP_NOFS.
lustre potentially used vmalloc in GFP_NOFS context, and so now calls
kvmalloc() with GFP_NOFS, which triggers a warning.

I haven't yet looking into how to remove the warning.  Maybe all
GFP_NOFS usages should use kmalloc(), not kvmalloc(), and accept the
increased chance of failure.
I'll dig deeper and hope to have a clearer opinion soon.

As it is only a warning, it is probably OK to keep the patch in
staging-next, but I wouldn't object if it was dropped.

Thanks,
NeilBrown

 

>
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../lustre/include/linux/libcfs/libcfs_private.h   |   42 ++++++--------------
>  1 file changed, 12 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> index 2f4ff595fac9..c874f9d15c72 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> @@ -84,14 +84,6 @@ do {								    \
>  	lbug_with_loc(&msgdata);					\
>  } while (0)
>  
> -#ifndef LIBCFS_VMALLOC_SIZE
> -#define LIBCFS_VMALLOC_SIZE	(2 << PAGE_SHIFT) /* 2 pages */
> -#endif
> -
> -#define LIBCFS_ALLOC_PRE(size, mask)					\
> -	LASSERT(!in_interrupt() || ((size) <= LIBCFS_VMALLOC_SIZE &&	\
> -				    !gfpflags_allow_blocking(mask)))
> -
>  #define LIBCFS_ALLOC_POST(ptr, size)					    \
>  do {									    \
>  	if (unlikely(!(ptr))) {						    \
> @@ -103,46 +95,36 @@ do {									    \
>  } while (0)
>  
>  /**
> - * allocate memory with GFP flags @mask
> + * default allocator
>   */
> -#define LIBCFS_ALLOC_GFP(ptr, size, mask)				    \
> +#define LIBCFS_ALLOC(ptr, size)						    \
>  do {									    \
> -	LIBCFS_ALLOC_PRE((size), (mask));				    \
> -	(ptr) = (size) <= LIBCFS_VMALLOC_SIZE ?				    \
> -		kmalloc((size), (mask)) : vmalloc(size);	    \
> +	LASSERT(!in_interrupt());					    \
> +	(ptr) = kvmalloc((size), GFP_NOFS);				    \
>  	LIBCFS_ALLOC_POST((ptr), (size));				    \
>  } while (0)
>  
> -/**
> - * default allocator
> - */
> -#define LIBCFS_ALLOC(ptr, size) \
> -	LIBCFS_ALLOC_GFP(ptr, size, GFP_NOFS)
> -
>  /**
>   * non-sleeping allocator
>   */
> -#define LIBCFS_ALLOC_ATOMIC(ptr, size) \
> -	LIBCFS_ALLOC_GFP(ptr, size, GFP_ATOMIC)
> +#define LIBCFS_ALLOC_ATOMIC(ptr, size)					\
> +do {									\
> +	(ptr) = kmalloc((size), GFP_ATOMIC);				\
> +	LIBCFS_ALLOC_POST(ptr, size);					\
> +} while (0)
>  
>  /**
>   * allocate memory for specified CPU partition
>   *   \a cptab != NULL, \a cpt is CPU partition id of \a cptab
>   *   \a cptab == NULL, \a cpt is HW NUMA node id
>   */
> -#define LIBCFS_CPT_ALLOC_GFP(ptr, cptab, cpt, size, mask)		    \
> +#define LIBCFS_CPT_ALLOC(ptr, cptab, cpt, size)				    \
>  do {									    \
> -	LIBCFS_ALLOC_PRE((size), (mask));				    \
> -	(ptr) = (size) <= LIBCFS_VMALLOC_SIZE ?				    \
> -		kmalloc_node((size), (mask), cfs_cpt_spread_node(cptab, cpt)) :\
> -		vmalloc_node(size, cfs_cpt_spread_node(cptab, cpt));	    \
> +	LASSERT(!in_interrupt());					    \
> +	(ptr) = kvmalloc_node((size), GFP_NOFS, cfs_cpt_spread_node(cptab, cpt)); \
>  	LIBCFS_ALLOC_POST((ptr), (size));				    \
>  } while (0)
>  
> -/** default numa allocator */
> -#define LIBCFS_CPT_ALLOC(ptr, cptab, cpt, size)				    \
> -	LIBCFS_CPT_ALLOC_GFP(ptr, cptab, cpt, size, GFP_NOFS)
> -
>  #define LIBCFS_FREE(ptr, size)					  \
>  do {								    \
>  	if (unlikely(!(ptr))) {						\

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[PATCH 09/12] staging: lustre: libcfs: discard MKSTR() macro

[NeilBrown]

This is only used for tracing when some strings might
be NULL.  NULL strings are not a problem for tracing,
vnsprintf() will report them as "(null)" which is probably
better (easier to parse) than an empty string.

Also remove a nearby comment that doesn't relate to the
(remaining) code at all.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/include/linux/libcfs/libcfs_private.h   |    8 --------
 .../staging/lustre/lustre/obdclass/obd_config.c    |    2 +-
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index dee5f650197f..27d40a7589d4 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -235,14 +235,6 @@ do {							    \
 /* logical equivalence */
 #define equi(a, b) (!!(a) == !!(b))
 
-/* --------------------------------------------------------------------
- * Light-weight trace
- * Support for temporary event tracing with minimal Heisenberg effect.
- * --------------------------------------------------------------------
- */
-
-#define MKSTR(ptr) ((ptr)) ? (ptr) : ""
-
 #ifndef HAVE_CFS_SIZE_ROUND
 static inline size_t cfs_size_round(int val)
 {
diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
index c0e192ae22a9..997c0f9aafb5 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
@@ -236,7 +236,7 @@ static int class_attach(struct lustre_cfg *lcfg)
 	uuid = lustre_cfg_string(lcfg, 2);
 
 	CDEBUG(D_IOCTL, "attach type %s name: %s uuid: %s\n",
-	       MKSTR(typename), MKSTR(name), MKSTR(uuid));
+	       typename, name, uuid);
 
 	obd = class_newdev(typename, name);
 	if (IS_ERR(obd)) {

[PATCH 11/12] staging: lustre: libcfs: discard KLASSERT()

[NeilBrown]

This appears to be intended for assertions that only make
sense in the kernel, but as this code is now kernel-only,
it doesn't make sense any more.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/include/linux/libcfs/libcfs_private.h   |    2 --
 drivers/staging/lustre/lustre/obdclass/cl_page.c   |    4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index d32ce68e6c58..31403667be6b 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -74,8 +74,6 @@ do {									\
 # define LINVRNT(exp) ((void)sizeof !!(exp))
 #endif
 
-#define KLASSERT(e) LASSERT(e)
-
 void __noreturn lbug_with_loc(struct libcfs_debug_msg_data *msg);
 
 #define LBUG()							  \
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
index 7f65439f9b95..d3b25667bc3a 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
@@ -202,7 +202,7 @@ struct cl_page *cl_page_find(const struct lu_env *env,
 		 * vmpage lock is used to protect the child/parent
 		 * relationship
 		 */
-		KLASSERT(PageLocked(vmpage));
+		LASSERT(PageLocked(vmpage));
 		/*
 		 * cl_vmpage_page() can be called here without any locks as
 		 *
@@ -340,7 +340,7 @@ struct cl_page *cl_vmpage_page(struct page *vmpage, struct cl_object *obj)
 {
 	struct cl_page *page;
 
-	KLASSERT(PageLocked(vmpage));
+	LASSERT(PageLocked(vmpage));
 
 	/*
 	 * NOTE: absence of races and liveness of data are guaranteed by page

[PATCH 12/12] staging: lustre: libcfs: discard LASSERT_CHECKED

[NeilBrown]

This macro isn't used, and comment is about some earlier version
of the lustre code that never reached the mainline kernel.
Just discard it.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/include/linux/libcfs/libcfs_private.h   |    7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index 31403667be6b..940200ee632e 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -43,13 +43,6 @@
 # define DEBUG_SUBSYSTEM S_UNDEFINED
 #endif
 
-/*
- * When this is on, LASSERT macro includes check for assignment used instead
- * of equality check, but doesn't have unlikely(). Turn this on from time to
- * time to make test-builds. This shouldn't be on for production release.
- */
-#define LASSERT_CHECKED (0)
-
 #define LASSERTF(cond, fmt, ...)					\
 do {									\
 	if (unlikely(!(cond))) {					\

Re: [lustre-devel] [PATCH 12/12] staging: lustre: libcfs: discard LASSERT_CHECKED

[Luis de Bethencourt]

On 12/13/2017 03:33 AM, Patrick Farrell wrote:
> Thanks, Neil - lots of small but nice cleanups.
> 
> Full series acked.
> ________________________________
> From: lustre-devel <lustre-devel-bounces@lists.lustre.org> on behalf of NeilBrown <neilb@suse.com>
> Sent: Tuesday, December 12, 2017 9:15:55 PM
> To: Oleg Drokin; James Simmons; Andreas Dilger; Greg Kroah-Hartman
> Cc: linux-kernel@vger.kernel.org; lustre-devel@lists.lustre.org
> Subject: [lustre-devel] [PATCH 12/12] staging: lustre: libcfs: discard LASSERT_CHECKED
> 
> This macro isn't used, and comment is about some earlier version
> of the lustre code that never reached the mainline kernel.
> Just discard it.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---

Thanks Neil! Nice cleanup indeed.

Acked-by: Luis de Bethencourt <luisbg@kernel.org>

[PATCH 10/12] staging: lustre: libcfs: discard MAX_NUMERIC_VALUE

[NeilBrown]

This is unused.
drivers/staging/lustre/lnet/lnet/nidstrings.c does use the name,
but it includes its own local definition.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/include/linux/libcfs/libcfs_private.h   |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index 27d40a7589d4..d32ce68e6c58 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -227,9 +227,6 @@ do {							    \
 #define CFS_ALLOC_PTR(ptr)      LIBCFS_ALLOC(ptr, sizeof(*(ptr)))
 #define CFS_FREE_PTR(ptr)       LIBCFS_FREE(ptr, sizeof(*(ptr)))
 
-/* max value for numeric network address */
-#define MAX_NUMERIC_VALUE 0xffffffff
-
 /* implication */
 #define ergo(a, b) (!(a) || (b))
 /* logical equivalence */