[Devel] [PATCH rh7 2/2] Revert "ms/fs/seq_file.c: simplify seq_file iteration code and interface"

Konstantin Khorenko khorenko at virtuozzo.com
Wed Apr 22 21:06:04 MSK 2020


This reverts commit ef05e968fcbad6ba646e309242ff3eec71f39888.

RedHat has fixed the issue in another way,
so no need in backporting mainstream patches, revert them.

https://jira.sw.ru/browse/PSBM-99399

Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 Documentation/filesystems/seq_file.txt | 63 ++++++++++++----------------------
 fs/seq_file.c                          | 54 +++++++++++++++++------------
 2 files changed, 54 insertions(+), 63 deletions(-)

diff --git a/Documentation/filesystems/seq_file.txt b/Documentation/filesystems/seq_file.txt
index a0e6bb263ec73..a285660a7bdfa 100644
--- a/Documentation/filesystems/seq_file.txt
+++ b/Documentation/filesystems/seq_file.txt
@@ -57,39 +57,23 @@ http://lwn.net/Articles/22359/).
 
 The iterator interface
 
-Modules implementing a virtual file with seq_file must implement an
-iterator object that allows stepping through the data of interest
-during a "session" (roughly one read() system call).  If the iterator
-is able to move to a specific position - like the file they implement,
-though with freedom to map the position number to a sequence location
-in whatever way is convenient - the iterator need only exist
-transiently during a session.  If the iterator cannot easily find a
-numerical position but works well with a first/next interface, the
-iterator can be stored in the private data area and continue from one
-session to the next.
-
-A seq_file implementation that is formatting firewall rules from a
-table, for example, could provide a simple iterator that interprets
-position N as the Nth rule in the chain.  A seq_file implementation
-that presents the content of a, potentially volatile, linked list
-might record a pointer into that list, providing that can be done
-without risk of the current location being removed.
-
-Positioning can thus be done in whatever way makes the most sense for
-the generator of the data, which need not be aware of how a position
-translates to an offset in the virtual file. The one obvious exception
-is that a position of zero should indicate the beginning of the file.
+Modules implementing a virtual file with seq_file must implement a simple
+iterator object that allows stepping through the data of interest.
+Iterators must be able to move to a specific position - like the file they
+implement - but the interpretation of that position is up to the iterator
+itself. A seq_file implementation that is formatting firewall rules, for
+example, could interpret position N as the Nth rule in the chain.
+Positioning can thus be done in whatever way makes the most sense for the
+generator of the data, which need not be aware of how a position translates
+to an offset in the virtual file. The one obvious exception is that a
+position of zero should indicate the beginning of the file.
 
 The /proc/sequence iterator just uses the count of the next number it
 will output as its position.
 
-Four functions must be implemented to make the iterator work. The
-first, called start(), starts a session and takes a position as an
-argument, returning an iterator which will start reading at that
-position.  The pos passed to start() will always be either zero, or
-the most recent pos used in the previous session.
-
-For our simple sequence example,
+Four functions must be implemented to make the iterator work. The first,
+called start() takes a position as an argument and returns an iterator
+which will start reading at that position. For our simple sequence example,
 the start() function looks like:
 
 	static void *ct_seq_start(struct seq_file *s, loff_t *pos)
@@ -108,12 +92,11 @@ implementations; in most cases the start() function should check for a
 "past end of file" condition and return NULL if need be.
 
 For more complicated applications, the private field of the seq_file
-structure can be used to hold state from session to session.  There is
-also a special value which can be returned by the start() function
-called SEQ_START_TOKEN; it can be used if you wish to instruct your
-show() function (described below) to print a header at the top of the
-output. SEQ_START_TOKEN should only be used if the offset is zero,
-however.
+structure can be used. There is also a special value which can be returned
+by the start() function called SEQ_START_TOKEN; it can be used if you wish
+to instruct your show() function (described below) to print a header at the
+top of the output. SEQ_START_TOKEN should only be used if the offset is
+zero, however.
 
 The next function to implement is called, amazingly, next(); its job is to
 move the iterator forward to the next position in the sequence.  The
@@ -129,13 +112,9 @@ next() function returns a new iterator, or NULL if the sequence is
 	        return spos;
 	}
 
-The stop() function closes a session; its job, of course, is to clean
-up. If dynamic memory is allocated for the iterator, stop() is the
-place to free it; if a lock was taken by start(), stop() must release
-that lock.  The value that *pos was set to by the last next() call
-before stop() is remembered, and used for the first start() call of
-the next session unless lseek() has been called on the file; in that
-case next start() will be asked to start at position zero.
+The stop() function is called when iteration is complete; its job, of
+course, is to clean up. If dynamic memory is allocated for the iterator,
+stop() is the place to free it.
 
 	static void ct_seq_stop(struct seq_file *s, void *v)
 	{
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 61fa5f8e8a58b..98b234fd70cc6 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -95,22 +95,23 @@ EXPORT_SYMBOL(seq_open);
 
 static int traverse(struct seq_file *m, loff_t offset)
 {
-	loff_t pos = 0;
+	loff_t pos = 0, index;
 	int error = 0;
 	void *p;
 
 	m->version = 0;
-	m->index = 0;
+	index = 0;
 	m->count = m->from = 0;
-	if (!offset)
+	if (!offset) {
+		m->index = index;
 		return 0;
-
+	}
 	if (!m->buf) {
 		m->buf = seq_buf_alloc(m->size = PAGE_SIZE);
 		if (!m->buf)
 			return -ENOMEM;
 	}
-	p = m->op->start(m, &m->index);
+	p = m->op->start(m, &index);
 	while (p) {
 		error = PTR_ERR(p);
 		if (IS_ERR(p))
@@ -127,15 +128,20 @@ static int traverse(struct seq_file *m, loff_t offset)
 		if (pos + m->count > offset) {
 			m->from = offset - pos;
 			m->count -= m->from;
+			m->index = index;
 			break;
 		}
 		pos += m->count;
 		m->count = 0;
-		p = m->op->next(m, p, &m->index);
-		if (pos == offset)
+		if (pos == offset) {
+			index++;
+			m->index = index;
 			break;
+		}
+		p = m->op->next(m, p, &index);
 	}
 	m->op->stop(m, p);
+	m->index = index;
 	return error;
 
 Eoverflow:
@@ -159,6 +165,7 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
 {
 	struct seq_file *m = file->private_data;
 	size_t copied = 0;
+	loff_t pos;
 	size_t n;
 	void *p;
 	int err = 0;
@@ -218,12 +225,16 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
 		size -= n;
 		buf += n;
 		copied += n;
+		if (!m->count) {
+			m->from = 0;
+			m->index++;
+		}
 		if (!size)
 			goto Done;
 	}
 	/* we need at least one record in buffer */
-	m->from = 0;
-	p = m->op->start(m, &m->index);
+	pos = m->index;
+	p = m->op->start(m, &pos);
 	while (1) {
 		err = PTR_ERR(p);
 		if (!p || IS_ERR(p))
@@ -234,7 +245,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
 		if (unlikely(err))
 			m->count = 0;
 		if (unlikely(!m->count)) {
-			p = m->op->next(m, p, &m->index);
+			p = m->op->next(m, p, &pos);
+			m->index = pos;
 			continue;
 		}
 		if (m->count < m->size)
@@ -246,33 +258,29 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
 		if (!m->buf)
 			goto Enomem;
 		m->version = 0;
-		p = m->op->start(m, &m->index);
+		pos = m->index;
+		p = m->op->start(m, &pos);
 	}
 	m->op->stop(m, p);
 	m->count = 0;
 	goto Done;
 Fill:
 	/* they want more? let's try to get some more */
-	while (1) {
+	while (m->count < size) {
 		size_t offs = m->count;
-		loff_t pos = m->index;
-
-		p = m->op->next(m, p, &m->index);
-		if (pos == m->index)
-			/* Buggy ->next function */
-			m->index++;
+		loff_t next = pos;
+		p = m->op->next(m, p, &next);
 		if (!p || IS_ERR(p)) {
 			err = PTR_ERR(p);
 			break;
 		}
-		if (m->count >= size)
-			break;
 		err = m->op->show(m, p);
 		if (seq_has_overflowed(m) || err) {
 			m->count = offs;
 			if (likely(err <= 0))
 				break;
 		}
+		pos = next;
 	}
 	m->op->stop(m, p);
 	n = min(m->count, size);
@@ -281,7 +289,11 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
 		goto Efault;
 	copied += n;
 	m->count -= n;
-	m->from = n;
+	if (m->count)
+		m->from = n;
+	else
+		pos++;
+	m->index = pos;
 Done:
 	if (!copied)
 		copied = err;
-- 
2.15.1



More information about the Devel mailing list