Re: [patch 2/2] Sort module list by pointer address to get coherent sleepable seq_file iterators

[Fengguang Wu <wfg@mail.ustc.edu.cn>]

On Sun, Aug 12, 2007 at 11:08:46AM -0400, Mathieu Desnoyers wrote:
>  
>  static void *m_next(struct seq_file *m, void *p, loff_t *pos)
>  {
> -	return seq_list_next(p, &modules, pos);
> +	return seq_sorted_list_next(p, &modules, &m->private);
>  }

In theory it is not safe to use something other than the passed in
*pos as an position indicator. Because seq_file do not always call
->next() to advance to the next item. Look at seq_file.c, it sometimes
increase the pos/index directly! Which also prevents pos to skip
forward, which is preferred in your case.

The attached patch tries to fix it.

The seq_file.c is so twisted!

Fengguang
===

seqfile: remove seq_file's assumption about iterators

The seq_file implementation has some hardcoded index++/pos++ lines,
which assumes iterators to be *continuous* integers.

This patch replaces the index++ lines with calls to m->next(), so that
seq_file users can freely use discrete forms of iterators, such as
ascending addresses.

Cc: Randy Dunlap <rddunlap@osdl.org>
Cc: Martin Bligh <mbligh@google.com>
Signed-off-by: Wu Fengguang <wfg@mail.ustc.edu.cn>

---
 fs/seq_file.c |   22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

--- linux-2.6.22-git15.orig/fs/seq_file.c
+++ linux-2.6.22-git15/fs/seq_file.c
@@ -13,6 +13,8 @@
 #include <asm/uaccess.h>
 #include <asm/page.h>
 
+#define SEQFILE_SHOW_NEXT	LONG_MAX
+
 /**
  *	seq_open -	initialize sequential file
  *	@file: file we initialize
@@ -93,6 +95,7 @@ ssize_t seq_read(struct file *file, char
 	/* if not empty - flush it first */
 	if (m->count) {
 		n = min(m->count, size);
+		BUG_ON(m->from == SEQFILE_SHOW_NEXT);
 		err = copy_to_user(buf, m->buf + m->from, n);
 		if (err)
 			goto Efault;
@@ -102,7 +105,7 @@ ssize_t seq_read(struct file *file, char
 		buf += n;
 		copied += n;
 		if (!m->count)
-			m->index++;
+			m->from = SEQFILE_SHOW_NEXT;
 		if (!size)
 			goto Done;
 	}
@@ -113,9 +116,11 @@ ssize_t seq_read(struct file *file, char
 		err = PTR_ERR(p);
 		if (!p || IS_ERR(p))
 			break;
-		err = m->op->show(m, p);
-		if (err)
-			break;
+		if (m->from != SEQFILE_SHOW_NEXT) {
+			err = m->op->show(m, p);
+			if (err)
+				break;
+		}
 		if (m->count < m->size)
 			goto Fill;
 		m->op->stop(m, p);
@@ -156,7 +161,7 @@ Fill:
 	if (m->count)
 		m->from = n;
 	else
-		pos++;
+		m->from = SEQFILE_SHOW_NEXT;
 	m->index = pos;
 Done:
 	if (!copied)
@@ -211,12 +216,9 @@ static int traverse(struct seq_file *m, 
 		}
 		pos += m->count;
 		m->count = 0;
-		if (pos == offset) {
-			index++;
-			m->index = index;
-			break;
-		}
 		p = m->op->next(m, p, &index);
+		if (pos == offset)
+			break;
 	}
 	m->op->stop(m, p);
 	return error;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/