2009-09-07 08:38:56

by Miklos Szeredi

[permalink] [raw]
Subject: [patch 1/2] vfs: seq_file: add helpers for data filling

From: Miklos Szeredi <[email protected]>

Add two helpers that allow access to the seq_file's own buffer, but
hides the internal details of seq_files.

This allows easier implementation of special purpose filling
functions. It also cleans up some existing functions which duplicated
the seq_file logic.

Signed-off-by: Miklos Szeredi <[email protected]>
---
fs/seq_file.c | 112 ++++++++++++++++++++++++++++++++---------------
include/linux/seq_file.h | 3 +
2 files changed, 80 insertions(+), 35 deletions(-)

Index: linux-2.6/fs/seq_file.c
===================================================================
--- linux-2.6.orig/fs/seq_file.c 2009-09-03 19:40:10.000000000 +0200
+++ linux-2.6/fs/seq_file.c 2009-09-04 14:58:04.000000000 +0200
@@ -419,6 +419,44 @@ char *mangle_path(char *s, char *p, char
EXPORT_SYMBOL(mangle_path);

/**
+ * seq_get_buf - get buffer to write arbitrary data to
+ * @m: the seq_file handle
+ * @bufp: the beginning of the buffer is stored here
+ *
+ * Return the number of bytes available in the buffer, or zero if
+ * there's no space.
+ */
+size_t seq_get_buf(struct seq_file *m, char **bufp)
+{
+ BUG_ON(m->count > m->size);
+ if (m->count < m->size)
+ *bufp = m->buf + m->count;
+ else
+ *bufp = NULL;
+
+ return m->size - m->count;
+}
+
+/**
+ * seq_commit - commit data to the buffer
+ * @m: the seq_file handle
+ * @num: the number of bytes to commit
+ *
+ * Commit @num bytes of data written to a buffer previously acquired
+ * by seq_buf_get. To signal an error condition, or that the data
+ * didn't fit in the available space, pass a negative @num value.
+ */
+void seq_commit(struct seq_file *m, int num)
+{
+ if (num < 0) {
+ m->count = m->size;
+ } else {
+ BUG_ON(m->count + num > m->size);
+ m->count += num;
+ }
+}
+
+/**
* seq_path - seq_file interface to print a pathname
* @m: the seq_file handle
* @path: the struct path to print
@@ -429,20 +467,21 @@ EXPORT_SYMBOL(mangle_path);
*/
int seq_path(struct seq_file *m, struct path *path, char *esc)
{
- if (m->count < m->size) {
- char *s = m->buf + m->count;
- char *p = d_path(path, s, m->size - m->count);
+ char *buf;
+ size_t size = seq_get_buf(m, &buf);
+ int res = -1;
+
+ if (size) {
+ char *p = d_path(path, buf, size);
if (!IS_ERR(p)) {
- s = mangle_path(s, p, esc);
- if (s) {
- p = m->buf + m->count;
- m->count = s - m->buf;
- return s - p;
- }
+ char *end = mangle_path(buf, p, esc);
+ if (end)
+ res = end - buf;
}
}
- m->count = m->size;
- return -1;
+ seq_commit(m, res);
+
+ return res;
}
EXPORT_SYMBOL(seq_path);

@@ -454,26 +493,28 @@ EXPORT_SYMBOL(seq_path);
int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
char *esc)
{
- int err = -ENAMETOOLONG;
- if (m->count < m->size) {
- char *s = m->buf + m->count;
+ char *buf;
+ size_t size = seq_get_buf(m, &buf);
+ int res = -ENAMETOOLONG;
+
+ if (size) {
char *p;

spin_lock(&dcache_lock);
- p = __d_path(path, root, s, m->size - m->count);
+ p = __d_path(path, root, buf, size);
spin_unlock(&dcache_lock);
- err = PTR_ERR(p);
+ res = PTR_ERR(p);
if (!IS_ERR(p)) {
- s = mangle_path(s, p, esc);
- if (s) {
- p = m->buf + m->count;
- m->count = s - m->buf;
- return 0;
- }
+ char *end = mangle_path(buf, p, esc);
+ if (end)
+ res = end - buf;
+ else
+ res = -ENAMETOOLONG;
}
}
- m->count = m->size;
- return err;
+ seq_commit(m, res);
+
+ return res < 0 ? res : 0;
}

/*
@@ -481,20 +522,21 @@ int seq_path_root(struct seq_file *m, st
*/
int seq_dentry(struct seq_file *m, struct dentry *dentry, char *esc)
{
- if (m->count < m->size) {
- char *s = m->buf + m->count;
- char *p = dentry_path(dentry, s, m->size - m->count);
+ char *buf;
+ size_t size = seq_get_buf(m, &buf);
+ int res = -1;
+
+ if (size) {
+ char *p = dentry_path(dentry, buf, size);
if (!IS_ERR(p)) {
- s = mangle_path(s, p, esc);
- if (s) {
- p = m->buf + m->count;
- m->count = s - m->buf;
- return s - p;
- }
+ char *end = mangle_path(buf, p, esc);
+ if (end)
+ res = end - buf;
}
}
- m->count = m->size;
- return -1;
+ seq_commit(m, res);
+
+ return res;
}

int seq_bitmap(struct seq_file *m, const unsigned long *bits,
Index: linux-2.6/include/linux/seq_file.h
===================================================================
--- linux-2.6.orig/include/linux/seq_file.h 2009-09-04 13:17:03.000000000 +0200
+++ linux-2.6/include/linux/seq_file.h 2009-09-04 14:26:49.000000000 +0200
@@ -48,6 +48,9 @@ int seq_write(struct seq_file *seq, cons
int seq_printf(struct seq_file *, const char *, ...)
__attribute__ ((format (printf,2,3)));

+size_t seq_get_buf(struct seq_file *m, char **bufp);
+void seq_commit(struct seq_file *m, int num);
+
int seq_path(struct seq_file *, struct path *, char *);
int seq_dentry(struct seq_file *, struct dentry *, char *);
int seq_path_root(struct seq_file *m, struct path *path, struct path *root,


2009-09-07 08:41:54

by Miklos Szeredi

[permalink] [raw]
Subject: [patch 2/2] vfs: revert /proc/mounts to old behavior for unreachable mountpoints

From: Miklos Szeredi <[email protected]>

"vfs: fix d_path() for unreachable paths" prefixes unreachable paths
with "(unreachable)" in the result of getcwd(2), /proc/*/mounts,
/proc/*/cwd, /proc/*/fd/*, etc...

Hugh Dickins reported that an old version of gnome-vfs-daemon crashes
because it finds an entry in /proc/mounts where the mountpoint is
unreachable.

This patch reverts /proc/mounts to the old behavior (or rather a less
crazy version of the old behavior).

Reported-by: Hugh Dickins <[email protected]>
Reported-by: Valdis Kletnieks <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
---
fs/namespace.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 56 insertions(+), 1 deletion(-)

Index: linux-2.6/fs/namespace.c
===================================================================
--- linux-2.6.orig/fs/namespace.c 2009-09-04 15:25:36.000000000 +0200
+++ linux-2.6/fs/namespace.c 2009-09-04 15:40:25.000000000 +0200
@@ -789,6 +789,61 @@ static void show_type(struct seq_file *m
}
}

+/*
+ * Same as d_path() except it doesn't stick "(unreachable)" in front
+ * of unreachable paths.
+ */
+static char *d_path_compat(struct path *path, char *buf, int buflen)
+{
+ char *res;
+ struct path root;
+ struct path tmp;
+
+ read_lock(&current->fs->lock);
+ root = current->fs->root;
+ path_get(&root);
+ read_unlock(&current->fs->lock);
+ spin_lock(&dcache_lock);
+ tmp = root;
+ res = __d_path(path, &tmp, buf, buflen);
+ if (!IS_ERR(res) &&
+ (tmp.mnt != root.mnt || tmp.dentry != root.dentry)) {
+ /*
+ * Unreachable path found, redo with the global root
+ * so we get a normal looking path.
+ */
+ res = __d_path(path, &tmp, buf, buflen);
+ }
+ spin_unlock(&dcache_lock);
+ path_put(&root);
+
+ return res;
+}
+
+/*
+ * Some old programs break if /proc/mounts contains a mountpoint
+ * beginning with "(unreachable)". Revert this back to the old way of
+ * displaying the path from the global root instead.
+ */
+static int show_path_old(struct seq_file *m, struct path *path, char *esc)
+{
+ char *buf;
+ size_t size = seq_get_buf(m, &buf);
+ int res = -1;
+
+ if (size) {
+ char *p = d_path_compat(path, buf, size);
+ if (!IS_ERR(p)) {
+ char *end = mangle_path(buf, p, esc);
+ if (end)
+ res = end - buf;
+ }
+ }
+ seq_commit(m, res);
+
+ return res;
+}
+
static int show_vfsmnt(struct seq_file *m, void *v)
{
struct vfsmount *mnt = list_entry(v, struct vfsmount, mnt_list);
@@ -797,7 +852,7 @@ static int show_vfsmnt(struct seq_file *

mangle(m, mnt->mnt_devname ? mnt->mnt_devname : "none");
seq_putc(m, ' ');
- seq_path(m, &mnt_path, " \t\n\\");
+ show_path_old(m, &mnt_path, " \t\n\\");
seq_putc(m, ' ');
show_type(m, mnt->mnt_sb);
seq_puts(m, __mnt_is_readonly(mnt) ? " ro" : " rw");

2009-09-09 21:15:41

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/2] vfs: seq_file: add helpers for data filling

On Mon, 07 Sep 2009 10:38:24 +0200
Miklos Szeredi <[email protected]> wrote:

> Add two helpers that allow access to the seq_file's own buffer, but
> hides the internal details of seq_files.
>
> This allows easier implementation of special purpose filling
> functions. It also cleans up some existing functions which duplicated
> the seq_file logic.

This patch conflicts with
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch,
below.

I reworked your patch so that it removes the effects of
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch
and so that your patch is staged after
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch.

Your patch appears to fix the same thing as
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch,
so I _could_ have just dropped
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch.
However it's unclear to me at thsi stage that your patch will be
merged.


I have a mountain of VFS patches piled up here:

vfs-fix-vfs_rename_dir-for-fs_rename_does_d_move-filesystems.patch
raw-fix-rawctl-compat-ioctls-breakage-on-amd64-and-itanic.patch
vfs-improve-comment-describing-fget_light.patch
libfs-make-simple_read_from_buffer-conventional.patch
fs-inodec-add-dev-id-and-inode-number-for-debugging-in-init_special_inode.patch
vfs-split-generic_forget_inode-so-that-hugetlbfs-does-not-have-to-copy-it.patch
fs-fix-overflow-in-sys_mount-for-in-kernel-calls.patch
vfs-optimization-for-touch_atime.patch
vfs-optimize-touch_time-too.patch
vfs-optimize-touch_time-too-fix.patch
ecryptfs-another-lockdep-issue.patch
vfs-explicitly-cast-s_maxbytes-in-fiemap_check_ranges.patch
vfs-change-sb-s_maxbytes-to-a-loff_t.patch
vfs-remove-redundant-position-check-in-do_sendfile.patch
#vfs-fix-d_path-for-unreachable-paths.patch: [email protected] issues
fs-remove-unneeded-dcache_unhashed-tricks.patch
fs-improve-remountro-vs-buffercache-coherency.patch
fs-improve-remountro-vs-buffercache-coherency-fix.patch
#
#
#fs-new-truncate-helpers.patch and friends: TBU?
fs-new-truncate-helpers.patch
fs-use-new-truncate-helpers.patch
fs-introduce-new-truncate-sequence.patch
fs-convert-simple-fs-to-new-truncate.patch
tmpfs-convert-to-use-the-new-truncate-convention.patch
#ext2-convert-to-use-the-new-truncate-convention.patch: Jan wanted update. Nick agreed
ext2-convert-to-use-the-new-truncate-convention.patch
ext2-convert-to-use-the-new-truncate-convention-fix.patch
fat-convert-to-use-the-new-truncate-convention.patch
btrfs-convert-to-use-the-new-truncate-convention.patch
jfs-convert-to-use-the-new-truncate-convention.patch
udf-convert-to-use-the-new-truncate-convention.patch
minix-convert-to-use-the-new-truncate-convention.patch
#
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch
vfs-seq_file-add-helpers-for-data-filling.patch
vfs-revert-proc-mounts-to-old-behavior-for-unreachable-mountpoints.patch


We're going to have to work out what to do if viro remains disappeared. I
guess I'll just send them all out to you guys to review during the merge
window and we'll go through them.





From: Tetsuo Handa <[email protected]>

seq_path_root() is returning a return value of successful __d_path()
instead of returning a negative value when mangle_path() failed.

This is not a bug so far because nobody is using return value of
seq_path_root().

Signed-off-by: Tetsuo Handa <[email protected]>
Cc: Al Viro <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

fs/seq_file.c | 1 +
1 file changed, 1 insertion(+)

diff -puN fs/seq_file.c~seq_file-return-a-negative-error-code-when-seq_path_root-fails fs/seq_file.c
--- a/fs/seq_file.c~seq_file-return-a-negative-error-code-when-seq_path_root-fails
+++ a/fs/seq_file.c
@@ -470,6 +470,7 @@ int seq_path_root(struct seq_file *m, st
m->count = s - m->buf;
return 0;
}
+ err = -ENAMETOOLONG;
}
}
m->count = m->size;
_