All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] Use !x to check for kzalloc failure
@ 2015-06-20 16:58 ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-janitors, devel, HPDD-discuss, Greg Kroah-Hartman,
	Andreas Dilger, Oleg Drokin

Use !x to check for kzalloc failure, as is more normal in the kernel.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 00/12] Use !x to check for kzalloc failure
@ 2015-06-20 16:58 ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-janitors, devel, HPDD-discuss, Greg Kroah-Hartman,
	Andreas Dilger, Oleg Drokin

Use !x to check for kzalloc failure, as is more normal in the kernel.

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:58   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:58 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c b/drivers/staging/lustre/lustre/fid/fid_request.c
--- a/drivers/staging/lustre/lustre/fid/fid_request.c
+++ b/drivers/staging/lustre/lustre/fid/fid_request.c
@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o
 	int rc;
 
 	cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);
-	if (cli->cl_seq == NULL)
+	if (!cli->cl_seq)
 		return -ENOMEM;
 
 	prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);
-	if (prefix == NULL) {
+	if (!prefix) {
 		rc = -ENOMEM;
 		goto out_free_seq;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-20 16:58   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:58 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c b/drivers/staging/lustre/lustre/fid/fid_request.c
--- a/drivers/staging/lustre/lustre/fid/fid_request.c
+++ b/drivers/staging/lustre/lustre/fid/fid_request.c
@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o
 	int rc;
 
 	cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);
-	if (cli->cl_seq = NULL)
+	if (!cli->cl_seq)
 		return -ENOMEM;
 
 	prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);
-	if (prefix = NULL) {
+	if (!prefix) {
 		rc = -ENOMEM;
 		goto out_free_seq;
 	}

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 02/12] staging: lustre: fld: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/fld/fld_cache.c   |    2 +-
 drivers/staging/lustre/lustre/fld/fld_request.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/fld/fld_request.c b/drivers/staging/lustre/lustre/fld/fld_request.c
--- a/drivers/staging/lustre/lustre/fld/fld_request.c
+++ b/drivers/staging/lustre/lustre/fld/fld_request.c
@@ -222,7 +222,7 @@ int fld_client_add_target(struct lu_clie
 			fld->lcf_name, name, tar->ft_idx);
 
 	target = kzalloc(sizeof(*target), GFP_NOFS);
-	if (target == NULL)
+	if (!target)
 		return -ENOMEM;
 
 	spin_lock(&fld->lcf_lock);
diff -u -p a/drivers/staging/lustre/lustre/fld/fld_cache.c b/drivers/staging/lustre/lustre/fld/fld_cache.c
--- a/drivers/staging/lustre/lustre/fld/fld_cache.c
+++ b/drivers/staging/lustre/lustre/fld/fld_cache.c
@@ -70,7 +70,7 @@ struct fld_cache *fld_cache_init(const c
 	LASSERT(cache_threshold < cache_size);
 
 	cache = kzalloc(sizeof(*cache), GFP_NOFS);
-	if (cache == NULL)
+	if (!cache)
 		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&cache->fci_entries_head);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 02/12] staging: lustre: fld: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/fld/fld_cache.c   |    2 +-
 drivers/staging/lustre/lustre/fld/fld_request.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/fld/fld_request.c b/drivers/staging/lustre/lustre/fld/fld_request.c
--- a/drivers/staging/lustre/lustre/fld/fld_request.c
+++ b/drivers/staging/lustre/lustre/fld/fld_request.c
@@ -222,7 +222,7 @@ int fld_client_add_target(struct lu_clie
 			fld->lcf_name, name, tar->ft_idx);
 
 	target = kzalloc(sizeof(*target), GFP_NOFS);
-	if (target = NULL)
+	if (!target)
 		return -ENOMEM;
 
 	spin_lock(&fld->lcf_lock);
diff -u -p a/drivers/staging/lustre/lustre/fld/fld_cache.c b/drivers/staging/lustre/lustre/fld/fld_cache.c
--- a/drivers/staging/lustre/lustre/fld/fld_cache.c
+++ b/drivers/staging/lustre/lustre/fld/fld_cache.c
@@ -70,7 +70,7 @@ struct fld_cache *fld_cache_init(const c
 	LASSERT(cache_threshold < cache_size);
 
 	cache = kzalloc(sizeof(*cache), GFP_NOFS);
-	if (cache = NULL)
+	if (!cache)
 		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&cache->fci_entries_head);

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 03/12] staging: lustre: lclient: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/lclient/lcommon_cl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/drivers/staging/lustre/lustre/lclient/lcommon_cl.c b/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
--- a/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
+++ b/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
@@ -203,7 +203,7 @@ struct lu_device *ccc_device_alloc(const
 	int rc;
 
 	vdv = kzalloc(sizeof(*vdv), GFP_NOFS);
-	if (vdv == NULL)
+	if (!vdv)
 		return ERR_PTR(-ENOMEM);
 
 	lud = &vdv->cdv_cl.cd_lu_dev;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 03/12] staging: lustre: lclient: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/lclient/lcommon_cl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/drivers/staging/lustre/lustre/lclient/lcommon_cl.c b/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
--- a/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
+++ b/drivers/staging/lustre/lustre/lclient/lcommon_cl.c
@@ -203,7 +203,7 @@ struct lu_device *ccc_device_alloc(const
 	int rc;
 
 	vdv = kzalloc(sizeof(*vdv), GFP_NOFS);
-	if (vdv = NULL)
+	if (!vdv)
 		return ERR_PTR(-ENOMEM);
 
 	lud = &vdv->cdv_cl.cd_lu_dev;

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 04/12] staging: lustre: ldlm: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c  |    4 ++--
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c |    8 ++++----
 drivers/staging/lustre/lustre/ldlm/ldlm_pool.c  |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
@@ -1422,7 +1422,7 @@ static int ldlm_pools_thread_start(void)
 		return -EALREADY;
 
 	ldlm_pools_thread = kzalloc(sizeof(*ldlm_pools_thread), GFP_NOFS);
-	if (ldlm_pools_thread == NULL)
+	if (!ldlm_pools_thread)
 		return -ENOMEM;
 
 	init_completion(&ldlm_pools_comp);
diff -u -p a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
@@ -225,7 +225,7 @@ static void ldlm_handle_cp_callback(stru
 			void *lvb_data;
 
 			lvb_data = kzalloc(lvb_len, GFP_NOFS);
-			if (lvb_data == NULL) {
+			if (!lvb_data) {
 				LDLM_ERROR(lock, "No memory: %d.\n", lvb_len);
 				rc = -ENOMEM;
 				goto out;
@@ -453,7 +453,7 @@ static int ldlm_bl_to_thread(struct ldlm
 		struct ldlm_bl_work_item *blwi;
 
 		blwi = kzalloc(sizeof(*blwi), GFP_NOFS);
-		if (blwi == NULL)
+		if (!blwi)
 			return -ENOMEM;
 		init_blwi(blwi, ns, ld, cancels, count, lock, cancel_flags);
 
@@ -1053,7 +1053,7 @@ static int ldlm_setup(void)
 		return -EALREADY;
 
 	ldlm_state = kzalloc(sizeof(*ldlm_state), GFP_NOFS);
-	if (ldlm_state == NULL)
+	if (!ldlm_state)
 		return -ENOMEM;
 
 	ldlm_kobj = kobject_create_and_add("ldlm", lustre_kobj);
@@ -1123,7 +1123,7 @@ static int ldlm_setup(void)
 
 
 	blp = kzalloc(sizeof(*blp), GFP_NOFS);
-	if (blp == NULL) {
+	if (!blp) {
 		rc = -ENOMEM;
 		goto out;
 	}
diff -u -p a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1528,7 +1528,7 @@ struct ldlm_lock *ldlm_lock_create(struc
 	if (lvb_len) {
 		lock->l_lvb_len = lvb_len;
 		lock->l_lvb_data = kzalloc(lvb_len, GFP_NOFS);
-		if (lock->l_lvb_data == NULL)
+		if (!lock->l_lvb_data)
 			goto out;
 	}
 
@@ -1813,7 +1813,7 @@ int ldlm_run_ast_work(struct ldlm_namesp
 		return 0;
 
 	arg = kzalloc(sizeof(*arg), GFP_NOFS);
-	if (arg == NULL)
+	if (!arg)
 		return -ENOMEM;
 
 	atomic_set(&arg->restart, 0);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 04/12] staging: lustre: ldlm: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c  |    4 ++--
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c |    8 ++++----
 drivers/staging/lustre/lustre/ldlm/ldlm_pool.c  |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
@@ -1422,7 +1422,7 @@ static int ldlm_pools_thread_start(void)
 		return -EALREADY;
 
 	ldlm_pools_thread = kzalloc(sizeof(*ldlm_pools_thread), GFP_NOFS);
-	if (ldlm_pools_thread = NULL)
+	if (!ldlm_pools_thread)
 		return -ENOMEM;
 
 	init_completion(&ldlm_pools_comp);
diff -u -p a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
@@ -225,7 +225,7 @@ static void ldlm_handle_cp_callback(stru
 			void *lvb_data;
 
 			lvb_data = kzalloc(lvb_len, GFP_NOFS);
-			if (lvb_data = NULL) {
+			if (!lvb_data) {
 				LDLM_ERROR(lock, "No memory: %d.\n", lvb_len);
 				rc = -ENOMEM;
 				goto out;
@@ -453,7 +453,7 @@ static int ldlm_bl_to_thread(struct ldlm
 		struct ldlm_bl_work_item *blwi;
 
 		blwi = kzalloc(sizeof(*blwi), GFP_NOFS);
-		if (blwi = NULL)
+		if (!blwi)
 			return -ENOMEM;
 		init_blwi(blwi, ns, ld, cancels, count, lock, cancel_flags);
 
@@ -1053,7 +1053,7 @@ static int ldlm_setup(void)
 		return -EALREADY;
 
 	ldlm_state = kzalloc(sizeof(*ldlm_state), GFP_NOFS);
-	if (ldlm_state = NULL)
+	if (!ldlm_state)
 		return -ENOMEM;
 
 	ldlm_kobj = kobject_create_and_add("ldlm", lustre_kobj);
@@ -1123,7 +1123,7 @@ static int ldlm_setup(void)
 
 
 	blp = kzalloc(sizeof(*blp), GFP_NOFS);
-	if (blp = NULL) {
+	if (!blp) {
 		rc = -ENOMEM;
 		goto out;
 	}
diff -u -p a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1528,7 +1528,7 @@ struct ldlm_lock *ldlm_lock_create(struc
 	if (lvb_len) {
 		lock->l_lvb_len = lvb_len;
 		lock->l_lvb_data = kzalloc(lvb_len, GFP_NOFS);
-		if (lock->l_lvb_data = NULL)
+		if (!lock->l_lvb_data)
 			goto out;
 	}
 
@@ -1813,7 +1813,7 @@ int ldlm_run_ast_work(struct ldlm_namesp
 		return 0;
 
 	arg = kzalloc(sizeof(*arg), GFP_NOFS);
-	if (arg = NULL)
+	if (!arg)
 		return -ENOMEM;
 
 	atomic_set(&arg->restart, 0);

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 05/12] staging: lustre: lmv: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/lmv/lmv_intent.c |    2 +-
 drivers/staging/lustre/lustre/lmv/lmv_obd.c    |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/lmv/lmv_obd.c b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
--- a/drivers/staging/lustre/lustre/lmv/lmv_obd.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
@@ -716,7 +716,7 @@ repeat_fid2path:
 	if (remote_gf == NULL) {
 		remote_gf_size = sizeof(*remote_gf) + PATH_MAX;
 		remote_gf = kzalloc(remote_gf_size, GFP_NOFS);
-		if (remote_gf == NULL) {
+		if (!remote_gf) {
 			rc = -ENOMEM;
 			goto out_fid2path;
 		}
@@ -1398,7 +1398,7 @@ static int lmv_statfs(const struct lu_en
 		return rc;
 
 	temp = kzalloc(sizeof(*temp), GFP_NOFS);
-	if (temp == NULL)
+	if (!temp)
 		return -ENOMEM;
 
 	for (i = 0; i < lmv->desc.ld_tgt_count; i++) {
@@ -1730,7 +1730,7 @@ lmv_enqueue_remote(struct obd_export *ex
 	}
 
 	rdata = kzalloc(sizeof(*rdata), GFP_NOFS);
-	if (rdata == NULL) {
+	if (!rdata) {
 		rc = -ENOMEM;
 		goto out;
 	}
diff -u -p a/drivers/staging/lustre/lustre/lmv/lmv_intent.c b/drivers/staging/lustre/lustre/lmv/lmv_intent.c
--- a/drivers/staging/lustre/lustre/lmv/lmv_intent.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_intent.c
@@ -100,7 +100,7 @@ static int lmv_intent_remote(struct obd_
 	}
 
 	op_data = kzalloc(sizeof(*op_data), GFP_NOFS);
-	if (op_data == NULL) {
+	if (!op_data) {
 		rc = -ENOMEM;
 		goto out;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 05/12] staging: lustre: lmv: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/lmv/lmv_intent.c |    2 +-
 drivers/staging/lustre/lustre/lmv/lmv_obd.c    |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/lmv/lmv_obd.c b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
--- a/drivers/staging/lustre/lustre/lmv/lmv_obd.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
@@ -716,7 +716,7 @@ repeat_fid2path:
 	if (remote_gf = NULL) {
 		remote_gf_size = sizeof(*remote_gf) + PATH_MAX;
 		remote_gf = kzalloc(remote_gf_size, GFP_NOFS);
-		if (remote_gf = NULL) {
+		if (!remote_gf) {
 			rc = -ENOMEM;
 			goto out_fid2path;
 		}
@@ -1398,7 +1398,7 @@ static int lmv_statfs(const struct lu_en
 		return rc;
 
 	temp = kzalloc(sizeof(*temp), GFP_NOFS);
-	if (temp = NULL)
+	if (!temp)
 		return -ENOMEM;
 
 	for (i = 0; i < lmv->desc.ld_tgt_count; i++) {
@@ -1730,7 +1730,7 @@ lmv_enqueue_remote(struct obd_export *ex
 	}
 
 	rdata = kzalloc(sizeof(*rdata), GFP_NOFS);
-	if (rdata = NULL) {
+	if (!rdata) {
 		rc = -ENOMEM;
 		goto out;
 	}
diff -u -p a/drivers/staging/lustre/lustre/lmv/lmv_intent.c b/drivers/staging/lustre/lustre/lmv/lmv_intent.c
--- a/drivers/staging/lustre/lustre/lmv/lmv_intent.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_intent.c
@@ -100,7 +100,7 @@ static int lmv_intent_remote(struct obd_
 	}
 
 	op_data = kzalloc(sizeof(*op_data), GFP_NOFS);
-	if (op_data = NULL) {
+	if (!op_data) {
 		rc = -ENOMEM;
 		goto out;
 	}

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 06/12] staging: lustre: lov: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/lov/lov_dev.c     |    2 +-
 drivers/staging/lustre/lustre/lov/lov_io.c      |    2 +-
 drivers/staging/lustre/lustre/lov/lov_obd.c     |    2 +-
 drivers/staging/lustre/lustre/lov/lov_pool.c    |    2 +-
 drivers/staging/lustre/lustre/lov/lov_request.c |   18 +++++++++---------
 5 files changed, 13 insertions(+), 13 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/lov/lov_request.c b/drivers/staging/lustre/lustre/lov/lov_request.c
--- a/drivers/staging/lustre/lustre/lov/lov_request.c
+++ b/drivers/staging/lustre/lustre/lov/lov_request.c
@@ -275,7 +275,7 @@ int lov_prep_getattr_set(struct obd_expo
 	int rc = 0, i;
 
 	set = kzalloc(sizeof(*set), GFP_NOFS);
-	if (set == NULL)
+	if (!set)
 		return -ENOMEM;
 	lov_init_set(set);
 
@@ -301,7 +301,7 @@ int lov_prep_getattr_set(struct obd_expo
 		}
 
 		req = kzalloc(sizeof(*req), GFP_NOFS);
-		if (req == NULL) {
+		if (!req) {
 			rc = -ENOMEM;
 			goto out_set;
 		}
@@ -358,7 +358,7 @@ int lov_prep_destroy_set(struct obd_expo
 	int rc = 0, i;
 
 	set = kzalloc(sizeof(*set), GFP_NOFS);
-	if (set == NULL)
+	if (!set)
 		return -ENOMEM;
 	lov_init_set(set);
 
@@ -384,7 +384,7 @@ int lov_prep_destroy_set(struct obd_expo
 		}
 
 		req = kzalloc(sizeof(*req), GFP_NOFS);
-		if (req == NULL) {
+		if (!req) {
 			rc = -ENOMEM;
 			goto out_set;
 		}
@@ -477,7 +477,7 @@ int lov_prep_setattr_set(struct obd_expo
 	int rc = 0, i;
 
 	set = kzalloc(sizeof(*set), GFP_NOFS);
-	if (set == NULL)
+	if (!set)
 		return -ENOMEM;
 	lov_init_set(set);
 
@@ -500,7 +500,7 @@ int lov_prep_setattr_set(struct obd_expo
 		}
 
 		req = kzalloc(sizeof(*req), GFP_NOFS);
-		if (req == NULL) {
+		if (!req) {
 			rc = -ENOMEM;
 			goto out_set;
 		}
@@ -704,7 +704,7 @@ int lov_prep_statfs_set(struct obd_devic
 	int rc = 0, i;
 
 	set = kzalloc(sizeof(*set), GFP_NOFS);
-	if (set == NULL)
+	if (!set)
 		return -ENOMEM;
 	lov_init_set(set);
 
@@ -730,14 +730,14 @@ int lov_prep_statfs_set(struct obd_devic
 		}
 
 		req = kzalloc(sizeof(*req), GFP_NOFS);
-		if (req == NULL) {
+		if (!req) {
 			rc = -ENOMEM;
 			goto out_set;
 		}
 
 		req->rq_oi.oi_osfs = kzalloc(sizeof(*req->rq_oi.oi_osfs),
 					     GFP_NOFS);
-		if (req->rq_oi.oi_osfs == NULL) {
+		if (!req->rq_oi.oi_osfs) {
 			kfree(req);
 			rc = -ENOMEM;
 			goto out_set;
diff -u -p a/drivers/staging/lustre/lustre/lov/lov_pool.c b/drivers/staging/lustre/lustre/lov/lov_pool.c
--- a/drivers/staging/lustre/lustre/lov/lov_pool.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pool.c
@@ -431,7 +431,7 @@ int lov_pool_new(struct obd_device *obd,
 		return -ENAMETOOLONG;
 
 	new_pool = kzalloc(sizeof(*new_pool), GFP_NOFS);
-	if (new_pool == NULL)
+	if (!new_pool)
 		return -ENOMEM;
 
 	strncpy(new_pool->pool_name, poolname, LOV_MAXPOOLNAME);
diff -u -p a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
--- a/drivers/staging/lustre/lustre/lov/lov_obd.c
+++ b/drivers/staging/lustre/lustre/lov/lov_obd.c
@@ -976,7 +976,7 @@ static int lov_recreate(struct obd_expor
 		src_oa->o_flags & OBD_FL_RECREATE_OBJS);
 
 	obj_mdp = kzalloc(sizeof(*obj_mdp), GFP_NOFS);
-	if (obj_mdp == NULL)
+	if (!obj_mdp)
 		return -ENOMEM;
 
 	ost_idx = src_oa->o_nlink;
diff -u -p a/drivers/staging/lustre/lustre/lov/lov_io.c b/drivers/staging/lustre/lustre/lov/lov_io.c
--- a/drivers/staging/lustre/lustre/lov/lov_io.c
+++ b/drivers/staging/lustre/lustre/lov/lov_io.c
@@ -181,7 +181,7 @@ static int lov_io_sub_init(const struct
 			} else {
 				sub->sub_io = kzalloc(sizeof(*sub->sub_io),
 						      GFP_NOFS);
-				if (sub->sub_io == NULL)
+				if (!sub->sub_io)
 					result = -ENOMEM;
 			}
 		}
diff -u -p a/drivers/staging/lustre/lustre/lov/lov_dev.c b/drivers/staging/lustre/lustre/lov/lov_dev.c
--- a/drivers/staging/lustre/lustre/lov/lov_dev.c
+++ b/drivers/staging/lustre/lustre/lov/lov_dev.c
@@ -478,7 +478,7 @@ static struct lu_device *lov_device_allo
 	int rc;
 
 	ld = kzalloc(sizeof(*ld), GFP_NOFS);
-	if (ld == NULL)
+	if (!ld)
 		return ERR_PTR(-ENOMEM);
 
 	cl_device_init(&ld->ld_cl, t);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 06/12] staging: lustre: lov: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/lov/lov_dev.c     |    2 +-
 drivers/staging/lustre/lustre/lov/lov_io.c      |    2 +-
 drivers/staging/lustre/lustre/lov/lov_obd.c     |    2 +-
 drivers/staging/lustre/lustre/lov/lov_pool.c    |    2 +-
 drivers/staging/lustre/lustre/lov/lov_request.c |   18 +++++++++---------
 5 files changed, 13 insertions(+), 13 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/lov/lov_request.c b/drivers/staging/lustre/lustre/lov/lov_request.c
--- a/drivers/staging/lustre/lustre/lov/lov_request.c
+++ b/drivers/staging/lustre/lustre/lov/lov_request.c
@@ -275,7 +275,7 @@ int lov_prep_getattr_set(struct obd_expo
 	int rc = 0, i;
 
 	set = kzalloc(sizeof(*set), GFP_NOFS);
-	if (set = NULL)
+	if (!set)
 		return -ENOMEM;
 	lov_init_set(set);
 
@@ -301,7 +301,7 @@ int lov_prep_getattr_set(struct obd_expo
 		}
 
 		req = kzalloc(sizeof(*req), GFP_NOFS);
-		if (req = NULL) {
+		if (!req) {
 			rc = -ENOMEM;
 			goto out_set;
 		}
@@ -358,7 +358,7 @@ int lov_prep_destroy_set(struct obd_expo
 	int rc = 0, i;
 
 	set = kzalloc(sizeof(*set), GFP_NOFS);
-	if (set = NULL)
+	if (!set)
 		return -ENOMEM;
 	lov_init_set(set);
 
@@ -384,7 +384,7 @@ int lov_prep_destroy_set(struct obd_expo
 		}
 
 		req = kzalloc(sizeof(*req), GFP_NOFS);
-		if (req = NULL) {
+		if (!req) {
 			rc = -ENOMEM;
 			goto out_set;
 		}
@@ -477,7 +477,7 @@ int lov_prep_setattr_set(struct obd_expo
 	int rc = 0, i;
 
 	set = kzalloc(sizeof(*set), GFP_NOFS);
-	if (set = NULL)
+	if (!set)
 		return -ENOMEM;
 	lov_init_set(set);
 
@@ -500,7 +500,7 @@ int lov_prep_setattr_set(struct obd_expo
 		}
 
 		req = kzalloc(sizeof(*req), GFP_NOFS);
-		if (req = NULL) {
+		if (!req) {
 			rc = -ENOMEM;
 			goto out_set;
 		}
@@ -704,7 +704,7 @@ int lov_prep_statfs_set(struct obd_devic
 	int rc = 0, i;
 
 	set = kzalloc(sizeof(*set), GFP_NOFS);
-	if (set = NULL)
+	if (!set)
 		return -ENOMEM;
 	lov_init_set(set);
 
@@ -730,14 +730,14 @@ int lov_prep_statfs_set(struct obd_devic
 		}
 
 		req = kzalloc(sizeof(*req), GFP_NOFS);
-		if (req = NULL) {
+		if (!req) {
 			rc = -ENOMEM;
 			goto out_set;
 		}
 
 		req->rq_oi.oi_osfs = kzalloc(sizeof(*req->rq_oi.oi_osfs),
 					     GFP_NOFS);
-		if (req->rq_oi.oi_osfs = NULL) {
+		if (!req->rq_oi.oi_osfs) {
 			kfree(req);
 			rc = -ENOMEM;
 			goto out_set;
diff -u -p a/drivers/staging/lustre/lustre/lov/lov_pool.c b/drivers/staging/lustre/lustre/lov/lov_pool.c
--- a/drivers/staging/lustre/lustre/lov/lov_pool.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pool.c
@@ -431,7 +431,7 @@ int lov_pool_new(struct obd_device *obd,
 		return -ENAMETOOLONG;
 
 	new_pool = kzalloc(sizeof(*new_pool), GFP_NOFS);
-	if (new_pool = NULL)
+	if (!new_pool)
 		return -ENOMEM;
 
 	strncpy(new_pool->pool_name, poolname, LOV_MAXPOOLNAME);
diff -u -p a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
--- a/drivers/staging/lustre/lustre/lov/lov_obd.c
+++ b/drivers/staging/lustre/lustre/lov/lov_obd.c
@@ -976,7 +976,7 @@ static int lov_recreate(struct obd_expor
 		src_oa->o_flags & OBD_FL_RECREATE_OBJS);
 
 	obj_mdp = kzalloc(sizeof(*obj_mdp), GFP_NOFS);
-	if (obj_mdp = NULL)
+	if (!obj_mdp)
 		return -ENOMEM;
 
 	ost_idx = src_oa->o_nlink;
diff -u -p a/drivers/staging/lustre/lustre/lov/lov_io.c b/drivers/staging/lustre/lustre/lov/lov_io.c
--- a/drivers/staging/lustre/lustre/lov/lov_io.c
+++ b/drivers/staging/lustre/lustre/lov/lov_io.c
@@ -181,7 +181,7 @@ static int lov_io_sub_init(const struct
 			} else {
 				sub->sub_io = kzalloc(sizeof(*sub->sub_io),
 						      GFP_NOFS);
-				if (sub->sub_io = NULL)
+				if (!sub->sub_io)
 					result = -ENOMEM;
 			}
 		}
diff -u -p a/drivers/staging/lustre/lustre/lov/lov_dev.c b/drivers/staging/lustre/lustre/lov/lov_dev.c
--- a/drivers/staging/lustre/lustre/lov/lov_dev.c
+++ b/drivers/staging/lustre/lustre/lov/lov_dev.c
@@ -478,7 +478,7 @@ static struct lu_device *lov_device_allo
 	int rc;
 
 	ld = kzalloc(sizeof(*ld), GFP_NOFS);
-	if (ld = NULL)
+	if (!ld)
 		return ERR_PTR(-ENOMEM);
 
 	cl_device_init(&ld->ld_cl, t);

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 07/12] staging: lustre: mdc: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/mdc/mdc_request.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -1202,7 +1202,7 @@ static int mdc_ioc_fid2path(struct obd_e
 	/* Key is KEY_FID2PATH + getinfo_fid2path description */
 	keylen = cfs_size_round(sizeof(KEY_FID2PATH)) + sizeof(*gf);
 	key = kzalloc(keylen, GFP_NOFS);
-	if (key == NULL)
+	if (!key)
 		return -ENOMEM;
 	memcpy(key, KEY_FID2PATH, sizeof(KEY_FID2PATH));
 	memcpy(key + cfs_size_round(sizeof(KEY_FID2PATH)), gf, sizeof(*gf));
@@ -1605,7 +1605,7 @@ static int mdc_changelog_send_thread(voi
 	       cs->cs_fp, cs->cs_startrec);
 
 	cs->cs_buf = kzalloc(KUC_CHANGELOG_MSG_MAXSIZE, GFP_NOFS);
-	if (cs->cs_buf == NULL) {
+	if (!cs->cs_buf) {
 		rc = -ENOMEM;
 		goto out;
 	}
@@ -1934,7 +1934,7 @@ static int mdc_iocontrol(unsigned int cm
 		struct obd_quotactl *oqctl;
 
 		oqctl = kzalloc(sizeof(*oqctl), GFP_NOFS);
-		if (oqctl == NULL) {
+		if (!oqctl) {
 			rc = -ENOMEM;
 			goto out;
 		}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 07/12] staging: lustre: mdc: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/mdc/mdc_request.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -1202,7 +1202,7 @@ static int mdc_ioc_fid2path(struct obd_e
 	/* Key is KEY_FID2PATH + getinfo_fid2path description */
 	keylen = cfs_size_round(sizeof(KEY_FID2PATH)) + sizeof(*gf);
 	key = kzalloc(keylen, GFP_NOFS);
-	if (key = NULL)
+	if (!key)
 		return -ENOMEM;
 	memcpy(key, KEY_FID2PATH, sizeof(KEY_FID2PATH));
 	memcpy(key + cfs_size_round(sizeof(KEY_FID2PATH)), gf, sizeof(*gf));
@@ -1605,7 +1605,7 @@ static int mdc_changelog_send_thread(voi
 	       cs->cs_fp, cs->cs_startrec);
 
 	cs->cs_buf = kzalloc(KUC_CHANGELOG_MSG_MAXSIZE, GFP_NOFS);
-	if (cs->cs_buf = NULL) {
+	if (!cs->cs_buf) {
 		rc = -ENOMEM;
 		goto out;
 	}
@@ -1934,7 +1934,7 @@ static int mdc_iocontrol(unsigned int cm
 		struct obd_quotactl *oqctl;
 
 		oqctl = kzalloc(sizeof(*oqctl), GFP_NOFS);
-		if (oqctl = NULL) {
+		if (!oqctl) {
 			rc = -ENOMEM;
 			goto out;
 		}

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 08/12] staging: lustre: mgc: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/mgc/mgc_request.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1128,7 +1128,7 @@ static int mgc_apply_recover_logs(struct
 	LASSERT(cfg->cfg_sb == cfg->cfg_instance);
 
 	inst = kzalloc(PAGE_CACHE_SIZE, GFP_NOFS);
-	if (inst == NULL)
+	if (!inst)
 		return -ENOMEM;
 
 	if (!IS_SERVER(lsi)) {
@@ -1493,7 +1493,7 @@ static int mgc_process_cfg_log(struct ob
 		lsi = s2lsi(cld->cld_cfg.cfg_sb);
 
 	env = kzalloc(sizeof(*env), GFP_NOFS);
-	if (env == NULL)
+	if (!env)
 		return -ENOMEM;
 
 	rc = lu_env_init(env, LCT_MG_THREAD);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 08/12] staging: lustre: mgc: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/mgc/mgc_request.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1128,7 +1128,7 @@ static int mgc_apply_recover_logs(struct
 	LASSERT(cfg->cfg_sb = cfg->cfg_instance);
 
 	inst = kzalloc(PAGE_CACHE_SIZE, GFP_NOFS);
-	if (inst = NULL)
+	if (!inst)
 		return -ENOMEM;
 
 	if (!IS_SERVER(lsi)) {
@@ -1493,7 +1493,7 @@ static int mgc_process_cfg_log(struct ob
 		lsi = s2lsi(cld->cld_cfg.cfg_sb);
 
 	env = kzalloc(sizeof(*env), GFP_NOFS);
-	if (env = NULL)
+	if (!env)
 		return -ENOMEM;
 
 	rc = lu_env_init(env, LCT_MG_THREAD);

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 09/12] staging: lustre: obdclass: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/obdclass/class_obd.c      |    2 +-
 drivers/staging/lustre/lustre/obdclass/genops.c         |    6 +++---
 drivers/staging/lustre/lustre/obdclass/llog.c           |    6 +++---
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c |    2 +-
 drivers/staging/lustre/lustre/obdclass/lustre_peer.c    |    2 +-
 drivers/staging/lustre/lustre/obdclass/obd_config.c     |   10 +++++-----
 drivers/staging/lustre/lustre/obdclass/obd_mount.c      |   12 ++++++------
 7 files changed, 20 insertions(+), 20 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
--- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
@@ -85,7 +85,7 @@ int lustre_process_log(struct super_bloc
 	LASSERT(cfg);
 
 	bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
-	if (bufs == NULL)
+	if (!bufs)
 		return -ENOMEM;
 
 	/* mgc_process_config */
@@ -258,7 +258,7 @@ int lustre_start_mgc(struct super_block
 	mgssec = lsi->lsi_lmd->lmd_mgssec ? lsi->lsi_lmd->lmd_mgssec : "";
 
 	data = kzalloc(sizeof(*data), GFP_NOFS);
-	if (data == NULL) {
+	if (!data) {
 		rc = -ENOMEM;
 		goto out_free;
 	}
@@ -885,7 +885,7 @@ static int lmd_parse_mgssec(struct lustr
 		length = tail - ptr;
 
 	lmd->lmd_mgssec = kzalloc(length + 1, GFP_NOFS);
-	if (lmd->lmd_mgssec == NULL)
+	if (!lmd->lmd_mgssec)
 		return -ENOMEM;
 
 	memcpy(lmd->lmd_mgssec, ptr, length);
@@ -911,7 +911,7 @@ static int lmd_parse_string(char **handl
 		length = tail - ptr;
 
 	*handle = kzalloc(length + 1, GFP_NOFS);
-	if (*handle == NULL)
+	if (!*handle)
 		return -ENOMEM;
 
 	memcpy(*handle, ptr, length);
@@ -941,7 +941,7 @@ static int lmd_parse_mgs(struct lustre_m
 		oldlen = strlen(lmd->lmd_mgs) + 1;
 
 	mgsnid = kzalloc(oldlen + length + 1, GFP_NOFS);
-	if (mgsnid == NULL)
+	if (!mgsnid)
 		return -ENOMEM;
 
 	if (lmd->lmd_mgs != NULL) {
@@ -983,7 +983,7 @@ static int lmd_parse(char *options, stru
 	lmd->lmd_magic = LMD_MAGIC;
 
 	lmd->lmd_params = kzalloc(4096, GFP_NOFS);
-	if (lmd->lmd_params == NULL)
+	if (!lmd->lmd_params)
 		return -ENOMEM;
 	lmd->lmd_params[0] = '\0';
 
diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
--- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
@@ -835,7 +835,7 @@ int class_add_profile(int proflen, char
 	CDEBUG(D_CONFIG, "Add profile %s\n", prof);
 
 	lprof = kzalloc(sizeof(*lprof), GFP_NOFS);
-	if (lprof == NULL)
+	if (!lprof)
 		return -ENOMEM;
 	INIT_LIST_HEAD(&lprof->lp_list);
 
@@ -979,7 +979,7 @@ struct lustre_cfg *lustre_cfg_rename(str
 	new_len = LUSTRE_CFG_BUFLEN(cfg, 1) + strlen(new_name) - name_len;
 
 	new_param = kzalloc(new_len, GFP_NOFS);
-	if (new_param == NULL)
+	if (!new_param)
 		return ERR_PTR(-ENOMEM);
 
 	strcpy(new_param, new_name);
@@ -987,7 +987,7 @@ struct lustre_cfg *lustre_cfg_rename(str
 		strcat(new_param, value);
 
 	bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
-	if (bufs == NULL) {
+	if (!bufs) {
 		kfree(new_param);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -1461,7 +1461,7 @@ int class_config_llog_handler(const stru
 			inst_len = LUSTRE_CFG_BUFLEN(lcfg, 0) +
 				   sizeof(clli->cfg_instance) * 2 + 4;
 			inst_name = kzalloc(inst_len, GFP_NOFS);
-			if (inst_name == NULL) {
+			if (!inst_name) {
 				rc = -ENOMEM;
 				goto out;
 			}
@@ -1639,7 +1639,7 @@ int class_config_dump_handler(const stru
 	int	 rc = 0;
 
 	outstr = kzalloc(256, GFP_NOFS);
-	if (outstr == NULL)
+	if (!outstr)
 		return -ENOMEM;
 
 	if (rec->lrh_type == OBD_CFG_REC) {
diff -u -p a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
--- a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
@@ -105,7 +105,7 @@ int class_add_uuid(const char *uuid, __u
 		return -EOVERFLOW;
 
 	data = kzalloc(sizeof(*data), GFP_NOFS);
-	if (data == NULL)
+	if (!data)
 		return -ENOMEM;
 
 	obd_str2uuid(&data->un_uuid, uuid);
diff -u -p a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -275,7 +275,7 @@ struct dentry *ldebugfs_add_symlink(cons
 		return NULL;
 
 	dest = kzalloc(MAX_STRING_SIZE + 1, GFP_KERNEL);
-	if (dest == NULL)
+	if (!dest)
 		return NULL;
 
 	va_start(ap, format);
diff -u -p a/drivers/staging/lustre/lustre/obdclass/llog.c b/drivers/staging/lustre/lustre/obdclass/llog.c
--- a/drivers/staging/lustre/lustre/obdclass/llog.c
+++ b/drivers/staging/lustre/lustre/obdclass/llog.c
@@ -61,7 +61,7 @@ static struct llog_handle *llog_alloc_ha
 	struct llog_handle *loghandle;
 
 	loghandle = kzalloc(sizeof(*loghandle), GFP_NOFS);
-	if (loghandle == NULL)
+	if (!loghandle)
 		return NULL;
 
 	init_rwsem(&loghandle->lgh_lock);
@@ -208,7 +208,7 @@ int llog_init_handle(const struct lu_env
 	LASSERT(handle->lgh_hdr == NULL);
 
 	llh = kzalloc(sizeof(*llh), GFP_NOFS);
-	if (llh == NULL)
+	if (!llh)
 		return -ENOMEM;
 	handle->lgh_hdr = llh;
 	/* first assign flags to use llog_client_ops */
@@ -435,7 +435,7 @@ int llog_process_or_fork(const struct lu
 	int		      rc;
 
 	lpi = kzalloc(sizeof(*lpi), GFP_NOFS);
-	if (lpi == NULL) {
+	if (!lpi) {
 		CERROR("cannot alloc pointer\n");
 		return -ENOMEM;
 	}
diff -u -p a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -172,7 +172,7 @@ int class_register_type(struct obd_ops *
 
 	rc = -ENOMEM;
 	type = kzalloc(sizeof(*type), GFP_NOFS);
-	if (type == NULL)
+	if (!type)
 		return rc;
 
 	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
@@ -1016,7 +1016,7 @@ struct obd_import *class_new_import(stru
 	struct obd_import *imp;
 
 	imp = kzalloc(sizeof(*imp), GFP_NOFS);
-	if (imp == NULL)
+	if (!imp)
 		return NULL;
 
 	INIT_LIST_HEAD(&imp->imp_pinger_chain);
@@ -1819,7 +1819,7 @@ void *kuc_alloc(int payload_len, int tra
 	int len = kuc_len(payload_len);
 
 	lh = kzalloc(len, GFP_NOFS);
-	if (lh == NULL)
+	if (!lh)
 		return ERR_PTR(-ENOMEM);
 
 	lh->kuc_magic = KUC_MAGIC;
diff -u -p a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
--- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
+++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
@@ -232,7 +232,7 @@ int class_handle_ioctl(unsigned int cmd,
 			goto out;
 		}
 		lcfg = kzalloc(data->ioc_plen1, GFP_NOFS);
-		if (lcfg == NULL) {
+		if (!lcfg) {
 			err = -ENOMEM;
 			goto out;
 		}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 09/12] staging: lustre: obdclass: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/obdclass/class_obd.c      |    2 +-
 drivers/staging/lustre/lustre/obdclass/genops.c         |    6 +++---
 drivers/staging/lustre/lustre/obdclass/llog.c           |    6 +++---
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c |    2 +-
 drivers/staging/lustre/lustre/obdclass/lustre_peer.c    |    2 +-
 drivers/staging/lustre/lustre/obdclass/obd_config.c     |   10 +++++-----
 drivers/staging/lustre/lustre/obdclass/obd_mount.c      |   12 ++++++------
 7 files changed, 20 insertions(+), 20 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
--- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
@@ -85,7 +85,7 @@ int lustre_process_log(struct super_bloc
 	LASSERT(cfg);
 
 	bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
-	if (bufs = NULL)
+	if (!bufs)
 		return -ENOMEM;
 
 	/* mgc_process_config */
@@ -258,7 +258,7 @@ int lustre_start_mgc(struct super_block
 	mgssec = lsi->lsi_lmd->lmd_mgssec ? lsi->lsi_lmd->lmd_mgssec : "";
 
 	data = kzalloc(sizeof(*data), GFP_NOFS);
-	if (data = NULL) {
+	if (!data) {
 		rc = -ENOMEM;
 		goto out_free;
 	}
@@ -885,7 +885,7 @@ static int lmd_parse_mgssec(struct lustr
 		length = tail - ptr;
 
 	lmd->lmd_mgssec = kzalloc(length + 1, GFP_NOFS);
-	if (lmd->lmd_mgssec = NULL)
+	if (!lmd->lmd_mgssec)
 		return -ENOMEM;
 
 	memcpy(lmd->lmd_mgssec, ptr, length);
@@ -911,7 +911,7 @@ static int lmd_parse_string(char **handl
 		length = tail - ptr;
 
 	*handle = kzalloc(length + 1, GFP_NOFS);
-	if (*handle = NULL)
+	if (!*handle)
 		return -ENOMEM;
 
 	memcpy(*handle, ptr, length);
@@ -941,7 +941,7 @@ static int lmd_parse_mgs(struct lustre_m
 		oldlen = strlen(lmd->lmd_mgs) + 1;
 
 	mgsnid = kzalloc(oldlen + length + 1, GFP_NOFS);
-	if (mgsnid = NULL)
+	if (!mgsnid)
 		return -ENOMEM;
 
 	if (lmd->lmd_mgs != NULL) {
@@ -983,7 +983,7 @@ static int lmd_parse(char *options, stru
 	lmd->lmd_magic = LMD_MAGIC;
 
 	lmd->lmd_params = kzalloc(4096, GFP_NOFS);
-	if (lmd->lmd_params = NULL)
+	if (!lmd->lmd_params)
 		return -ENOMEM;
 	lmd->lmd_params[0] = '\0';
 
diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
--- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
@@ -835,7 +835,7 @@ int class_add_profile(int proflen, char
 	CDEBUG(D_CONFIG, "Add profile %s\n", prof);
 
 	lprof = kzalloc(sizeof(*lprof), GFP_NOFS);
-	if (lprof = NULL)
+	if (!lprof)
 		return -ENOMEM;
 	INIT_LIST_HEAD(&lprof->lp_list);
 
@@ -979,7 +979,7 @@ struct lustre_cfg *lustre_cfg_rename(str
 	new_len = LUSTRE_CFG_BUFLEN(cfg, 1) + strlen(new_name) - name_len;
 
 	new_param = kzalloc(new_len, GFP_NOFS);
-	if (new_param = NULL)
+	if (!new_param)
 		return ERR_PTR(-ENOMEM);
 
 	strcpy(new_param, new_name);
@@ -987,7 +987,7 @@ struct lustre_cfg *lustre_cfg_rename(str
 		strcat(new_param, value);
 
 	bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
-	if (bufs = NULL) {
+	if (!bufs) {
 		kfree(new_param);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -1461,7 +1461,7 @@ int class_config_llog_handler(const stru
 			inst_len = LUSTRE_CFG_BUFLEN(lcfg, 0) +
 				   sizeof(clli->cfg_instance) * 2 + 4;
 			inst_name = kzalloc(inst_len, GFP_NOFS);
-			if (inst_name = NULL) {
+			if (!inst_name) {
 				rc = -ENOMEM;
 				goto out;
 			}
@@ -1639,7 +1639,7 @@ int class_config_dump_handler(const stru
 	int	 rc = 0;
 
 	outstr = kzalloc(256, GFP_NOFS);
-	if (outstr = NULL)
+	if (!outstr)
 		return -ENOMEM;
 
 	if (rec->lrh_type = OBD_CFG_REC) {
diff -u -p a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
--- a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
@@ -105,7 +105,7 @@ int class_add_uuid(const char *uuid, __u
 		return -EOVERFLOW;
 
 	data = kzalloc(sizeof(*data), GFP_NOFS);
-	if (data = NULL)
+	if (!data)
 		return -ENOMEM;
 
 	obd_str2uuid(&data->un_uuid, uuid);
diff -u -p a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -275,7 +275,7 @@ struct dentry *ldebugfs_add_symlink(cons
 		return NULL;
 
 	dest = kzalloc(MAX_STRING_SIZE + 1, GFP_KERNEL);
-	if (dest = NULL)
+	if (!dest)
 		return NULL;
 
 	va_start(ap, format);
diff -u -p a/drivers/staging/lustre/lustre/obdclass/llog.c b/drivers/staging/lustre/lustre/obdclass/llog.c
--- a/drivers/staging/lustre/lustre/obdclass/llog.c
+++ b/drivers/staging/lustre/lustre/obdclass/llog.c
@@ -61,7 +61,7 @@ static struct llog_handle *llog_alloc_ha
 	struct llog_handle *loghandle;
 
 	loghandle = kzalloc(sizeof(*loghandle), GFP_NOFS);
-	if (loghandle = NULL)
+	if (!loghandle)
 		return NULL;
 
 	init_rwsem(&loghandle->lgh_lock);
@@ -208,7 +208,7 @@ int llog_init_handle(const struct lu_env
 	LASSERT(handle->lgh_hdr = NULL);
 
 	llh = kzalloc(sizeof(*llh), GFP_NOFS);
-	if (llh = NULL)
+	if (!llh)
 		return -ENOMEM;
 	handle->lgh_hdr = llh;
 	/* first assign flags to use llog_client_ops */
@@ -435,7 +435,7 @@ int llog_process_or_fork(const struct lu
 	int		      rc;
 
 	lpi = kzalloc(sizeof(*lpi), GFP_NOFS);
-	if (lpi = NULL) {
+	if (!lpi) {
 		CERROR("cannot alloc pointer\n");
 		return -ENOMEM;
 	}
diff -u -p a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -172,7 +172,7 @@ int class_register_type(struct obd_ops *
 
 	rc = -ENOMEM;
 	type = kzalloc(sizeof(*type), GFP_NOFS);
-	if (type = NULL)
+	if (!type)
 		return rc;
 
 	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
@@ -1016,7 +1016,7 @@ struct obd_import *class_new_import(stru
 	struct obd_import *imp;
 
 	imp = kzalloc(sizeof(*imp), GFP_NOFS);
-	if (imp = NULL)
+	if (!imp)
 		return NULL;
 
 	INIT_LIST_HEAD(&imp->imp_pinger_chain);
@@ -1819,7 +1819,7 @@ void *kuc_alloc(int payload_len, int tra
 	int len = kuc_len(payload_len);
 
 	lh = kzalloc(len, GFP_NOFS);
-	if (lh = NULL)
+	if (!lh)
 		return ERR_PTR(-ENOMEM);
 
 	lh->kuc_magic = KUC_MAGIC;
diff -u -p a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
--- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
+++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
@@ -232,7 +232,7 @@ int class_handle_ioctl(unsigned int cmd,
 			goto out;
 		}
 		lcfg = kzalloc(data->ioc_plen1, GFP_NOFS);
-		if (lcfg = NULL) {
+		if (!lcfg) {
 			err = -ENOMEM;
 			goto out;
 		}

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 10/12] staging: lustre: obdecho: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/obdecho/echo_client.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c
--- a/drivers/staging/lustre/lustre/obdecho/echo_client.c
+++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c
@@ -480,11 +480,11 @@ static int echo_alloc_memmd(struct echo_
 
 	LASSERT(*lsmp == NULL);
 	*lsmp = kzalloc(lsm_size, GFP_NOFS);
-	if (*lsmp == NULL)
+	if (!*lsmp)
 		return -ENOMEM;
 
 	(*lsmp)->lsm_oinfo[0] = kzalloc(sizeof(struct lov_oinfo), GFP_NOFS);
-	if ((*lsmp)->lsm_oinfo[0] == NULL) {
+	if (!(*lsmp)->lsm_oinfo[0]) {
 		kfree(*lsmp);
 		return -ENOMEM;
 	}
@@ -701,7 +701,7 @@ static struct lu_device *echo_device_all
 	int cleanup = 0;
 
 	ed = kzalloc(sizeof(*ed), GFP_NOFS);
-	if (ed == NULL) {
+	if (!ed) {
 		rc = -ENOMEM;
 		goto out;
 	}
@@ -1878,7 +1878,7 @@ echo_client_iocontrol(unsigned int cmd,
 		return rc;
 
 	env = kzalloc(sizeof(*env), GFP_NOFS);
-	if (env == NULL)
+	if (!env)
 		return -ENOMEM;
 
 	rc = lu_env_init(env, LCT_DT_THREAD);
@@ -2049,7 +2049,7 @@ static int echo_client_setup(const struc
 	ec->ec_nstripes = 0;
 
 	ocd = kzalloc(sizeof(*ocd), GFP_NOFS);
-	if (ocd == NULL) {
+	if (!ocd) {
 		CERROR("Can't alloc ocd connecting to %s\n",
 		       lustre_cfg_string(lcfg, 1));
 		return -ENOMEM;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 10/12] staging: lustre: obdecho: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/obdecho/echo_client.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c
--- a/drivers/staging/lustre/lustre/obdecho/echo_client.c
+++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c
@@ -480,11 +480,11 @@ static int echo_alloc_memmd(struct echo_
 
 	LASSERT(*lsmp = NULL);
 	*lsmp = kzalloc(lsm_size, GFP_NOFS);
-	if (*lsmp = NULL)
+	if (!*lsmp)
 		return -ENOMEM;
 
 	(*lsmp)->lsm_oinfo[0] = kzalloc(sizeof(struct lov_oinfo), GFP_NOFS);
-	if ((*lsmp)->lsm_oinfo[0] = NULL) {
+	if (!(*lsmp)->lsm_oinfo[0]) {
 		kfree(*lsmp);
 		return -ENOMEM;
 	}
@@ -701,7 +701,7 @@ static struct lu_device *echo_device_all
 	int cleanup = 0;
 
 	ed = kzalloc(sizeof(*ed), GFP_NOFS);
-	if (ed = NULL) {
+	if (!ed) {
 		rc = -ENOMEM;
 		goto out;
 	}
@@ -1878,7 +1878,7 @@ echo_client_iocontrol(unsigned int cmd,
 		return rc;
 
 	env = kzalloc(sizeof(*env), GFP_NOFS);
-	if (env = NULL)
+	if (!env)
 		return -ENOMEM;
 
 	rc = lu_env_init(env, LCT_DT_THREAD);
@@ -2049,7 +2049,7 @@ static int echo_client_setup(const struc
 	ec->ec_nstripes = 0;
 
 	ocd = kzalloc(sizeof(*ocd), GFP_NOFS);
-	if (ocd = NULL) {
+	if (!ocd) {
 		CERROR("Can't alloc ocd connecting to %s\n",
 		       lustre_cfg_string(lcfg, 1));
 		return -ENOMEM;

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 11/12] staging: lustre: osc: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/osc/osc_dev.c     |    2 +-
 drivers/staging/lustre/lustre/osc/osc_request.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -119,7 +119,7 @@ static int osc_packmd(struct obd_export
 
 	if (*lmmp == NULL) {
 		*lmmp = kzalloc(lmm_size, GFP_NOFS);
-		if (*lmmp == NULL)
+		if (!*lmmp)
 			return -ENOMEM;
 	}
 
@@ -1909,7 +1909,7 @@ int osc_build_rpc(const struct lu_env *e
 		mpflag = cfs_memory_pressure_get_and_set();
 
 	crattr = kzalloc(sizeof(*crattr), GFP_NOFS);
-	if (crattr == NULL) {
+	if (!crattr) {
 		rc = -ENOMEM;
 		goto out;
 	}
diff -u -p a/drivers/staging/lustre/lustre/osc/osc_dev.c b/drivers/staging/lustre/lustre/osc/osc_dev.c
--- a/drivers/staging/lustre/lustre/osc/osc_dev.c
+++ b/drivers/staging/lustre/lustre/osc/osc_dev.c
@@ -218,7 +218,7 @@ static struct lu_device *osc_device_allo
 	int rc;
 
 	od = kzalloc(sizeof(*od), GFP_NOFS);
-	if (od == NULL)
+	if (!od)
 		return ERR_PTR(-ENOMEM);
 
 	cl_device_init(&od->od_cl, t);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 11/12] staging: lustre: osc: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/osc/osc_dev.c     |    2 +-
 drivers/staging/lustre/lustre/osc/osc_request.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -119,7 +119,7 @@ static int osc_packmd(struct obd_export
 
 	if (*lmmp = NULL) {
 		*lmmp = kzalloc(lmm_size, GFP_NOFS);
-		if (*lmmp = NULL)
+		if (!*lmmp)
 			return -ENOMEM;
 	}
 
@@ -1909,7 +1909,7 @@ int osc_build_rpc(const struct lu_env *e
 		mpflag = cfs_memory_pressure_get_and_set();
 
 	crattr = kzalloc(sizeof(*crattr), GFP_NOFS);
-	if (crattr = NULL) {
+	if (!crattr) {
 		rc = -ENOMEM;
 		goto out;
 	}
diff -u -p a/drivers/staging/lustre/lustre/osc/osc_dev.c b/drivers/staging/lustre/lustre/osc/osc_dev.c
--- a/drivers/staging/lustre/lustre/osc/osc_dev.c
+++ b/drivers/staging/lustre/lustre/osc/osc_dev.c
@@ -218,7 +218,7 @@ static struct lu_device *osc_device_allo
 	int rc;
 
 	od = kzalloc(sizeof(*od), GFP_NOFS);
-	if (od = NULL)
+	if (!od)
 		return ERR_PTR(-ENOMEM);
 
 	cl_device_init(&od->od_cl, t);

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 12/12] staging: lustre: ptlrpc: Use !x to check for kzalloc failure
  2015-06-20 16:58 ` Julia Lawall
@ 2015-06-20 16:59   ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/ptlrpc/client.c       |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c |    8 ++++----
 drivers/staging/lustre/lustre/ptlrpc/nrs.c          |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c      |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c     |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/sec_config.c   |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/sec_plain.c    |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/service.c      |    4 ++--
 8 files changed, 12 insertions(+), 12 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
--- a/drivers/staging/lustre/lustre/ptlrpc/service.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
@@ -732,7 +732,7 @@ ptlrpc_register_service(struct ptlrpc_se
 
 	service = kzalloc(offsetof(struct ptlrpc_service, srv_parts[ncpts]),
 			  GFP_NOFS);
-	if (service == NULL) {
+	if (!service) {
 		kfree(cpts);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -2298,7 +2298,7 @@ static int ptlrpc_main(void *arg)
 	}
 
 	env = kzalloc(sizeof(*env), GFP_NOFS);
-	if (env == NULL) {
+	if (!env) {
 		rc = -ENOMEM;
 		goto out_srv_fini;
 	}
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
@@ -444,7 +444,7 @@ struct ptlrpc_sec *plain_create_sec(stru
 	LASSERT(SPTLRPC_FLVR_POLICY(sf->sf_rpc) == SPTLRPC_POLICY_PLAIN);
 
 	plsec = kzalloc(sizeof(*plsec), GFP_NOFS);
-	if (plsec == NULL)
+	if (!plsec)
 		return NULL;
 
 	/*
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/sec_config.c b/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
@@ -564,7 +564,7 @@ struct sptlrpc_conf *sptlrpc_conf_get(co
 		return NULL;
 
 	conf = kzalloc(sizeof(*conf), GFP_NOFS);
-	if (conf == NULL)
+	if (!conf)
 		return NULL;
 
 	strcpy(conf->sc_fsname, fsname);
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
@@ -415,7 +415,7 @@ static int enc_pools_add_pages(int npage
 
 	for (i = 0; i < npools; i++) {
 		pools[i] = kzalloc(PAGE_CACHE_SIZE, GFP_NOFS);
-		if (pools[i] == NULL)
+		if (!pools[i])
 			goto out_pools;
 
 		for (j = 0; j < PAGES_PER_POOL && alloced < npages; j++) {
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
--- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
@@ -739,7 +739,7 @@ static int ptlrpcd_init(void)
 
 	size = offsetof(struct ptlrpcd, pd_threads[nthreads]);
 	ptlrpcds = kzalloc(size, GFP_NOFS);
-	if (ptlrpcds == NULL) {
+	if (!ptlrpcds) {
 		rc = -ENOMEM;
 		goto out;
 	}
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/nrs.c b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
@@ -1156,7 +1156,7 @@ int ptlrpc_nrs_policy_register(struct pt
 	}
 
 	desc = kzalloc(sizeof(*desc), GFP_NOFS);
-	if (desc == NULL) {
+	if (!desc) {
 		rc = -ENOMEM;
 		goto fail;
 	}
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c b/drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c
--- a/drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c
@@ -652,7 +652,7 @@ static ssize_t ptlrpc_lprocfs_nrs_seq_wr
 		return -EINVAL;
 
 	cmd = kzalloc(LPROCFS_NRS_WR_MAX_CMD, GFP_NOFS);
-	if (cmd == NULL)
+	if (!cmd)
 		return -ENOMEM;
 	/**
 	 * strsep() modifies its argument, so keep a copy
@@ -819,7 +819,7 @@ ptlrpc_lprocfs_svc_req_history_start(str
 	}
 
 	srhi = kzalloc(sizeof(*srhi), GFP_NOFS);
-	if (srhi == NULL)
+	if (!srhi)
 		return NULL;
 
 	srhi->srhi_seq = 0;
@@ -1219,7 +1219,7 @@ int lprocfs_wr_evict_client(struct file
 	char *tmpbuf;
 
 	kbuf = kzalloc(BUFLEN, GFP_NOFS);
-	if (kbuf == NULL)
+	if (!kbuf)
 		return -ENOMEM;
 
 	/*
@@ -1303,7 +1303,7 @@ int lprocfs_wr_import(struct file *file,
 		return -EINVAL;
 
 	kbuf = kzalloc(count + 1, GFP_NOFS);
-	if (kbuf == NULL)
+	if (!kbuf)
 		return -ENOMEM;
 
 	if (copy_from_user(kbuf, buffer, count)) {
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
--- a/drivers/staging/lustre/lustre/ptlrpc/client.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
@@ -971,7 +971,7 @@ int ptlrpc_set_add_cb(struct ptlrpc_requ
 	struct ptlrpc_set_cbdata *cbdata;
 
 	cbdata = kzalloc(sizeof(*cbdata), GFP_NOFS);
-	if (cbdata == NULL)
+	if (!cbdata)
 		return -ENOMEM;
 
 	cbdata->psc_interpret = fn;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [PATCH 12/12] staging: lustre: ptlrpc: Use !x to check for kzalloc failure
@ 2015-06-20 16:59   ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-20 16:59 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel

!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x = NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/ptlrpc/client.c       |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c |    8 ++++----
 drivers/staging/lustre/lustre/ptlrpc/nrs.c          |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c      |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c     |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/sec_config.c   |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/sec_plain.c    |    2 +-
 drivers/staging/lustre/lustre/ptlrpc/service.c      |    4 ++--
 8 files changed, 12 insertions(+), 12 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
--- a/drivers/staging/lustre/lustre/ptlrpc/service.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
@@ -732,7 +732,7 @@ ptlrpc_register_service(struct ptlrpc_se
 
 	service = kzalloc(offsetof(struct ptlrpc_service, srv_parts[ncpts]),
 			  GFP_NOFS);
-	if (service = NULL) {
+	if (!service) {
 		kfree(cpts);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -2298,7 +2298,7 @@ static int ptlrpc_main(void *arg)
 	}
 
 	env = kzalloc(sizeof(*env), GFP_NOFS);
-	if (env = NULL) {
+	if (!env) {
 		rc = -ENOMEM;
 		goto out_srv_fini;
 	}
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
@@ -444,7 +444,7 @@ struct ptlrpc_sec *plain_create_sec(stru
 	LASSERT(SPTLRPC_FLVR_POLICY(sf->sf_rpc) = SPTLRPC_POLICY_PLAIN);
 
 	plsec = kzalloc(sizeof(*plsec), GFP_NOFS);
-	if (plsec = NULL)
+	if (!plsec)
 		return NULL;
 
 	/*
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/sec_config.c b/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
@@ -564,7 +564,7 @@ struct sptlrpc_conf *sptlrpc_conf_get(co
 		return NULL;
 
 	conf = kzalloc(sizeof(*conf), GFP_NOFS);
-	if (conf = NULL)
+	if (!conf)
 		return NULL;
 
 	strcpy(conf->sc_fsname, fsname);
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
@@ -415,7 +415,7 @@ static int enc_pools_add_pages(int npage
 
 	for (i = 0; i < npools; i++) {
 		pools[i] = kzalloc(PAGE_CACHE_SIZE, GFP_NOFS);
-		if (pools[i] = NULL)
+		if (!pools[i])
 			goto out_pools;
 
 		for (j = 0; j < PAGES_PER_POOL && alloced < npages; j++) {
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
--- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
@@ -739,7 +739,7 @@ static int ptlrpcd_init(void)
 
 	size = offsetof(struct ptlrpcd, pd_threads[nthreads]);
 	ptlrpcds = kzalloc(size, GFP_NOFS);
-	if (ptlrpcds = NULL) {
+	if (!ptlrpcds) {
 		rc = -ENOMEM;
 		goto out;
 	}
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/nrs.c b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
@@ -1156,7 +1156,7 @@ int ptlrpc_nrs_policy_register(struct pt
 	}
 
 	desc = kzalloc(sizeof(*desc), GFP_NOFS);
-	if (desc = NULL) {
+	if (!desc) {
 		rc = -ENOMEM;
 		goto fail;
 	}
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c b/drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c
--- a/drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c
@@ -652,7 +652,7 @@ static ssize_t ptlrpc_lprocfs_nrs_seq_wr
 		return -EINVAL;
 
 	cmd = kzalloc(LPROCFS_NRS_WR_MAX_CMD, GFP_NOFS);
-	if (cmd = NULL)
+	if (!cmd)
 		return -ENOMEM;
 	/**
 	 * strsep() modifies its argument, so keep a copy
@@ -819,7 +819,7 @@ ptlrpc_lprocfs_svc_req_history_start(str
 	}
 
 	srhi = kzalloc(sizeof(*srhi), GFP_NOFS);
-	if (srhi = NULL)
+	if (!srhi)
 		return NULL;
 
 	srhi->srhi_seq = 0;
@@ -1219,7 +1219,7 @@ int lprocfs_wr_evict_client(struct file
 	char *tmpbuf;
 
 	kbuf = kzalloc(BUFLEN, GFP_NOFS);
-	if (kbuf = NULL)
+	if (!kbuf)
 		return -ENOMEM;
 
 	/*
@@ -1303,7 +1303,7 @@ int lprocfs_wr_import(struct file *file,
 		return -EINVAL;
 
 	kbuf = kzalloc(count + 1, GFP_NOFS);
-	if (kbuf = NULL)
+	if (!kbuf)
 		return -ENOMEM;
 
 	if (copy_from_user(kbuf, buffer, count)) {
diff -u -p a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
--- a/drivers/staging/lustre/lustre/ptlrpc/client.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
@@ -971,7 +971,7 @@ int ptlrpc_set_add_cb(struct ptlrpc_requ
 	struct ptlrpc_set_cbdata *cbdata;
 
 	cbdata = kzalloc(sizeof(*cbdata), GFP_NOFS);
-	if (cbdata = NULL)
+	if (!cbdata)
 		return -ENOMEM;
 
 	cbdata->psc_interpret = fn;

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 09/12] staging: lustre: obdclass: Use !x to check for kzalloc failure
  2015-06-20 16:59   ` Julia Lawall
@ 2015-06-21 10:02     ` walter harms
  -1 siblings, 0 replies; 80+ messages in thread
From: walter harms @ 2015-06-21 10:02 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Oleg Drokin, kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel



Am 20.06.2015 18:59, schrieb Julia Lawall:
> !x is more normal for kzalloc failure in the kernel.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression x;
> statement S1, S2;
> @@
> 
> x = kzalloc(...);
> if (
> - x == NULL
> + !x
>  ) S1 else S2
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
>  drivers/staging/lustre/lustre/obdclass/class_obd.c      |    2 +-
>  drivers/staging/lustre/lustre/obdclass/genops.c         |    6 +++---
>  drivers/staging/lustre/lustre/obdclass/llog.c           |    6 +++---
>  drivers/staging/lustre/lustre/obdclass/lprocfs_status.c |    2 +-
>  drivers/staging/lustre/lustre/obdclass/lustre_peer.c    |    2 +-
>  drivers/staging/lustre/lustre/obdclass/obd_config.c     |   10 +++++-----
>  drivers/staging/lustre/lustre/obdclass/obd_mount.c      |   12 ++++++------
>  7 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> @@ -85,7 +85,7 @@ int lustre_process_log(struct super_bloc
>  	LASSERT(cfg);
>  
>  	bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
> -	if (bufs == NULL)
> +	if (!bufs)
>  		return -ENOMEM;
>  
>  	/* mgc_process_config */
> @@ -258,7 +258,7 @@ int lustre_start_mgc(struct super_block
>  	mgssec = lsi->lsi_lmd->lmd_mgssec ? lsi->lsi_lmd->lmd_mgssec : "";
>  
>  	data = kzalloc(sizeof(*data), GFP_NOFS);
> -	if (data == NULL) {
> +	if (!data) {
>  		rc = -ENOMEM;
>  		goto out_free;
>  	}
> @@ -885,7 +885,7 @@ static int lmd_parse_mgssec(struct lustr
>  		length = tail - ptr;
>  
>  	lmd->lmd_mgssec = kzalloc(length + 1, GFP_NOFS);
> -	if (lmd->lmd_mgssec == NULL)
> +	if (!lmd->lmd_mgssec)
>  		return -ENOMEM;
>  
>  	memcpy(lmd->lmd_mgssec, ptr, length);
looks like memdup()

> @@ -911,7 +911,7 @@ static int lmd_parse_string(char **handl
>  		length = tail - ptr;
>  
>  	*handle = kzalloc(length + 1, GFP_NOFS);
> -	if (*handle == NULL)
> +	if (!*handle)
>  		return -ENOMEM;
>  
>  	memcpy(*handle, ptr, length);

looks like memdup()


> @@ -941,7 +941,7 @@ static int lmd_parse_mgs(struct lustre_m
>  		oldlen = strlen(lmd->lmd_mgs) + 1;
>  
>  	mgsnid = kzalloc(oldlen + length + 1, GFP_NOFS);
> -	if (mgsnid == NULL)
> +	if (!mgsnid)
>  		return -ENOMEM;
>  
>  	if (lmd->lmd_mgs != NULL) {
> @@ -983,7 +983,7 @@ static int lmd_parse(char *options, stru
>  	lmd->lmd_magic = LMD_MAGIC;
>  
>  	lmd->lmd_params = kzalloc(4096, GFP_NOFS);
> -	if (lmd->lmd_params == NULL)
> +	if (!lmd->lmd_params)
>  		return -ENOMEM;
>  	lmd->lmd_params[0] = '\0';
>  
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
> --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
> @@ -835,7 +835,7 @@ int class_add_profile(int proflen, char
>  	CDEBUG(D_CONFIG, "Add profile %s\n", prof);
>  
>  	lprof = kzalloc(sizeof(*lprof), GFP_NOFS);
> -	if (lprof == NULL)
> +	if (!lprof)
>  		return -ENOMEM;
>  	INIT_LIST_HEAD(&lprof->lp_list);
>  
> @@ -979,7 +979,7 @@ struct lustre_cfg *lustre_cfg_rename(str
>  	new_len = LUSTRE_CFG_BUFLEN(cfg, 1) + strlen(new_name) - name_len;
>  
>  	new_param = kzalloc(new_len, GFP_NOFS);
> -	if (new_param == NULL)
> +	if (!new_param)
>  		return ERR_PTR(-ENOMEM);
>  
>  	strcpy(new_param, new_name);
> @@ -987,7 +987,7 @@ struct lustre_cfg *lustre_cfg_rename(str
>  		strcat(new_param, value);
>  
>  	bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
> -	if (bufs == NULL) {
> +	if (!bufs) {
>  		kfree(new_param);
>  		return ERR_PTR(-ENOMEM);
>  	}
> @@ -1461,7 +1461,7 @@ int class_config_llog_handler(const stru
>  			inst_len = LUSTRE_CFG_BUFLEN(lcfg, 0) +
>  				   sizeof(clli->cfg_instance) * 2 + 4;
>  			inst_name = kzalloc(inst_len, GFP_NOFS);
> -			if (inst_name == NULL) {
> +			if (!inst_name) {
>  				rc = -ENOMEM;
>  				goto out;
>  			}
> @@ -1639,7 +1639,7 @@ int class_config_dump_handler(const stru
>  	int	 rc = 0;
>  
>  	outstr = kzalloc(256, GFP_NOFS);
> -	if (outstr == NULL)
> +	if (!outstr)
>  		return -ENOMEM;
>  
>  	if (rec->lrh_type == OBD_CFG_REC) {
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> --- a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> @@ -105,7 +105,7 @@ int class_add_uuid(const char *uuid, __u
>  		return -EOVERFLOW;
>  
>  	data = kzalloc(sizeof(*data), GFP_NOFS);
> -	if (data == NULL)
> +	if (!data)
>  		return -ENOMEM;
>  
>  	obd_str2uuid(&data->un_uuid, uuid);
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> @@ -275,7 +275,7 @@ struct dentry *ldebugfs_add_symlink(cons
>  		return NULL;
>  
>  	dest = kzalloc(MAX_STRING_SIZE + 1, GFP_KERNEL);
> -	if (dest == NULL)
> +	if (!dest)
>  		return NULL;
>  
>  	va_start(ap, format);
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/llog.c b/drivers/staging/lustre/lustre/obdclass/llog.c
> --- a/drivers/staging/lustre/lustre/obdclass/llog.c
> +++ b/drivers/staging/lustre/lustre/obdclass/llog.c
> @@ -61,7 +61,7 @@ static struct llog_handle *llog_alloc_ha
>  	struct llog_handle *loghandle;
>  
>  	loghandle = kzalloc(sizeof(*loghandle), GFP_NOFS);
> -	if (loghandle == NULL)
> +	if (!loghandle)
>  		return NULL;
>  
>  	init_rwsem(&loghandle->lgh_lock);
> @@ -208,7 +208,7 @@ int llog_init_handle(const struct lu_env
>  	LASSERT(handle->lgh_hdr == NULL);
>  
>  	llh = kzalloc(sizeof(*llh), GFP_NOFS);
> -	if (llh == NULL)
> +	if (!llh)
>  		return -ENOMEM;
>  	handle->lgh_hdr = llh;
>  	/* first assign flags to use llog_client_ops */
> @@ -435,7 +435,7 @@ int llog_process_or_fork(const struct lu
>  	int		      rc;
>  
>  	lpi = kzalloc(sizeof(*lpi), GFP_NOFS);
> -	if (lpi == NULL) {
> +	if (!lpi) {
>  		CERROR("cannot alloc pointer\n");
>  		return -ENOMEM;
>  	}
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -172,7 +172,7 @@ int class_register_type(struct obd_ops *
>  
>  	rc = -ENOMEM;
>  	type = kzalloc(sizeof(*type), GFP_NOFS);
> -	if (type == NULL)
> +	if (!type)
>  		return rc;
>  
>  	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
> @@ -1016,7 +1016,7 @@ struct obd_import *class_new_import(stru
>  	struct obd_import *imp;
>  
>  	imp = kzalloc(sizeof(*imp), GFP_NOFS);
> -	if (imp == NULL)
> +	if (!imp)
>  		return NULL;
>  
>  	INIT_LIST_HEAD(&imp->imp_pinger_chain);
> @@ -1819,7 +1819,7 @@ void *kuc_alloc(int payload_len, int tra
>  	int len = kuc_len(payload_len);
>  
>  	lh = kzalloc(len, GFP_NOFS);
> -	if (lh == NULL)
> +	if (!lh)
>  		return ERR_PTR(-ENOMEM);
>  
>  	lh->kuc_magic = KUC_MAGIC;
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> @@ -232,7 +232,7 @@ int class_handle_ioctl(unsigned int cmd,
>  			goto out;
>  		}
>  		lcfg = kzalloc(data->ioc_plen1, GFP_NOFS);
> -		if (lcfg == NULL) {
> +		if (!lcfg) {
>  			err = -ENOMEM;
>  			goto out;
>  		}
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 09/12] staging: lustre: obdclass: Use !x to check for kzalloc failure
@ 2015-06-21 10:02     ` walter harms
  0 siblings, 0 replies; 80+ messages in thread
From: walter harms @ 2015-06-21 10:02 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Oleg Drokin, kernel-janitors, Andreas Dilger, Greg Kroah-Hartman,
	HPDD-discuss, devel, linux-kernel



Am 20.06.2015 18:59, schrieb Julia Lawall:
> !x is more normal for kzalloc failure in the kernel.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression x;
> statement S1, S2;
> @@
> 
> x = kzalloc(...);
> if (
> - x = NULL
> + !x
>  ) S1 else S2
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
>  drivers/staging/lustre/lustre/obdclass/class_obd.c      |    2 +-
>  drivers/staging/lustre/lustre/obdclass/genops.c         |    6 +++---
>  drivers/staging/lustre/lustre/obdclass/llog.c           |    6 +++---
>  drivers/staging/lustre/lustre/obdclass/lprocfs_status.c |    2 +-
>  drivers/staging/lustre/lustre/obdclass/lustre_peer.c    |    2 +-
>  drivers/staging/lustre/lustre/obdclass/obd_config.c     |   10 +++++-----
>  drivers/staging/lustre/lustre/obdclass/obd_mount.c      |   12 ++++++------
>  7 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> @@ -85,7 +85,7 @@ int lustre_process_log(struct super_bloc
>  	LASSERT(cfg);
>  
>  	bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
> -	if (bufs = NULL)
> +	if (!bufs)
>  		return -ENOMEM;
>  
>  	/* mgc_process_config */
> @@ -258,7 +258,7 @@ int lustre_start_mgc(struct super_block
>  	mgssec = lsi->lsi_lmd->lmd_mgssec ? lsi->lsi_lmd->lmd_mgssec : "";
>  
>  	data = kzalloc(sizeof(*data), GFP_NOFS);
> -	if (data = NULL) {
> +	if (!data) {
>  		rc = -ENOMEM;
>  		goto out_free;
>  	}
> @@ -885,7 +885,7 @@ static int lmd_parse_mgssec(struct lustr
>  		length = tail - ptr;
>  
>  	lmd->lmd_mgssec = kzalloc(length + 1, GFP_NOFS);
> -	if (lmd->lmd_mgssec = NULL)
> +	if (!lmd->lmd_mgssec)
>  		return -ENOMEM;
>  
>  	memcpy(lmd->lmd_mgssec, ptr, length);
looks like memdup()

> @@ -911,7 +911,7 @@ static int lmd_parse_string(char **handl
>  		length = tail - ptr;
>  
>  	*handle = kzalloc(length + 1, GFP_NOFS);
> -	if (*handle = NULL)
> +	if (!*handle)
>  		return -ENOMEM;
>  
>  	memcpy(*handle, ptr, length);

looks like memdup()


> @@ -941,7 +941,7 @@ static int lmd_parse_mgs(struct lustre_m
>  		oldlen = strlen(lmd->lmd_mgs) + 1;
>  
>  	mgsnid = kzalloc(oldlen + length + 1, GFP_NOFS);
> -	if (mgsnid = NULL)
> +	if (!mgsnid)
>  		return -ENOMEM;
>  
>  	if (lmd->lmd_mgs != NULL) {
> @@ -983,7 +983,7 @@ static int lmd_parse(char *options, stru
>  	lmd->lmd_magic = LMD_MAGIC;
>  
>  	lmd->lmd_params = kzalloc(4096, GFP_NOFS);
> -	if (lmd->lmd_params = NULL)
> +	if (!lmd->lmd_params)
>  		return -ENOMEM;
>  	lmd->lmd_params[0] = '\0';
>  
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
> --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
> @@ -835,7 +835,7 @@ int class_add_profile(int proflen, char
>  	CDEBUG(D_CONFIG, "Add profile %s\n", prof);
>  
>  	lprof = kzalloc(sizeof(*lprof), GFP_NOFS);
> -	if (lprof = NULL)
> +	if (!lprof)
>  		return -ENOMEM;
>  	INIT_LIST_HEAD(&lprof->lp_list);
>  
> @@ -979,7 +979,7 @@ struct lustre_cfg *lustre_cfg_rename(str
>  	new_len = LUSTRE_CFG_BUFLEN(cfg, 1) + strlen(new_name) - name_len;
>  
>  	new_param = kzalloc(new_len, GFP_NOFS);
> -	if (new_param = NULL)
> +	if (!new_param)
>  		return ERR_PTR(-ENOMEM);
>  
>  	strcpy(new_param, new_name);
> @@ -987,7 +987,7 @@ struct lustre_cfg *lustre_cfg_rename(str
>  		strcat(new_param, value);
>  
>  	bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
> -	if (bufs = NULL) {
> +	if (!bufs) {
>  		kfree(new_param);
>  		return ERR_PTR(-ENOMEM);
>  	}
> @@ -1461,7 +1461,7 @@ int class_config_llog_handler(const stru
>  			inst_len = LUSTRE_CFG_BUFLEN(lcfg, 0) +
>  				   sizeof(clli->cfg_instance) * 2 + 4;
>  			inst_name = kzalloc(inst_len, GFP_NOFS);
> -			if (inst_name = NULL) {
> +			if (!inst_name) {
>  				rc = -ENOMEM;
>  				goto out;
>  			}
> @@ -1639,7 +1639,7 @@ int class_config_dump_handler(const stru
>  	int	 rc = 0;
>  
>  	outstr = kzalloc(256, GFP_NOFS);
> -	if (outstr = NULL)
> +	if (!outstr)
>  		return -ENOMEM;
>  
>  	if (rec->lrh_type = OBD_CFG_REC) {
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> --- a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> @@ -105,7 +105,7 @@ int class_add_uuid(const char *uuid, __u
>  		return -EOVERFLOW;
>  
>  	data = kzalloc(sizeof(*data), GFP_NOFS);
> -	if (data = NULL)
> +	if (!data)
>  		return -ENOMEM;
>  
>  	obd_str2uuid(&data->un_uuid, uuid);
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> @@ -275,7 +275,7 @@ struct dentry *ldebugfs_add_symlink(cons
>  		return NULL;
>  
>  	dest = kzalloc(MAX_STRING_SIZE + 1, GFP_KERNEL);
> -	if (dest = NULL)
> +	if (!dest)
>  		return NULL;
>  
>  	va_start(ap, format);
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/llog.c b/drivers/staging/lustre/lustre/obdclass/llog.c
> --- a/drivers/staging/lustre/lustre/obdclass/llog.c
> +++ b/drivers/staging/lustre/lustre/obdclass/llog.c
> @@ -61,7 +61,7 @@ static struct llog_handle *llog_alloc_ha
>  	struct llog_handle *loghandle;
>  
>  	loghandle = kzalloc(sizeof(*loghandle), GFP_NOFS);
> -	if (loghandle = NULL)
> +	if (!loghandle)
>  		return NULL;
>  
>  	init_rwsem(&loghandle->lgh_lock);
> @@ -208,7 +208,7 @@ int llog_init_handle(const struct lu_env
>  	LASSERT(handle->lgh_hdr = NULL);
>  
>  	llh = kzalloc(sizeof(*llh), GFP_NOFS);
> -	if (llh = NULL)
> +	if (!llh)
>  		return -ENOMEM;
>  	handle->lgh_hdr = llh;
>  	/* first assign flags to use llog_client_ops */
> @@ -435,7 +435,7 @@ int llog_process_or_fork(const struct lu
>  	int		      rc;
>  
>  	lpi = kzalloc(sizeof(*lpi), GFP_NOFS);
> -	if (lpi = NULL) {
> +	if (!lpi) {
>  		CERROR("cannot alloc pointer\n");
>  		return -ENOMEM;
>  	}
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -172,7 +172,7 @@ int class_register_type(struct obd_ops *
>  
>  	rc = -ENOMEM;
>  	type = kzalloc(sizeof(*type), GFP_NOFS);
> -	if (type = NULL)
> +	if (!type)
>  		return rc;
>  
>  	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
> @@ -1016,7 +1016,7 @@ struct obd_import *class_new_import(stru
>  	struct obd_import *imp;
>  
>  	imp = kzalloc(sizeof(*imp), GFP_NOFS);
> -	if (imp = NULL)
> +	if (!imp)
>  		return NULL;
>  
>  	INIT_LIST_HEAD(&imp->imp_pinger_chain);
> @@ -1819,7 +1819,7 @@ void *kuc_alloc(int payload_len, int tra
>  	int len = kuc_len(payload_len);
>  
>  	lh = kzalloc(len, GFP_NOFS);
> -	if (lh = NULL)
> +	if (!lh)
>  		return ERR_PTR(-ENOMEM);
>  
>  	lh->kuc_magic = KUC_MAGIC;
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> @@ -232,7 +232,7 @@ int class_handle_ioctl(unsigned int cmd,
>  			goto out;
>  		}
>  		lcfg = kzalloc(data->ioc_plen1, GFP_NOFS);
> -		if (lcfg = NULL) {
> +		if (!lcfg) {
>  			err = -ENOMEM;
>  			goto out;
>  		}
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> 
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 09/12] staging: lustre: obdclass: Use !x to check for kzalloc failure
  2015-06-21 10:02     ` walter harms
@ 2015-06-21 10:29       ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-21 10:29 UTC (permalink / raw)
  To: walter harms
  Cc: Julia Lawall, Oleg Drokin, kernel-janitors, Andreas Dilger,
	Greg Kroah-Hartman, HPDD-discuss, devel, linux-kernel

> > @@ -885,7 +885,7 @@ static int lmd_parse_mgssec(struct lustr
> >  		length = tail - ptr;
> >  
> >  	lmd->lmd_mgssec = kzalloc(length + 1, GFP_NOFS);
> > -	if (lmd->lmd_mgssec == NULL)
> > +	if (!lmd->lmd_mgssec)
> >  		return -ENOMEM;
> >  
> >  	memcpy(lmd->lmd_mgssec, ptr, length);
> looks like memdup()

kmemdup has the same length for both calls.  There is kstrndup, but it
recalculates the length, which looks unnecessary here.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 09/12] staging: lustre: obdclass: Use !x to check for kzalloc failure
@ 2015-06-21 10:29       ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-21 10:29 UTC (permalink / raw)
  To: walter harms
  Cc: Julia Lawall, Oleg Drokin, kernel-janitors, Andreas Dilger,
	Greg Kroah-Hartman, HPDD-discuss, devel, linux-kernel

> > @@ -885,7 +885,7 @@ static int lmd_parse_mgssec(struct lustr
> >  		length = tail - ptr;
> >  
> >  	lmd->lmd_mgssec = kzalloc(length + 1, GFP_NOFS);
> > -	if (lmd->lmd_mgssec = NULL)
> > +	if (!lmd->lmd_mgssec)
> >  		return -ENOMEM;
> >  
> >  	memcpy(lmd->lmd_mgssec, ptr, length);
> looks like memdup()

kmemdup has the same length for both calls.  There is kstrndup, but it
recalculates the length, which looks unnecessary here.

julia
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 09/12] staging: lustre: obdclass: Use !x to check for kzalloc failure
  2015-06-20 16:59   ` Julia Lawall
  (?)
  (?)
@ 2015-06-21 11:58   ` walter harms
  -1 siblings, 0 replies; 80+ messages in thread
From: walter harms @ 2015-06-21 11:58 UTC (permalink / raw)
  To: kernel-janitors



Am 21.06.2015 12:29, schrieb Julia Lawall:
>>> @@ -885,7 +885,7 @@ static int lmd_parse_mgssec(struct lustr
>>>  		length = tail - ptr;
>>>  
>>>  	lmd->lmd_mgssec = kzalloc(length + 1, GFP_NOFS);
>>> -	if (lmd->lmd_mgssec = NULL)
>>> +	if (!lmd->lmd_mgssec)
>>>  		return -ENOMEM;
>>>  
>>>  	memcpy(lmd->lmd_mgssec, ptr, length);
>> looks like memdup()
> 
> kmemdup has the same length for both calls.  There is kstrndup, but it
> recalculates the length, which looks unnecessary here.
> 
> julia
> 

mmh, it seem to be use only while mount,
i am wonderingparsing a comma sperareted list must be commen problem for
all FS (and others) maybe i am looking in thew wrong place, there should be
some helpers available ? are they ?

re,
 wh
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] Fwd: [HPDD-discuss] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-20 16:58   ` Julia Lawall
  (?)
@ 2015-06-22 15:46   ` Patrick Farrell
  2015-06-22 17:18     ` Simmons, James A.
  2015-06-23  8:05     ` Dilger, Andreas
  -1 siblings, 2 replies; 80+ messages in thread
From: Patrick Farrell @ 2015-06-22 15:46 UTC (permalink / raw)
  To: lustre-devel

Question for lustre-devel...  Isn't this the opposite of the style that 
we're currently trying to use in Gerrit submissions?  Is my memory faulty?


-------- Original Message --------
Subject: 	[HPDD-discuss] [PATCH 01/12] staging: lustre: fid: Use !x to 
check for kzalloc failure
Date: 	Sat, 20 Jun 2015 18:58:59 +0200
From: 	Julia Lawall <Julia.Lawall@lip6.fr>
To: 	Oleg Drokin <oleg.drokin@intel.com>
CC: 	<devel@driverdev.osuosl.org>, Greg Kroah-Hartman 
<gregkh@linuxfoundation.org>, <kernel-janitors@vger.kernel.org>, 
<linux-kernel@vger.kernel.org>, <HPDD-discuss@lists.01.org>



!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
  ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
  drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c b/drivers/staging/lustre/lustre/fid/fid_request.c
--- a/drivers/staging/lustre/lustre/fid/fid_request.c
+++ b/drivers/staging/lustre/lustre/fid/fid_request.c
@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o
  	int rc;
  
  	cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);
-	if (cli->cl_seq == NULL)
+	if (!cli->cl_seq)
  		return -ENOMEM;
  
  	prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);
-	if (prefix == NULL) {
+	if (!prefix) {
  		rc = -ENOMEM;
  		goto out_free_seq;
  	}

_______________________________________________
HPDD-discuss mailing list
HPDD-discuss at lists.01.org
https://lists.01.org/mailman/listinfo/hpdd-discuss



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20150622/efb39765/attachment.htm>

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] Fwd: [HPDD-discuss] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-22 15:46   ` [lustre-devel] Fwd: [HPDD-discuss] " Patrick Farrell
@ 2015-06-22 17:18     ` Simmons, James A.
  2015-06-23  8:05     ` Dilger, Andreas
  1 sibling, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-06-22 17:18 UTC (permalink / raw)
  To: lustre-devel

>Question for lustre-devel...  Isn't this the opposite of the style that we're currently trying to use in Gerrit submissions?  Is my memory faulty?

You are correct. Is this change really necessary? I will respone shortly.


-------- Original Message --------
Subject:

[HPDD-discuss] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure

Date:

Sat, 20 Jun 2015 18:58:59 +0200

From:

Julia Lawall <Julia.Lawall@lip6.fr><mailto:Julia.Lawall@lip6.fr>

To:

Oleg Drokin <oleg.drokin@intel.com><mailto:oleg.drokin@intel.com>

CC:

<devel@driverdev.osuosl.org><mailto:devel@driverdev.osuosl.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org><mailto:gregkh@linuxfoundation.org>, <kernel-janitors@vger.kernel.org><mailto:kernel-janitors@vger.kernel.org>, <linux-kernel@vger.kernel.org><mailto:linux-kernel@vger.kernel.org>, <HPDD-discuss@lists.01.org><mailto:HPDD-discuss@lists.01.org>



!x is more normal for kzalloc failure in the kernel.



The semantic patch that makes this change is as follows:

(http://coccinelle.lip6.fr/)



// <smpl>

@@

expression x;

statement S1, S2;

@@



x = kzalloc(...);

if (

- x == NULL

+ !x

 ) S1 else S2

// </smpl>



Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr><mailto:Julia.Lawall@lip6.fr>



---

 drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--

 1 file changed, 2 insertions(+), 2 deletions(-)



diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c b/drivers/staging/lustre/lustre/fid/fid_request.c

--- a/drivers/staging/lustre/lustre/fid/fid_request.c

+++ b/drivers/staging/lustre/lustre/fid/fid_request.c

@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o

        int rc;



        cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);

-       if (cli->cl_seq == NULL)

+       if (!cli->cl_seq)

               return -ENOMEM;



        prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);

-       if (prefix == NULL) {

+       if (!prefix) {

               rc = -ENOMEM;

               goto out_free_seq;

        }



_______________________________________________

HPDD-discuss mailing list

HPDD-discuss at lists.01.org<mailto:HPDD-discuss@lists.01.org>

https://lists.01.org/mailman/listinfo/hpdd-discuss


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20150622/cc615a1b/attachment.htm>

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] Fwd: [HPDD-discuss] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-22 15:46   ` [lustre-devel] Fwd: [HPDD-discuss] " Patrick Farrell
  2015-06-22 17:18     ` Simmons, James A.
@ 2015-06-23  8:05     ` Dilger, Andreas
  1 sibling, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-06-23  8:05 UTC (permalink / raw)
  To: lustre-devel

Yes, and IMHO this set of patches is not an improvement.

Cheers, Andreas

On Jun 22, 2015, at 08:46, Patrick Farrell <paf at cray.com<mailto:paf@cray.com>> wrote:

Question for lustre-devel...  Isn't this the opposite of the style that we're currently trying to use in Gerrit submissions?  Is my memory faulty?


-------- Original Message --------
Subject:        [HPDD-discuss] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
Date:   Sat, 20 Jun 2015 18:58:59 +0200
From:   Julia Lawall <Julia.Lawall@lip6.fr><mailto:Julia.Lawall@lip6.fr>
To:     Oleg Drokin <oleg.drokin@intel.com><mailto:oleg.drokin@intel.com>
CC:     <devel@driverdev.osuosl.org><mailto:devel@driverdev.osuosl.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org><mailto:gregkh@linuxfoundation.org>, <kernel-janitors@vger.kernel.org><mailto:kernel-janitors@vger.kernel.org>, <linux-kernel@vger.kernel.org><mailto:linux-kernel@vger.kernel.org>, <HPDD-discuss@lists.01.org><mailto:HPDD-discuss@lists.01.org>



!x is more normal for kzalloc failure in the kernel.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x;
statement S1, S2;
@@

x = kzalloc(...);
if (
- x == NULL
+ !x
 ) S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr><mailto:Julia.Lawall@lip6.fr>

---
 drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c b/drivers/staging/lustre/lustre/fid/fid_request.c
--- a/drivers/staging/lustre/lustre/fid/fid_request.c
+++ b/drivers/staging/lustre/lustre/fid/fid_request.c
@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o
        int rc;

        cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);
-       if (cli->cl_seq == NULL)
+       if (!cli->cl_seq)
                return -ENOMEM;

        prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);
-       if (prefix == NULL) {
+       if (!prefix) {
                rc = -ENOMEM;
                goto out_free_seq;
        }

_______________________________________________
HPDD-discuss mailing list
HPDD-discuss at lists.01.org<mailto:HPDD-discuss@lists.01.org>
https://lists.01.org/mailman/listinfo/hpdd-discuss



_______________________________________________
lustre-devel mailing list
lustre-devel at lists.lustre.org<mailto:lustre-devel@lists.lustre.org>
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-20 16:58   ` Julia Lawall
  (?)
@ 2015-06-23  8:25     ` Dilger, Andreas
  -1 siblings, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-06-23  8:25 UTC (permalink / raw)
  To: Julia Lawall, Drokin, Oleg
  Cc: kernel-janitors, Greg Kroah-Hartman, lustre-devel, devel, linux-kernel

On 2015/06/20, 10:58 AM, "Julia Lawall" <Julia.Lawall@lip6.fr> wrote:

>!x is more normal for kzalloc failure in the kernel.

While "!x" might be more normal for kzalloc(), I don't see that as an
improvement over explicitly checking against NULL, which is what kzalloc()
and other memory-allocating functions return on error.

I've found in the past that developers can introduce bugs when they treat
return values as boolean when they really aren't.  I'd prefer that the
code is kept with explicit comparisons against NULL, as it is today.
Most of the cases that are now using "!x" are from your previous patches.

Cheers, Andreas

>Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
>---
> drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c
>b/drivers/staging/lustre/lustre/fid/fid_request.c
>--- a/drivers/staging/lustre/lustre/fid/fid_request.c
>+++ b/drivers/staging/lustre/lustre/fid/fid_request.c
>@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o
> 	int rc;
> 
> 	cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);
>-	if (cli->cl_seq == NULL)
>+	if (!cli->cl_seq)
> 		return -ENOMEM;
> 
> 	prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);
>-	if (prefix == NULL) {
>+	if (!prefix) {
> 		rc = -ENOMEM;
> 		goto out_free_seq;
> 	}
>
>


Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division



^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23  8:25     ` Dilger, Andreas
  0 siblings, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-06-23  8:25 UTC (permalink / raw)
  To: Julia Lawall, Drokin, Oleg
  Cc: kernel-janitors, Greg Kroah-Hartman, lustre-devel, devel, linux-kernel

On 2015/06/20, 10:58 AM, "Julia Lawall" <Julia.Lawall@lip6.fr> wrote:

>!x is more normal for kzalloc failure in the kernel.

While "!x" might be more normal for kzalloc(), I don't see that as an
improvement over explicitly checking against NULL, which is what kzalloc()
and other memory-allocating functions return on error.

I've found in the past that developers can introduce bugs when they treat
return values as boolean when they really aren't.  I'd prefer that the
code is kept with explicit comparisons against NULL, as it is today.
Most of the cases that are now using "!x" are from your previous patches.

Cheers, Andreas

>Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
>---
> drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c
>b/drivers/staging/lustre/lustre/fid/fid_request.c
>--- a/drivers/staging/lustre/lustre/fid/fid_request.c
>+++ b/drivers/staging/lustre/lustre/fid/fid_request.c
>@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o
> 	int rc;
> 
> 	cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);
>-	if (cli->cl_seq = NULL)
>+	if (!cli->cl_seq)
> 		return -ENOMEM;
> 
> 	prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);
>-	if (prefix = NULL) {
>+	if (!prefix) {
> 		rc = -ENOMEM;
> 		goto out_free_seq;
> 	}
>
>


Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division



^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23  8:25     ` Dilger, Andreas
  0 siblings, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-06-23  8:25 UTC (permalink / raw)
  To: lustre-devel

On 2015/06/20, 10:58 AM, "Julia Lawall" <Julia.Lawall@lip6.fr> wrote:

>!x is more normal for kzalloc failure in the kernel.

While "!x" might be more normal for kzalloc(), I don't see that as an
improvement over explicitly checking against NULL, which is what kzalloc()
and other memory-allocating functions return on error.

I've found in the past that developers can introduce bugs when they treat
return values as boolean when they really aren't.  I'd prefer that the
code is kept with explicit comparisons against NULL, as it is today.
Most of the cases that are now using "!x" are from your previous patches.

Cheers, Andreas

>Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
>---
> drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c
>b/drivers/staging/lustre/lustre/fid/fid_request.c
>--- a/drivers/staging/lustre/lustre/fid/fid_request.c
>+++ b/drivers/staging/lustre/lustre/fid/fid_request.c
>@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o
> 	int rc;
> 
> 	cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);
>-	if (cli->cl_seq == NULL)
>+	if (!cli->cl_seq)
> 		return -ENOMEM;
> 
> 	prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);
>-	if (prefix == NULL) {
>+	if (!prefix) {
> 		rc = -ENOMEM;
> 		goto out_free_seq;
> 	}
>
>


Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-23  8:25     ` Dilger, Andreas
  (?)
@ 2015-06-23  9:23       ` Dan Carpenter
  -1 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-23  9:23 UTC (permalink / raw)
  To: Dilger, Andreas
  Cc: Julia Lawall, Drokin, Oleg, devel, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, lustre-devel

On Tue, Jun 23, 2015 at 08:25:05AM +0000, Dilger, Andreas wrote:
> I've found in the past that developers can introduce bugs when they treat
> return values as boolean when they really aren't.

I can imagine a bug like that where a function can return 0-2 and people
do:

	if (ret)

instead of:

	if (ret == 1)

but that bug is something else besides pointers so it doesn't apply
here.

What someone should do is try to measure it scientifically where we
flash some code on the screen and you have to press J for NULL and K for
non-NULL and we time it to the hundredth of a second.  I have a feeling
that (NULL != foo) is the worst way to write it because of the double
negative Yoda code.

Yoda code is the most useless thing ever.  I have actually measured this
and we introduce about 2 = vs == bugs per year.  It's probably less now
that we have so many static checks against it.  But people decided that
Yoda code was a good idea based on their gut instead of using statistics
and measurements and science.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23  9:23       ` Dan Carpenter
  0 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-23  9:23 UTC (permalink / raw)
  To: Dilger, Andreas
  Cc: Julia Lawall, Drokin, Oleg, devel, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, lustre-devel

On Tue, Jun 23, 2015 at 08:25:05AM +0000, Dilger, Andreas wrote:
> I've found in the past that developers can introduce bugs when they treat
> return values as boolean when they really aren't.

I can imagine a bug like that where a function can return 0-2 and people
do:

	if (ret)

instead of:

	if (ret = 1)

but that bug is something else besides pointers so it doesn't apply
here.

What someone should do is try to measure it scientifically where we
flash some code on the screen and you have to press J for NULL and K for
non-NULL and we time it to the hundredth of a second.  I have a feeling
that (NULL != foo) is the worst way to write it because of the double
negative Yoda code.

Yoda code is the most useless thing ever.  I have actually measured this
and we introduce about 2 = vs = bugs per year.  It's probably less now
that we have so many static checks against it.  But people decided that
Yoda code was a good idea based on their gut instead of using statistics
and measurements and science.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23  9:23       ` Dan Carpenter
  0 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-23  9:23 UTC (permalink / raw)
  To: lustre-devel

On Tue, Jun 23, 2015 at 08:25:05AM +0000, Dilger, Andreas wrote:
> I've found in the past that developers can introduce bugs when they treat
> return values as boolean when they really aren't.

I can imagine a bug like that where a function can return 0-2 and people
do:

	if (ret)

instead of:

	if (ret == 1)

but that bug is something else besides pointers so it doesn't apply
here.

What someone should do is try to measure it scientifically where we
flash some code on the screen and you have to press J for NULL and K for
non-NULL and we time it to the hundredth of a second.  I have a feeling
that (NULL != foo) is the worst way to write it because of the double
negative Yoda code.

Yoda code is the most useless thing ever.  I have actually measured this
and we introduce about 2 = vs == bugs per year.  It's probably less now
that we have so many static checks against it.  But people decided that
Yoda code was a good idea based on their gut instead of using statistics
and measurements and science.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-23  9:23       ` Dan Carpenter
  (?)
@ 2015-06-23  9:35         ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-23  9:35 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Dilger, Andreas, Julia Lawall, Drokin, Oleg, devel,
	Greg Kroah-Hartman, kernel-janitors, linux-kernel, lustre-devel

On Tue, 23 Jun 2015, Dan Carpenter wrote:

> On Tue, Jun 23, 2015 at 08:25:05AM +0000, Dilger, Andreas wrote:
> > I've found in the past that developers can introduce bugs when they treat
> > return values as boolean when they really aren't.
>
> I can imagine a bug like that where a function can return 0-2 and people
> do:
>
> 	if (ret)
>
> instead of:
>
> 	if (ret == 1)
>
> but that bug is something else besides pointers so it doesn't apply
> here.
>
> What someone should do is try to measure it scientifically where we
> flash some code on the screen and you have to press J for NULL and K for
> non-NULL and we time it to the hundredth of a second.  I have a feeling
> that (NULL != foo) is the worst way to write it because of the double
> negative Yoda code.
>
> Yoda code is the most useless thing ever.  I have actually measured this
> and we introduce about 2 = vs == bugs per year.  It's probably less now
> that we have so many static checks against it.  But people decided that
> Yoda code was a good idea based on their gut instead of using statistics
> and measurements and science.

In 2007, Al Viro said (https://lkml.org/lkml/2007/7/27/103):

Idiomatic form for "has allocation succeeded?" is neither "if (p != 0)"
nor "if (p != NULL)".  It's simply "if (p)".


>From the point of view of looking at kernel code, x == NULL for the result
of kmalloc etc looks verbose and distracting.

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23  9:35         ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-23  9:35 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Dilger, Andreas, Julia Lawall, Drokin, Oleg, devel,
	Greg Kroah-Hartman, kernel-janitors, linux-kernel, lustre-devel

On Tue, 23 Jun 2015, Dan Carpenter wrote:

> On Tue, Jun 23, 2015 at 08:25:05AM +0000, Dilger, Andreas wrote:
> > I've found in the past that developers can introduce bugs when they treat
> > return values as boolean when they really aren't.
>
> I can imagine a bug like that where a function can return 0-2 and people
> do:
>
> 	if (ret)
>
> instead of:
>
> 	if (ret = 1)
>
> but that bug is something else besides pointers so it doesn't apply
> here.
>
> What someone should do is try to measure it scientifically where we
> flash some code on the screen and you have to press J for NULL and K for
> non-NULL and we time it to the hundredth of a second.  I have a feeling
> that (NULL != foo) is the worst way to write it because of the double
> negative Yoda code.
>
> Yoda code is the most useless thing ever.  I have actually measured this
> and we introduce about 2 = vs = bugs per year.  It's probably less now
> that we have so many static checks against it.  But people decided that
> Yoda code was a good idea based on their gut instead of using statistics
> and measurements and science.

In 2007, Al Viro said (https://lkml.org/lkml/2007/7/27/103):

Idiomatic form for "has allocation succeeded?" is neither "if (p != 0)"
nor "if (p != NULL)".  It's simply "if (p)".


From the point of view of looking at kernel code, x = NULL for the result
of kmalloc etc looks verbose and distracting.

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23  9:35         ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-23  9:35 UTC (permalink / raw)
  To: lustre-devel

On Tue, 23 Jun 2015, Dan Carpenter wrote:

> On Tue, Jun 23, 2015 at 08:25:05AM +0000, Dilger, Andreas wrote:
> > I've found in the past that developers can introduce bugs when they treat
> > return values as boolean when they really aren't.
>
> I can imagine a bug like that where a function can return 0-2 and people
> do:
>
> 	if (ret)
>
> instead of:
>
> 	if (ret == 1)
>
> but that bug is something else besides pointers so it doesn't apply
> here.
>
> What someone should do is try to measure it scientifically where we
> flash some code on the screen and you have to press J for NULL and K for
> non-NULL and we time it to the hundredth of a second.  I have a feeling
> that (NULL != foo) is the worst way to write it because of the double
> negative Yoda code.
>
> Yoda code is the most useless thing ever.  I have actually measured this
> and we introduce about 2 = vs == bugs per year.  It's probably less now
> that we have so many static checks against it.  But people decided that
> Yoda code was a good idea based on their gut instead of using statistics
> and measurements and science.

In 2007, Al Viro said (https://lkml.org/lkml/2007/7/27/103):

Idiomatic form for "has allocation succeeded?" is neither "if (p != 0)"
nor "if (p != NULL)".  It's simply "if (p)".


From the point of view of looking at kernel code, x == NULL for the result
of kmalloc etc looks verbose and distracting.

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-23  9:35         ` Julia Lawall
  (?)
@ 2015-06-23  9:57           ` Dan Carpenter
  -1 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-23  9:57 UTC (permalink / raw)
  To: Julia Lawall
  Cc: devel, Dilger, Andreas, Greg Kroah-Hartman, kernel-janitors,
	linux-kernel, Drokin, Oleg, lustre-devel

Yes.  I know Al's thoughts and kernel style.

But Alan Cox and Andreas have both said they think (x == NULL) can help
you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
did massive seds changing !foo to foo == NULL on our code base.  But
I've never seen a real life example of a bug this fixes.

To be honest, I've never seen a real life proof that (!foo) code is less
buggy.  I should look through the kbuild mailbox...  Hm...  But my other
idea of setting up code style readability testing website is also a good
one.

Linux kernel style is based on Joe Perches finding that 80% of the code
prefers one way or the other.  That's a valid method for determining
code style.  I bet it normally picks the more readable style but it
would be interesting to measure it more formally.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23  9:57           ` Dan Carpenter
  0 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-23  9:57 UTC (permalink / raw)
  To: Julia Lawall
  Cc: devel, Dilger, Andreas, Greg Kroah-Hartman, kernel-janitors,
	linux-kernel, Drokin, Oleg, lustre-devel

Yes.  I know Al's thoughts and kernel style.

But Alan Cox and Andreas have both said they think (x = NULL) can help
you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
did massive seds changing !foo to foo = NULL on our code base.  But
I've never seen a real life example of a bug this fixes.

To be honest, I've never seen a real life proof that (!foo) code is less
buggy.  I should look through the kbuild mailbox...  Hm...  But my other
idea of setting up code style readability testing website is also a good
one.

Linux kernel style is based on Joe Perches finding that 80% of the code
prefers one way or the other.  That's a valid method for determining
code style.  I bet it normally picks the more readable style but it
would be interesting to measure it more formally.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23  9:57           ` Dan Carpenter
  0 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-23  9:57 UTC (permalink / raw)
  To: lustre-devel

Yes.  I know Al's thoughts and kernel style.

But Alan Cox and Andreas have both said they think (x == NULL) can help
you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
did massive seds changing !foo to foo == NULL on our code base.  But
I've never seen a real life example of a bug this fixes.

To be honest, I've never seen a real life proof that (!foo) code is less
buggy.  I should look through the kbuild mailbox...  Hm...  But my other
idea of setting up code style readability testing website is also a good
one.

Linux kernel style is based on Joe Perches finding that 80% of the code
prefers one way or the other.  That's a valid method for determining
code style.  I bet it normally picks the more readable style but it
would be interesting to measure it more formally.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-23  9:57           ` Dan Carpenter
  (?)
@ 2015-06-23 10:51             ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-23 10:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, devel, Dilger, Andreas, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, Drokin, Oleg, lustre-devel



On Tue, 23 Jun 2015, Dan Carpenter wrote:

> Yes.  I know Al's thoughts and kernel style.
>
> But Alan Cox and Andreas have both said they think (x == NULL) can help
> you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
> did massive seds changing !foo to foo == NULL on our code base.  But
> I've never seen a real life example of a bug this fixes.
>
> To be honest, I've never seen a real life proof that (!foo) code is less
> buggy.  I should look through the kbuild mailbox...  Hm...  But my other
> idea of setting up code style readability testing website is also a good
> one.
>
> Linux kernel style is based on Joe Perches finding that 80% of the code
> prefers one way or the other.  That's a valid method for determining
> code style.  I bet it normally picks the more readable style but it
> would be interesting to measure it more formally.

On today's linux-next, I find 3218 tests on the result of kmalloc etc
using NULL and 14429 without, making 82% without.  The complete semantic
patch is shown below.

julia

@initialize:ocaml@
@@

let withnull = ref 0
let withoutnull = ref 0

@r1 disable is_null, isnt_null1 exists@
expression x,e;
position p;
statement S1,S2;
@@

x = \(kmalloc\|kzalloc\|kcalloc\|devm_kmalloc\|devm_kzalloc\)(...)
... when != x = e
    when != &x
if@p (<+...\(x == NULL\|x != NULL\|NULL == x\|NULL != x\)...+>) S1 else S2

@r2 disable not_ptr1, not_ptr2 exists@
expression x,e;
position p;
statement S1,S2;
@@

x = \(kmalloc\|kzalloc\|kcalloc\|devm_kmalloc\|devm_kzalloc\)(...)
... when != x = e
    when != &x
if@p (<+...\(!x\|x && ...\|x || ...\)...+>) S1 else S2

@script:ocaml@
_p << r1.p;
@@
withnull := !withnull + 1

@script:ocaml@
_p << r2.p;
@@
withoutnull := !withoutnull + 1

@finalize:ocaml@
@@

Printf.printf "withnull %d withoutnull %d\n" !withnull !withoutnull


^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23 10:51             ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-23 10:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, devel, Dilger, Andreas, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, Drokin, Oleg, lustre-devel



On Tue, 23 Jun 2015, Dan Carpenter wrote:

> Yes.  I know Al's thoughts and kernel style.
>
> But Alan Cox and Andreas have both said they think (x = NULL) can help
> you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
> did massive seds changing !foo to foo = NULL on our code base.  But
> I've never seen a real life example of a bug this fixes.
>
> To be honest, I've never seen a real life proof that (!foo) code is less
> buggy.  I should look through the kbuild mailbox...  Hm...  But my other
> idea of setting up code style readability testing website is also a good
> one.
>
> Linux kernel style is based on Joe Perches finding that 80% of the code
> prefers one way or the other.  That's a valid method for determining
> code style.  I bet it normally picks the more readable style but it
> would be interesting to measure it more formally.

On today's linux-next, I find 3218 tests on the result of kmalloc etc
using NULL and 14429 without, making 82% without.  The complete semantic
patch is shown below.

julia

@initialize:ocaml@
@@

let withnull = ref 0
let withoutnull = ref 0

@r1 disable is_null, isnt_null1 exists@
expression x,e;
position p;
statement S1,S2;
@@

x = \(kmalloc\|kzalloc\|kcalloc\|devm_kmalloc\|devm_kzalloc\)(...)
... when != x = e
    when != &x
if@p (<+...\(x = NULL\|x != NULL\|NULL = x\|NULL != x\)...+>) S1 else S2

@r2 disable not_ptr1, not_ptr2 exists@
expression x,e;
position p;
statement S1,S2;
@@

x = \(kmalloc\|kzalloc\|kcalloc\|devm_kmalloc\|devm_kzalloc\)(...)
... when != x = e
    when != &x
if@p (<+...\(!x\|x && ...\|x || ...\)...+>) S1 else S2

@script:ocaml@
_p << r1.p;
@@
withnull := !withnull + 1

@script:ocaml@
_p << r2.p;
@@
withoutnull := !withoutnull + 1

@finalize:ocaml@
@@

Printf.printf "withnull %d withoutnull %d\n" !withnull !withoutnull


^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23 10:51             ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-23 10:51 UTC (permalink / raw)
  To: lustre-devel



On Tue, 23 Jun 2015, Dan Carpenter wrote:

> Yes.  I know Al's thoughts and kernel style.
>
> But Alan Cox and Andreas have both said they think (x == NULL) can help
> you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
> did massive seds changing !foo to foo == NULL on our code base.  But
> I've never seen a real life example of a bug this fixes.
>
> To be honest, I've never seen a real life proof that (!foo) code is less
> buggy.  I should look through the kbuild mailbox...  Hm...  But my other
> idea of setting up code style readability testing website is also a good
> one.
>
> Linux kernel style is based on Joe Perches finding that 80% of the code
> prefers one way or the other.  That's a valid method for determining
> code style.  I bet it normally picks the more readable style but it
> would be interesting to measure it more formally.

On today's linux-next, I find 3218 tests on the result of kmalloc etc
using NULL and 14429 without, making 82% without.  The complete semantic
patch is shown below.

julia

@initialize:ocaml@
@@

let withnull = ref 0
let withoutnull = ref 0

@r1 disable is_null, isnt_null1 exists@
expression x,e;
position p;
statement S1,S2;
@@

x = \(kmalloc\|kzalloc\|kcalloc\|devm_kmalloc\|devm_kzalloc\)(...)
... when != x = e
    when != &x
if at p (<+...\(x == NULL\|x != NULL\|NULL == x\|NULL != x\)...+>) S1 else S2

@r2 disable not_ptr1, not_ptr2 exists@
expression x,e;
position p;
statement S1,S2;
@@

x = \(kmalloc\|kzalloc\|kcalloc\|devm_kmalloc\|devm_kzalloc\)(...)
... when != x = e
    when != &x
if at p (<+...\(!x\|x && ...\|x || ...\)...+>) S1 else S2

@script:ocaml@
_p << r1.p;
@@
withnull := !withnull + 1

@script:ocaml@
_p << r2.p;
@@
withoutnull := !withoutnull + 1

@finalize:ocaml@
@@

Printf.printf "withnull %d withoutnull %d\n" !withnull !withoutnull

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-23  9:57           ` Dan Carpenter
  (?)
@ 2015-06-23 22:03             ` Joe Perches
  -1 siblings, 0 replies; 80+ messages in thread
From: Joe Perches @ 2015-06-23 22:03 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, devel, Dilger, Andreas, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, Drokin, Oleg, lustre-devel

On Tue, 2015-06-23 at 12:57 +0300, Dan Carpenter wrote:
> I've never seen a real life proof that (!foo) code is less
> buggy.

Nor have I.

> I should look through the kbuild mailbox...  Hm...  But my other
> idea of setting up code style readability testing website is also a good
> one.
> 
> Linux kernel style is based on Joe Perches finding that 80% of the code
> prefers one way or the other.  That's a valid method for determining
> code style.  I bet it normally picks the more readable style but it
> would be interesting to measure it more formally.

That might be hard to do well.

A code readability testing website is going to be
fundamentally biased by the experiences of the coder
that is tested.

Flashing code for millisecond type readability tests
has more correlation to quantity of white to black
than code content does of correctness to memorability.



^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23 22:03             ` Joe Perches
  0 siblings, 0 replies; 80+ messages in thread
From: Joe Perches @ 2015-06-23 22:03 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, devel, Dilger, Andreas, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, Drokin, Oleg, lustre-devel

On Tue, 2015-06-23 at 12:57 +0300, Dan Carpenter wrote:
> I've never seen a real life proof that (!foo) code is less
> buggy.

Nor have I.

> I should look through the kbuild mailbox...  Hm...  But my other
> idea of setting up code style readability testing website is also a good
> one.
> 
> Linux kernel style is based on Joe Perches finding that 80% of the code
> prefers one way or the other.  That's a valid method for determining
> code style.  I bet it normally picks the more readable style but it
> would be interesting to measure it more formally.

That might be hard to do well.

A code readability testing website is going to be
fundamentally biased by the experiences of the coder
that is tested.

Flashing code for millisecond type readability tests
has more correlation to quantity of white to black
than code content does of correctness to memorability.



^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23 22:03             ` Joe Perches
  0 siblings, 0 replies; 80+ messages in thread
From: Joe Perches @ 2015-06-23 22:03 UTC (permalink / raw)
  To: lustre-devel

On Tue, 2015-06-23 at 12:57 +0300, Dan Carpenter wrote:
> I've never seen a real life proof that (!foo) code is less
> buggy.

Nor have I.

> I should look through the kbuild mailbox...  Hm...  But my other
> idea of setting up code style readability testing website is also a good
> one.
> 
> Linux kernel style is based on Joe Perches finding that 80% of the code
> prefers one way or the other.  That's a valid method for determining
> code style.  I bet it normally picks the more readable style but it
> would be interesting to measure it more formally.

That might be hard to do well.

A code readability testing website is going to be
fundamentally biased by the experiences of the coder
that is tested.

Flashing code for millisecond type readability tests
has more correlation to quantity of white to black
than code content does of correctness to memorability.

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-23  9:23       ` Dan Carpenter
  (?)
@ 2015-06-23 22:11         ` Joe Perches
  -1 siblings, 0 replies; 80+ messages in thread
From: Joe Perches @ 2015-06-23 22:11 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Dilger, Andreas, Julia Lawall, Drokin, Oleg, devel,
	Greg Kroah-Hartman, kernel-janitors, linux-kernel, lustre-devel

On Tue, 2015-06-23 at 12:23 +0300, Dan Carpenter wrote:
> people decided that
> Yoda code was a good idea based on their gut instead of using statistics
> and measurements and science.

I think that style exists because compilers
disallow CONST = val assignment typos.



^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23 22:11         ` Joe Perches
  0 siblings, 0 replies; 80+ messages in thread
From: Joe Perches @ 2015-06-23 22:11 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Dilger, Andreas, Julia Lawall, Drokin, Oleg, devel,
	Greg Kroah-Hartman, kernel-janitors, linux-kernel, lustre-devel

On Tue, 2015-06-23 at 12:23 +0300, Dan Carpenter wrote:
> people decided that
> Yoda code was a good idea based on their gut instead of using statistics
> and measurements and science.

I think that style exists because compilers
disallow CONST = val assignment typos.



^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-23 22:11         ` Joe Perches
  0 siblings, 0 replies; 80+ messages in thread
From: Joe Perches @ 2015-06-23 22:11 UTC (permalink / raw)
  To: lustre-devel

On Tue, 2015-06-23 at 12:23 +0300, Dan Carpenter wrote:
> people decided that
> Yoda code was a good idea based on their gut instead of using statistics
> and measurements and science.

I think that style exists because compilers
disallow CONST = val assignment typos.

^ permalink raw reply	[flat|nested] 80+ messages in thread
* RE: [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
  2015-06-23 10:51             ` Julia Lawall
  (?)
@ 2015-06-24 20:14               ` Simmons, James A.
  -1 siblings, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-06-24 20:14 UTC (permalink / raw)
  To: 'Julia Lawall', Dan Carpenter
  Cc: devel, Greg Kroah-Hartman, kernel-janitors, linux-kernel, Drokin,
	Oleg, lustre-devel

>> Yes.  I know Al's thoughts and kernel style.
>>
>> But Alan Cox and Andreas have both said they think (x == NULL) can help
>> you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
>> did massive seds changing !foo to foo == NULL on our code base.  But
>> I've never seen a real life example of a bug this fixes.
>>
>> To be honest, I've never seen a real life proof that (!foo) code is less
>> buggy.  I should look through the kbuild mailbox...  Hm...  But my other
>> idea of setting up code style readability testing website is also a good
>> one.
>>
>> Linux kernel style is based on Joe Perches finding that 80% of the code
>> prefers one way or the other.  That's a valid method for determining
>> code style.  I bet it normally picks the more readable style but it
>> would be interesting to measure it more formally.
>
>On today's linux-next, I find 3218 tests on the result of kmalloc etc
>using NULL and 14429 without, making 82% without.  The complete semantic
>patch is shown below.

Most people doing something a certain way is not a technical argument. Usually
people do what they are taught. From most people's comments their seems to 
be no technical reason to us one over another. I do have one technical reason not
to accept these patches. It is too easy to make a mistake and break things very badly.
I don't think it is worth the risk for a non-hard requirement.


^ permalink raw reply	[flat|nested] 80+ messages in thread
* RE: [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-24 20:14               ` Simmons, James A.
  0 siblings, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-06-24 20:14 UTC (permalink / raw)
  To: 'Julia Lawall', Dan Carpenter
  Cc: devel, Greg Kroah-Hartman, kernel-janitors, linux-kernel, Drokin,
	Oleg, lustre-devel

>> Yes.  I know Al's thoughts and kernel style.
>>
>> But Alan Cox and Andreas have both said they think (x = NULL) can help
>> you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
>> did massive seds changing !foo to foo = NULL on our code base.  But
>> I've never seen a real life example of a bug this fixes.
>>
>> To be honest, I've never seen a real life proof that (!foo) code is less
>> buggy.  I should look through the kbuild mailbox...  Hm...  But my other
>> idea of setting up code style readability testing website is also a good
>> one.
>>
>> Linux kernel style is based on Joe Perches finding that 80% of the code
>> prefers one way or the other.  That's a valid method for determining
>> code style.  I bet it normally picks the more readable style but it
>> would be interesting to measure it more formally.
>
>On today's linux-next, I find 3218 tests on the result of kmalloc etc
>using NULL and 14429 without, making 82% without.  The complete semantic
>patch is shown below.

Most people doing something a certain way is not a technical argument. Usually
people do what they are taught. From most people's comments their seems to 
be no technical reason to us one over another. I do have one technical reason not
to accept these patches. It is too easy to make a mistake and break things very badly.
I don't think it is worth the risk for a non-hard requirement.


^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
@ 2015-06-24 20:14               ` Simmons, James A.
  0 siblings, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-06-24 20:14 UTC (permalink / raw)
  To: lustre-devel

>> Yes.  I know Al's thoughts and kernel style.
>>
>> But Alan Cox and Andreas have both said they think (x == NULL) can help
>> you avoid some kind of boolean vs pointer bugs.  I've had co-workers who
>> did massive seds changing !foo to foo == NULL on our code base.  But
>> I've never seen a real life example of a bug this fixes.
>>
>> To be honest, I've never seen a real life proof that (!foo) code is less
>> buggy.  I should look through the kbuild mailbox...  Hm...  But my other
>> idea of setting up code style readability testing website is also a good
>> one.
>>
>> Linux kernel style is based on Joe Perches finding that 80% of the code
>> prefers one way or the other.  That's a valid method for determining
>> code style.  I bet it normally picks the more readable style but it
>> would be interesting to measure it more formally.
>
>On today's linux-next, I find 3218 tests on the result of kmalloc etc
>using NULL and 14429 without, making 82% without.  The complete semantic
>patch is shown below.

Most people doing something a certain way is not a technical argument. Usually
people do what they are taught. From most people's comments their seems to 
be no technical reason to us one over another. I do have one technical reason not
to accept these patches. It is too easy to make a mistake and break things very badly.
I don't think it is worth the risk for a non-hard requirement.

^ permalink raw reply	[flat|nested] 80+ messages in thread
* LIBCFS_ALLOC
  2015-06-23  8:25     ` Dilger, Andreas
  (?)
@ 2015-06-28  6:52       ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-28  6:52 UTC (permalink / raw)
  To: Dilger, Andreas
  Cc: Julia Lawall, Drokin, Oleg, kernel-janitors, Greg Kroah-Hartman,
	lustre-devel, devel, linux-kernel

It is not clear that all of the uses of LIBCFS_ALLOC really risk needing 
vmalloc.  For example:

lnet/klnds/socklnd/socklnd.c, function ksocknal_accept:

ksock_connreq_t *cr;
...
LIBCFS_ALLOC(cr, sizeof(*cr));

The definition of ksock_connreq_t is:

typedef struct ksock_connreq {
        struct list_head ksncr_list;  /* stash on ksnd_connd_connreqs */
        lnet_ni_t        *ksncr_ni;   /* chosen NI */
        struct socket    *ksncr_sock; /* accepted socket */
} ksock_connreq_t;

This looks like a very small structure.

LIBCFS_ALLOC relies on a test on the size, which should be able to be 
compiled away.  libcfs_kvzalloc on the other hand relies on the failure of 
kmalloc and so the test for that won't be compiled away.

Does it matter?

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* LIBCFS_ALLOC
@ 2015-06-28  6:52       ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-28  6:52 UTC (permalink / raw)
  To: Dilger, Andreas
  Cc: Julia Lawall, Drokin, Oleg, kernel-janitors, Greg Kroah-Hartman,
	lustre-devel, devel, linux-kernel

It is not clear that all of the uses of LIBCFS_ALLOC really risk needing 
vmalloc.  For example:

lnet/klnds/socklnd/socklnd.c, function ksocknal_accept:

ksock_connreq_t *cr;
...
LIBCFS_ALLOC(cr, sizeof(*cr));

The definition of ksock_connreq_t is:

typedef struct ksock_connreq {
        struct list_head ksncr_list;  /* stash on ksnd_connd_connreqs */
        lnet_ni_t        *ksncr_ni;   /* chosen NI */
        struct socket    *ksncr_sock; /* accepted socket */
} ksock_connreq_t;

This looks like a very small structure.

LIBCFS_ALLOC relies on a test on the size, which should be able to be 
compiled away.  libcfs_kvzalloc on the other hand relies on the failure of 
kmalloc and so the test for that won't be compiled away.

Does it matter?

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] LIBCFS_ALLOC
@ 2015-06-28  6:52       ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-28  6:52 UTC (permalink / raw)
  To: lustre-devel

It is not clear that all of the uses of LIBCFS_ALLOC really risk needing 
vmalloc.  For example:

lnet/klnds/socklnd/socklnd.c, function ksocknal_accept:

ksock_connreq_t *cr;
...
LIBCFS_ALLOC(cr, sizeof(*cr));

The definition of ksock_connreq_t is:

typedef struct ksock_connreq {
        struct list_head ksncr_list;  /* stash on ksnd_connd_connreqs */
        lnet_ni_t        *ksncr_ni;   /* chosen NI */
        struct socket    *ksncr_sock; /* accepted socket */
} ksock_connreq_t;

This looks like a very small structure.

LIBCFS_ALLOC relies on a test on the size, which should be able to be 
compiled away.  libcfs_kvzalloc on the other hand relies on the failure of 
kmalloc and so the test for that won't be compiled away.

Does it matter?

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: LIBCFS_ALLOC
  2015-06-28  6:52       ` LIBCFS_ALLOC Julia Lawall
  (?)
@ 2015-06-28 21:54         ` Dan Carpenter
  -1 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-28 21:54 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Dilger, Andreas, devel, Greg Kroah-Hartman, kernel-janitors,
	linux-kernel, Drokin, Oleg, lustre-devel

Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
a tiny sliver of RAM isn't going to work.  It's easier to use
libcfs_kvzalloc() everywhere, but it's probably the wrong thing.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: LIBCFS_ALLOC
@ 2015-06-28 21:54         ` Dan Carpenter
  0 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-28 21:54 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Dilger, Andreas, devel, Greg Kroah-Hartman, kernel-janitors,
	linux-kernel, Drokin, Oleg, lustre-devel

Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
a tiny sliver of RAM isn't going to work.  It's easier to use
libcfs_kvzalloc() everywhere, but it's probably the wrong thing.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] LIBCFS_ALLOC
@ 2015-06-28 21:54         ` Dan Carpenter
  0 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-28 21:54 UTC (permalink / raw)
  To: lustre-devel

Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
a tiny sliver of RAM isn't going to work.  It's easier to use
libcfs_kvzalloc() everywhere, but it's probably the wrong thing.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 80+ messages in thread
* RE: LIBCFS_ALLOC
  2015-06-28 21:54         ` LIBCFS_ALLOC Dan Carpenter
@ 2015-06-30 14:56           ` Simmons, James A.
  -1 siblings, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-06-30 14:56 UTC (permalink / raw)
  To: 'Dan Carpenter', Julia Lawall
  Cc: devel, Dilger, Andreas, Greg Kroah-Hartman, kernel-janitors,
	linux-kernel, Drokin, Oleg, lustre-devel

>Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
>a tiny sliver of RAM isn't going to work.  It's easier to use
>libcfs_kvzalloc() everywhere, but it's probably the wrong thing.

The original  reason we have the vmalloc water mark wasn't so much the
issue of memory exhaustion but to handle the case of memory fragmentation. 
Some sites had after a extended period of time started to see failures of
allocating even 32K using kmalloc.  In our latest development branch we moved
away from using a water mark to always try kmalloc first and if it fails then we
try vmalloc. At ORNL we ran into severe performance issues when we entered
vmalloc territory. It has been discussed before on what might replace vmalloc
handling in the case of kmalloc fails but no solution has been worked out.

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] LIBCFS_ALLOC
@ 2015-06-30 14:56           ` Simmons, James A.
  0 siblings, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-06-30 14:56 UTC (permalink / raw)
  To: lustre-devel

>Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
>a tiny sliver of RAM isn't going to work.  It's easier to use
>libcfs_kvzalloc() everywhere, but it's probably the wrong thing.

The original  reason we have the vmalloc water mark wasn't so much the
issue of memory exhaustion but to handle the case of memory fragmentation. 
Some sites had after a extended period of time started to see failures of
allocating even 32K using kmalloc.  In our latest development branch we moved
away from using a water mark to always try kmalloc first and if it fails then we
try vmalloc. At ORNL we ran into severe performance issues when we entered
vmalloc territory. It has been discussed before on what might replace vmalloc
handling in the case of kmalloc fails but no solution has been worked out.

^ permalink raw reply	[flat|nested] 80+ messages in thread
* RE: LIBCFS_ALLOC
  2015-06-30 14:56           ` [lustre-devel] LIBCFS_ALLOC Simmons, James A.
  (?)
@ 2015-06-30 15:01             ` Julia Lawall
  -1 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-30 15:01 UTC (permalink / raw)
  To: Simmons, James A.
  Cc: 'Dan Carpenter',
	Julia Lawall, devel, Dilger, Andreas, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, Drokin, Oleg, lustre-devel



On Tue, 30 Jun 2015, Simmons, James A. wrote:

> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
> >a tiny sliver of RAM isn't going to work.  It's easier to use
> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>
> The original  reason we have the vmalloc water mark wasn't so much the
> issue of memory exhaustion but to handle the case of memory fragmentation.
> Some sites had after a extended period of time started to see failures of
> allocating even 32K using kmalloc.  In our latest development branch we moved
> away from using a water mark to always try kmalloc first and if it fails then we
> try vmalloc. At ORNL we ran into severe performance issues when we entered
> vmalloc territory. It has been discussed before on what might replace vmalloc
> handling in the case of kmalloc fails but no solution has been worked out.

OK, but if a structure contains only 4 words, would it be better to just
use kzalloc?  Or does it not matter?  It would only save trying vmalloc in
a case that it is guaranteed to fail, but if a structure with 4 words
can't be allocatted, the system has other problems.  Another argument is
that kzalloc is a well known function that people and bug-finding tools
understand, so it is better to use it whenever possible.

Some of the other structures contain a lot more fields, as well as small
arrays.  They are probably acceptable for kzalloc too, but I wouldn't know
the exact dividing line.

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* RE: LIBCFS_ALLOC
@ 2015-06-30 15:01             ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-30 15:01 UTC (permalink / raw)
  To: Simmons, James A.
  Cc: 'Dan Carpenter',
	Julia Lawall, devel, Dilger, Andreas, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, Drokin, Oleg, lustre-devel



On Tue, 30 Jun 2015, Simmons, James A. wrote:

> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
> >a tiny sliver of RAM isn't going to work.  It's easier to use
> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>
> The original  reason we have the vmalloc water mark wasn't so much the
> issue of memory exhaustion but to handle the case of memory fragmentation.
> Some sites had after a extended period of time started to see failures of
> allocating even 32K using kmalloc.  In our latest development branch we moved
> away from using a water mark to always try kmalloc first and if it fails then we
> try vmalloc. At ORNL we ran into severe performance issues when we entered
> vmalloc territory. It has been discussed before on what might replace vmalloc
> handling in the case of kmalloc fails but no solution has been worked out.

OK, but if a structure contains only 4 words, would it be better to just
use kzalloc?  Or does it not matter?  It would only save trying vmalloc in
a case that it is guaranteed to fail, but if a structure with 4 words
can't be allocatted, the system has other problems.  Another argument is
that kzalloc is a well known function that people and bug-finding tools
understand, so it is better to use it whenever possible.

Some of the other structures contain a lot more fields, as well as small
arrays.  They are probably acceptable for kzalloc too, but I wouldn't know
the exact dividing line.

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] LIBCFS_ALLOC
@ 2015-06-30 15:01             ` Julia Lawall
  0 siblings, 0 replies; 80+ messages in thread
From: Julia Lawall @ 2015-06-30 15:01 UTC (permalink / raw)
  To: lustre-devel



On Tue, 30 Jun 2015, Simmons, James A. wrote:

> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
> >a tiny sliver of RAM isn't going to work.  It's easier to use
> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>
> The original  reason we have the vmalloc water mark wasn't so much the
> issue of memory exhaustion but to handle the case of memory fragmentation.
> Some sites had after a extended period of time started to see failures of
> allocating even 32K using kmalloc.  In our latest development branch we moved
> away from using a water mark to always try kmalloc first and if it fails then we
> try vmalloc. At ORNL we ran into severe performance issues when we entered
> vmalloc territory. It has been discussed before on what might replace vmalloc
> handling in the case of kmalloc fails but no solution has been worked out.

OK, but if a structure contains only 4 words, would it be better to just
use kzalloc?  Or does it not matter?  It would only save trying vmalloc in
a case that it is guaranteed to fail, but if a structure with 4 words
can't be allocatted, the system has other problems.  Another argument is
that kzalloc is a well known function that people and bug-finding tools
understand, so it is better to use it whenever possible.

Some of the other structures contain a lot more fields, as well as small
arrays.  They are probably acceptable for kzalloc too, but I wouldn't know
the exact dividing line.

julia

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: LIBCFS_ALLOC
  2015-06-30 14:56           ` [lustre-devel] LIBCFS_ALLOC Simmons, James A.
  (?)
@ 2015-06-30 17:38             ` Dan Carpenter
  -1 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-30 17:38 UTC (permalink / raw)
  To: Simmons, James A.
  Cc: Julia Lawall, devel, Dilger, Andreas, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, Drokin, Oleg, lustre-devel

All that you are saying is true and stuff that Julia and I have
discussed before.  For this call site though we are not allocating 32k,
we're allocating 4 pointers so libcfs_kvzalloc() doesn't make sense.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: LIBCFS_ALLOC
@ 2015-06-30 17:38             ` Dan Carpenter
  0 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-30 17:38 UTC (permalink / raw)
  To: Simmons, James A.
  Cc: Julia Lawall, devel, Dilger, Andreas, Greg Kroah-Hartman,
	kernel-janitors, linux-kernel, Drokin, Oleg, lustre-devel

All that you are saying is true and stuff that Julia and I have
discussed before.  For this call site though we are not allocating 32k,
we're allocating 4 pointers so libcfs_kvzalloc() doesn't make sense.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] LIBCFS_ALLOC
@ 2015-06-30 17:38             ` Dan Carpenter
  0 siblings, 0 replies; 80+ messages in thread
From: Dan Carpenter @ 2015-06-30 17:38 UTC (permalink / raw)
  To: lustre-devel

All that you are saying is true and stuff that Julia and I have
discussed before.  For this call site though we are not allocating 32k,
we're allocating 4 pointers so libcfs_kvzalloc() doesn't make sense.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [lustre-devel] LIBCFS_ALLOC
  2015-06-28  6:52       ` LIBCFS_ALLOC Julia Lawall
@ 2015-06-30 21:26         ` Dilger, Andreas
  -1 siblings, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-06-30 21:26 UTC (permalink / raw)
  To: Julia Lawall
  Cc: devel, Greg Kroah-Hartman, kernel-janitors, linux-kernel, Drokin,
	Oleg, lustre-devel

On 2015/06/28, 12:52 AM, "Julia Lawall" <julia.lawall@lip6.fr> wrote:

>It is not clear that all of the uses of LIBCFS_ALLOC really risk needing
>vmalloc.  For example:
>
>lnet/klnds/socklnd/socklnd.c, function ksocknal_accept:
>
>ksock_connreq_t *cr;
>...
>LIBCFS_ALLOC(cr, sizeof(*cr));
>
>The definition of ksock_connreq_t is:
>
>typedef struct ksock_connreq {
>        struct list_head ksncr_list;  /* stash on ksnd_connd_connreqs */
>        lnet_ni_t        *ksncr_ni;   /* chosen NI */
>        struct socket    *ksncr_sock; /* accepted socket */
>} ksock_connreq_t;
>
>This looks like a very small structure.
>
>LIBCFS_ALLOC relies on a test on the size, which should be able to be
>compiled away.  libcfs_kvzalloc on the other hand relies on the failure
>of 
>kmalloc and so the test for that won't be compiled away.

There are probably only a handful of places where trying vmalloc() even
makes sense.  In most cases, LIBCFS_ALLOC() can be replaced by a straight
call to kmalloc() because the allocation size is small enough, and only a
few need to use libcfs_kvzalloc().  Anything at PAGE_SIZE or less will
either work with kmalloc() or it won't work at all.

I do agree with James' comments that vmalloc() was needed for both very
large allocations that can't be satisfied by kmalloc() at all, as well as
smaller allocations (anything over a couple of pages) due to fragmentation
of free pages after running for a long time.  I think libcfs_kvzalloc() is
at least as good as the size-based threshold used previously, since using
kmalloc() is going to be faster than vmalloc() and would work better on
32-bit platforms with limited vmalloc() size.

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division



^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] LIBCFS_ALLOC
@ 2015-06-30 21:26         ` Dilger, Andreas
  0 siblings, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-06-30 21:26 UTC (permalink / raw)
  To: lustre-devel

On 2015/06/28, 12:52 AM, "Julia Lawall" <julia.lawall@lip6.fr> wrote:

>It is not clear that all of the uses of LIBCFS_ALLOC really risk needing
>vmalloc.  For example:
>
>lnet/klnds/socklnd/socklnd.c, function ksocknal_accept:
>
>ksock_connreq_t *cr;
>...
>LIBCFS_ALLOC(cr, sizeof(*cr));
>
>The definition of ksock_connreq_t is:
>
>typedef struct ksock_connreq {
>        struct list_head ksncr_list;  /* stash on ksnd_connd_connreqs */
>        lnet_ni_t        *ksncr_ni;   /* chosen NI */
>        struct socket    *ksncr_sock; /* accepted socket */
>} ksock_connreq_t;
>
>This looks like a very small structure.
>
>LIBCFS_ALLOC relies on a test on the size, which should be able to be
>compiled away.  libcfs_kvzalloc on the other hand relies on the failure
>of 
>kmalloc and so the test for that won't be compiled away.

There are probably only a handful of places where trying vmalloc() even
makes sense.  In most cases, LIBCFS_ALLOC() can be replaced by a straight
call to kmalloc() because the allocation size is small enough, and only a
few need to use libcfs_kvzalloc().  Anything at PAGE_SIZE or less will
either work with kmalloc() or it won't work at all.

I do agree with James' comments that vmalloc() was needed for both very
large allocations that can't be satisfied by kmalloc() at all, as well as
smaller allocations (anything over a couple of pages) due to fragmentation
of free pages after running for a long time.  I think libcfs_kvzalloc() is
at least as good as the size-based threshold used previously, since using
kmalloc() is going to be faster than vmalloc() and would work better on
32-bit platforms with limited vmalloc() size.

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division

^ permalink raw reply	[flat|nested] 80+ messages in thread
* RE: [lustre-devel] LIBCFS_ALLOC
  2015-06-30 15:01             ` LIBCFS_ALLOC Julia Lawall
  (?)
@ 2015-07-02 22:25               ` Simmons, James A.
  -1 siblings, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-07-02 22:25 UTC (permalink / raw)
  To: 'Julia Lawall'
  Cc: devel, Greg Kroah-Hartman, kernel-janitors, linux-kernel, Drokin,
	Oleg, 'Dan Carpenter',
	lustre-devel


>> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
>> >a tiny sliver of RAM isn't going to work.  It's easier to use
>> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>>
>> The original  reason we have the vmalloc water mark wasn't so much the
>> issue of memory exhaustion but to handle the case of memory fragmentation.
>> Some sites had after a extended period of time started to see failures of
>> allocating even 32K using kmalloc.  In our latest development branch we moved
>> away from using a water mark to always try kmalloc first and if it fails then we
>> try vmalloc. At ORNL we ran into severe performance issues when we entered
>> vmalloc territory. It has been discussed before on what might replace vmalloc
>> handling in the case of kmalloc fails but no solution has been worked out.
>
>OK, but if a structure contains only 4 words, would it be better to just
>use kzalloc?  Or does it not matter?  It would only save trying vmalloc in
>a case that it is guaranteed to fail, but if a structure with 4 words
>can't be allocated, the system has other problems.  Another argument is
>that kzalloc is a well known function that people and bug-finding tools
>understand, so it is better to use it whenever possible.
>
>Some of the other structures contain a lot more fields, as well as small
>arrays.  They are probably acceptable for kzalloc too, but I wouldn't know
>the exact dividing line.

The reason I bring this up is to discuss sorting this out. Once long ago we had
just  LIBCFS_ALLOC. For some reason before my time OBD_ALLOC got spawned
off of that.  Currently LIBCFS_ALLOC is used just by the libcfs/LNet layer. Now 
OBD_ALLOC in our development branch has moved to a try kmalloc first and 
if it fails try vmalloc for any size memory allocation.  LIBCFS_ALLOC still does the
original approach.   So we have two possible solutions depending on if libcfs/LNet
needs to ever do a vmalloc.

One solution if libcfs/LNet never needs a vmalloc is remove LIBCFS_ALLOC and replace
it with kzalloc everywhere. We can then move libcfs_kzvalloc to the lustre layer and
port the change we did in the development branch to here of the try kmalloc then
vmalloc approach. 

The other approach is if libcfs/LNet does in some case need to use vmalloc we could 
then update LIBCFS_ALLOC to first try kmalloc then vmalloc. Once this is implemented
we can nuke the OBD_ALLOC system.

Either way I like to see it consolidated down to one system.

^ permalink raw reply	[flat|nested] 80+ messages in thread
* RE: [lustre-devel] LIBCFS_ALLOC
@ 2015-07-02 22:25               ` Simmons, James A.
  0 siblings, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-07-02 22:25 UTC (permalink / raw)
  To: 'Julia Lawall'
  Cc: devel, Greg Kroah-Hartman, kernel-janitors, linux-kernel, Drokin,
	Oleg, 'Dan Carpenter',
	lustre-devel


>> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
>> >a tiny sliver of RAM isn't going to work.  It's easier to use
>> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>>
>> The original  reason we have the vmalloc water mark wasn't so much the
>> issue of memory exhaustion but to handle the case of memory fragmentation.
>> Some sites had after a extended period of time started to see failures of
>> allocating even 32K using kmalloc.  In our latest development branch we moved
>> away from using a water mark to always try kmalloc first and if it fails then we
>> try vmalloc. At ORNL we ran into severe performance issues when we entered
>> vmalloc territory. It has been discussed before on what might replace vmalloc
>> handling in the case of kmalloc fails but no solution has been worked out.
>
>OK, but if a structure contains only 4 words, would it be better to just
>use kzalloc?  Or does it not matter?  It would only save trying vmalloc in
>a case that it is guaranteed to fail, but if a structure with 4 words
>can't be allocated, the system has other problems.  Another argument is
>that kzalloc is a well known function that people and bug-finding tools
>understand, so it is better to use it whenever possible.
>
>Some of the other structures contain a lot more fields, as well as small
>arrays.  They are probably acceptable for kzalloc too, but I wouldn't know
>the exact dividing line.

The reason I bring this up is to discuss sorting this out. Once long ago we had
just  LIBCFS_ALLOC. For some reason before my time OBD_ALLOC got spawned
off of that.  Currently LIBCFS_ALLOC is used just by the libcfs/LNet layer. Now 
OBD_ALLOC in our development branch has moved to a try kmalloc first and 
if it fails try vmalloc for any size memory allocation.  LIBCFS_ALLOC still does the
original approach.   So we have two possible solutions depending on if libcfs/LNet
needs to ever do a vmalloc.

One solution if libcfs/LNet never needs a vmalloc is remove LIBCFS_ALLOC and replace
it with kzalloc everywhere. We can then move libcfs_kzvalloc to the lustre layer and
port the change we did in the development branch to here of the try kmalloc then
vmalloc approach. 

The other approach is if libcfs/LNet does in some case need to use vmalloc we could 
then update LIBCFS_ALLOC to first try kmalloc then vmalloc. Once this is implemented
we can nuke the OBD_ALLOC system.

Either way I like to see it consolidated down to one system.

^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] LIBCFS_ALLOC
@ 2015-07-02 22:25               ` Simmons, James A.
  0 siblings, 0 replies; 80+ messages in thread
From: Simmons, James A. @ 2015-07-02 22:25 UTC (permalink / raw)
  To: lustre-devel


>> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have even
>> >a tiny sliver of RAM isn't going to work.  It's easier to use
>> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>>
>> The original  reason we have the vmalloc water mark wasn't so much the
>> issue of memory exhaustion but to handle the case of memory fragmentation.
>> Some sites had after a extended period of time started to see failures of
>> allocating even 32K using kmalloc.  In our latest development branch we moved
>> away from using a water mark to always try kmalloc first and if it fails then we
>> try vmalloc. At ORNL we ran into severe performance issues when we entered
>> vmalloc territory. It has been discussed before on what might replace vmalloc
>> handling in the case of kmalloc fails but no solution has been worked out.
>
>OK, but if a structure contains only 4 words, would it be better to just
>use kzalloc?  Or does it not matter?  It would only save trying vmalloc in
>a case that it is guaranteed to fail, but if a structure with 4 words
>can't be allocated, the system has other problems.  Another argument is
>that kzalloc is a well known function that people and bug-finding tools
>understand, so it is better to use it whenever possible.
>
>Some of the other structures contain a lot more fields, as well as small
>arrays.  They are probably acceptable for kzalloc too, but I wouldn't know
>the exact dividing line.

The reason I bring this up is to discuss sorting this out. Once long ago we had
just  LIBCFS_ALLOC. For some reason before my time OBD_ALLOC got spawned
off of that.  Currently LIBCFS_ALLOC is used just by the libcfs/LNet layer. Now 
OBD_ALLOC in our development branch has moved to a try kmalloc first and 
if it fails try vmalloc for any size memory allocation.  LIBCFS_ALLOC still does the
original approach.   So we have two possible solutions depending on if libcfs/LNet
needs to ever do a vmalloc.

One solution if libcfs/LNet never needs a vmalloc is remove LIBCFS_ALLOC and replace
it with kzalloc everywhere. We can then move libcfs_kzvalloc to the lustre layer and
port the change we did in the development branch to here of the try kmalloc then
vmalloc approach. 

The other approach is if libcfs/LNet does in some case need to use vmalloc we could 
then update LIBCFS_ALLOC to first try kmalloc then vmalloc. Once this is implemented
we can nuke the OBD_ALLOC system.

Either way I like to see it consolidated down to one system.

^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [lustre-devel] LIBCFS_ALLOC
  2015-07-02 22:25               ` Simmons, James A.
  (?)
@ 2015-07-03 11:52                 ` Dilger, Andreas
  -1 siblings, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-07-03 11:52 UTC (permalink / raw)
  To: Simmons, James A., 'Julia Lawall'
  Cc: devel, Greg Kroah-Hartman, kernel-janitors, linux-kernel, Drokin,
	Oleg, 'Dan Carpenter',
	lustre-devel

On 2015/07/02, 4:25 PM, "Simmons, James A." <simmonsja@ornl.gov> wrote:

>
>>> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have
>>>even
>>> >a tiny sliver of RAM isn't going to work.  It's easier to use
>>> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>>>
>>> The original  reason we have the vmalloc water mark wasn't so much the
>>> issue of memory exhaustion but to handle the case of memory
>>>fragmentation.
>>> Some sites had after a extended period of time started to see failures
>>>of
>>> allocating even 32K using kmalloc.  In our latest development branch
>>>we moved
>>> away from using a water mark to always try kmalloc first and if it
>>>fails then we
>>> try vmalloc. At ORNL we ran into severe performance issues when we
>>>entered
>>> vmalloc territory. It has been discussed before on what might replace
>>>vmalloc
>>> handling in the case of kmalloc fails but no solution has been worked
>>>out.
>>
>>OK, but if a structure contains only 4 words, would it be better to just
>>use kzalloc?  Or does it not matter?  It would only save trying vmalloc
>>in
>>a case that it is guaranteed to fail, but if a structure with 4 words
>>can't be allocated, the system has other problems.  Another argument is
>>that kzalloc is a well known function that people and bug-finding tools
>>understand, so it is better to use it whenever possible.
>>
>>Some of the other structures contain a lot more fields, as well as small
>>arrays.  They are probably acceptable for kzalloc too, but I wouldn't
>>know
>>the exact dividing line.
>
>The reason I bring this up is to discuss sorting this out. Once long ago
>we had just LIBCFS_ALLOC. For some reason before my time OBD_ALLOC got
> spawned off of that.  Currently LIBCFS_ALLOC is used just by the
>libcfs/LNet
> layer.

That is because there was (is?) interest from Cray and others to use LNet
independently from Lustre (Zest and DVS, for example) so LNet should be
self-contained and not depend on anything from Lustre.

> Now OBD_ALLOC in our development branch has moved to a try kmalloc first
>and
>if it fails try vmalloc for any size memory allocation.  LIBCFS_ALLOC
>still
> does the original approach.   So we have two possible solutions
>depending on if libcfs/LNet needs to ever do a vmalloc.
>
>One solution if libcfs/LNet never needs a vmalloc is remove LIBCFS_ALLOC
>and replace it with kzalloc everywhere. We can then move libcfs_kzvalloc
>to
> the lustre layer and port the change we did in the development branch to
> here of the try kmalloc then vmalloc approach.
>
>The other approach is if libcfs/LNet does in some case need to use vmalloc
> we could then update LIBCFS_ALLOC to first try kmalloc then vmalloc. Once
> this is implemented we can nuke the OBD_ALLOC system.

I don't agree.  I think there are a few places where vmalloc() makes sense
to try (if the allocation may be large), but in most places LIBCFS_ALLOC()
should only use kmalloc().

Unfortunately, there wasn't a separate LIBCFS_ALLOC_LARGE() like there was
for OBD_ALLOC_LARGE() that made it clear which callsites are (potentially)
large and which are small.  The macro approach allowed the compile-time
optimization of the small callsites, but that needs to be done by hand now.

>Either way I like to see it consolidated down to one system.

Given the proliferation of foo_kvmalloc() and foo_kvzalloc() helpers
(ext4_, kvm_, dm_, apparmor, ceph_, __aa_), maybe it it is time to move
these to common kernel code instead of introducing yet another new one?

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division



^ permalink raw reply	[flat|nested] 80+ messages in thread
* Re: [lustre-devel] LIBCFS_ALLOC
@ 2015-07-03 11:52                 ` Dilger, Andreas
  0 siblings, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-07-03 11:52 UTC (permalink / raw)
  To: Simmons, James A., 'Julia Lawall'
  Cc: devel, Greg Kroah-Hartman, kernel-janitors, linux-kernel, Drokin,
	Oleg, 'Dan Carpenter',
	lustre-devel

On 2015/07/02, 4:25 PM, "Simmons, James A." <simmonsja@ornl.gov> wrote:

>
>>> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have
>>>even
>>> >a tiny sliver of RAM isn't going to work.  It's easier to use
>>> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>>>
>>> The original  reason we have the vmalloc water mark wasn't so much the
>>> issue of memory exhaustion but to handle the case of memory
>>>fragmentation.
>>> Some sites had after a extended period of time started to see failures
>>>of
>>> allocating even 32K using kmalloc.  In our latest development branch
>>>we moved
>>> away from using a water mark to always try kmalloc first and if it
>>>fails then we
>>> try vmalloc. At ORNL we ran into severe performance issues when we
>>>entered
>>> vmalloc territory. It has been discussed before on what might replace
>>>vmalloc
>>> handling in the case of kmalloc fails but no solution has been worked
>>>out.
>>
>>OK, but if a structure contains only 4 words, would it be better to just
>>use kzalloc?  Or does it not matter?  It would only save trying vmalloc
>>in
>>a case that it is guaranteed to fail, but if a structure with 4 words
>>can't be allocated, the system has other problems.  Another argument is
>>that kzalloc is a well known function that people and bug-finding tools
>>understand, so it is better to use it whenever possible.
>>
>>Some of the other structures contain a lot more fields, as well as small
>>arrays.  They are probably acceptable for kzalloc too, but I wouldn't
>>know
>>the exact dividing line.
>
>The reason I bring this up is to discuss sorting this out. Once long ago
>we had just LIBCFS_ALLOC. For some reason before my time OBD_ALLOC got
> spawned off of that.  Currently LIBCFS_ALLOC is used just by the
>libcfs/LNet
> layer.

That is because there was (is?) interest from Cray and others to use LNet
independently from Lustre (Zest and DVS, for example) so LNet should be
self-contained and not depend on anything from Lustre.

> Now OBD_ALLOC in our development branch has moved to a try kmalloc first
>and
>if it fails try vmalloc for any size memory allocation.  LIBCFS_ALLOC
>still
> does the original approach.   So we have two possible solutions
>depending on if libcfs/LNet needs to ever do a vmalloc.
>
>One solution if libcfs/LNet never needs a vmalloc is remove LIBCFS_ALLOC
>and replace it with kzalloc everywhere. We can then move libcfs_kzvalloc
>to
> the lustre layer and port the change we did in the development branch to
> here of the try kmalloc then vmalloc approach.
>
>The other approach is if libcfs/LNet does in some case need to use vmalloc
> we could then update LIBCFS_ALLOC to first try kmalloc then vmalloc. Once
> this is implemented we can nuke the OBD_ALLOC system.

I don't agree.  I think there are a few places where vmalloc() makes sense
to try (if the allocation may be large), but in most places LIBCFS_ALLOC()
should only use kmalloc().

Unfortunately, there wasn't a separate LIBCFS_ALLOC_LARGE() like there was
for OBD_ALLOC_LARGE() that made it clear which callsites are (potentially)
large and which are small.  The macro approach allowed the compile-time
optimization of the small callsites, but that needs to be done by hand now.

>Either way I like to see it consolidated down to one system.

Given the proliferation of foo_kvmalloc() and foo_kvzalloc() helpers
(ext4_, kvm_, dm_, apparmor, ceph_, __aa_), maybe it it is time to move
these to common kernel code instead of introducing yet another new one?

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division



^ permalink raw reply	[flat|nested] 80+ messages in thread
* [lustre-devel] LIBCFS_ALLOC
@ 2015-07-03 11:52                 ` Dilger, Andreas
  0 siblings, 0 replies; 80+ messages in thread
From: Dilger, Andreas @ 2015-07-03 11:52 UTC (permalink / raw)
  To: lustre-devel

On 2015/07/02, 4:25 PM, "Simmons, James A." <simmonsja@ornl.gov> wrote:

>
>>> >Yeah.  You're right.  Doing a vmalloc() when kmalloc() doesn't have
>>>even
>>> >a tiny sliver of RAM isn't going to work.  It's easier to use
>>> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing.
>>>
>>> The original  reason we have the vmalloc water mark wasn't so much the
>>> issue of memory exhaustion but to handle the case of memory
>>>fragmentation.
>>> Some sites had after a extended period of time started to see failures
>>>of
>>> allocating even 32K using kmalloc.  In our latest development branch
>>>we moved
>>> away from using a water mark to always try kmalloc first and if it
>>>fails then we
>>> try vmalloc. At ORNL we ran into severe performance issues when we
>>>entered
>>> vmalloc territory. It has been discussed before on what might replace
>>>vmalloc
>>> handling in the case of kmalloc fails but no solution has been worked
>>>out.
>>
>>OK, but if a structure contains only 4 words, would it be better to just
>>use kzalloc?  Or does it not matter?  It would only save trying vmalloc
>>in
>>a case that it is guaranteed to fail, but if a structure with 4 words
>>can't be allocated, the system has other problems.  Another argument is
>>that kzalloc is a well known function that people and bug-finding tools
>>understand, so it is better to use it whenever possible.
>>
>>Some of the other structures contain a lot more fields, as well as small
>>arrays.  They are probably acceptable for kzalloc too, but I wouldn't
>>know
>>the exact dividing line.
>
>The reason I bring this up is to discuss sorting this out. Once long ago
>we had just LIBCFS_ALLOC. For some reason before my time OBD_ALLOC got
> spawned off of that.  Currently LIBCFS_ALLOC is used just by the
>libcfs/LNet
> layer.

That is because there was (is?) interest from Cray and others to use LNet
independently from Lustre (Zest and DVS, for example) so LNet should be
self-contained and not depend on anything from Lustre.

> Now OBD_ALLOC in our development branch has moved to a try kmalloc first
>and
>if it fails try vmalloc for any size memory allocation.  LIBCFS_ALLOC
>still
> does the original approach.   So we have two possible solutions
>depending on if libcfs/LNet needs to ever do a vmalloc.
>
>One solution if libcfs/LNet never needs a vmalloc is remove LIBCFS_ALLOC
>and replace it with kzalloc everywhere. We can then move libcfs_kzvalloc
>to
> the lustre layer and port the change we did in the development branch to
> here of the try kmalloc then vmalloc approach.
>
>The other approach is if libcfs/LNet does in some case need to use vmalloc
> we could then update LIBCFS_ALLOC to first try kmalloc then vmalloc. Once
> this is implemented we can nuke the OBD_ALLOC system.

I don't agree.  I think there are a few places where vmalloc() makes sense
to try (if the allocation may be large), but in most places LIBCFS_ALLOC()
should only use kmalloc().

Unfortunately, there wasn't a separate LIBCFS_ALLOC_LARGE() like there was
for OBD_ALLOC_LARGE() that made it clear which callsites are (potentially)
large and which are small.  The macro approach allowed the compile-time
optimization of the small callsites, but that needs to be done by hand now.

>Either way I like to see it consolidated down to one system.

Given the proliferation of foo_kvmalloc() and foo_kvzalloc() helpers
(ext4_, kvm_, dm_, apparmor, ceph_, __aa_), maybe it it is time to move
these to common kernel code instead of introducing yet another new one?

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division

^ permalink raw reply	[flat|nested] 80+ messages in thread
end of thread, other threads:[~2015-07-03 11:52 UTC | newest]

Thread overview: 80+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-20 16:58 [PATCH 00/12] Use !x to check for kzalloc failure Julia Lawall
2015-06-20 16:58 ` Julia Lawall
2015-06-20 16:58 ` [PATCH 01/12] staging: lustre: fid: " Julia Lawall
2015-06-20 16:58   ` Julia Lawall
2015-06-22 15:46   ` [lustre-devel] Fwd: [HPDD-discuss] " Patrick Farrell
2015-06-22 17:18     ` Simmons, James A.
2015-06-23  8:05     ` Dilger, Andreas
2015-06-23  8:25   ` Dilger, Andreas
2015-06-23  8:25     ` [lustre-devel] " Dilger, Andreas
2015-06-23  8:25     ` Dilger, Andreas
2015-06-23  9:23     ` Dan Carpenter
2015-06-23  9:23       ` [lustre-devel] " Dan Carpenter
2015-06-23  9:23       ` Dan Carpenter
2015-06-23  9:35       ` Julia Lawall
2015-06-23  9:35         ` [lustre-devel] " Julia Lawall
2015-06-23  9:35         ` Julia Lawall
2015-06-23  9:57         ` Dan Carpenter
2015-06-23  9:57           ` [lustre-devel] " Dan Carpenter
2015-06-23  9:57           ` Dan Carpenter
2015-06-23 10:51           ` Julia Lawall
2015-06-23 10:51             ` [lustre-devel] " Julia Lawall
2015-06-23 10:51             ` Julia Lawall
2015-06-24 20:14             ` [lustre-devel] " Simmons, James A.
2015-06-24 20:14               ` Simmons, James A.
2015-06-24 20:14               ` Simmons, James A.
2015-06-23 22:03           ` Joe Perches
2015-06-23 22:03             ` [lustre-devel] " Joe Perches
2015-06-23 22:03             ` Joe Perches
2015-06-23 22:11       ` Joe Perches
2015-06-23 22:11         ` [lustre-devel] " Joe Perches
2015-06-23 22:11         ` Joe Perches
2015-06-28  6:52     ` LIBCFS_ALLOC Julia Lawall
2015-06-28  6:52       ` [lustre-devel] LIBCFS_ALLOC Julia Lawall
2015-06-28  6:52       ` LIBCFS_ALLOC Julia Lawall
2015-06-28 21:54       ` LIBCFS_ALLOC Dan Carpenter
2015-06-28 21:54         ` [lustre-devel] LIBCFS_ALLOC Dan Carpenter
2015-06-28 21:54         ` LIBCFS_ALLOC Dan Carpenter
2015-06-30 14:56         ` LIBCFS_ALLOC Simmons, James A.
2015-06-30 14:56           ` [lustre-devel] LIBCFS_ALLOC Simmons, James A.
2015-06-30 15:01           ` LIBCFS_ALLOC Julia Lawall
2015-06-30 15:01             ` [lustre-devel] LIBCFS_ALLOC Julia Lawall
2015-06-30 15:01             ` LIBCFS_ALLOC Julia Lawall
2015-07-02 22:25             ` [lustre-devel] LIBCFS_ALLOC Simmons, James A.
2015-07-02 22:25               ` Simmons, James A.
2015-07-02 22:25               ` Simmons, James A.
2015-07-03 11:52               ` Dilger, Andreas
2015-07-03 11:52                 ` Dilger, Andreas
2015-07-03 11:52                 ` Dilger, Andreas
2015-06-30 17:38           ` LIBCFS_ALLOC Dan Carpenter
2015-06-30 17:38             ` [lustre-devel] LIBCFS_ALLOC Dan Carpenter
2015-06-30 17:38             ` LIBCFS_ALLOC Dan Carpenter
2015-06-30 21:26       ` [lustre-devel] LIBCFS_ALLOC Dilger, Andreas
2015-06-30 21:26         ` Dilger, Andreas
2015-06-20 16:59 ` [PATCH 02/12] staging: lustre: fld: Use !x to check for kzalloc failure Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 03/12] staging: lustre: lclient: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 04/12] staging: lustre: ldlm: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 05/12] staging: lustre: lmv: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 06/12] staging: lustre: lov: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 07/12] staging: lustre: mdc: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 08/12] staging: lustre: mgc: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 09/12] staging: lustre: obdclass: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-21 10:02   ` walter harms
2015-06-21 10:02     ` walter harms
2015-06-21 10:29     ` Julia Lawall
2015-06-21 10:29       ` Julia Lawall
2015-06-21 11:58   ` walter harms
2015-06-20 16:59 ` [PATCH 10/12] staging: lustre: obdecho: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 11/12] staging: lustre: osc: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall
2015-06-20 16:59 ` [PATCH 12/12] staging: lustre: ptlrpc: " Julia Lawall
2015-06-20 16:59   ` Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.