2009-03-19 05:13:28

by J. R. Okajima

[permalink] [raw]
Subject: [RFC Aufs2 #3 0/2] 'debug' module parm and /sys/fs/aufs entries

Some entires under /sys/fs/aufs still print an abosule path. While sysfs
(and procfs?) limits the length of an entry upto PAGE_SIZE, the abs path
can be any length. I don't think it is important to support longer path
than PAGE_SIZE, so aufs simply returns -EFBIG (regardless this patch).

Following the comments from Greg KH, split /sys/fs/aufs/si_<id>/xino
into some files in order to keep "one value per file".

J. R. Okajima (2):
replace /sys/fs/aufs/debug by /sys/module/aufs/parmaters/debug
split 'xino' entry under sysfs

Documentation/ABI/testing/sysfs-aufs | 62 ++++++++
Documentation/filesystems/aufs/aufs.5 | 8 +
fs/aufs/branch.h | 6 +
fs/aufs/debug.c | 4 +-
fs/aufs/debug.h | 12 +-
fs/aufs/sysaufs.c | 6 +-
fs/aufs/sysaufs.h | 13 +-
fs/aufs/sysfs.c | 260 ++++++++++++++++-----------------
8 files changed, 224 insertions(+), 147 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-aufs


2009-03-19 05:13:12

by J. R. Okajima

[permalink] [raw]
Subject: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs

Follow the comments from Greg KH on LKML.
- make all entries "one value per file".
- split 'xino' into 'xi_path', 'xib' and 'xi0'...'xiN'.
- new members in struct au_xino_file for xi[0-9]* entry.
- remove sysaufs_si_attr_xino.
- sysaufs_si_xino() shows only a size info of one xino file.
- new variables sysaufs_si_attr_xi_path and sysaufs_si_attr_xib.
- new functions sysaufs_si_xi_path() and sysaufs_si_xib().
- rename sysaufs_sbi_xi() to sysaufs_xi_attr().
- sysaufs_si_show() supports xiN.
- sysaufs_br_init() initializes br->br_xino.xi_attr too.
- sysaufs_brs_del() and sysaufs_brs_add() always handle
br->br_xino.xi_attr.
- new static function sysaufs_brs_do_add() to generate brN and xiN
names.

Signed-off-by: J. R. Okajima <[email protected]>
---
Documentation/ABI/testing/sysfs-aufs | 62 +++++++++++
fs/aufs/branch.h | 6 +
fs/aufs/sysaufs.c | 6 +-
fs/aufs/sysaufs.h | 11 ++-
fs/aufs/sysfs.c | 195 +++++++++++++++++++++++-----------
5 files changed, 212 insertions(+), 68 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-aufs

diff --git a/Documentation/ABI/testing/sysfs-aufs b/Documentation/ABI/testing/sysfs-aufs
new file mode 100644
index 0000000..1552d3e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-aufs
@@ -0,0 +1,62 @@
+What: /sys/fs/aufs/si_<id>/
+Date: March 2009
+Contact: J. R. Okajima <[email protected]>
+Description:
+ Under /sys/fs/aufs, a directory named si_<id> is created
+ per aufs mount, where <id> is a unique id generated
+ internally.
+
+What: /sys/fs/aufs/si_<id>/br0, br1 ... brN
+Date: March 2009
+Contact: J. R. Okajima <[email protected]>
+Description:
+ It shows the abolute path of a member directory (which
+ is called branch) in aufs, and its permission.
+
+What: /sys/fs/aufs/si_<id>/xi_path
+Date: March 2009
+Contact: J. R. Okajima <[email protected]>
+Description:
+ It shows the abolute path of XINO (External Inode Number
+ Bitmap, Translation Table and Generation Table) file
+ even if it is the default path.
+ When the aufs mount option 'noxino' is specified, it
+ will be empty. About XINO files, see
+ Documentation/filesystems/aufs/aufs.5 in detail.
+
+What: /sys/fs/aufs/si_<id>/xib
+Date: March 2009
+Contact: J. R. Okajima <[email protected]>
+Description:
+ It shows the consumed blocks by xib (External Inode Number
+ Bitmap), its block size and file size.
+ When the aufs mount option 'noxino' is specified, it
+ will be empty. About XINO files, see
+ Documentation/filesystems/aufs/aufs.5 in detail.
+
+What: /sys/fs/aufs/si_<id>/xino0, xino1 ... xinoN
+Date: March 2009
+Contact: J. R. Okajima <[email protected]>
+Description:
+ It shows the consumed blocks by xino (External Inode Number
+ Translation Table), its link count, block size and file
+ size.
+ When the aufs mount option 'noxino' is specified, it
+ will be empty. About XINO files, see
+ Documentation/filesystems/aufs/aufs.5 in detail.
+
+What: /sys/fs/aufs/si_<id>/xigen
+Date: March 2009
+Contact: J. R. Okajima <[email protected]>
+Description:
+ It shows the consumed blocks by xigen (External Inode
+ Generation Table), its block size and file size.
+ If CONFIG_AUFS_EXPORT is disabled, this entry will not
+ be created.
+ When the aufs mount option 'noxino' is specified, it
+ will be empty. About XINO files, see
+ Documentation/filesystems/aufs/aufs.5 in detail.
+
+# Local variables: ;
+# mode: text;
+# End: ;
diff --git a/fs/aufs/branch.h b/fs/aufs/branch.h
index 5a7fed4..838648f 100644
--- a/fs/aufs/branch.h
+++ b/fs/aufs/branch.h
@@ -31,6 +31,12 @@ struct au_xino_file {
struct mutex xi_nondir_mtx;

/* todo: make xino files an array to support huge inode number */
+
+#ifdef CONFIG_SYSFS
+ /* an entry under sysfs per mount-point */
+ char xi_name[8];
+ struct attribute xi_attr;
+#endif
};

/* members for writable branch only */
diff --git a/fs/aufs/sysaufs.c b/fs/aufs/sysaufs.c
index 47ffbfd..c6fde2c 100644
--- a/fs/aufs/sysaufs.c
+++ b/fs/aufs/sysaufs.c
@@ -25,9 +25,11 @@ struct kset *sysaufs_ket;
.show = sysaufs_si_##_name, \
}

-static struct sysaufs_si_attr sysaufs_si_attr_xino = AuSiAttr(xino);
+static struct sysaufs_si_attr sysaufs_si_attr_xi_path = AuSiAttr(xi_path),
+ sysaufs_si_attr_xib = AuSiAttr(xib);
struct attribute *sysaufs_si_attrs[] = {
- &sysaufs_si_attr_xino.attr,
+ &sysaufs_si_attr_xi_path.attr,
+ &sysaufs_si_attr_xib.attr,
NULL,
};

diff --git a/fs/aufs/sysaufs.h b/fs/aufs/sysaufs.h
index 60b18a8..71a0bba 100644
--- a/fs/aufs/sysaufs.h
+++ b/fs/aufs/sysaufs.h
@@ -48,7 +48,8 @@ struct au_branch;
/* sysfs.c */
extern struct attribute_group *sysaufs_attr_group;

-int sysaufs_si_xino(struct seq_file *seq, struct super_block *sb);
+int sysaufs_si_xi_path(struct seq_file *seq, struct super_block *sb);
+int sysaufs_si_xib(struct seq_file *seq, struct super_block *sb);
ssize_t sysaufs_si_show(struct kobject *kobj, struct attribute *attr,
char *buf);

@@ -62,7 +63,13 @@ void sysaufs_brs_del(struct super_block *sb, aufs_bindex_t bindex);
#define sysaufs_attr_group NULL

static inline
-int sysaufs_si_xino(struct seq_file *seq, struct super_block *sb)
+int sysaufs_si_xi_path(struct seq_file *seq, struct super_block *sb)
+{
+ return 0;
+}
+
+static inline
+int sysaufs_si_xib(struct seq_file *seq, struct super_block *sb)
{
return 0;
}
diff --git a/fs/aufs/sysfs.c b/fs/aufs/sysfs.c
index 2112f3b..9bcdce2 100644
--- a/fs/aufs/sysfs.c
+++ b/fs/aufs/sysfs.c
@@ -28,55 +28,65 @@ struct attribute_group *sysaufs_attr_group = &sysaufs_attr_group_body;

/* ---------------------------------------------------------------------- */

-static int sysaufs_sbi_xi(struct seq_file *seq, struct file *xf,
- struct kstat *st)
+int sysaufs_si_xi_path(struct seq_file *seq, struct super_block *sb)
+{
+ int err;
+
+ err = 0;
+ if (au_opt_test(au_mntflags(sb), XINO)) {
+ err = au_xino_path(seq, au_sbi(sb)->si_xib);
+ seq_putc(seq, '\n');
+ }
+ return err;
+}
+
+static int sysaufs_xi_attr(struct seq_file *seq, struct file *xf,
+ struct kstat *st)
{
int err;

err = vfs_getattr(xf->f_vfsmnt, xf->f_dentry, st);
- if (!err) {
- seq_printf(seq, "%llux%lu %lld",
+ if (!err)
+ seq_printf(seq, "%llux%lu %lld\n",
st->blocks, st->blksize, (long long)st->size);
- seq_putc(seq, '\n');
- } else
+ else
seq_printf(seq, "err %d\n", err);

return err;
}

-int sysaufs_si_xino(struct seq_file *seq, struct super_block *sb)
+int sysaufs_si_xib(struct seq_file *seq, struct super_block *sb)
+{
+ int err;
+ struct kstat st;
+
+ err = 0;
+ if (au_opt_test(au_mntflags(sb), XINO))
+ err = sysaufs_xi_attr(seq, au_sbi(sb)->si_xib, &st);
+ return err;
+}
+
+static int sysaufs_si_xino(struct seq_file *seq, struct super_block *sb,
+ aufs_bindex_t bindex)
{
int err;
- aufs_bindex_t bend, bindex;
struct kstat st;
- struct au_sbinfo *sbinfo;
struct file *xf;

err = 0;
- sbinfo = au_sbi(sb);
if (!au_opt_test(au_mntflags(sb), XINO))
goto out; /* success */

- xf = sbinfo->si_xib;
- err = au_xino_path(seq, xf);
- seq_putc(seq, '\n');
- if (!err)
- err = sysaufs_sbi_xi(seq, xf, &st);
-
- bend = au_sbend(sb);
- for (bindex = 0; !err && bindex <= bend; bindex++) {
- xf = au_sbr(sb, bindex)->br_xino.xi_file;
- if (!xf)
- continue;
+ AuDbg("b%d\n", bindex);

- seq_printf(seq, "%d: ", bindex);
+ xf = au_sbr(sb, bindex)->br_xino.xi_file;
+ if (xf) {
err = vfs_getattr(xf->f_vfsmnt, xf->f_dentry, &st);
- if (!err) {
- seq_printf(seq, "%ld, %llux%lu %lld",
+ if (!err)
+ seq_printf(seq, "%ld, %llux%lu %lld\n",
(long)file_count(xf), st.blocks, st.blksize,
(long long)st.size);
- seq_putc(seq, '\n');
- } else
+ else
seq_printf(seq, "err %d\n", err);
}

@@ -89,20 +99,15 @@ int sysaufs_si_xino(struct seq_file *seq, struct super_block *sb)
* sysfs handles the lifetime of the entry, and never call ->show() after it is
* unlinked.
*/
-#define SysaufsBr_PREFIX "br"
-static int sysaufs_sbi_br(struct seq_file *seq, struct super_block *sb,
- aufs_bindex_t bindex)
+static int sysaufs_si_br(struct seq_file *seq, struct super_block *sb,
+ aufs_bindex_t bindex)
{
- int err;
struct path path;
struct dentry *root;
struct au_branch *br;

- err = -ENOENT;
- if (unlikely(au_sbend(sb) < bindex))
- goto out;
+ AuDbg("b%d\n", bindex);

- err = 0;
root = sb->s_root;
di_read_lock_parent(root, !AuLock_IR);
br = au_sbr(sb, bindex);
@@ -111,9 +116,7 @@ static int sysaufs_sbi_br(struct seq_file *seq, struct super_block *sb,
au_seq_path(seq, &path);
di_read_unlock(root, !AuLock_IR);
seq_printf(seq, "=%s\n", au_optstr_br_perm(br->br_perm));
-
- out:
- return err;
+ return 0;
}

/* ---------------------------------------------------------------------- */
@@ -134,17 +137,41 @@ static struct seq_file *au_seq(char *p, ssize_t len)
return seq;
}

+#define SysaufsBr_PREFIX "br"
+#define SysaufsXi_PREFIX "xi"
+
/* todo: file size may exceed PAGE_SIZE */
ssize_t sysaufs_si_show(struct kobject *kobj, struct attribute *attr,
char *buf)
{
ssize_t err;
long l;
+ aufs_bindex_t bend;
struct au_sbinfo *sbinfo;
struct super_block *sb;
struct seq_file *seq;
char *name;
struct attribute **cattr;
+ static struct {
+ const int prefix_len;
+ char *prefix;
+ int (*func)(struct seq_file *seq, struct super_block *sb,
+ aufs_bindex_t bindex);
+ } a[] = {
+ {
+ .prefix_len = sizeof(SysaufsBr_PREFIX) - 1,
+ .prefix = SysaufsBr_PREFIX,
+ .func = sysaufs_si_br
+ },
+ {
+ .prefix_len = sizeof(SysaufsXi_PREFIX) - 1,
+ .prefix = SysaufsXi_PREFIX,
+ .func = sysaufs_si_xino
+ },
+ {
+ .prefix_len = 0
+ }
+ }, *p;

sbinfo = container_of(kobj, struct au_sbinfo, si_kobj);
sb = sbinfo->si_sb;
@@ -166,12 +193,22 @@ ssize_t sysaufs_si_show(struct kobject *kobj, struct attribute *attr,
cattr++;
}

- if (!strncmp(name, SysaufsBr_PREFIX, sizeof(SysaufsBr_PREFIX) - 1)) {
- name += sizeof(SysaufsBr_PREFIX) - 1;
- err = strict_strtol(name, 10, &l);
- if (!err)
- err = sysaufs_sbi_br(seq, sb, (aufs_bindex_t)l);
- goto out_seq;
+ p = a;
+ bend = au_sbend(sb);
+ while (p->prefix_len) {
+ if (!strncmp(name, p->prefix, p->prefix_len)) {
+ name += p->prefix_len;
+ err = strict_strtol(name, 10, &l);
+ if (!err) {
+ if (l <= bend)
+ err = p->func(seq, sb,
+ (aufs_bindex_t)l);
+ else
+ err = -ENOENT;
+ }
+ goto out_seq;
+ }
+ p++;
}
BUG();

@@ -192,44 +229,74 @@ ssize_t sysaufs_si_show(struct kobject *kobj, struct attribute *attr,

void sysaufs_br_init(struct au_branch *br)
{
+ struct au_xino_file *xi;
+
br->br_attr.name = br->br_name;
br->br_attr.mode = S_IRUGO;
br->br_attr.owner = THIS_MODULE;
+
+ xi = &br->br_xino;
+ xi->xi_attr.name = xi->xi_name;
+ xi->xi_attr.mode = S_IRUGO;
+ xi->xi_attr.owner = THIS_MODULE;
}

void sysaufs_brs_del(struct super_block *sb, aufs_bindex_t bindex)
{
- struct au_sbinfo *sbinfo;
- aufs_bindex_t bend;
-
- if (!sysaufs_brs)
- return;
+ struct au_branch *br;
+ struct au_xino_file *xi;
+ struct kobject *kobj;
+ aufs_bindex_t bend, bi;

- sbinfo = au_sbi(sb);
+ kobj = &au_sbi(sb)->si_kobj;
bend = au_sbend(sb);
- for (; bindex <= bend; bindex++)
- sysfs_remove_file(&sbinfo->si_kobj,
- &au_sbr(sb, bindex)->br_attr);
+ for (bi = bindex; bi <= bend; bi++) {
+ br = au_sbr(sb, bi);
+ xi = &br->br_xino;
+ sysfs_remove_file(kobj, &xi->xi_attr);
+ }
+
+ if (sysaufs_brs)
+ for (; bindex <= bend; bindex++) {
+ br = au_sbr(sb, bindex);
+ sysfs_remove_file(kobj, &br->br_attr);
+ }
}

-void sysaufs_brs_add(struct super_block *sb, aufs_bindex_t bindex)
+static void sysaufs_brs_do_add(struct kobject *kobj, struct attribute *attr,
+ char name[], int nlen, char prefix[],
+ aufs_bindex_t bindex)
{
int err;
- aufs_bindex_t bend;
+
+ snprintf(name, nlen, "%s%d", prefix, bindex);
+ err = sysfs_create_file(kobj, attr);
+ if (unlikely(err))
+ AuWarn("failed %s under sysfs(%d)\n", name, err);
+}
+
+void sysaufs_brs_add(struct super_block *sb, aufs_bindex_t bindex)
+{
+ aufs_bindex_t bend, bi;
struct kobject *kobj;
struct au_branch *br;
-
- if (!sysaufs_brs)
- return;
+ struct au_xino_file *xi;

kobj = &au_sbi(sb)->si_kobj;
bend = au_sbend(sb);
- for (; bindex <= bend; bindex++) {
- br = au_sbr(sb, bindex);
- snprintf(br->br_name, sizeof(br->br_name),
- SysaufsBr_PREFIX "%d", bindex);
- err = sysfs_create_file(kobj, &br->br_attr);
- if (unlikely(err))
- AuWarn("failed %s under sysfs(%d)\n", br->br_name, err);
+ for (bi = bindex; bi <= bend; bi++) {
+ br = au_sbr(sb, bi);
+ xi = &br->br_xino;
+ /* todo: create link for shared xino */
+ sysaufs_brs_do_add(kobj, &xi->xi_attr, xi->xi_name,
+ sizeof(xi->xi_name), SysaufsXi_PREFIX, bi);
}
+
+ if (sysaufs_brs)
+ for (; bindex <= bend; bindex++) {
+ br = au_sbr(sb, bindex);
+ sysaufs_brs_do_add(kobj, &br->br_attr, br->br_name,
+ sizeof(br->br_name),
+ SysaufsBr_PREFIX, bindex);
+ }
}
--
1.6.1.284.g5dc13

2009-03-19 05:33:19

by J. R. Okajima

[permalink] [raw]
Subject: [RFC Aufs2 #3 1/2] replace /sys/fs/aufs/debug by /sys/module/aufs/parmaters/debug

Follow the comments from Greg KH on LKML.
- remove the global variable 'au_cond'.
- new global variable 'aufs_debug' as a module parameter.
- describe the new module parameter 'debug' in the manual.
- remove the global variable 'sysaufs_ktype', debug_show(),
debug_store(), au_debug_attr, au_attr_show(), au_attr_store(), and
sysaufs_ktype_body.
- make au_attr empty.

Signed-off-by: J. R. Okajima <[email protected]>
---
Documentation/filesystems/aufs/aufs.5 | 8 ++++
fs/aufs/debug.c | 4 +-
fs/aufs/debug.h | 12 ++---
fs/aufs/sysaufs.h | 2 -
fs/aufs/sysfs.c | 73 ---------------------------------
5 files changed, 16 insertions(+), 83 deletions(-)

diff --git a/Documentation/filesystems/aufs/aufs.5 b/Documentation/filesystems/aufs/aufs.5
index 0b485ea..b3bca05 100644
--- a/Documentation/filesystems/aufs/aufs.5
+++ b/Documentation/filesystems/aufs/aufs.5
@@ -361,6 +361,14 @@ If your linux version is linux\-2.6.24 and earlier, you need to enable
CONFIG_AUFS_SYSAUFS too.
Currently this is for developers only.
The default is \[oq]a\[cq].
+.
+.TP
+.B debug= 0 | 1
+Specifies disable(0) or enable(1) debug print in aufs.
+This parameter can be changed dynamically.
+You need to enable CONFIG_AUFS_DEBUG.
+Currently this is for developers only.
+The default is \[oq]0\[cq] (disable).

.\" ----------------------------------------------------------------------
.SH Branch Syntax
diff --git a/fs/aufs/debug.c b/fs/aufs/debug.c
index e8bc59c..63ec24f 100644
--- a/fs/aufs/debug.c
+++ b/fs/aufs/debug.c
@@ -13,7 +13,9 @@

#include "aufs.h"

-atomic_t au_cond = ATOMIC_INIT(0);
+int aufs_debug;
+MODULE_PARM_DESC(debug, "debug print");
+module_param_named(debug, aufs_debug, int, S_IRUGO | S_IWUSR | S_IWGRP);

char *au_plevel = KERN_DEBUG;
#define dpri(fmt, arg...) do { \
diff --git a/fs/aufs/debug.h b/fs/aufs/debug.h
index 0b377a8..36ff0ea 100644
--- a/fs/aufs/debug.h
+++ b/fs/aufs/debug.h
@@ -25,20 +25,18 @@

#ifdef CONFIG_AUFS_DEBUG
#define AuDebugOn(a) BUG_ON(a)
-extern atomic_t au_cond;
+
+/* module parameter */
+extern int aufs_debug;
static inline void au_debug(int n)
{
- atomic_set(&au_cond, n);
+ aufs_debug = n;
smp_mb();
}

static inline int au_debug_test(void)
{
- int ret;
-
- ret = atomic_read(&au_cond);
- smp_mb();
- return ret;
+ return aufs_debug;
}
#else
#define AuDebugOn(a) do {} while (0)
diff --git a/fs/aufs/sysaufs.h b/fs/aufs/sysaufs.h
index 04ffab2..60b18a8 100644
--- a/fs/aufs/sysaufs.h
+++ b/fs/aufs/sysaufs.h
@@ -47,7 +47,6 @@ struct au_branch;
#ifdef CONFIG_SYSFS
/* sysfs.c */
extern struct attribute_group *sysaufs_attr_group;
-extern struct kobj_type *sysaufs_ktype;

int sysaufs_si_xino(struct seq_file *seq, struct super_block *sb);
ssize_t sysaufs_si_show(struct kobject *kobj, struct attribute *attr,
@@ -61,7 +60,6 @@ void sysaufs_brs_del(struct super_block *sb, aufs_bindex_t bindex);

#else
#define sysaufs_attr_group NULL
-#define sysaufs_ktype NULL

static inline
int sysaufs_si_xino(struct seq_file *seq, struct super_block *sb)
diff --git a/fs/aufs/sysfs.c b/fs/aufs/sysfs.c
index b03c2f7..2112f3b 100644
--- a/fs/aufs/sysfs.c
+++ b/fs/aufs/sysfs.c
@@ -16,36 +16,7 @@
#include <linux/sysfs.h>
#include "aufs.h"

-#ifdef CONFIG_AUFS_DEBUG
-static ssize_t debug_show(struct kobject *kobj __maybe_unused,
- struct kobj_attribute *attr __maybe_unused,
- char *buf)
-{
- return sprintf(buf, "%d\n", au_debug_test());
-}
-
-static ssize_t debug_store(struct kobject *kobj __maybe_unused,
- struct kobj_attribute *attr __maybe_unused,
- const char *buf, size_t sz)
-{
- if (unlikely(!sz || (*buf != '0' && *buf != '1')))
- return -EOPNOTSUPP;
-
- if (*buf == '0')
- au_debug(0);
- else if (*buf == '1')
- au_debug(1);
- return sz;
-}
-
-static struct kobj_attribute au_debug_attr = __ATTR(debug, S_IRUGO | S_IWUSR,
- debug_show, debug_store);
-#endif
-
static struct attribute *au_attr[] = {
-#ifdef CONFIG_AUFS_DEBUG
- &au_debug_attr.attr,
-#endif
NULL, /* need to NULL terminate the list of attributes */
};

@@ -57,50 +28,6 @@ struct attribute_group *sysaufs_attr_group = &sysaufs_attr_group_body;

/* ---------------------------------------------------------------------- */

-/*
- * they are copied from linux/lib/kobject.c,
- * and will be exported in the future.
- */
-static ssize_t au_attr_show(struct kobject *kobj, struct attribute *attr,
- char *buf)
-{
- struct kobj_attribute *kattr;
- ssize_t ret = -EIO;
-
- kattr = container_of(attr, struct kobj_attribute, attr);
- if (kattr->show)
- ret = kattr->show(kobj, kattr, buf);
- return ret;
-}
-
-#ifdef CONFIG_AUFS_DEBUG
-static ssize_t au_attr_store(struct kobject *kobj, struct attribute *attr,
- const char *buf, size_t count)
-{
- struct kobj_attribute *kattr;
- ssize_t ret = -EIO;
-
- kattr = container_of(attr, struct kobj_attribute, attr);
- if (kattr->store)
- ret = kattr->store(kobj, kattr, buf, count);
- return ret;
-}
-#endif
-
-static struct sysfs_ops sysaufs_ops = {
- .show = au_attr_show,
-#ifdef CONFIG_AUFS_DEBUG
- .store = au_attr_store
-#endif
-};
-
-static struct kobj_type sysaufs_ktype_body = {
- .sysfs_ops = &sysaufs_ops
-};
-struct kobj_type *sysaufs_ktype = &sysaufs_ktype_body;
-
-/* ---------------------------------------------------------------------- */
-
static int sysaufs_sbi_xi(struct seq_file *seq, struct file *xf,
struct kstat *st)
{
--
1.6.1.284.g5dc13

2009-03-20 00:46:50

by Greg KH

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs

On Thu, Mar 19, 2009 at 02:12:45PM +0900, J. R. Okajima wrote:
> Follow the comments from Greg KH on LKML.
> - make all entries "one value per file".
> - split 'xino' into 'xi_path', 'xib' and 'xi0'...'xiN'.
> - new members in struct au_xino_file for xi[0-9]* entry.
> - remove sysaufs_si_attr_xino.
> - sysaufs_si_xino() shows only a size info of one xino file.
> - new variables sysaufs_si_attr_xi_path and sysaufs_si_attr_xib.
> - new functions sysaufs_si_xi_path() and sysaufs_si_xib().
> - rename sysaufs_sbi_xi() to sysaufs_xi_attr().
> - sysaufs_si_show() supports xiN.
> - sysaufs_br_init() initializes br->br_xino.xi_attr too.
> - sysaufs_brs_del() and sysaufs_brs_add() always handle
> br->br_xino.xi_attr.
> - new static function sysaufs_brs_do_add() to generate brN and xiN
> names.
>
> Signed-off-by: J. R. Okajima <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-aufs | 62 +++++++++++
> fs/aufs/branch.h | 6 +
> fs/aufs/sysaufs.c | 6 +-
> fs/aufs/sysaufs.h | 11 ++-
> fs/aufs/sysfs.c | 195 +++++++++++++++++++++++-----------
> 5 files changed, 212 insertions(+), 68 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-aufs
>
> diff --git a/Documentation/ABI/testing/sysfs-aufs b/Documentation/ABI/testing/sysfs-aufs
> new file mode 100644
> index 0000000..1552d3e
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-aufs
> @@ -0,0 +1,62 @@
> +What: /sys/fs/aufs/si_<id>/
> +Date: March 2009
> +Contact: J. R. Okajima <[email protected]>
> +Description:
> + Under /sys/fs/aufs, a directory named si_<id> is created
> + per aufs mount, where <id> is a unique id generated
> + internally.
> +
> +What: /sys/fs/aufs/si_<id>/br0, br1 ... brN
> +Date: March 2009
> +Contact: J. R. Okajima <[email protected]>
> +Description:
> + It shows the abolute path of a member directory (which
> + is called branch) in aufs, and its permission.
> +
> +What: /sys/fs/aufs/si_<id>/xi_path
> +Date: March 2009
> +Contact: J. R. Okajima <[email protected]>
> +Description:
> + It shows the abolute path of XINO (External Inode Number
> + Bitmap, Translation Table and Generation Table) file
> + even if it is the default path.
> + When the aufs mount option 'noxino' is specified, it
> + will be empty. About XINO files, see
> + Documentation/filesystems/aufs/aufs.5 in detail.
> +
> +What: /sys/fs/aufs/si_<id>/xib
> +Date: March 2009
> +Contact: J. R. Okajima <[email protected]>
> +Description:
> + It shows the consumed blocks by xib (External Inode Number
> + Bitmap), its block size and file size.
> + When the aufs mount option 'noxino' is specified, it
> + will be empty. About XINO files, see
> + Documentation/filesystems/aufs/aufs.5 in detail.

Sysfs files are one value per file. This violates that rule.

> +What: /sys/fs/aufs/si_<id>/xino0, xino1 ... xinoN
> +Date: March 2009
> +Contact: J. R. Okajima <[email protected]>
> +Description:
> + It shows the consumed blocks by xino (External Inode Number
> + Translation Table), its link count, block size and file
> + size.
> + When the aufs mount option 'noxino' is specified, it
> + will be empty. About XINO files, see
> + Documentation/filesystems/aufs/aufs.5 in detail.
> +
> +What: /sys/fs/aufs/si_<id>/xigen
> +Date: March 2009
> +Contact: J. R. Okajima <[email protected]>
> +Description:
> + It shows the consumed blocks by xigen (External Inode
> + Generation Table), its block size and file size.
> + If CONFIG_AUFS_EXPORT is disabled, this entry will not
> + be created.
> + When the aufs mount option 'noxino' is specified, it
> + will be empty. About XINO files, see
> + Documentation/filesystems/aufs/aufs.5 in detail.

Are all of these things something that a "normal" user would care about?
or are they development / debugging things?

> +# Local variables: ;
> +# mode: text;
> +# End: ;

I don't think you ment to add this to the file :)

And why are you using seq_file for a sysfs file? That's not allowed,
and a sure sign you are doing something wrong, please remove all of
that.

thanks,

greg k-h

2009-03-20 02:26:08

by J. R. Okajima

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs


Greg KH:
> > +Description:
> > + It shows the consumed blocks by xib (External Inode Number
> > + Bitmap), its block size and file size.
> > + When the aufs mount option 'noxino' is specified, it
> > + will be empty. About XINO files, see
> > + Documentation/filesystems/aufs/aufs.5 in detail.
>
> Sysfs files are one value per file. This violates that rule.

Current print format is
"%llux%lu %lld\n", st->blocks, st->blksize, (long long)st->size

Do you mean this has three values and violates the rule?
And aufs should create three entries such like xib/blocks, xib/blksize
and xib/size?
If I change it "<blocks>x<blksize>", is it still violation?


> Are all of these things something that a "normal" user would care about?
> or are they development / debugging things?

Normal users want to care them, I guess.
Since XINO files grow only, some heavy users had met ENOSPC
actually. Currently aufs supports automatic truncation for XINO files in
tmpfs only.


> And why are you using seq_file for a sysfs file? That's not allowed,
> and a sure sign you are doing something wrong, please remove all of
> that.

I just wanted to set limit its size to PAGE_SIZE to print the absolute
path. Is there another better approach?


Thank you for reviewing
J. R. Okajima

2009-03-20 02:44:14

by Greg KH

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs

On Fri, Mar 20, 2009 at 11:25:49AM +0900, [email protected] wrote:
>
> Greg KH:
> > > +Description:
> > > + It shows the consumed blocks by xib (External Inode Number
> > > + Bitmap), its block size and file size.
> > > + When the aufs mount option 'noxino' is specified, it
> > > + will be empty. About XINO files, see
> > > + Documentation/filesystems/aufs/aufs.5 in detail.
> >
> > Sysfs files are one value per file. This violates that rule.
>
> Current print format is
> "%llux%lu %lld\n", st->blocks, st->blksize, (long long)st->size
>
> Do you mean this has three values and violates the rule?

Hm, rule is "one value per file", since this file has 3 values, what do
you think? :)

> And aufs should create three entries such like xib/blocks, xib/blksize
> and xib/size?

Yes.

> If I change it "<blocks>x<blksize>", is it still violation?

I don't understand.

> > Are all of these things something that a "normal" user would care about?
> > or are they development / debugging things?
>
> Normal users want to care them, I guess.

Really? Try leaving them out for now and see if anyone notices :)

> > And why are you using seq_file for a sysfs file? That's not allowed,
> > and a sure sign you are doing something wrong, please remove all of
> > that.
>
> I just wanted to set limit its size to PAGE_SIZE to print the absolute
> path. Is there another better approach?

Do you really need to print these paths? And are they going to bigger
than PATH_MAX?

thanks,

greg k-h

2009-03-20 02:55:28

by J. R. Okajima

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs


Greg KH:
> Really? Try leaving them out for now and see if anyone notices :)

Ok, then I will move them to debugfs.
Does the rule "one value per file" applied to debugfs too?


> Do you really need to print these paths? And are they going to bigger
> than PATH_MAX?

Printing is necessary, and PATH_MAX will be sufficient.
I just wanted to forbid over-running when the path is incredibly longer.


J. R. Okajima

2009-03-20 03:21:11

by Greg KH

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs

On Fri, Mar 20, 2009 at 11:55:09AM +0900, [email protected] wrote:
>
> Greg KH:
> > Really? Try leaving them out for now and see if anyone notices :)
>
> Ok, then I will move them to debugfs.
> Does the rule "one value per file" applied to debugfs too?

The only rule that debugfs has is:
debugfs has no rules!

You can do whatever you want in there, have fun.

thanks,

greg k-h

2009-03-20 03:59:57

by J. R. Okajima

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs


Greg KH:
> > Greg KH:
> > > Really? Try leaving them out for now and see if anyone notices :)
> >
> > Ok, then I will move them to debugfs.
> > Does the rule "one value per file" applied to debugfs too?
>
> The only rule that debugfs has is:
> debugfs has no rules!

To make sure, how do you think this change?

(current)
/sys/fs/aufs/si_<id>/
+ br0 ... brN
+ xi0 ... xiN
+ xi_path
+ xib
+ xigen

(future)
/sys/fs/aufs/si_<id>/
+ br0 ... brN
+ xi_path

/debug/aufs/si_<id>/
+ xi0 ... xiN
+ xib
+ xigen
xiN and xib may be consolidated into a single file.


J. R. Okajima

2009-03-20 04:48:37

by Greg KH

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs

On Fri, Mar 20, 2009 at 12:59:41PM +0900, [email protected] wrote:
>
> Greg KH:
> > > Greg KH:
> > > > Really? Try leaving them out for now and see if anyone notices :)
> > >
> > > Ok, then I will move them to debugfs.
> > > Does the rule "one value per file" applied to debugfs too?
> >
> > The only rule that debugfs has is:
> > debugfs has no rules!
>
> To make sure, how do you think this change?
>
> (current)
> /sys/fs/aufs/si_<id>/
> + br0 ... brN
> + xi0 ... xiN
> + xi_path
> + xib
> + xigen
>
> (future)
> /sys/fs/aufs/si_<id>/
> + br0 ... brN
> + xi_path
>
> /debug/aufs/si_<id>/
> + xi0 ... xiN
> + xib
> + xigen
> xiN and xib may be consolidated into a single file.

Looks like a good start, if you are _sure_ you need sysfs files :)

thanks,

greg k-h

2009-03-20 05:16:14

by J. R. Okajima

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs


Greg KH:
> Looks like a good start, if you are _sure_ you need sysfs files :)

I believe those paths are important to users.
When CONFIG_SYSFS is disabled, they are printed in /proc/mounts (and has
severer limit).

Addition to set limit, there is one more reason to adopt seq_file.
Because the printed string is a path, it may contain unprintable
characters. seq_file has a good interface seq_path() which supports
escaping such characters.


J. R. Okajima

2009-03-20 05:47:53

by Greg KH

[permalink] [raw]
Subject: Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs

On Fri, Mar 20, 2009 at 02:15:49PM +0900, [email protected] wrote:
>
> Greg KH:
> > Looks like a good start, if you are _sure_ you need sysfs files :)
>
> I believe those paths are important to users.
> When CONFIG_SYSFS is disabled, they are printed in /proc/mounts (and has
> severer limit).

No one disables sysfs that I know of. Heck, my phone enables sysfs...

> Addition to set limit, there is one more reason to adopt seq_file.
> Because the printed string is a path, it may contain unprintable
> characters. seq_file has a good interface seq_path() which supports
> escaping such characters.

That is true. Ok, let's see the final result and I'll be glad to look
at it.

thanks,

greg k-h

2009-03-24 08:17:04

by J. R. Okajima

[permalink] [raw]
Subject: Q. DEBUG_FS and SYSFS config (Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs)


Greg KH:
> No one disables sysfs that I know of. Heck, my phone enables sysfs...
>
> > Addition to set limit, there is one more reason to adopt seq_file.
> > Because the printed string is a path, it may contain unprintable
> > characters. seq_file has a good interface seq_path() which supports
> > escaping such characters.
>
> That is true. Ok, let's see the final result and I'll be glad to look
> at it.

Hi Greg,

Currently I am testing locally about new aufs entries under debugfs, and
I found a strange issue about configuration.

First, configure "normaly".
General setup
[ ] Configure standard kernel features (for small systems) --->

$ egrep -i '(sysfs|debug_fs)' .config
CONFIG_SYSFS_DEPRECATED=y
CONFIG_SYSFS_DEPRECATED_V2=y
CONFIG_SYSFS=y
CONFIG_DEBUG_FS=y
# CONFIG_FAULT_INJECTION_DEBUG_FS is not set

Next, enable EMBEDED, and then disable SYSFS.
General setup
[*] Configure standard kernel features (for small systems) --->
File systems
Pseudo filesystems --->
[ ] sysfs file system support

$ egrep -i '(sysfs|debug_fs)' .config
# CONFIG_SYSFS is not set
CONFIG_DEBUG_FS=y

SYSFS is disabled expectedly, but DEBUG_FS is still enabled,
while "depends on SYSFS" is specified for DEBUG_FS.

Is this expected behaviour?
In other words, can I assume "when DEBUG_FS is enabled, SYSFS must be
enabled too" safely?


J. R. Okajima

2009-03-24 15:36:22

by Greg KH

[permalink] [raw]
Subject: Re: Q. DEBUG_FS and SYSFS config (Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs)

On Tue, Mar 24, 2009 at 05:16:40PM +0900, [email protected] wrote:
>
> Greg KH:
> > No one disables sysfs that I know of. Heck, my phone enables sysfs...
> >
> > > Addition to set limit, there is one more reason to adopt seq_file.
> > > Because the printed string is a path, it may contain unprintable
> > > characters. seq_file has a good interface seq_path() which supports
> > > escaping such characters.
> >
> > That is true. Ok, let's see the final result and I'll be glad to look
> > at it.
>
> Hi Greg,
>
> Currently I am testing locally about new aufs entries under debugfs, and
> I found a strange issue about configuration.
>
> First, configure "normaly".
> General setup
> [ ] Configure standard kernel features (for small systems) --->
>
> $ egrep -i '(sysfs|debug_fs)' .config
> CONFIG_SYSFS_DEPRECATED=y
> CONFIG_SYSFS_DEPRECATED_V2=y

You should disable those two options, no modern distro needs them
anymore.

> CONFIG_SYSFS=y
> CONFIG_DEBUG_FS=y
> # CONFIG_FAULT_INJECTION_DEBUG_FS is not set
>
> Next, enable EMBEDED, and then disable SYSFS.
> General setup
> [*] Configure standard kernel features (for small systems) --->
> File systems
> Pseudo filesystems --->
> [ ] sysfs file system support
>
> $ egrep -i '(sysfs|debug_fs)' .config
> # CONFIG_SYSFS is not set
> CONFIG_DEBUG_FS=y
>
> SYSFS is disabled expectedly, but DEBUG_FS is still enabled,
> while "depends on SYSFS" is specified for DEBUG_FS.
>
> Is this expected behaviour?
> In other words, can I assume "when DEBUG_FS is enabled, SYSFS must be
> enabled too" safely?

Your code should not worry about either of these, it will "just work"
either way.

As for if there is a bug here, I don't know, the kbuild dependancies
should fix it up if you run "make oldconfig" right?

thanks,

greg k-h

2009-03-24 15:57:45

by Kay Sievers

[permalink] [raw]
Subject: Re: Q. DEBUG_FS and SYSFS config (Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs)

On Tue, Mar 24, 2009 at 16:33, Greg KH <[email protected]> wrote:

>> First, configure "normaly".
>> General setup
>>   [ ] Configure standard kernel features (for small systems)  --->
>>
>> $ egrep -i '(sysfs|debug_fs)' .config
>> CONFIG_SYSFS_DEPRECATED=y
>> CONFIG_SYSFS_DEPRECATED_V2=y
>
> You should disable those two options, no modern distro needs them
> anymore.

In fact, all recent distros will no longer work correctly, because
DEPRECATED disables features which are expected to be there today. I
guess it's time to remove the *_V2 hack to reset the default value,
and set the default of CONFIG_SYSFS_DEPRECATED=n?

Thanks,
Kay

2009-03-24 23:51:37

by Greg KH

[permalink] [raw]
Subject: Re: Q. DEBUG_FS and SYSFS config (Re: [RFC Aufs2 #3 2/2] split 'xino' entry under sysfs)

On Tue, Mar 24, 2009 at 04:57:16PM +0100, Kay Sievers wrote:
> On Tue, Mar 24, 2009 at 16:33, Greg KH <[email protected]> wrote:
>
> >> First, configure "normaly".
> >> General setup
> >> ? [ ] Configure standard kernel features (for small systems) ?--->
> >>
> >> $ egrep -i '(sysfs|debug_fs)' .config
> >> CONFIG_SYSFS_DEPRECATED=y
> >> CONFIG_SYSFS_DEPRECATED_V2=y
> >
> > You should disable those two options, no modern distro needs them
> > anymore.
>
> In fact, all recent distros will no longer work correctly, because
> DEPRECATED disables features which are expected to be there today. I
> guess it's time to remove the *_V2 hack to reset the default value,
> and set the default of CONFIG_SYSFS_DEPRECATED=n?

Sounds like a good idea for 2.6.31. Care to send a patch so we can keep
it in -next for a few months?

thanks,

greg k-h