All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/34] dprintk() cleanup (part 1)
@ 2017-04-07 18:14 Anna.Schumaker
  2017-04-07 18:14 ` [PATCH 01/34] NFS: Clean up do_callback_layoutrecall() Anna.Schumaker
                   ` (29 more replies)
  0 siblings, 30 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:14 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

We have around 750 dprintk()s in the NFS client alone, and many of these
simply tell us when we're entering or leaving a function (usually both).
This can create a lot of noise when you're trying to debug a single issues,
and I think many dprintk()s can be cut out in favor of tracepoints or dynamic
tracing through SystemTap.

This patch set does just that. I went through some of our files one by one
and removed dprintk()s that seemed to be unnecessary. Occasionally this
allowed me to refactor the surrounding code, so I tried to make individual
patches for each function I refactored.

These patches make changes to the following files:
    - callback_proc.c
    - callback_xdr.c
    - client.c
    - direct.c
    - namespace.c
    - nfs42proc.c
    - nfs4client.c
    - nfs4getroot.c
    - nfs4namespace.c
    - nfs4proc.c

What does everybody think?  Are any of these dprintk()s worth holding on to?

Thanks,
Anna


Anna Schumaker (34):
  NFS: Clean up do_callback_layoutrecall()
  NFS: Clean up nfs4_callback_layoutrecall()
  NFS: Remove extra dprintk()s from callback_proc.c
  NFS: Clean up decode_getattr_args()
  NFS: Clean up decode_recall_args()
  NFS: Clean up decode_layoutrecall_args()
  NFS: Clean up decode_cb_sequence_args()
  NFS: Clean up decode_notify_lock_args()
  NFS: Clean up encode_cb_sequence_res()
  NFS: Remove extra dprintk()s from callback_xdr.c
  NFS: Clean up nfs_init_client()
  NFS: Clean up extra dprintk()s in client.c
  NFS: Remove nfs_direct_readpage_release()
  NFS: Clean up nfs_direct_commit_complete()
  NFS: Remove extra dprintk()s from namespace.c
  NFS: Clean up nfs42_layoutstat_done()
  NFS: Clean up nfs4_match_clientids()
  NFS: Clean up nfs4_check_serverowner_minor_id()
  NFS: Create a common nfs4_match_client() function
  NFS: Clean up nfs4_check_serverowner_major_id()
  NFS: Clean up nfs4_check_server_scope()
  NFS: Clean up nfs4_set_client()
  NFS: Clean up nfs4_init_server()
  NFS: Remove extra dprintk()s from nfs4client.c
  NFS: Clean up nfs4_get_rootfh()
  NFS: Remove extra dprintk()s from nfs4namespace.c
  NFS: Clean up nfs4_proc_bind_one_conn_to_session()
  NFS: Clean up _nfs4_proc_exchange_id()
  NFS: Clean up nfs4_proc_get_lease_time()
  NFS: Clean up nfs41_proc_async_sequence()
  NFS: Clean up nfs4_proc_sequence()
  NFS: Clean up nfs41_proc_reclaim_complete()
  NFS: Clean up nfs4_layoutget_handle_exception()
  NFS: Remove extra dprintk()s from nfs4proc.c

 fs/nfs/callback_proc.c |  41 +------
 fs/nfs/callback_xdr.c  | 109 +++++--------------
 fs/nfs/client.c        |  65 ++----------
 fs/nfs/direct.c        |  21 +---
 fs/nfs/namespace.c     |  34 ++----
 fs/nfs/nfs42proc.c     |   2 -
 fs/nfs/nfs4client.c    | 283 ++++++++++++++-----------------------------------
 fs/nfs/nfs4getroot.c   |   3 -
 fs/nfs/nfs4namespace.c |   7 +-
 fs/nfs/nfs4proc.c      | 274 +++++++----------------------------------------
 10 files changed, 167 insertions(+), 672 deletions(-)

-- 
2.12.2


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 01/34] NFS: Clean up do_callback_layoutrecall()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
@ 2017-04-07 18:14 ` Anna.Schumaker
  2017-04-07 18:14 ` [PATCH 02/34] NFS: Clean up nfs4_callback_layoutrecall() Anna.Schumaker
                   ` (28 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:14 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Removing the dprintk()s lets us simplify the function by removing the
else condition entirely and returning the status of
initiate_{file,bulk}_draining() directly.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_proc.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index f073a6d2c6a5..c52408055721 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -317,16 +317,9 @@ static u32 initiate_bulk_draining(struct nfs_client *clp,
 static u32 do_callback_layoutrecall(struct nfs_client *clp,
 				    struct cb_layoutrecallargs *args)
 {
-	u32 res;
-
-	dprintk("%s enter, type=%i\n", __func__, args->cbl_recall_type);
 	if (args->cbl_recall_type == RETURN_FILE)
-		res = initiate_file_draining(clp, args);
-	else
-		res = initiate_bulk_draining(clp, args);
-	dprintk("%s returning %i\n", __func__, res);
-	return res;
-
+		return initiate_file_draining(clp, args);
+	return initiate_bulk_draining(clp, args);
 }
 
 __be32 nfs4_callback_layoutrecall(struct cb_layoutrecallargs *args,
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 02/34] NFS: Clean up nfs4_callback_layoutrecall()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
  2017-04-07 18:14 ` [PATCH 01/34] NFS: Clean up do_callback_layoutrecall() Anna.Schumaker
@ 2017-04-07 18:14 ` Anna.Schumaker
  2017-04-07 18:14 ` [PATCH 03/34] NFS: Remove extra dprintk()s from callback_proc.c Anna.Schumaker
                   ` (27 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:14 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

In addition to removing the dprintk(), this patch also initializes "res"
to the default return value instead of doing this through an else
condition.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_proc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index c52408055721..e61e4ccc181a 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -325,16 +325,10 @@ static u32 do_callback_layoutrecall(struct nfs_client *clp,
 __be32 nfs4_callback_layoutrecall(struct cb_layoutrecallargs *args,
 				  void *dummy, struct cb_process_state *cps)
 {
-	u32 res;
-
-	dprintk("%s: -->\n", __func__);
+	u32 res = NFS4ERR_OP_NOT_IN_SESSION;
 
 	if (cps->clp)
 		res = do_callback_layoutrecall(cps->clp, args);
-	else
-		res = NFS4ERR_OP_NOT_IN_SESSION;
-
-	dprintk("%s: exit with status = %d\n", __func__, res);
 	return cpu_to_be32(res);
 }
 
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 03/34] NFS: Remove extra dprintk()s from callback_proc.c
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
  2017-04-07 18:14 ` [PATCH 01/34] NFS: Clean up do_callback_layoutrecall() Anna.Schumaker
  2017-04-07 18:14 ` [PATCH 02/34] NFS: Clean up nfs4_callback_layoutrecall() Anna.Schumaker
@ 2017-04-07 18:14 ` Anna.Schumaker
  2017-04-07 18:14 ` [PATCH 04/34] NFS: Clean up decode_getattr_args() Anna.Schumaker
                   ` (26 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:14 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_proc.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index e61e4ccc181a..e7f041447afd 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -351,8 +351,6 @@ __be32 nfs4_callback_devicenotify(struct cb_devicenotifyargs *args,
 	struct nfs_client *clp = cps->clp;
 	struct nfs_server *server = NULL;
 
-	dprintk("%s: -->\n", __func__);
-
 	if (!clp) {
 		res = cpu_to_be32(NFS4ERR_OP_NOT_IN_SESSION);
 		goto out;
@@ -371,8 +369,6 @@ __be32 nfs4_callback_devicenotify(struct cb_devicenotifyargs *args,
 					goto found;
 				}
 			rcu_read_unlock();
-			dprintk("%s: layout type %u not found\n",
-				__func__, dev->cbd_layout_type);
 			continue;
 		}
 
@@ -382,8 +378,6 @@ __be32 nfs4_callback_devicenotify(struct cb_devicenotifyargs *args,
 
 out:
 	kfree(args->devs);
-	dprintk("%s: exit with status = %u\n",
-		__func__, be32_to_cpu(res));
 	return res;
 }
 
@@ -404,16 +398,11 @@ static __be32
 validate_seqid(const struct nfs4_slot_table *tbl, const struct nfs4_slot *slot,
 		const struct cb_sequenceargs * args)
 {
-	dprintk("%s enter. slotid %u seqid %u, slot table seqid: %u\n",
-		__func__, args->csa_slotid, args->csa_sequenceid, slot->seq_nr);
-
 	if (args->csa_slotid > tbl->server_highest_slotid)
 		return htonl(NFS4ERR_BADSLOT);
 
 	/* Replay */
 	if (args->csa_sequenceid == slot->seq_nr) {
-		dprintk("%s seqid %u is a replay\n",
-			__func__, args->csa_sequenceid);
 		if (nfs4_test_locked_slot(tbl, slot->slot_nr))
 			return htonl(NFS4ERR_DELAY);
 		/* Signal process_op to set this error on next op */
@@ -467,15 +456,6 @@ static bool referring_call_exists(struct nfs_client *clp,
 
 		for (j = 0; j < rclist->rcl_nrefcalls; j++) {
 			ref = &rclist->rcl_refcalls[j];
-
-			dprintk("%s: sessionid %x:%x:%x:%x sequenceid %u "
-				"slotid %u\n", __func__,
-				((u32 *)&rclist->rcl_sessionid.data)[0],
-				((u32 *)&rclist->rcl_sessionid.data)[1],
-				((u32 *)&rclist->rcl_sessionid.data)[2],
-				((u32 *)&rclist->rcl_sessionid.data)[3],
-				ref->rc_sequenceid, ref->rc_slotid);
-
 			status = nfs4_slot_wait_on_seqid(tbl, ref->rc_slotid,
 					ref->rc_sequenceid, HZ >> 1) < 0;
 			if (status)
@@ -580,8 +560,6 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
 		res->csr_status = status;
 
 	trace_nfs4_cb_sequence(args, res, status);
-	dprintk("%s: exit with status = %d res->csr_status %d\n", __func__,
-		ntohl(status), ntohl(res->csr_status));
 	return status;
 }
 
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 04/34] NFS: Clean up decode_getattr_args()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (2 preceding siblings ...)
  2017-04-07 18:14 ` [PATCH 03/34] NFS: Remove extra dprintk()s from callback_proc.c Anna.Schumaker
@ 2017-04-07 18:14 ` Anna.Schumaker
  2017-04-07 18:14 ` [PATCH 05/34] NFS: Clean up decode_recall_args() Anna.Schumaker
                   ` (25 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:14 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Removing the dprintk() lets us return the status value directly, rather
than jumping to a label if an error occurs.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_xdr.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index d051fc3583a9..c782261d86d5 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -192,11 +192,8 @@ static __be32 decode_getattr_args(struct svc_rqst *rqstp, struct xdr_stream *xdr
 
 	status = decode_fh(xdr, &args->fh);
 	if (unlikely(status != 0))
-		goto out;
-	status = decode_bitmap(xdr, args->bitmap);
-out:
-	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
-	return status;
+		return status;
+	return decode_bitmap(xdr, args->bitmap);
 }
 
 static __be32 decode_recall_args(struct svc_rqst *rqstp, struct xdr_stream *xdr, struct cb_recallargs *args)
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 05/34] NFS: Clean up decode_recall_args()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (3 preceding siblings ...)
  2017-04-07 18:14 ` [PATCH 04/34] NFS: Clean up decode_getattr_args() Anna.Schumaker
@ 2017-04-07 18:14 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 06/34] NFS: Clean up decode_layoutrecall_args() Anna.Schumaker
                   ` (24 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:14 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Removing the dprintk() lets us simplify the function by returning status
codes directly, rather than using a goto.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_xdr.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index c782261d86d5..0319230fe1a8 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -203,17 +203,12 @@ static __be32 decode_recall_args(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 
 	status = decode_delegation_stateid(xdr, &args->stateid);
 	if (unlikely(status != 0))
-		goto out;
+		return status;
 	p = read_buf(xdr, 4);
-	if (unlikely(p == NULL)) {
-		status = htonl(NFS4ERR_RESOURCE);
-		goto out;
-	}
+	if (unlikely(p == NULL))
+		return htonl(NFS4ERR_RESOURCE);
 	args->truncate = ntohl(*p);
-	status = decode_fh(xdr, &args->fh);
-out:
-	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
-	return status;
+	return decode_fh(xdr, &args->fh);
 }
 
 #if defined(CONFIG_NFS_V4_1)
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 06/34] NFS: Clean up decode_layoutrecall_args()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (4 preceding siblings ...)
  2017-04-07 18:14 ` [PATCH 05/34] NFS: Clean up decode_recall_args() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 07/34] NFS: Clean up decode_cb_sequence_args() Anna.Schumaker
                   ` (23 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Additionally, this change lets us cut out the goto by returning errors
immediately.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_xdr.c | 38 +++++++++++---------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 0319230fe1a8..d5ddceb91a9a 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -227,10 +227,8 @@ static __be32 decode_layoutrecall_args(struct svc_rqst *rqstp,
 	uint32_t iomode;
 
 	p = read_buf(xdr, 4 * sizeof(uint32_t));
-	if (unlikely(p == NULL)) {
-		status = htonl(NFS4ERR_BADXDR);
-		goto out;
-	}
+	if (unlikely(p == NULL))
+		return htonl(NFS4ERR_BADXDR);
 
 	args->cbl_layout_type = ntohl(*p++);
 	/* Depite the spec's xdr, iomode really belongs in the FILE switch,
@@ -244,37 +242,23 @@ static __be32 decode_layoutrecall_args(struct svc_rqst *rqstp,
 		args->cbl_range.iomode = iomode;
 		status = decode_fh(xdr, &args->cbl_fh);
 		if (unlikely(status != 0))
-			goto out;
+			return status;
 
 		p = read_buf(xdr, 2 * sizeof(uint64_t));
-		if (unlikely(p == NULL)) {
-			status = htonl(NFS4ERR_BADXDR);
-			goto out;
-		}
+		if (unlikely(p == NULL))
+			return htonl(NFS4ERR_BADXDR);
 		p = xdr_decode_hyper(p, &args->cbl_range.offset);
 		p = xdr_decode_hyper(p, &args->cbl_range.length);
-		status = decode_layout_stateid(xdr, &args->cbl_stateid);
-		if (unlikely(status != 0))
-			goto out;
+		return decode_layout_stateid(xdr, &args->cbl_stateid);
 	} else if (args->cbl_recall_type == RETURN_FSID) {
 		p = read_buf(xdr, 2 * sizeof(uint64_t));
-		if (unlikely(p == NULL)) {
-			status = htonl(NFS4ERR_BADXDR);
-			goto out;
-		}
+		if (unlikely(p == NULL))
+			return htonl(NFS4ERR_BADXDR);
 		p = xdr_decode_hyper(p, &args->cbl_fsid.major);
 		p = xdr_decode_hyper(p, &args->cbl_fsid.minor);
-	} else if (args->cbl_recall_type != RETURN_ALL) {
-		status = htonl(NFS4ERR_BADXDR);
-		goto out;
-	}
-	dprintk("%s: ltype 0x%x iomode %d changed %d recall_type %d\n",
-		__func__,
-		args->cbl_layout_type, iomode,
-		args->cbl_layoutchanged, args->cbl_recall_type);
-out:
-	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
-	return status;
+	} else if (args->cbl_recall_type != RETURN_ALL)
+		return htonl(NFS4ERR_BADXDR);
+	return 0;
 }
 
 static
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 07/34] NFS: Clean up decode_cb_sequence_args()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (5 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 06/34] NFS: Clean up decode_layoutrecall_args() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 08/34] NFS: Clean up decode_notify_lock_args() Anna.Schumaker
                   ` (22 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_xdr.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index d5ddceb91a9a..ce1e293b8a18 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -413,12 +413,11 @@ static __be32 decode_cb_sequence_args(struct svc_rqst *rqstp,
 
 	status = decode_sessionid(xdr, &args->csa_sessionid);
 	if (status)
-		goto out;
+		return status;
 
-	status = htonl(NFS4ERR_RESOURCE);
 	p = read_buf(xdr, 5 * sizeof(uint32_t));
 	if (unlikely(p == NULL))
-		goto out;
+		return htonl(NFS4ERR_RESOURCE);
 
 	args->csa_addr = svc_addr(rqstp);
 	args->csa_sequenceid = ntohl(*p++);
@@ -432,7 +431,7 @@ static __be32 decode_cb_sequence_args(struct svc_rqst *rqstp,
 						  sizeof(*args->csa_rclists),
 						  GFP_KERNEL);
 		if (unlikely(args->csa_rclists == NULL))
-			goto out;
+			return htonl(NFS4ERR_RESOURCE);
 
 		for (i = 0; i < args->csa_nrclists; i++) {
 			status = decode_rc_list(xdr, &args->csa_rclists[i]);
@@ -442,27 +441,13 @@ static __be32 decode_cb_sequence_args(struct svc_rqst *rqstp,
 			}
 		}
 	}
-	status = 0;
-
-	dprintk("%s: sessionid %x:%x:%x:%x sequenceid %u slotid %u "
-		"highestslotid %u cachethis %d nrclists %u\n",
-		__func__,
-		((u32 *)&args->csa_sessionid)[0],
-		((u32 *)&args->csa_sessionid)[1],
-		((u32 *)&args->csa_sessionid)[2],
-		((u32 *)&args->csa_sessionid)[3],
-		args->csa_sequenceid, args->csa_slotid,
-		args->csa_highestslotid, args->csa_cachethis,
-		args->csa_nrclists);
-out:
-	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
-	return status;
+	return 0;
 
 out_free:
 	for (i = 0; i < args->csa_nrclists; i++)
 		kfree(args->csa_rclists[i].rcl_refcalls);
 	kfree(args->csa_rclists);
-	goto out;
+	return status;
 }
 
 static __be32 decode_recallany_args(struct svc_rqst *rqstp,
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 08/34] NFS: Clean up decode_notify_lock_args()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (6 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 07/34] NFS: Clean up decode_cb_sequence_args() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 09/34] NFS: Clean up encode_cb_sequence_res() Anna.Schumaker
                   ` (21 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Let's cut out the goto and return any errors immedately

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_xdr.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index ce1e293b8a18..4cf70dc59933 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -518,11 +518,8 @@ static __be32 decode_notify_lock_args(struct svc_rqst *rqstp, struct xdr_stream
 
 	status = decode_fh(xdr, &args->cbnl_fh);
 	if (unlikely(status != 0))
-		goto out;
-	status = decode_lockowner(xdr, args);
-out:
-	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
-	return status;
+		return status;
+	return decode_lockowner(xdr, args);
 }
 
 #endif /* CONFIG_NFS_V4_1 */
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 09/34] NFS: Clean up encode_cb_sequence_res()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (7 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 08/34] NFS: Clean up decode_notify_lock_args() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 10/34] NFS: Remove extra dprintk()s from callback_xdr.c Anna.Schumaker
                   ` (20 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_xdr.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 4cf70dc59933..e54e697340a4 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -692,11 +692,11 @@ static __be32 encode_cb_sequence_res(struct svc_rqst *rqstp,
 	__be32 status = res->csr_status;
 
 	if (unlikely(status != 0))
-		goto out;
+		return status;
 
 	status = encode_sessionid(xdr, &res->csr_sessionid);
 	if (status)
-		goto out;
+		return status;
 
 	p = xdr_reserve_space(xdr, 4 * sizeof(uint32_t));
 	if (unlikely(p == NULL))
@@ -706,9 +706,7 @@ static __be32 encode_cb_sequence_res(struct svc_rqst *rqstp,
 	*p++ = htonl(res->csr_slotid);
 	*p++ = htonl(res->csr_highestslotid);
 	*p++ = htonl(res->csr_target_highestslotid);
-out:
-	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
-	return status;
+	return 0;
 }
 
 static __be32
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 10/34] NFS: Remove extra dprintk()s from callback_xdr.c
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (8 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 09/34] NFS: Clean up encode_cb_sequence_res() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 11/34] NFS: Clean up nfs_init_client() Anna.Schumaker
                   ` (19 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_xdr.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index e54e697340a4..c14758e08d73 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -171,8 +171,6 @@ static __be32 decode_compound_hdr_arg(struct xdr_stream *xdr, struct cb_compound
 		return htonl(NFS4ERR_MINOR_VERS_MISMATCH);
 	}
 	hdr->nops = ntohl(*p);
-	dprintk("%s: minorversion %d nops %d\n", __func__,
-		hdr->minorversion, hdr->nops);
 	return 0;
 }
 
@@ -665,7 +663,6 @@ static __be32 encode_getattr_res(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 	status = encode_attr_mtime(xdr, res->bitmap, &res->mtime);
 	*savep = htonl((unsigned int)((char *)xdr->p - (char *)(savep+1)));
 out:
-	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
 	return status;
 }
 
@@ -827,14 +824,10 @@ static __be32 process_op(int nop, struct svc_rqst *rqstp,
 	long maxlen;
 	__be32 res;
 
-	dprintk("%s: start\n", __func__);
 	status = decode_op_hdr(xdr_in, &op_nr);
 	if (unlikely(status))
 		return status;
 
-	dprintk("%s: minorversion=%d nop=%d op_nr=%u\n",
-		__func__, cps->minorversion, nop, op_nr);
-
 	switch (cps->minorversion) {
 	case 0:
 		status = preprocess_nfs4_op(op_nr, &op);
@@ -873,7 +866,6 @@ static __be32 process_op(int nop, struct svc_rqst *rqstp,
 		return res;
 	if (op->encode_res != NULL && status == 0)
 		status = op->encode_res(rqstp, xdr_out, resp);
-	dprintk("%s: done, status = %d\n", __func__, ntohl(status));
 	return status;
 }
 
@@ -893,8 +885,6 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
 	};
 	unsigned int nops = 0;
 
-	dprintk("%s: start\n", __func__);
-
 	xdr_init_decode(&xdr_in, &rqstp->rq_arg, rqstp->rq_arg.head[0].iov_base);
 
 	p = (__be32*)((char *)rqstp->rq_res.head[0].iov_base + rqstp->rq_res.head[0].iov_len);
@@ -933,7 +923,6 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
 	*hdr_res.nops = htonl(nops);
 	nfs4_cb_free_slot(&cps);
 	nfs_put_client(cps.clp);
-	dprintk("%s: done, status = %u\n", __func__, ntohl(status));
 	return rpc_success;
 
 out_invalidcred:
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 11/34] NFS: Clean up nfs_init_client()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (9 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 10/34] NFS: Remove extra dprintk()s from callback_xdr.c Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 12/34] NFS: Clean up extra dprintk()s in client.c Anna.Schumaker
                   ` (18 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

We always call nfs_mark_client_ready() even if nfs_create_rpc_client()
returns an error, so we can rearrange nfs_init_client() to mark the
client ready from a single place.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/client.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 390ada8741bc..675142189181 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -624,27 +624,21 @@ struct nfs_client *nfs_init_client(struct nfs_client *clp,
 {
 	int error;
 
-	if (clp->cl_cons_state == NFS_CS_READY) {
-		/* the client is already initialised */
-		dprintk("<-- nfs_init_client() = 0 [already %p]\n", clp);
+	/* the client is already initialised */
+	if (clp->cl_cons_state == NFS_CS_READY)
 		return clp;
-	}
 
 	/*
 	 * Create a client RPC handle for doing FSSTAT with UNIX auth only
 	 * - RFC 2623, sec 2.3.2
 	 */
 	error = nfs_create_rpc_client(clp, cl_init, RPC_AUTH_UNIX);
-	if (error < 0)
-		goto error;
-	nfs_mark_client_ready(clp, NFS_CS_READY);
+	nfs_mark_client_ready(clp, error == 0 ? NFS_CS_READY : error);
+	if (error < 0) {
+		nfs_put_client(clp);
+		clp = ERR_PTR(error);
+	}
 	return clp;
-
-error:
-	nfs_mark_client_ready(clp, error);
-	nfs_put_client(clp);
-	dprintk("<-- nfs_init_client() = xerror %d\n", error);
-	return ERR_PTR(error);
 }
 EXPORT_SYMBOL_GPL(nfs_init_client);
 
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 12/34] NFS: Clean up extra dprintk()s in client.c
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (10 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 11/34] NFS: Clean up nfs_init_client() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 13/34] NFS: Remove nfs_direct_readpage_release() Anna.Schumaker
                   ` (17 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/client.c | 45 +++------------------------------------------
 1 file changed, 3 insertions(+), 42 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 675142189181..3ffbffe8f39f 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -240,8 +240,6 @@ static void pnfs_init_server(struct nfs_server *server)
  */
 void nfs_free_client(struct nfs_client *clp)
 {
-	dprintk("--> nfs_free_client(%u)\n", clp->rpc_ops->version);
-
 	nfs_fscache_release_client_cookie(clp);
 
 	/* -EIO all pending I/O */
@@ -256,8 +254,6 @@ void nfs_free_client(struct nfs_client *clp)
 	kfree(clp->cl_hostname);
 	kfree(clp->cl_acceptor);
 	kfree(clp);
-
-	dprintk("<-- nfs_free_client()\n");
 }
 EXPORT_SYMBOL_GPL(nfs_free_client);
 
@@ -271,7 +267,6 @@ void nfs_put_client(struct nfs_client *clp)
 	if (!clp)
 		return;
 
-	dprintk("--> nfs_put_client({%d})\n", atomic_read(&clp->cl_count));
 	nn = net_generic(clp->cl_net, nfs_net_id);
 
 	if (atomic_dec_and_lock(&clp->cl_count, &nn->nfs_client_lock)) {
@@ -382,9 +377,6 @@ nfs_found_client(const struct nfs_client_initdata *cl_init,
 	}
 
 	smp_rmb();
-
-	dprintk("<-- %s found nfs_client %p for %s\n",
-		__func__, clp, cl_init->hostname ?: "");
 	return clp;
 }
 
@@ -403,9 +395,6 @@ struct nfs_client *nfs_get_client(const struct nfs_client_initdata *cl_init)
 		return NULL;
 	}
 
-	dprintk("--> nfs_get_client(%s,v%u)\n",
-		cl_init->hostname, rpc_ops->version);
-
 	/* see if the client already exists */
 	do {
 		spin_lock(&nn->nfs_client_lock);
@@ -430,8 +419,6 @@ struct nfs_client *nfs_get_client(const struct nfs_client_initdata *cl_init)
 		new = rpc_ops->alloc_client(cl_init);
 	} while (!IS_ERR(new));
 
-	dprintk("<-- nfs_get_client() Failed to find %s (%ld)\n",
-		cl_init->hostname, PTR_ERR(new));
 	return new;
 }
 EXPORT_SYMBOL_GPL(nfs_get_client);
@@ -662,8 +649,6 @@ static int nfs_init_server(struct nfs_server *server,
 	struct nfs_client *clp;
 	int error;
 
-	dprintk("--> nfs_init_server()\n");
-
 	nfs_init_timeout_values(&timeparms, data->nfs_server.protocol,
 			data->timeo, data->retrans);
 	if (data->flags & NFS_MOUNT_NORESVPORT)
@@ -671,10 +656,8 @@ static int nfs_init_server(struct nfs_server *server,
 
 	/* Allocate or find a client reference we can use */
 	clp = nfs_get_client(&cl_init);
-	if (IS_ERR(clp)) {
-		dprintk("<-- nfs_init_server() = error %ld\n", PTR_ERR(clp));
+	if (IS_ERR(clp))
 		return PTR_ERR(clp);
-	}
 
 	server->nfs_client = clp;
 
@@ -719,13 +702,11 @@ static int nfs_init_server(struct nfs_server *server,
 	server->mountd_protocol = data->mount_server.protocol;
 
 	server->namelen  = data->namlen;
-	dprintk("<-- nfs_init_server() = 0 [new %p]\n", clp);
 	return 0;
 
 error:
 	server->nfs_client = NULL;
 	nfs_put_client(clp);
-	dprintk("<-- nfs_init_server() = xerror %d\n", error);
 	return error;
 }
 
@@ -795,12 +776,10 @@ int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, struct nfs
 	struct nfs_client *clp = server->nfs_client;
 	int error;
 
-	dprintk("--> nfs_probe_fsinfo()\n");
-
 	if (clp->rpc_ops->set_capabilities != NULL) {
 		error = clp->rpc_ops->set_capabilities(server, mntfh);
 		if (error < 0)
-			goto out_error;
+			return error;
 	}
 
 	fsinfo.fattr = fattr;
@@ -808,7 +787,7 @@ int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, struct nfs
 	memset(fsinfo.layouttype, 0, sizeof(fsinfo.layouttype));
 	error = clp->rpc_ops->fsinfo(server, mntfh, &fsinfo);
 	if (error < 0)
-		goto out_error;
+		return error;
 
 	nfs_server_set_fsinfo(server, &fsinfo);
 
@@ -823,12 +802,7 @@ int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, struct nfs
 			server->namelen = pathinfo.max_namelen;
 	}
 
-	dprintk("<-- nfs_probe_fsinfo() = 0\n");
 	return 0;
-
-out_error:
-	dprintk("nfs_probe_fsinfo: error = %d\n", -error);
-	return error;
 }
 EXPORT_SYMBOL_GPL(nfs_probe_fsinfo);
 
@@ -930,8 +904,6 @@ EXPORT_SYMBOL_GPL(nfs_alloc_server);
  */
 void nfs_free_server(struct nfs_server *server)
 {
-	dprintk("--> nfs_free_server()\n");
-
 	nfs_server_remove_lists(server);
 
 	if (server->destroy != NULL)
@@ -950,7 +922,6 @@ void nfs_free_server(struct nfs_server *server)
 	bdi_destroy(&server->backing_dev_info);
 	kfree(server);
 	nfs_release_automount_timer();
-	dprintk("<-- nfs_free_server()\n");
 }
 EXPORT_SYMBOL_GPL(nfs_free_server);
 
@@ -1030,10 +1001,6 @@ struct nfs_server *nfs_clone_server(struct nfs_server *source,
 	struct nfs_fattr *fattr_fsinfo;
 	int error;
 
-	dprintk("--> nfs_clone_server(,%llx:%llx,)\n",
-		(unsigned long long) fattr->fsid.major,
-		(unsigned long long) fattr->fsid.minor);
-
 	server = nfs_alloc_server();
 	if (!server)
 		return ERR_PTR(-ENOMEM);
@@ -1065,10 +1032,6 @@ struct nfs_server *nfs_clone_server(struct nfs_server *source,
 	if (server->namelen == 0 || server->namelen > NFS4_MAXNAMLEN)
 		server->namelen = NFS4_MAXNAMLEN;
 
-	dprintk("Cloned FSID: %llx:%llx\n",
-		(unsigned long long) server->fsid.major,
-		(unsigned long long) server->fsid.minor);
-
 	error = nfs_start_lockd(server);
 	if (error < 0)
 		goto out_free_server;
@@ -1077,13 +1040,11 @@ struct nfs_server *nfs_clone_server(struct nfs_server *source,
 	server->mount_time = jiffies;
 
 	nfs_free_fattr(fattr_fsinfo);
-	dprintk("<-- nfs_clone_server() = %p\n", server);
 	return server;
 
 out_free_server:
 	nfs_free_fattr(fattr_fsinfo);
 	nfs_free_server(server);
-	dprintk("<-- nfs_clone_server() = error %d\n", error);
 	return ERR_PTR(error);
 }
 EXPORT_SYMBOL_GPL(nfs_clone_server);
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 13/34] NFS: Remove nfs_direct_readpage_release()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (11 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 12/34] NFS: Clean up extra dprintk()s in client.c Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 14/34] NFS: Clean up nfs_direct_commit_complete() Anna.Schumaker
                   ` (16 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Just remove the function and have the caller use nfs_release_request()
instead.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/direct.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index aab32fc3d6a8..0b65e25b71ac 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -392,16 +392,6 @@ static void nfs_direct_complete(struct nfs_direct_req *dreq)
 	nfs_direct_req_release(dreq);
 }
 
-static void nfs_direct_readpage_release(struct nfs_page *req)
-{
-	dprintk("NFS: direct read done (%s/%llu %d@%lld)\n",
-		req->wb_context->dentry->d_sb->s_id,
-		(unsigned long long)NFS_FILEID(d_inode(req->wb_context->dentry)),
-		req->wb_bytes,
-		(long long)req_offset(req));
-	nfs_release_request(req);
-}
-
 static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
 {
 	unsigned long bytes = 0;
@@ -426,7 +416,7 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
 			set_page_dirty(page);
 		bytes += req->wb_bytes;
 		nfs_list_remove_request(req);
-		nfs_direct_readpage_release(req);
+		nfs_release_request(req);
 	}
 out_put:
 	if (put_dreq(dreq))
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 14/34] NFS: Clean up nfs_direct_commit_complete()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (12 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 13/34] NFS: Remove nfs_direct_readpage_release() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 15/34] NFS: Remove extra dprintk()s from namespace.c Anna.Schumaker
                   ` (15 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/direct.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 0b65e25b71ac..18d0868ee274 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -685,16 +685,9 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)
 	int status = data->task.tk_status;
 
 	nfs_init_cinfo_from_dreq(&cinfo, dreq);
-	if (status < 0) {
-		dprintk("NFS: %5u commit failed with error %d.\n",
-			data->task.tk_pid, status);
+	if (status < 0 || nfs_direct_cmp_commit_data_verf(dreq, data))
 		dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
-	} else if (nfs_direct_cmp_commit_data_verf(dreq, data)) {
-		dprintk("NFS: %5u commit verify failed\n", data->task.tk_pid);
-		dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
-	}
 
-	dprintk("NFS: %5u commit returned %d\n", data->task.tk_pid, status);
 	while (!list_empty(&data->pages)) {
 		req = nfs_list_entry(data->pages.next);
 		nfs_list_remove_request(req);
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 15/34] NFS: Remove extra dprintk()s from namespace.c
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (13 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 14/34] NFS: Clean up nfs_direct_commit_complete() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 16/34] NFS: Clean up nfs42_layoutstat_done() Anna.Schumaker
                   ` (14 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/namespace.c | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 786f17580582..1a224a33a6c2 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -143,11 +143,8 @@ struct vfsmount *nfs_d_automount(struct path *path)
 	struct nfs_fh *fh = NULL;
 	struct nfs_fattr *fattr = NULL;
 
-	dprintk("--> nfs_d_automount()\n");
-
-	mnt = ERR_PTR(-ESTALE);
 	if (IS_ROOT(path->dentry))
-		goto out_nofree;
+		return ERR_PTR(-ESTALE);
 
 	mnt = ERR_PTR(-ENOMEM);
 	fh = nfs_alloc_fhandle();
@@ -155,13 +152,10 @@ struct vfsmount *nfs_d_automount(struct path *path)
 	if (fh == NULL || fattr == NULL)
 		goto out;
 
-	dprintk("%s: enter\n", __func__);
-
 	mnt = server->nfs_client->rpc_ops->submount(server, path->dentry, fh, fattr);
 	if (IS_ERR(mnt))
 		goto out;
 
-	dprintk("%s: done, success\n", __func__);
 	mntget(mnt); /* prevent immediate expiration */
 	mnt_set_expiry(mnt, &nfs_automount_list);
 	schedule_delayed_work(&nfs_automount_task, nfs_mountpoint_expiry_timeout);
@@ -169,11 +163,6 @@ struct vfsmount *nfs_d_automount(struct path *path)
 out:
 	nfs_free_fattr(fattr);
 	nfs_free_fhandle(fh);
-out_nofree:
-	if (IS_ERR(mnt))
-		dprintk("<-- %s(): error %ld\n", __func__, PTR_ERR(mnt));
-	else
-		dprintk("<-- %s() = %p\n", __func__, mnt);
 	return mnt;
 }
 
@@ -248,27 +237,20 @@ struct vfsmount *nfs_do_submount(struct dentry *dentry, struct nfs_fh *fh,
 		.fattr = fattr,
 		.authflavor = authflavor,
 	};
-	struct vfsmount *mnt = ERR_PTR(-ENOMEM);
+	struct vfsmount *mnt;
 	char *page = (char *) __get_free_page(GFP_USER);
 	char *devname;
 
-	dprintk("--> nfs_do_submount()\n");
-
-	dprintk("%s: submounting on %pd2\n", __func__,
-			dentry);
 	if (page == NULL)
-		goto out;
+		return ERR_PTR(-ENOMEM);
+
 	devname = nfs_devname(dentry, page, PAGE_SIZE);
-	mnt = (struct vfsmount *)devname;
 	if (IS_ERR(devname))
-		goto free_page;
-	mnt = nfs_do_clone_mount(NFS_SB(dentry->d_sb), devname, &mountdata);
-free_page:
-	free_page((unsigned long)page);
-out:
-	dprintk("%s: done\n", __func__);
+		mnt = (struct vfsmount *)devname;
+	else
+		mnt = nfs_do_clone_mount(NFS_SB(dentry->d_sb), devname, &mountdata);
 
-	dprintk("<-- nfs_do_submount() = %p\n", mnt);
+	free_page((unsigned long)page);
 	return mnt;
 }
 EXPORT_SYMBOL_GPL(nfs_do_submount);
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 16/34] NFS: Clean up nfs42_layoutstat_done()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (14 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 15/34] NFS: Remove extra dprintk()s from namespace.c Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 17/34] NFS: Clean up nfs4_match_clientids() Anna.Schumaker
                   ` (13 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs42proc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 1e486c73ec94..c81c61971625 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -400,8 +400,6 @@ nfs42_layoutstat_done(struct rpc_task *task, void *calldata)
 	case -EOPNOTSUPP:
 		NFS_SERVER(inode)->caps &= ~NFS_CAP_LAYOUTSTATS;
 	}
-
-	dprintk("%s server returns %d\n", __func__, task->tk_status);
 }
 
 static void
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 17/34] NFS: Clean up nfs4_match_clientids()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (15 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 16/34] NFS: Clean up nfs42_layoutstat_done() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 18/34] NFS: Clean up nfs4_check_serverowner_minor_id() Anna.Schumaker
                   ` (12 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

If we cut out the dprintk()s, then we don't even need this to be a
separate function.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4client.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 8346ccbf2d52..39b8c38b6a68 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -583,21 +583,6 @@ int nfs40_walk_client_list(struct nfs_client *new,
 
 #ifdef CONFIG_NFS_V4_1
 /*
- * Returns true if the client IDs match
- */
-static bool nfs4_match_clientids(u64 a, u64 b)
-{
-	if (a != b) {
-		dprintk("NFS: --> %s client ID %llx does not match %llx\n",
-			__func__, a, b);
-		return false;
-	}
-	dprintk("NFS: --> %s client ID %llx matches %llx\n",
-		__func__, a, b);
-	return true;
-}
-
-/*
  * Returns true if the server major ids match
  */
 static bool
@@ -680,7 +665,7 @@ int nfs4_detect_session_trunking(struct nfs_client *clp,
 				 struct rpc_xprt *xprt)
 {
 	/* Check eir_clientid */
-	if (!nfs4_match_clientids(clp->cl_clientid, res->clientid))
+	if (clp->cl_clientid != res->clientid)
 		goto out_err;
 
 	/* Check eir_server_owner so_major_id */
@@ -765,7 +750,7 @@ int nfs41_walk_client_list(struct nfs_client *new,
 		if (pos->cl_cons_state != NFS_CS_READY)
 			continue;
 
-		if (!nfs4_match_clientids(pos->cl_clientid, new->cl_clientid))
+		if (pos->cl_clientid != new->cl_clientid)
 			continue;
 
 		/*
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 18/34] NFS: Clean up nfs4_check_serverowner_minor_id()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (16 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 17/34] NFS: Clean up nfs4_match_clientids() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 19/34] NFS: Create a common nfs4_match_client() function Anna.Schumaker
                   ` (11 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Once again, we can remove the function and compare integer values
directly.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4client.c | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 39b8c38b6a68..4f4f179cb849 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -604,25 +604,6 @@ nfs4_check_serverowner_major_id(struct nfs41_server_owner *o1,
 }
 
 /*
- * Returns true if server minor ids match
- */
-static bool
-nfs4_check_serverowner_minor_id(struct nfs41_server_owner *o1,
-				struct nfs41_server_owner *o2)
-{
-	/* Check eir_server_owner so_minor_id */
-	if (o1->minor_id != o2->minor_id)
-		goto out_minor_mismatch;
-
-	dprintk("NFS: --> %s server owner minor IDs match\n", __func__);
-	return true;
-
-out_minor_mismatch:
-	dprintk("NFS: --> %s server owner minor IDs do not match\n", __func__);
-	return false;
-}
-
-/*
  * Returns true if the server scopes match
  */
 static bool
@@ -674,8 +655,7 @@ int nfs4_detect_session_trunking(struct nfs_client *clp,
 		goto out_err;
 
 	/* Check eir_server_owner so_minor_id */
-	if (!nfs4_check_serverowner_minor_id(clp->cl_serverowner,
-					     res->server_owner))
+	if (clp->cl_serverowner->minor_id != res->server_owner->minor_id)
 		goto out_err;
 
 	/* Check eir_server_scope */
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 19/34] NFS: Create a common nfs4_match_client() function
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (17 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 18/34] NFS: Clean up nfs4_check_serverowner_minor_id() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 20/34] NFS: Clean up nfs4_check_serverowner_major_id() Anna.Schumaker
                   ` (10 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

This puts all the common code in a single place for the
walk_client_list() functions.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4client.c | 119 ++++++++++++++++++++++++----------------------------
 1 file changed, 55 insertions(+), 64 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 4f4f179cb849..8853c32eedf5 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -469,6 +469,50 @@ static bool nfs4_same_verifier(nfs4_verifier *v1, nfs4_verifier *v2)
 	return memcmp(v1->data, v2->data, sizeof(v1->data)) == 0;
 }
 
+static int nfs4_match_client(struct nfs_client  *pos,  struct nfs_client *new,
+			     struct nfs_client **prev, struct nfs_net *nn)
+{
+	int status;
+
+	if (pos->rpc_ops != new->rpc_ops)
+		return 1;
+
+	if (pos->cl_minorversion != new->cl_minorversion)
+		return 1;
+
+	/* If "pos" isn't marked ready, we can't trust the
+	 * remaining fields in "pos", especially the client
+	 * ID and serverowner fields.  Wait for CREATE_SESSION
+	 * to finish. */
+	if (pos->cl_cons_state > NFS_CS_READY) {
+		atomic_inc(&pos->cl_count);
+		spin_unlock(&nn->nfs_client_lock);
+
+		nfs_put_client(*prev);
+		*prev = pos;
+
+		status = nfs_wait_client_init_complete(pos);
+		spin_lock(&nn->nfs_client_lock);
+
+		if (status < 0)
+			return status;
+	}
+
+	if (pos->cl_cons_state != NFS_CS_READY)
+		return 1;
+
+	if (pos->cl_clientid != new->cl_clientid)
+		return 1;
+
+	/* NFSv4.1 always uses the uniform string, however someone
+	 * might switch the uniquifier string on us.
+	 */
+	if (!nfs4_match_client_owner_id(pos, new))
+		return 1;
+
+	return 0;
+}
+
 /**
  * nfs40_walk_client_list - Find server that recognizes a client ID
  *
@@ -497,34 +541,10 @@ int nfs40_walk_client_list(struct nfs_client *new,
 	spin_lock(&nn->nfs_client_lock);
 	list_for_each_entry(pos, &nn->nfs_client_list, cl_share_link) {
 
-		if (pos->rpc_ops != new->rpc_ops)
-			continue;
-
-		if (pos->cl_minorversion != new->cl_minorversion)
-			continue;
-
-		/* If "pos" isn't marked ready, we can't trust the
-		 * remaining fields in "pos" */
-		if (pos->cl_cons_state > NFS_CS_READY) {
-			atomic_inc(&pos->cl_count);
-			spin_unlock(&nn->nfs_client_lock);
-
-			nfs_put_client(prev);
-			prev = pos;
-
-			status = nfs_wait_client_init_complete(pos);
-			if (status < 0)
-				goto out;
-			status = -NFS4ERR_STALE_CLIENTID;
-			spin_lock(&nn->nfs_client_lock);
-		}
-		if (pos->cl_cons_state != NFS_CS_READY)
-			continue;
-
-		if (pos->cl_clientid != new->cl_clientid)
-			continue;
-
-		if (!nfs4_match_client_owner_id(pos, new))
+		status = nfs4_match_client(pos, new, &prev, nn);
+		if (status < 0)
+			goto out_unlock;
+		if (status != 0)
 			continue;
 		/*
 		 * We just sent a new SETCLIENTID, which should have
@@ -567,11 +587,13 @@ int nfs40_walk_client_list(struct nfs_client *new,
 			 */
 			nfs4_schedule_path_down_recovery(pos);
 		default:
+			spin_lock(&nn->nfs_client_lock);
 			goto out;
 		}
 
 		spin_lock(&nn->nfs_client_lock);
 	}
+out_unlock:
 	spin_unlock(&nn->nfs_client_lock);
 
 	/* No match found. The server lost our clientid */
@@ -704,33 +726,10 @@ int nfs41_walk_client_list(struct nfs_client *new,
 		if (pos == new)
 			goto found;
 
-		if (pos->rpc_ops != new->rpc_ops)
-			continue;
-
-		if (pos->cl_minorversion != new->cl_minorversion)
-			continue;
-
-		/* If "pos" isn't marked ready, we can't trust the
-		 * remaining fields in "pos", especially the client
-		 * ID and serverowner fields.  Wait for CREATE_SESSION
-		 * to finish. */
-		if (pos->cl_cons_state > NFS_CS_READY) {
-			atomic_inc(&pos->cl_count);
-			spin_unlock(&nn->nfs_client_lock);
-
-			nfs_put_client(prev);
-			prev = pos;
-
-			status = nfs_wait_client_init_complete(pos);
-			spin_lock(&nn->nfs_client_lock);
-			if (status < 0)
-				break;
-			status = -NFS4ERR_STALE_CLIENTID;
-		}
-		if (pos->cl_cons_state != NFS_CS_READY)
-			continue;
-
-		if (pos->cl_clientid != new->cl_clientid)
+		status = nfs4_match_client(pos, new, &prev, nn);
+		if (status < 0)
+			goto out;
+		if (status != 0)
 			continue;
 
 		/*
@@ -742,23 +741,15 @@ int nfs41_walk_client_list(struct nfs_client *new,
 						     new->cl_serverowner))
 			continue;
 
-		/* Unlike NFSv4.0, we know that NFSv4.1 always uses the
-		 * uniform string, however someone might switch the
-		 * uniquifier string on us.
-		 */
-		if (!nfs4_match_client_owner_id(pos, new))
-			continue;
 found:
 		atomic_inc(&pos->cl_count);
 		*result = pos;
 		status = 0;
-		dprintk("NFS: <-- %s using nfs_client = %p ({%d})\n",
-			__func__, pos, atomic_read(&pos->cl_count));
 		break;
 	}
 
+out:
 	spin_unlock(&nn->nfs_client_lock);
-	dprintk("NFS: <-- %s status = %d\n", __func__, status);
 	nfs_put_client(prev);
 	return status;
 }
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 20/34] NFS: Clean up nfs4_check_serverowner_major_id()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (18 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 19/34] NFS: Create a common nfs4_match_client() function Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 21/34] NFS: Clean up nfs4_check_server_scope() Anna.Schumaker
                   ` (9 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4client.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 8853c32eedf5..ce6f2ef62595 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -612,17 +612,8 @@ nfs4_check_serverowner_major_id(struct nfs41_server_owner *o1,
 				struct nfs41_server_owner *o2)
 {
 	if (o1->major_id_sz != o2->major_id_sz)
-		goto out_major_mismatch;
-	if (memcmp(o1->major_id, o2->major_id, o1->major_id_sz) != 0)
-		goto out_major_mismatch;
-
-	dprintk("NFS: --> %s server owner major IDs match\n", __func__);
-	return true;
-
-out_major_mismatch:
-	dprintk("NFS: --> %s server owner major IDs do not match\n",
-		__func__);
-	return false;
+		return false;
+	return memcmp(o1->major_id, o2->major_id, o1->major_id_sz) == 0;
 }
 
 /*
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 21/34] NFS: Clean up nfs4_check_server_scope()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (19 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 20/34] NFS: Clean up nfs4_check_serverowner_major_id() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 22/34] NFS: Clean up nfs4_set_client() Anna.Schumaker
                   ` (8 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4client.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index ce6f2ef62595..c9016de3e5bd 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -624,18 +624,9 @@ nfs4_check_server_scope(struct nfs41_server_scope *s1,
 			struct nfs41_server_scope *s2)
 {
 	if (s1->server_scope_sz != s2->server_scope_sz)
-		goto out_scope_mismatch;
-	if (memcmp(s1->server_scope, s2->server_scope,
-		   s1->server_scope_sz) != 0)
-		goto out_scope_mismatch;
-
-	dprintk("NFS: --> %s server scopes match\n", __func__);
-	return true;
-
-out_scope_mismatch:
-	dprintk("NFS: --> %s server scopes do not match\n",
-		__func__);
-	return false;
+		return false;
+	return memcmp(s1->server_scope, s2->server_scope,
+					s1->server_scope_sz) == 0;
 }
 
 /**
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 22/34] NFS: Clean up nfs4_set_client()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (20 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 21/34] NFS: Clean up nfs4_check_server_scope() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 23/34] NFS: Clean up nfs4_init_server() Anna.Schumaker
                   ` (7 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

If we cut out the dprintk()s, then we can return error codes directly
and cut out the goto.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4client.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index c9016de3e5bd..a380255e85ed 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -854,9 +854,6 @@ static int nfs4_set_client(struct nfs_server *server,
 		.timeparms = timeparms,
 	};
 	struct nfs_client *clp;
-	int error;
-
-	dprintk("--> nfs4_set_client()\n");
 
 	if (server->flags & NFS_MOUNT_NORESVPORT)
 		set_bit(NFS_CS_NORESVPORT, &cl_init.init_flags);
@@ -865,15 +862,11 @@ static int nfs4_set_client(struct nfs_server *server,
 
 	/* Allocate or find a client reference we can use */
 	clp = nfs_get_client(&cl_init);
-	if (IS_ERR(clp)) {
-		error = PTR_ERR(clp);
-		goto error;
-	}
+	if (IS_ERR(clp))
+		return PTR_ERR(clp);
 
-	if (server->nfs_client == clp) {
-		error = -ELOOP;
-		goto error;
-	}
+	if (server->nfs_client == clp)
+		return -ELOOP;
 
 	/*
 	 * Query for the lease time on clientid setup or renewal
@@ -885,11 +878,7 @@ static int nfs4_set_client(struct nfs_server *server,
 	set_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state);
 
 	server->nfs_client = clp;
-	dprintk("<-- nfs4_set_client() = 0 [new %p]\n", clp);
 	return 0;
-error:
-	dprintk("<-- nfs4_set_client() = xerror %d\n", error);
-	return error;
 }
 
 /*
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 23/34] NFS: Clean up nfs4_init_server()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (21 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 22/34] NFS: Clean up nfs4_set_client() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 24/34] NFS: Remove extra dprintk()s from nfs4client.c Anna.Schumaker
                   ` (6 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4client.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index a380255e85ed..2e7fc76cbd4b 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -1025,8 +1025,6 @@ static int nfs4_init_server(struct nfs_server *server,
 	struct rpc_timeout timeparms;
 	int error;
 
-	dprintk("--> nfs4_init_server()\n");
-
 	nfs_init_timeout_values(&timeparms, data->nfs_server.protocol,
 			data->timeo, data->retrans);
 
@@ -1054,7 +1052,7 @@ static int nfs4_init_server(struct nfs_server *server,
 			data->minorversion,
 			data->net);
 	if (error < 0)
-		goto error;
+		return error;
 
 	if (data->rsize)
 		server->rsize = nfs_block_size(data->rsize, NULL);
@@ -1065,16 +1063,10 @@ static int nfs4_init_server(struct nfs_server *server,
 	server->acregmax = data->acregmax * HZ;
 	server->acdirmin = data->acdirmin * HZ;
 	server->acdirmax = data->acdirmax * HZ;
+	server->port     = data->nfs_server.port;
 
-	server->port = data->nfs_server.port;
-
-	error = nfs_init_server_rpcclient(server, &timeparms,
-					  data->selected_flavor);
-
-error:
-	/* Done */
-	dprintk("<-- nfs4_init_server() = %d\n", error);
-	return error;
+	return nfs_init_server_rpcclient(server, &timeparms,
+					 data->selected_flavor);
 }
 
 /*
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 24/34] NFS: Remove extra dprintk()s from nfs4client.c
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (22 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 23/34] NFS: Clean up nfs4_init_server() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 25/34] NFS: Clean up nfs4_get_rootfh() Anna.Schumaker
                   ` (5 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4client.c | 62 +++++++++--------------------------------------------
 1 file changed, 10 insertions(+), 52 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 2e7fc76cbd4b..692a7a8bfc7a 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -359,11 +359,9 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
 	struct nfs_client *old;
 	int error;
 
-	if (clp->cl_cons_state == NFS_CS_READY) {
+	if (clp->cl_cons_state == NFS_CS_READY)
 		/* the client is initialised already */
-		dprintk("<-- nfs4_init_client() = 0 [already %p]\n", clp);
 		return clp;
-	}
 
 	/* Check NFS protocol revision and initialize RPC op vector */
 	clp->rpc_ops = &nfs_v4_clientops;
@@ -421,7 +419,6 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
 error:
 	nfs_mark_client_ready(clp, error);
 	nfs_put_client(clp);
-	dprintk("<-- nfs4_init_client() = xerror %d\n", error);
 	return ERR_PTR(error);
 }
 
@@ -577,8 +574,6 @@ int nfs40_walk_client_list(struct nfs_client *new,
 
 			prev = NULL;
 			*result = pos;
-			dprintk("NFS: <-- %s using nfs_client = %p ({%d})\n",
-				__func__, pos, atomic_read(&pos->cl_count));
 			goto out;
 		case -ERESTARTSYS:
 		case -ETIMEDOUT:
@@ -599,7 +594,6 @@ int nfs40_walk_client_list(struct nfs_client *new,
 	/* No match found. The server lost our clientid */
 out:
 	nfs_put_client(prev);
-	dprintk("NFS: <-- %s status = %d\n", __func__, status);
 	return status;
 }
 
@@ -909,7 +903,6 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv,
 		.net = mds_clp->cl_net,
 		.timeparms = &ds_timeout,
 	};
-	struct nfs_client *clp;
 	char buf[INET6_ADDRSTRLEN + 1];
 
 	if (rpc_ntop(ds_addr, buf, sizeof(buf)) <= 0)
@@ -925,10 +918,7 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv,
 	 * (section 13.1 RFC 5661).
 	 */
 	nfs_init_timeout_values(&ds_timeout, ds_proto, ds_timeo, ds_retrans);
-	clp = nfs_get_client(&cl_init);
-
-	dprintk("<-- %s %p\n", __func__, clp);
-	return clp;
+	return nfs_get_client(&cl_init);
 }
 EXPORT_SYMBOL_GPL(nfs4_set_ds_client);
 
@@ -1082,8 +1072,6 @@ struct nfs_server *nfs4_create_server(struct nfs_mount_info *mount_info,
 	bool auth_probe;
 	int error;
 
-	dprintk("--> nfs4_create_server()\n");
-
 	server = nfs_alloc_server();
 	if (!server)
 		return ERR_PTR(-ENOMEM);
@@ -1099,12 +1087,10 @@ struct nfs_server *nfs4_create_server(struct nfs_mount_info *mount_info,
 	if (error < 0)
 		goto error;
 
-	dprintk("<-- nfs4_create_server() = %p\n", server);
 	return server;
 
 error:
 	nfs_free_server(server);
-	dprintk("<-- nfs4_create_server() = error %d\n", error);
 	return ERR_PTR(error);
 }
 
@@ -1119,8 +1105,6 @@ struct nfs_server *nfs4_create_referral_server(struct nfs_clone_mount *data,
 	bool auth_probe;
 	int error;
 
-	dprintk("--> nfs4_create_referral_server()\n");
-
 	server = nfs_alloc_server();
 	if (!server)
 		return ERR_PTR(-ENOMEM);
@@ -1154,12 +1138,10 @@ struct nfs_server *nfs4_create_referral_server(struct nfs_clone_mount *data,
 	if (error < 0)
 		goto error;
 
-	dprintk("<-- nfs_create_referral_server() = %p\n", server);
 	return server;
 
 error:
 	nfs_free_server(server);
-	dprintk("<-- nfs4_create_referral_server() = error %d\n", error);
 	return ERR_PTR(error);
 }
 
@@ -1219,31 +1201,16 @@ int nfs4_update_server(struct nfs_server *server, const char *hostname,
 	struct sockaddr *localaddr = (struct sockaddr *)&address;
 	int error;
 
-	dprintk("--> %s: move FSID %llx:%llx to \"%s\")\n", __func__,
-			(unsigned long long)server->fsid.major,
-			(unsigned long long)server->fsid.minor,
-			hostname);
-
 	error = rpc_switch_client_transport(clnt, &xargs, clnt->cl_timeout);
-	if (error != 0) {
-		dprintk("<-- %s(): rpc_switch_client_transport returned %d\n",
-			__func__, error);
-		goto out;
-	}
+	if (error != 0)
+		return error;
 
 	error = rpc_localaddr(clnt, localaddr, sizeof(address));
-	if (error != 0) {
-		dprintk("<-- %s(): rpc_localaddr returned %d\n",
-			__func__, error);
-		goto out;
-	}
+	if (error != 0)
+		return error;
 
-	error = -EAFNOSUPPORT;
-	if (rpc_ntop(localaddr, buf, sizeof(buf)) == 0) {
-		dprintk("<-- %s(): rpc_ntop returned %d\n",
-			__func__, error);
-		goto out;
-	}
+	if (rpc_ntop(localaddr, buf, sizeof(buf)) == 0)
+		return -EAFNOSUPPORT;
 
 	nfs_server_remove_lists(server);
 	error = nfs4_set_client(server, hostname, sap, salen, buf,
@@ -1252,21 +1219,12 @@ int nfs4_update_server(struct nfs_server *server, const char *hostname,
 	nfs_put_client(clp);
 	if (error != 0) {
 		nfs_server_insert_lists(server);
-		dprintk("<-- %s(): nfs4_set_client returned %d\n",
-			__func__, error);
-		goto out;
+		return error;
 	}
 
 	if (server->nfs_client->cl_hostname == NULL)
 		server->nfs_client->cl_hostname = kstrdup(hostname, GFP_KERNEL);
 	nfs_server_insert_lists(server);
 
-	error = nfs_probe_destination(server);
-	if (error < 0)
-		goto out;
-
-	dprintk("<-- %s() succeeded\n", __func__);
-
-out:
-	return error;
+	return nfs_probe_destination(server);
 }
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 25/34] NFS: Clean up nfs4_get_rootfh()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (23 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 24/34] NFS: Remove extra dprintk()s from nfs4client.c Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 26/34] NFS: Remove extra dprintk()s from nfs4namespace.c Anna.Schumaker
                   ` (4 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4getroot.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/nfs/nfs4getroot.c b/fs/nfs/nfs4getroot.c
index 039b3eb6d834..ac8406018962 100644
--- a/fs/nfs/nfs4getroot.c
+++ b/fs/nfs/nfs4getroot.c
@@ -14,8 +14,6 @@ int nfs4_get_rootfh(struct nfs_server *server, struct nfs_fh *mntfh, bool auth_p
 	struct nfs_fsinfo fsinfo;
 	int ret = -ENOMEM;
 
-	dprintk("--> nfs4_get_rootfh()\n");
-
 	fsinfo.fattr = nfs_alloc_fattr();
 	if (fsinfo.fattr == NULL)
 		goto out;
@@ -38,6 +36,5 @@ int nfs4_get_rootfh(struct nfs_server *server, struct nfs_fh *mntfh, bool auth_p
 	memcpy(&server->fsid, &fsinfo.fattr->fsid, sizeof(server->fsid));
 out:
 	nfs_free_fattr(fsinfo.fattr);
-	dprintk("<-- nfs4_get_rootfh() = %d\n", ret);
 	return ret;
 }
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 26/34] NFS: Remove extra dprintk()s from nfs4namespace.c
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (24 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 25/34] NFS: Clean up nfs4_get_rootfh() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 27/34] NFS: Clean up nfs4_proc_bind_one_conn_to_session() Anna.Schumaker
                   ` (3 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4namespace.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index d8b040bd9814..7d531da1bae3 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -340,7 +340,6 @@ static struct vfsmount *nfs_follow_referral(struct dentry *dentry,
 out:
 	free_page((unsigned long) page);
 	free_page((unsigned long) page2);
-	dprintk("%s: done\n", __func__);
 	return mnt;
 }
 
@@ -358,11 +357,9 @@ static struct vfsmount *nfs_do_refmount(struct rpc_clnt *client, struct dentry *
 	int err;
 
 	/* BUG_ON(IS_ROOT(dentry)); */
-	dprintk("%s: enter\n", __func__);
-
 	page = alloc_page(GFP_KERNEL);
 	if (page == NULL)
-		goto out;
+		return mnt;
 
 	fs_locations = kmalloc(sizeof(struct nfs4_fs_locations), GFP_KERNEL);
 	if (fs_locations == NULL)
@@ -386,8 +383,6 @@ static struct vfsmount *nfs_do_refmount(struct rpc_clnt *client, struct dentry *
 out_free:
 	__free_page(page);
 	kfree(fs_locations);
-out:
-	dprintk("%s: done\n", __func__);
 	return mnt;
 }
 
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 27/34] NFS: Clean up nfs4_proc_bind_one_conn_to_session()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (25 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 26/34] NFS: Remove extra dprintk()s from nfs4namespace.c Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 28/34] NFS: Clean up _nfs4_proc_exchange_id() Anna.Schumaker
                   ` (2 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Returning errors directly even lets us remove the goto

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4proc.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 201ca3f2c4ba..184d57a5098d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7155,8 +7155,6 @@ int nfs4_proc_bind_one_conn_to_session(struct rpc_clnt *clnt,
 	};
 	struct rpc_task *task;
 
-	dprintk("--> %s\n", __func__);
-
 	nfs4_copy_sessionid(&args.sessionid, &clp->cl_session->sess_id);
 	if (!(clp->cl_session->flags & SESSION4_BACK_CHAN))
 		args.dir = NFS4_CDFC4_FORE;
@@ -7176,24 +7174,20 @@ int nfs4_proc_bind_one_conn_to_session(struct rpc_clnt *clnt,
 		if (memcmp(res.sessionid.data,
 		    clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) {
 			dprintk("NFS: %s: Session ID mismatch\n", __func__);
-			status = -EIO;
-			goto out;
+			return -EIO;
 		}
 		if ((res.dir & args.dir) != res.dir || res.dir == 0) {
 			dprintk("NFS: %s: Unexpected direction from server\n",
 				__func__);
-			status = -EIO;
-			goto out;
+			return -EIO;
 		}
 		if (res.use_conn_in_rdma_mode != args.use_conn_in_rdma_mode) {
 			dprintk("NFS: %s: Server returned RDMA mode = true\n",
 				__func__);
-			status = -EIO;
-			goto out;
+			return -EIO;
 		}
 	}
-out:
-	dprintk("<-- %s status= %d\n", __func__, status);
+
 	return status;
 }
 
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 28/34] NFS: Clean up _nfs4_proc_exchange_id()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (26 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 27/34] NFS: Clean up nfs4_proc_bind_one_conn_to_session() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-07 18:15 ` [PATCH 29/34] NFS: Clean up nfs4_proc_get_lease_time() Anna.Schumaker
  2017-04-08 14:24 ` [PATCH 00/34] dprintk() cleanup (part 1) Chuck Lever
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4proc.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 184d57a5098d..a3808826b655 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7453,15 +7453,14 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	};
 	struct nfs41_exchange_id_data *calldata;
 	struct rpc_task *task;
-	int status = -EIO;
+	int status;
 
 	if (!atomic_inc_not_zero(&clp->cl_count))
-		goto out;
+		return -EIO;
 
-	status = -ENOMEM;
 	calldata = kzalloc(sizeof(*calldata), GFP_NOFS);
 	if (!calldata)
-		goto out;
+		return -ENOMEM;
 
 	if (!xprt)
 		nfs4_init_boot_verifier(clp, &verifier);
@@ -7470,10 +7469,6 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	if (status)
 		goto out_calldata;
 
-	dprintk("NFS call  exchange_id auth=%s, '%s'\n",
-		clp->cl_rpcclient->cl_auth->au_ops->au_name,
-		clp->cl_owner_id);
-
 	calldata->res.server_owner = kzalloc(sizeof(struct nfs41_server_owner),
 						GFP_NOFS);
 	status = -ENOMEM;
@@ -7539,13 +7534,6 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 
 	rpc_put_task(task);
 out:
-	if (clp->cl_implid != NULL)
-		dprintk("NFS reply exchange_id: Server Implementation ID: "
-			"domain: %s, name: %s, date: %llu,%u\n",
-			clp->cl_implid->domain, clp->cl_implid->name,
-			clp->cl_implid->date.seconds,
-			clp->cl_implid->date.nseconds);
-	dprintk("NFS reply exchange_id: %d\n", status);
 	return status;
 
 out_impl_id:
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 29/34] NFS: Clean up nfs4_proc_get_lease_time()
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (27 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 28/34] NFS: Clean up _nfs4_proc_exchange_id() Anna.Schumaker
@ 2017-04-07 18:15 ` Anna.Schumaker
  2017-04-08 14:24 ` [PATCH 00/34] dprintk() cleanup (part 1) Chuck Lever
  29 siblings, 0 replies; 31+ messages in thread
From: Anna.Schumaker @ 2017-04-07 18:15 UTC (permalink / raw)
  To: Trond.Myklebust, linux-nfs

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4proc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a3808826b655..dda19a35ad9e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7751,17 +7751,13 @@ int nfs4_proc_get_lease_time(struct nfs_client *clp, struct nfs_fsinfo *fsinfo)
 
 	nfs4_init_sequence(&args.la_seq_args, &res.lr_seq_res, 0);
 	nfs4_set_sequence_privileged(&args.la_seq_args);
-	dprintk("--> %s\n", __func__);
 	task = rpc_run_task(&task_setup);
 
 	if (IS_ERR(task))
-		status = PTR_ERR(task);
-	else {
-		status = task->tk_status;
-		rpc_put_task(task);
-	}
-	dprintk("<-- %s return %d\n", __func__, status);
+		return PTR_ERR(task);
 
+	status = task->tk_status;
+	rpc_put_task(task);
 	return status;
 }
 
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 00/34] dprintk() cleanup (part 1)
  2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
                   ` (28 preceding siblings ...)
  2017-04-07 18:15 ` [PATCH 29/34] NFS: Clean up nfs4_proc_get_lease_time() Anna.Schumaker
@ 2017-04-08 14:24 ` Chuck Lever
  29 siblings, 0 replies; 31+ messages in thread
From: Chuck Lever @ 2017-04-08 14:24 UTC (permalink / raw)
  To: Anna Schumaker; +Cc: Trond Myklebust, Linux NFS Mailing List


> On Apr 7, 2017, at 2:14 PM, Anna.Schumaker@netapp.com wrote:
> 
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> We have around 750 dprintk()s in the NFS client alone, and many of these
> simply tell us when we're entering or leaving a function (usually both).
> This can create a lot of noise when you're trying to debug a single issues,
> and I think many dprintk()s can be cut out in favor of tracepoints or dynamic
> tracing through SystemTap.

Is there a description somewhere under Documentation for how
to enable and use each of these mechanisms?


> This patch set does just that. I went through some of our files one by one
> and removed dprintk()s that seemed to be unnecessary. Occasionally this
> allowed me to refactor the surrounding code, so I tried to make individual
> patches for each function I refactored.
> 
> These patches make changes to the following files:
>    - callback_proc.c
>    - callback_xdr.c
>    - client.c
>    - direct.c
>    - namespace.c
>    - nfs42proc.c
>    - nfs4client.c
>    - nfs4getroot.c
>    - nfs4namespace.c
>    - nfs4proc.c
> 
> What does everybody think?

I'm OK with clean up. Some random thoughts:

A more recent problem with dprintk debugging is the god-damned
journal rate limiting with imjournald and systemd. So IMO the
Linux NFS community has to come up with debugging tools that
do not run afoul of log message rate limiting.

If you're going to move forward with this project, should we
have a plan for removing dprintk in NFSD and the RPC layer as
well? (I've had a task on my to-do list for a while to replace
dprintk in rpcrdma.ko, but always there's something higher in
priority to work on).

Is there a plan for updating or replacing user space tooling
(ie. rpcdebug)?

Is there a desire to preserve the ability to compile out the
new debugging apparatus (tracepoints and so on)?

I think we want a "ruling" on whether dprintk and tracepoints
are considered part of a long-term kernel ABI/API. If they
are then such a conversion would mean a deeper commitment to
maintain specific tracepoints.


> Are any of these dprintk()s worth holding on to?
> 
> Thanks,
> Anna
> 
> 
> Anna Schumaker (34):
>  NFS: Clean up do_callback_layoutrecall()
>  NFS: Clean up nfs4_callback_layoutrecall()
>  NFS: Remove extra dprintk()s from callback_proc.c
>  NFS: Clean up decode_getattr_args()
>  NFS: Clean up decode_recall_args()
>  NFS: Clean up decode_layoutrecall_args()
>  NFS: Clean up decode_cb_sequence_args()
>  NFS: Clean up decode_notify_lock_args()
>  NFS: Clean up encode_cb_sequence_res()
>  NFS: Remove extra dprintk()s from callback_xdr.c
>  NFS: Clean up nfs_init_client()
>  NFS: Clean up extra dprintk()s in client.c
>  NFS: Remove nfs_direct_readpage_release()
>  NFS: Clean up nfs_direct_commit_complete()
>  NFS: Remove extra dprintk()s from namespace.c
>  NFS: Clean up nfs42_layoutstat_done()
>  NFS: Clean up nfs4_match_clientids()
>  NFS: Clean up nfs4_check_serverowner_minor_id()
>  NFS: Create a common nfs4_match_client() function
>  NFS: Clean up nfs4_check_serverowner_major_id()
>  NFS: Clean up nfs4_check_server_scope()
>  NFS: Clean up nfs4_set_client()
>  NFS: Clean up nfs4_init_server()
>  NFS: Remove extra dprintk()s from nfs4client.c
>  NFS: Clean up nfs4_get_rootfh()
>  NFS: Remove extra dprintk()s from nfs4namespace.c
>  NFS: Clean up nfs4_proc_bind_one_conn_to_session()
>  NFS: Clean up _nfs4_proc_exchange_id()
>  NFS: Clean up nfs4_proc_get_lease_time()
>  NFS: Clean up nfs41_proc_async_sequence()
>  NFS: Clean up nfs4_proc_sequence()
>  NFS: Clean up nfs41_proc_reclaim_complete()
>  NFS: Clean up nfs4_layoutget_handle_exception()
>  NFS: Remove extra dprintk()s from nfs4proc.c
> 
> fs/nfs/callback_proc.c |  41 +------
> fs/nfs/callback_xdr.c  | 109 +++++--------------
> fs/nfs/client.c        |  65 ++----------
> fs/nfs/direct.c        |  21 +---
> fs/nfs/namespace.c     |  34 ++----
> fs/nfs/nfs42proc.c     |   2 -
> fs/nfs/nfs4client.c    | 283 ++++++++++++++-----------------------------------
> fs/nfs/nfs4getroot.c   |   3 -
> fs/nfs/nfs4namespace.c |   7 +-
> fs/nfs/nfs4proc.c      | 274 +++++++----------------------------------------
> 10 files changed, 167 insertions(+), 672 deletions(-)
> 
> -- 
> 2.12.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever




^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2017-04-08 14:24 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-07 18:14 [PATCH 00/34] dprintk() cleanup (part 1) Anna.Schumaker
2017-04-07 18:14 ` [PATCH 01/34] NFS: Clean up do_callback_layoutrecall() Anna.Schumaker
2017-04-07 18:14 ` [PATCH 02/34] NFS: Clean up nfs4_callback_layoutrecall() Anna.Schumaker
2017-04-07 18:14 ` [PATCH 03/34] NFS: Remove extra dprintk()s from callback_proc.c Anna.Schumaker
2017-04-07 18:14 ` [PATCH 04/34] NFS: Clean up decode_getattr_args() Anna.Schumaker
2017-04-07 18:14 ` [PATCH 05/34] NFS: Clean up decode_recall_args() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 06/34] NFS: Clean up decode_layoutrecall_args() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 07/34] NFS: Clean up decode_cb_sequence_args() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 08/34] NFS: Clean up decode_notify_lock_args() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 09/34] NFS: Clean up encode_cb_sequence_res() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 10/34] NFS: Remove extra dprintk()s from callback_xdr.c Anna.Schumaker
2017-04-07 18:15 ` [PATCH 11/34] NFS: Clean up nfs_init_client() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 12/34] NFS: Clean up extra dprintk()s in client.c Anna.Schumaker
2017-04-07 18:15 ` [PATCH 13/34] NFS: Remove nfs_direct_readpage_release() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 14/34] NFS: Clean up nfs_direct_commit_complete() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 15/34] NFS: Remove extra dprintk()s from namespace.c Anna.Schumaker
2017-04-07 18:15 ` [PATCH 16/34] NFS: Clean up nfs42_layoutstat_done() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 17/34] NFS: Clean up nfs4_match_clientids() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 18/34] NFS: Clean up nfs4_check_serverowner_minor_id() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 19/34] NFS: Create a common nfs4_match_client() function Anna.Schumaker
2017-04-07 18:15 ` [PATCH 20/34] NFS: Clean up nfs4_check_serverowner_major_id() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 21/34] NFS: Clean up nfs4_check_server_scope() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 22/34] NFS: Clean up nfs4_set_client() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 23/34] NFS: Clean up nfs4_init_server() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 24/34] NFS: Remove extra dprintk()s from nfs4client.c Anna.Schumaker
2017-04-07 18:15 ` [PATCH 25/34] NFS: Clean up nfs4_get_rootfh() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 26/34] NFS: Remove extra dprintk()s from nfs4namespace.c Anna.Schumaker
2017-04-07 18:15 ` [PATCH 27/34] NFS: Clean up nfs4_proc_bind_one_conn_to_session() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 28/34] NFS: Clean up _nfs4_proc_exchange_id() Anna.Schumaker
2017-04-07 18:15 ` [PATCH 29/34] NFS: Clean up nfs4_proc_get_lease_time() Anna.Schumaker
2017-04-08 14:24 ` [PATCH 00/34] dprintk() cleanup (part 1) Chuck Lever

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.