[Cluster-devel] [PATCH 0/7] seq_printf cleanups

[Cluster-devel] [PATCH 0/7] seq_printf cleanups

[Joe Perches]

seq_printf should return void.

Add a public bool seq_is_full function that can be used to shortcut
unnecesary seq_printf/seq_puts calls when the seq buffer is full.

Start removing the misuses of the seq_printf/seq_puts return value.

Patchset brought forward from an unreplied to set of changes from
back in December 2013.

https://lkml.org/lkml/2013/12/11/713

Renamed seq_is_buf_full to seq_is_full.

Joe Perches (7):
  seq_file: Rename static bool seq_overflow to public bool seq_is_full
  netfilter: Convert print_tuple functions to return void
  dlm: Use seq_is_full - remove seq_printf returns
  dlm: Use seq_puts, remove unnecessary trailing spaces
  fs: Convert show_fdinfo functions to void
  debugfs: Fix misuse of seq_printf return value
  docg3: Fix mixuse of seq_printf return value

 Documentation/filesystems/seq_file.txt       |  28 +--
 Documentation/filesystems/vfs.txt            |   2 +-
 drivers/mtd/devices/docg3.c                  | 112 ++++++------
 drivers/net/tun.c                            |   4 +-
 fs/debugfs/file.c                            |  14 +-
 fs/dlm/debug_fs.c                            | 260 +++++++++++++--------------
 fs/eventfd.c                                 |  15 +-
 fs/eventpoll.c                               |  19 +-
 fs/notify/fdinfo.c                           |  76 ++++----
 fs/notify/fdinfo.h                           |   4 +-
 fs/proc/fd.c                                 |   2 +-
 fs/seq_file.c                                |  28 +--
 fs/signalfd.c                                |  10 +-
 fs/timerfd.c                                 |  27 +--
 include/linux/fs.h                           |   2 +-
 include/linux/seq_file.h                     |   8 +
 include/net/netfilter/nf_conntrack_core.h    |   2 +-
 include/net/netfilter/nf_conntrack_l3proto.h |   4 +-
 include/net/netfilter/nf_conntrack_l4proto.h |   4 +-
 net/netfilter/nf_conntrack_l3proto_generic.c |   5 +-
 net/netfilter/nf_conntrack_proto_dccp.c      |  10 +-
 net/netfilter/nf_conntrack_proto_generic.c   |   5 +-
 net/netfilter/nf_conntrack_proto_gre.c       |  10 +-
 net/netfilter/nf_conntrack_proto_sctp.c      |  10 +-
 net/netfilter/nf_conntrack_proto_tcp.c       |  10 +-
 net/netfilter/nf_conntrack_proto_udp.c       |  10 +-
 net/netfilter/nf_conntrack_proto_udplite.c   |  10 +-
 net/netfilter/nf_conntrack_standalone.c      |  15 +-
 28 files changed, 333 insertions(+), 373 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

[Cluster-devel] [PATCH 3/7] dlm: Use seq_is_full - remove seq_printf returns

[Joe Perches]

The seq_printf return should be ignored and the
seq_is_full function should be tested instead.

Convert functions returning int to void where
seq_printf is used.

Signed-off-by: Joe Perches <joe@perches.com>
---
 fs/dlm/debug_fs.c | 248 +++++++++++++++++++++++++-----------------------------
 1 file changed, 116 insertions(+), 132 deletions(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index 1323c56..27c8335 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -48,8 +48,8 @@ static char *print_lockmode(int mode)
 	}
 }
 
-static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
-			      struct dlm_rsb *res)
+static void print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
+			       struct dlm_rsb *res)
 {
 	seq_printf(s, "%08x %s", lkb->lkb_id, print_lockmode(lkb->lkb_grmode));
 
@@ -68,21 +68,17 @@ static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
 	if (lkb->lkb_wait_type)
 		seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
 
-	return seq_puts(s, "\n");
+	seq_puts(s, "\n");
 }
 
-static int print_format1(struct dlm_rsb *res, struct seq_file *s)
+static void print_format1(struct dlm_rsb *res, struct seq_file *s)
 {
 	struct dlm_lkb *lkb;
 	int i, lvblen = res->res_ls->ls_lvblen, recover_list, root_list;
-	int rv;
 
 	lock_rsb(res);
 
-	rv = seq_printf(s, "\nResource %p Name (len=%d) \"",
-			res, res->res_length);
-	if (rv)
-		goto out;
+	seq_printf(s, "\nResource %p Name (len=%d) \"", res, res->res_length);
 
 	for (i = 0; i < res->res_length; i++) {
 		if (isprint(res->res_name[i]))
@@ -92,17 +88,16 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
 	}
 
 	if (res->res_nodeid > 0)
-		rv = seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
-				res->res_nodeid);
+		seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
+			   res->res_nodeid);
 	else if (res->res_nodeid == 0)
-		rv = seq_puts(s, "\"\nMaster Copy\n");
+		seq_puts(s, "\"\nMaster Copy\n");
 	else if (res->res_nodeid == -1)
-		rv = seq_printf(s, "\"\nLooking up master (lkid %x)\n",
-			   	res->res_first_lkid);
+		seq_printf(s, "\"\nLooking up master (lkid %x)\n",
+			   res->res_first_lkid);
 	else
-		rv = seq_printf(s, "\"\nInvalid master %d\n",
-				res->res_nodeid);
-	if (rv)
+		seq_printf(s, "\"\nInvalid master %d\n", res->res_nodeid);
+	if (seq_is_full(s))
 		goto out;
 
 	/* Print the LVB: */
@@ -116,8 +111,8 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
 		}
 		if (rsb_flag(res, RSB_VALNOTVALID))
 			seq_puts(s, " (INVALID)");
-		rv = seq_puts(s, "\n");
-		if (rv)
+		seq_puts(s, "\n");
+		if (seq_is_full(s))
 			goto out;
 	}
 
@@ -125,32 +120,30 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
 	recover_list = !list_empty(&res->res_recover_list);
 
 	if (root_list || recover_list) {
-		rv = seq_printf(s, "Recovery: root %d recover %d flags %lx "
-				"count %d\n", root_list, recover_list,
-			   	res->res_flags, res->res_recover_locks_count);
-		if (rv)
-			goto out;
+		seq_printf(s, "Recovery: root %d recover %d flags %lx count %d\n",
+			   root_list, recover_list,
+			   res->res_flags, res->res_recover_locks_count);
 	}
 
 	/* Print the locks attached to this resource */
 	seq_puts(s, "Granted Queue\n");
 	list_for_each_entry(lkb, &res->res_grantqueue, lkb_statequeue) {
-		rv = print_format1_lock(s, lkb, res);
-		if (rv)
+		print_format1_lock(s, lkb, res);
+		if (seq_is_full(s))
 			goto out;
 	}
 
 	seq_puts(s, "Conversion Queue\n");
 	list_for_each_entry(lkb, &res->res_convertqueue, lkb_statequeue) {
-		rv = print_format1_lock(s, lkb, res);
-		if (rv)
+		print_format1_lock(s, lkb, res);
+		if (seq_is_full(s))
 			goto out;
 	}
 
 	seq_puts(s, "Waiting Queue\n");
 	list_for_each_entry(lkb, &res->res_waitqueue, lkb_statequeue) {
-		rv = print_format1_lock(s, lkb, res);
-		if (rv)
+		print_format1_lock(s, lkb, res);
+		if (seq_is_full(s))
 			goto out;
 	}
 
@@ -159,23 +152,23 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
 
 	seq_puts(s, "Lookup Queue\n");
 	list_for_each_entry(lkb, &res->res_lookup, lkb_rsb_lookup) {
-		rv = seq_printf(s, "%08x %s", lkb->lkb_id,
-				print_lockmode(lkb->lkb_rqmode));
+		seq_printf(s, "%08x %s",
+			   lkb->lkb_id, print_lockmode(lkb->lkb_rqmode));
 		if (lkb->lkb_wait_type)
 			seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
-		rv = seq_puts(s, "\n");
+		seq_puts(s, "\n");
+		if (seq_is_full(s))
+			goto out;
 	}
  out:
 	unlock_rsb(res);
-	return rv;
 }
 
-static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
-			      struct dlm_rsb *r)
+static void print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
+			       struct dlm_rsb *r)
 {
 	u64 xid = 0;
 	u64 us;
-	int rv;
 
 	if (lkb->lkb_flags & DLM_IFL_USER) {
 		if (lkb->lkb_ua)
@@ -188,103 +181,97 @@ static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
 	/* id nodeid remid pid xid exflags flags sts grmode rqmode time_us
 	   r_nodeid r_len r_name */
 
-	rv = seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
-			lkb->lkb_id,
-			lkb->lkb_nodeid,
-			lkb->lkb_remid,
-			lkb->lkb_ownpid,
-			(unsigned long long)xid,
-			lkb->lkb_exflags,
-			lkb->lkb_flags,
-			lkb->lkb_status,
-			lkb->lkb_grmode,
-			lkb->lkb_rqmode,
-			(unsigned long long)us,
-			r->res_nodeid,
-			r->res_length,
-			r->res_name);
-	return rv;
+	seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
+		   lkb->lkb_id,
+		   lkb->lkb_nodeid,
+		   lkb->lkb_remid,
+		   lkb->lkb_ownpid,
+		   (unsigned long long)xid,
+		   lkb->lkb_exflags,
+		   lkb->lkb_flags,
+		   lkb->lkb_status,
+		   lkb->lkb_grmode,
+		   lkb->lkb_rqmode,
+		   (unsigned long long)us,
+		   r->res_nodeid,
+		   r->res_length,
+		   r->res_name);
 }
 
-static int print_format2(struct dlm_rsb *r, struct seq_file *s)
+static void print_format2(struct dlm_rsb *r, struct seq_file *s)
 {
 	struct dlm_lkb *lkb;
-	int rv = 0;
 
 	lock_rsb(r);
 
 	list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
-		rv = print_format2_lock(s, lkb, r);
-		if (rv)
+		print_format2_lock(s, lkb, r);
+		if (seq_is_full(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
-		rv = print_format2_lock(s, lkb, r);
-		if (rv)
+		print_format2_lock(s, lkb, r);
+		if (seq_is_full(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
-		rv = print_format2_lock(s, lkb, r);
-		if (rv)
+		print_format2_lock(s, lkb, r);
+		if (seq_is_full(s))
 			goto out;
 	}
  out:
 	unlock_rsb(r);
-	return rv;
 }
 
-static int print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
+static void print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
 			      int rsb_lookup)
 {
 	u64 xid = 0;
-	int rv;
 
 	if (lkb->lkb_flags & DLM_IFL_USER) {
 		if (lkb->lkb_ua)
 			xid = lkb->lkb_ua->xid;
 	}
 
-	rv = seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
-			lkb->lkb_id,
-			lkb->lkb_nodeid,
-			lkb->lkb_remid,
-			lkb->lkb_ownpid,
-			(unsigned long long)xid,
-			lkb->lkb_exflags,
-			lkb->lkb_flags,
-			lkb->lkb_status,
-			lkb->lkb_grmode,
-			lkb->lkb_rqmode,
-			lkb->lkb_last_bast.mode,
-			rsb_lookup,
-			lkb->lkb_wait_type,
-			lkb->lkb_lvbseq,
-			(unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
-			(unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
-	return rv;
+	seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
+		   lkb->lkb_id,
+		   lkb->lkb_nodeid,
+		   lkb->lkb_remid,
+		   lkb->lkb_ownpid,
+		   (unsigned long long)xid,
+		   lkb->lkb_exflags,
+		   lkb->lkb_flags,
+		   lkb->lkb_status,
+		   lkb->lkb_grmode,
+		   lkb->lkb_rqmode,
+		   lkb->lkb_last_bast.mode,
+		   rsb_lookup,
+		   lkb->lkb_wait_type,
+		   lkb->lkb_lvbseq,
+		   (unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
+		   (unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
 }
 
-static int print_format3(struct dlm_rsb *r, struct seq_file *s)
+static void print_format3(struct dlm_rsb *r, struct seq_file *s)
 {
 	struct dlm_lkb *lkb;
 	int i, lvblen = r->res_ls->ls_lvblen;
 	int print_name = 1;
-	int rv;
 
 	lock_rsb(r);
 
-	rv = seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
-			r,
-			r->res_nodeid,
-			r->res_first_lkid,
-			r->res_flags,
-			!list_empty(&r->res_root_list),
-			!list_empty(&r->res_recover_list),
-			r->res_recover_locks_count,
-			r->res_length);
-	if (rv)
+	seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
+		   r,
+		   r->res_nodeid,
+		   r->res_first_lkid,
+		   r->res_flags,
+		   !list_empty(&r->res_root_list),
+		   !list_empty(&r->res_recover_list),
+		   r->res_recover_locks_count,
+		   r->res_length);
+	if (seq_is_full(s))
 		goto out;
 
 	for (i = 0; i < r->res_length; i++) {
@@ -300,8 +287,8 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
 		else
 			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
 	}
-	rv = seq_puts(s, "\n");
-	if (rv)
+	seq_puts(s, "\n");
+	if (seq_is_full(s))
 		goto out;
 
 	if (!r->res_lvbptr)
@@ -311,58 +298,55 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
 
 	for (i = 0; i < lvblen; i++)
 		seq_printf(s, " %02x", (unsigned char)r->res_lvbptr[i]);
-	rv = seq_puts(s, "\n");
-	if (rv)
+	seq_puts(s, "\n");
+	if (seq_is_full(s))
 		goto out;
 
  do_locks:
 	list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
-		rv = print_format3_lock(s, lkb, 0);
-		if (rv)
+		print_format3_lock(s, lkb, 0);
+		if (seq_is_full(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
-		rv = print_format3_lock(s, lkb, 0);
-		if (rv)
+		print_format3_lock(s, lkb, 0);
+		if (seq_is_full(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
-		rv = print_format3_lock(s, lkb, 0);
-		if (rv)
+		print_format3_lock(s, lkb, 0);
+		if (seq_is_full(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_lookup, lkb_rsb_lookup) {
-		rv = print_format3_lock(s, lkb, 1);
-		if (rv)
+		print_format3_lock(s, lkb, 1);
+		if (seq_is_full(s))
 			goto out;
 	}
  out:
 	unlock_rsb(r);
-	return rv;
 }
 
-static int print_format4(struct dlm_rsb *r, struct seq_file *s)
+static void print_format4(struct dlm_rsb *r, struct seq_file *s)
 {
 	int our_nodeid = dlm_our_nodeid();
 	int print_name = 1;
-	int i, rv;
+	int i;
 
 	lock_rsb(r);
 
-	rv = seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
-			r,
-			r->res_nodeid,
-			r->res_master_nodeid,
-			r->res_dir_nodeid,
-			our_nodeid,
-			r->res_toss_time,
-			r->res_flags,
-			r->res_length);
-	if (rv)
-		goto out;
+	seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
+		   r,
+		   r->res_nodeid,
+		   r->res_master_nodeid,
+		   r->res_dir_nodeid,
+		   our_nodeid,
+		   r->res_toss_time,
+		   r->res_flags,
+		   r->res_length);
 
 	for (i = 0; i < r->res_length; i++) {
 		if (!isascii(r->res_name[i]) || !isprint(r->res_name[i]))
@@ -377,10 +361,9 @@ static int print_format4(struct dlm_rsb *r, struct seq_file *s)
 		else
 			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
 	}
-	rv = seq_puts(s, "\n");
- out:
+	seq_puts(s, "\n");
+
 	unlock_rsb(r);
-	return rv;
 }
 
 struct rsbtbl_iter {
@@ -390,11 +373,12 @@ struct rsbtbl_iter {
 	int header;
 };
 
-/* seq_printf returns -1 if the buffer is full, and 0 otherwise.
-   If the buffer is full, seq_printf can be called again, but it
-   does nothing and just returns -1.  So, the these printing routines
-   periodically check the return value to avoid wasting too much time
-   trying to print to a full buffer. */
+/*
+ * If the buffer is full, seq_printf can be called again, but it
+ * does nothing.  So, the these printing routines periodically check
+ * seq_is_full to avoid wasting too much time trying to print to
+ * a full buffer.
+ */
 
 static int table_seq_show(struct seq_file *seq, void *iter_ptr)
 {
@@ -403,7 +387,7 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
 
 	switch (ri->format) {
 	case 1:
-		rv = print_format1(ri->rsb, seq);
+		print_format1(ri->rsb, seq);
 		break;
 	case 2:
 		if (ri->header) {
@@ -412,21 +396,21 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
 					"r_nodeid r_len r_name\n");
 			ri->header = 0;
 		}
-		rv = print_format2(ri->rsb, seq);
+		print_format2(ri->rsb, seq);
 		break;
 	case 3:
 		if (ri->header) {
 			seq_printf(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
 			ri->header = 0;
 		}
-		rv = print_format3(ri->rsb, seq);
+		print_format3(ri->rsb, seq);
 		break;
 	case 4:
 		if (ri->header) {
 			seq_printf(seq, "version 4 rsb 2\n");
 			ri->header = 0;
 		}
-		rv = print_format4(ri->rsb, seq);
+		print_format4(ri->rsb, seq);
 		break;
 	}
 
-- 
1.8.1.2.459.gbcd45b4.dirty

[Cluster-devel] [PATCH 3/7] dlm: Use seq_is_full - remove seq_printf returns

[Petr Mladek]

On Mon 29-09-14 16:08:23, Joe Perches wrote:
> The seq_printf return should be ignored and the
> seq_is_full function should be tested instead.
> 
> Convert functions returning int to void where
> seq_printf is used.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  fs/dlm/debug_fs.c | 248 +++++++++++++++++++++++++-----------------------------
>  1 file changed, 116 insertions(+), 132 deletions(-)
> 
> diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
> index 1323c56..27c8335 100644
> --- a/fs/dlm/debug_fs.c
> +++ b/fs/dlm/debug_fs.c
> @@ -48,8 +48,8 @@ static char *print_lockmode(int mode)
>  	}
>  }
>  
> -static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
> -			      struct dlm_rsb *res)
> +static void print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
> +			       struct dlm_rsb *res)
>  {
>  	seq_printf(s, "%08x %s", lkb->lkb_id, print_lockmode(lkb->lkb_grmode));
>  
> @@ -68,21 +68,17 @@ static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
>  	if (lkb->lkb_wait_type)
>  		seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
>  
> -	return seq_puts(s, "\n");
> +	seq_puts(s, "\n");
>  }
>  
> -static int print_format1(struct dlm_rsb *res, struct seq_file *s)
> +static void print_format1(struct dlm_rsb *res, struct seq_file *s)
>  {
>  	struct dlm_lkb *lkb;
>  	int i, lvblen = res->res_ls->ls_lvblen, recover_list, root_list;
> -	int rv;
>  
>  	lock_rsb(res);
>  
> -	rv = seq_printf(s, "\nResource %p Name (len=%d) \"",
> -			res, res->res_length);
> -	if (rv)
> -		goto out;
> +	seq_printf(s, "\nResource %p Name (len=%d) \"", res, res->res_length);

To keep the functionality, we should add

	if(seq_overflow(s))
		goto out;

  
>  	for (i = 0; i < res->res_length; i++) {
>  		if (isprint(res->res_name[i]))
> @@ -92,17 +88,16 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>  	}
>  
>  	if (res->res_nodeid > 0)
> -		rv = seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
> -				res->res_nodeid);
> +		seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
> +			   res->res_nodeid);
>  	else if (res->res_nodeid == 0)
> -		rv = seq_puts(s, "\"\nMaster Copy\n");
> +		seq_puts(s, "\"\nMaster Copy\n");
>  	else if (res->res_nodeid == -1)
> -		rv = seq_printf(s, "\"\nLooking up master (lkid %x)\n",
> -			   	res->res_first_lkid);
> +		seq_printf(s, "\"\nLooking up master (lkid %x)\n",
> +			   res->res_first_lkid);
>  	else
> -		rv = seq_printf(s, "\"\nInvalid master %d\n",
> -				res->res_nodeid);
> -	if (rv)
> +		seq_printf(s, "\"\nInvalid master %d\n", res->res_nodeid);
> +	if (seq_is_full(s))
>  		goto out;

I would check for seq_overflow()

Etc. There are needed many more changes if we agree on introducing
seq_is_full() and seq_overflow().

Well, we will need to touch most of these locations once again when
Steven removes return value from all the seq_* functions.

Best Regards,
Petr

>  	/* Print the LVB: */
> @@ -116,8 +111,8 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>  		}
>  		if (rsb_flag(res, RSB_VALNOTVALID))
>  			seq_puts(s, " (INVALID)");
> -		rv = seq_puts(s, "\n");
> -		if (rv)
> +		seq_puts(s, "\n");
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
> @@ -125,32 +120,30 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>  	recover_list = !list_empty(&res->res_recover_list);
>  
>  	if (root_list || recover_list) {
> -		rv = seq_printf(s, "Recovery: root %d recover %d flags %lx "
> -				"count %d\n", root_list, recover_list,
> -			   	res->res_flags, res->res_recover_locks_count);
> -		if (rv)
> -			goto out;
> +		seq_printf(s, "Recovery: root %d recover %d flags %lx count %d\n",
> +			   root_list, recover_list,
> +			   res->res_flags, res->res_recover_locks_count);
>  	}
>  
>  	/* Print the locks attached to this resource */
>  	seq_puts(s, "Granted Queue\n");
>  	list_for_each_entry(lkb, &res->res_grantqueue, lkb_statequeue) {
> -		rv = print_format1_lock(s, lkb, res);
> -		if (rv)
> +		print_format1_lock(s, lkb, res);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
>  	seq_puts(s, "Conversion Queue\n");
>  	list_for_each_entry(lkb, &res->res_convertqueue, lkb_statequeue) {
> -		rv = print_format1_lock(s, lkb, res);
> -		if (rv)
> +		print_format1_lock(s, lkb, res);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
>  	seq_puts(s, "Waiting Queue\n");
>  	list_for_each_entry(lkb, &res->res_waitqueue, lkb_statequeue) {
> -		rv = print_format1_lock(s, lkb, res);
> -		if (rv)
> +		print_format1_lock(s, lkb, res);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
> @@ -159,23 +152,23 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>  
>  	seq_puts(s, "Lookup Queue\n");
>  	list_for_each_entry(lkb, &res->res_lookup, lkb_rsb_lookup) {
> -		rv = seq_printf(s, "%08x %s", lkb->lkb_id,
> -				print_lockmode(lkb->lkb_rqmode));
> +		seq_printf(s, "%08x %s",
> +			   lkb->lkb_id, print_lockmode(lkb->lkb_rqmode));
>  		if (lkb->lkb_wait_type)
>  			seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
> -		rv = seq_puts(s, "\n");
> +		seq_puts(s, "\n");
> +		if (seq_is_full(s))
> +			goto out;
>  	}
>   out:
>  	unlock_rsb(res);
> -	return rv;
>  }
>  
> -static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
> -			      struct dlm_rsb *r)
> +static void print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
> +			       struct dlm_rsb *r)
>  {
>  	u64 xid = 0;
>  	u64 us;
> -	int rv;
>  
>  	if (lkb->lkb_flags & DLM_IFL_USER) {
>  		if (lkb->lkb_ua)
> @@ -188,103 +181,97 @@ static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
>  	/* id nodeid remid pid xid exflags flags sts grmode rqmode time_us
>  	   r_nodeid r_len r_name */
>  
> -	rv = seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
> -			lkb->lkb_id,
> -			lkb->lkb_nodeid,
> -			lkb->lkb_remid,
> -			lkb->lkb_ownpid,
> -			(unsigned long long)xid,
> -			lkb->lkb_exflags,
> -			lkb->lkb_flags,
> -			lkb->lkb_status,
> -			lkb->lkb_grmode,
> -			lkb->lkb_rqmode,
> -			(unsigned long long)us,
> -			r->res_nodeid,
> -			r->res_length,
> -			r->res_name);
> -	return rv;
> +	seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
> +		   lkb->lkb_id,
> +		   lkb->lkb_nodeid,
> +		   lkb->lkb_remid,
> +		   lkb->lkb_ownpid,
> +		   (unsigned long long)xid,
> +		   lkb->lkb_exflags,
> +		   lkb->lkb_flags,
> +		   lkb->lkb_status,
> +		   lkb->lkb_grmode,
> +		   lkb->lkb_rqmode,
> +		   (unsigned long long)us,
> +		   r->res_nodeid,
> +		   r->res_length,
> +		   r->res_name);
>  }
>  
> -static int print_format2(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format2(struct dlm_rsb *r, struct seq_file *s)
>  {
>  	struct dlm_lkb *lkb;
> -	int rv = 0;
>  
>  	lock_rsb(r);
>  
>  	list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
> -		rv = print_format2_lock(s, lkb, r);
> -		if (rv)
> +		print_format2_lock(s, lkb, r);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
> -		rv = print_format2_lock(s, lkb, r);
> -		if (rv)
> +		print_format2_lock(s, lkb, r);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
> -		rv = print_format2_lock(s, lkb, r);
> -		if (rv)
> +		print_format2_lock(s, lkb, r);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>   out:
>  	unlock_rsb(r);
> -	return rv;
>  }
>  
> -static int print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
> +static void print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
>  			      int rsb_lookup)
>  {
>  	u64 xid = 0;
> -	int rv;
>  
>  	if (lkb->lkb_flags & DLM_IFL_USER) {
>  		if (lkb->lkb_ua)
>  			xid = lkb->lkb_ua->xid;
>  	}
>  
> -	rv = seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
> -			lkb->lkb_id,
> -			lkb->lkb_nodeid,
> -			lkb->lkb_remid,
> -			lkb->lkb_ownpid,
> -			(unsigned long long)xid,
> -			lkb->lkb_exflags,
> -			lkb->lkb_flags,
> -			lkb->lkb_status,
> -			lkb->lkb_grmode,
> -			lkb->lkb_rqmode,
> -			lkb->lkb_last_bast.mode,
> -			rsb_lookup,
> -			lkb->lkb_wait_type,
> -			lkb->lkb_lvbseq,
> -			(unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
> -			(unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
> -	return rv;
> +	seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
> +		   lkb->lkb_id,
> +		   lkb->lkb_nodeid,
> +		   lkb->lkb_remid,
> +		   lkb->lkb_ownpid,
> +		   (unsigned long long)xid,
> +		   lkb->lkb_exflags,
> +		   lkb->lkb_flags,
> +		   lkb->lkb_status,
> +		   lkb->lkb_grmode,
> +		   lkb->lkb_rqmode,
> +		   lkb->lkb_last_bast.mode,
> +		   rsb_lookup,
> +		   lkb->lkb_wait_type,
> +		   lkb->lkb_lvbseq,
> +		   (unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
> +		   (unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
>  }
>  
> -static int print_format3(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format3(struct dlm_rsb *r, struct seq_file *s)
>  {
>  	struct dlm_lkb *lkb;
>  	int i, lvblen = r->res_ls->ls_lvblen;
>  	int print_name = 1;
> -	int rv;
>  
>  	lock_rsb(r);
>  
> -	rv = seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
> -			r,
> -			r->res_nodeid,
> -			r->res_first_lkid,
> -			r->res_flags,
> -			!list_empty(&r->res_root_list),
> -			!list_empty(&r->res_recover_list),
> -			r->res_recover_locks_count,
> -			r->res_length);
> -	if (rv)
> +	seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
> +		   r,
> +		   r->res_nodeid,
> +		   r->res_first_lkid,
> +		   r->res_flags,
> +		   !list_empty(&r->res_root_list),
> +		   !list_empty(&r->res_recover_list),
> +		   r->res_recover_locks_count,
> +		   r->res_length);
> +	if (seq_is_full(s))
>  		goto out;
>  
>  	for (i = 0; i < r->res_length; i++) {
> @@ -300,8 +287,8 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
>  		else
>  			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
>  	}
> -	rv = seq_puts(s, "\n");
> -	if (rv)
> +	seq_puts(s, "\n");
> +	if (seq_is_full(s))
>  		goto out;
>  
>  	if (!r->res_lvbptr)
> @@ -311,58 +298,55 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
>  
>  	for (i = 0; i < lvblen; i++)
>  		seq_printf(s, " %02x", (unsigned char)r->res_lvbptr[i]);
> -	rv = seq_puts(s, "\n");
> -	if (rv)
> +	seq_puts(s, "\n");
> +	if (seq_is_full(s))
>  		goto out;
>  
>   do_locks:
>  	list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
> -		rv = print_format3_lock(s, lkb, 0);
> -		if (rv)
> +		print_format3_lock(s, lkb, 0);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
> -		rv = print_format3_lock(s, lkb, 0);
> -		if (rv)
> +		print_format3_lock(s, lkb, 0);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
> -		rv = print_format3_lock(s, lkb, 0);
> -		if (rv)
> +		print_format3_lock(s, lkb, 0);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_lookup, lkb_rsb_lookup) {
> -		rv = print_format3_lock(s, lkb, 1);
> -		if (rv)
> +		print_format3_lock(s, lkb, 1);
> +		if (seq_is_full(s))
>  			goto out;
>  	}
>   out:
>  	unlock_rsb(r);
> -	return rv;
>  }
>  
> -static int print_format4(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format4(struct dlm_rsb *r, struct seq_file *s)
>  {
>  	int our_nodeid = dlm_our_nodeid();
>  	int print_name = 1;
> -	int i, rv;
> +	int i;
>  
>  	lock_rsb(r);
>  
> -	rv = seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
> -			r,
> -			r->res_nodeid,
> -			r->res_master_nodeid,
> -			r->res_dir_nodeid,
> -			our_nodeid,
> -			r->res_toss_time,
> -			r->res_flags,
> -			r->res_length);
> -	if (rv)
> -		goto out;
> +	seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
> +		   r,
> +		   r->res_nodeid,
> +		   r->res_master_nodeid,
> +		   r->res_dir_nodeid,
> +		   our_nodeid,
> +		   r->res_toss_time,
> +		   r->res_flags,
> +		   r->res_length);
>  
>  	for (i = 0; i < r->res_length; i++) {
>  		if (!isascii(r->res_name[i]) || !isprint(r->res_name[i]))
> @@ -377,10 +361,9 @@ static int print_format4(struct dlm_rsb *r, struct seq_file *s)
>  		else
>  			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
>  	}
> -	rv = seq_puts(s, "\n");
> - out:
> +	seq_puts(s, "\n");
> +
>  	unlock_rsb(r);
> -	return rv;
>  }
>  
>  struct rsbtbl_iter {
> @@ -390,11 +373,12 @@ struct rsbtbl_iter {
>  	int header;
>  };
>  
> -/* seq_printf returns -1 if the buffer is full, and 0 otherwise.
> -   If the buffer is full, seq_printf can be called again, but it
> -   does nothing and just returns -1.  So, the these printing routines
> -   periodically check the return value to avoid wasting too much time
> -   trying to print to a full buffer. */
> +/*
> + * If the buffer is full, seq_printf can be called again, but it
> + * does nothing.  So, the these printing routines periodically check
> + * seq_is_full to avoid wasting too much time trying to print to
> + * a full buffer.
> + */
>  
>  static int table_seq_show(struct seq_file *seq, void *iter_ptr)
>  {
> @@ -403,7 +387,7 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
>  
>  	switch (ri->format) {
>  	case 1:
> -		rv = print_format1(ri->rsb, seq);
> +		print_format1(ri->rsb, seq);
>  		break;
>  	case 2:
>  		if (ri->header) {
> @@ -412,21 +396,21 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
>  					"r_nodeid r_len r_name\n");
>  			ri->header = 0;
>  		}
> -		rv = print_format2(ri->rsb, seq);
> +		print_format2(ri->rsb, seq);
>  		break;
>  	case 3:
>  		if (ri->header) {
>  			seq_printf(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
>  			ri->header = 0;
>  		}
> -		rv = print_format3(ri->rsb, seq);
> +		print_format3(ri->rsb, seq);
>  		break;
>  	case 4:
>  		if (ri->header) {
>  			seq_printf(seq, "version 4 rsb 2\n");
>  			ri->header = 0;
>  		}
> -		rv = print_format4(ri->rsb, seq);
> +		print_format4(ri->rsb, seq);
>  		break;
>  	}
>  
> -- 
> 1.8.1.2.459.gbcd45b4.dirty
> 

[Cluster-devel] [PATCH 3/7] dlm: Use seq_is_full - remove seq_printf returns

[Steven Rostedt]

Note, I've started with Joe's patches and I'm massaging them for
something I can work with.

On Tue, 30 Sep 2014 12:34:35 +0200
Petr Mladek <pmladek@suse.cz> wrote:


> > -		rv = seq_printf(s, "\"\nInvalid master %d\n",
> > -				res->res_nodeid);
> > -	if (rv)
> > +		seq_printf(s, "\"\nInvalid master %d\n", res->res_nodeid);
> > +	if (seq_is_full(s))
> >  		goto out;
> 
> I would check for seq_overflow()
> 
> Etc. There are needed many more changes if we agree on introducing
> seq_is_full() and seq_overflow().

As I'm looking at this code, I'm thinking that we never
really care about seq_is_full(). We only really care if
seq_overflowed(), in which the contents will be discarded.

Rational? Because if we break when seq_is_full(), my new logic wont
throw away the result. If we break out of the function when it's full
and not when it has overflowed, then we may never print out the rest of
the content, as the seq_file code will still use a full buffer that
hasn't overflowed.

I'm thinking of switching everything to use seq_has_overflowed() and
try again.

Thoughts?

-- Steve

[Cluster-devel] [PATCH 3/7] dlm: Use seq_is_full - remove seq_printf returns

[Joe Perches]

On Mon, 2014-10-27 at 16:17 -0400, Steven Rostedt wrote:
> Note, I've started with Joe's patches and I'm massaging them for
> something I can work with.
[]
> I'm thinking of switching everything to use seq_has_overflowed() and
> try again.

Fine by me.

[Cluster-devel] [PATCH 3/7] dlm: Use seq_is_full - remove seq_printf returns

[Petr Mladek]

On Mon 2014-10-27 16:17:24, Steven Rostedt wrote:
> Note, I've started with Joe's patches and I'm massaging them for
> something I can work with.
> 
> On Tue, 30 Sep 2014 12:34:35 +0200
> Petr Mladek <pmladek@suse.cz> wrote:
> 
> 
> > > -		rv = seq_printf(s, "\"\nInvalid master %d\n",
> > > -				res->res_nodeid);
> > > -	if (rv)
> > > +		seq_printf(s, "\"\nInvalid master %d\n", res->res_nodeid);
> > > +	if (seq_is_full(s))
> > >  		goto out;
> > 
> > I would check for seq_overflow()
> > 
> > Etc. There are needed many more changes if we agree on introducing
> > seq_is_full() and seq_overflow().
> 
> As I'm looking at this code, I'm thinking that we never
> really care about seq_is_full(). We only really care if
> seq_overflowed(), in which the contents will be discarded.
> 
> Rational? Because if we break when seq_is_full(), my new logic wont
> throw away the result. If we break out of the function when it's full
> and not when it has overflowed, then we may never print out the rest of
> the content, as the seq_file code will still use a full buffer that
> hasn't overflowed.
> 
> I'm thinking of switching everything to use seq_has_overflowed() and
> try again.
> 
> Thoughts?

Sounds good to me.

Best Regards,
Petr

[Cluster-devel] [PATCH 4/7] dlm: Use seq_puts, remove unnecessary trailing spaces

[Joe Perches]

Convert the seq_printf output with constant strings to seq_puts.
Remove unnecessary trailing spaces from seq_(printf/puts) output.

Signed-off-by: Joe Perches <joe@perches.com>
---
 fs/dlm/debug_fs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index 27c8335..c69a766 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -279,7 +279,7 @@ static void print_format3(struct dlm_rsb *r, struct seq_file *s)
 			print_name = 0;
 	}
 
-	seq_printf(s, "%s", print_name ? "str " : "hex");
+	seq_puts(s, print_name ? "str " : "hex");
 
 	for (i = 0; i < r->res_length; i++) {
 		if (print_name)
@@ -353,7 +353,7 @@ static void print_format4(struct dlm_rsb *r, struct seq_file *s)
 			print_name = 0;
 	}
 
-	seq_printf(s, "%s", print_name ? "str " : "hex");
+	seq_puts(s, print_name ? "str " : "hex");
 
 	for (i = 0; i < r->res_length; i++) {
 		if (print_name)
@@ -391,23 +391,21 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
 		break;
 	case 2:
 		if (ri->header) {
-			seq_printf(seq, "id nodeid remid pid xid exflags "
-					"flags sts grmode rqmode time_ms "
-					"r_nodeid r_len r_name\n");
+			seq_puts(seq, "id nodeid remid pid xid exflags flags sts grmode rqmode time_ms r_nodeid r_len r_name\n");
 			ri->header = 0;
 		}
 		print_format2(ri->rsb, seq);
 		break;
 	case 3:
 		if (ri->header) {
-			seq_printf(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
+			seq_puts(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
 			ri->header = 0;
 		}
 		print_format3(ri->rsb, seq);
 		break;
 	case 4:
 		if (ri->header) {
-			seq_printf(seq, "version 4 rsb 2\n");
+			seq_puts(seq, "version 4 rsb 2\n");
 			ri->header = 0;
 		}
 		print_format4(ri->rsb, seq);
-- 
1.8.1.2.459.gbcd45b4.dirty

[Cluster-devel] [PATCH 0/7] seq_printf cleanups

[Steven Rostedt]

Joe,

If you haven't already done so, can you update checkpatch.pl to
complain if someone checks the return value of seq_printf(),
seq_puts(), or seq_putc().

It should state that those functions will soon be returning void.

Thanks!

-- Steve

[Cluster-devel] [PATCH 0/7] seq_printf cleanups

[Joe Perches]

On Tue, 2014-10-28 at 11:32 -0400, Steven Rostedt wrote:
> If you haven't already done so, can you update checkpatch.pl to
> complain if someone checks the return value of seq_printf(),
> seq_puts(), or seq_putc().

I'm not sure that matters much as a rule because I
hope soon the compiler will bleat when that happens.

There are several more too:

	seq_vprintf
	seq_escape
	seq_write
	seq_bitmap
	seq_cpumask/seq_nodemask (and _list variants),
	seq_put_decimal_xxx