xfs sysfs support

xfs_mountfs() has a couple failure conditions that do not jump to the correct labels. Specifically:

- xfs_initialize_perag_data() failure does not deallocate the log even though it occurs after log initialization
- xfs_mount_reset_sbqflags() failure returns the error directly rather than jump to the error sequence

Signed-off-by Brian Foster <[email protected]>

Create a sysfs kset to contain all sub-objects associated with the XFS module. The kset is created and removed on module initialization and removal respectively. The kset uses fs_obj as a parent. This leads to the creation of a /sys/fs/xfs directory when the kset exists.

Signed-off-by Brian Foster <[email protected]>

Embed a base kobject into xfs_mount. This creates a kobject associated with each XFS mount and a subdirectory in sysfs with the name of the filesystem. The subdirectory lifecycle matches that of the mount. Also add the new xfs_sysfs.[c,h] source files with some XFS sysfs infrastructure to facilitate attribute creation.

Note that there are currently no attributes exported as part of the xfs_mount kobject. It exists solely to serve as a per-mount container for child objects.

Signed-off-by Brian Foster <[email protected]>

fs/xfs/xfs_sysfs.c new file +10 -0

+#include <linux/sysfs.h>
+#include "xfs.h"
+#include "xfs_types.h"
+#include "xfs_sb.h"
+#include "xfs_trans_resv.h"
+#include "xfs_ag.h"
+#include "xfs_mount.h"
+
+struct xfs_sysfs_attr {
+	struct attribute attr;

Embed a kobject into the xfs log data structure (xlog). This creates a 'log' subdirectory for every XFS mount instance in sysfs. The lifecycle of the log kobject is tied to the lifecycle of the log.

Also define a set of generic attribute handlers associated with the log kobject in preparation for the addition of attributes.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_log.c      |  9 +++++++++
 fs/xfs/xfs_log_priv.h |  3 +++
 fs/xfs/xfs_sysfs.c    | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysfs.h    |  1 +
 4 files changed, 66 insertions(+)

fs/xfs/xfs_log.c +1 -0

index 292308d..8eb10d5 100644
@@ -34,6 +34,7 @@
 #include "xfs_trace.h"
 #include "xfs_fsops.h"
 #include "xfs_cksum.h"
+#include "xfs_sysfs.h"
 
 kmem_zone_t	*xfs_log_ticket_zone;
 

fs/xfs/xfs_log.c +1 -0

+
 	xlog_dealloc_log(mp->m_log);
 }
 

fs/xfs/xfs_log_priv.h +0 -0

index 9bc403a..ce1eee2 100644

fs/xfs/xfs_log_priv.h +1 -0

+
 	/* The following field are used for debugging; need to hold icloglock */
 #ifdef DEBUG
 	char			*l_iclog_bak[XLOG_MAX_ICLOGS];

fs/xfs/xfs_sysfs.c +9 -0

index 41365fe..f837527 100644
@@ -54,3 +54,56 @@ xfs_mp_release(struct kobject *kobj)
 struct kobj_type xfs_mp_ktype = {
 	.release = xfs_mp_release,
 };
+
+/* xlog */
+
+static struct attribute *xfs_log_attrs[] = {
+	NULL,
+};
+
+STATIC ssize_t
+xfs_log_show(

fs/xfs/xfs_sysfs.c +18 -0

+}
+
+STATIC ssize_t
+xfs_log_store(
+	struct kobject		*kobj,
+	struct attribute	*attr,
+	const char		*buf,
+	size_t			count)
+{
+	struct xlog *log = container_of(kobj, struct xlog, l_kobject);
+	struct xfs_sysfs_attr *xfs_attr = container_of(attr,
+		struct xfs_sysfs_attr, attr);
+
+	return xfs_attr->store ? xfs_attr->store(buf, count, log) : 0;
+}
+
+static struct sysfs_ops xfs_log_ops = {
+	.show = xfs_log_show,

Quoted 6 lines · Brian Foster

Embed a kobject into the xfs log data structure (xlog). This creates a 'log' subdirectory for every XFS mount instance in sysfs. The lifecycle of the log kobject is tied to the lifecycle of the log.

Also define a set of generic attribute handlers associated with the log kobject in preparation for the addition of attributes.

The code works fine, but....

Quoted 22 lines

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_log.c      |  9 +++++++++
 fs/xfs/xfs_log_priv.h |  3 +++
 fs/xfs/xfs_sysfs.c    | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysfs.h    |  1 +
 4 files changed, 66 insertions(+)

fs/xfs/xfs_log.c +1 -0

index 292308d..8eb10d5 100644
@@ -34,6 +34,7 @@
 #include "xfs_trace.h"
 #include "xfs_fsops.h"
 #include "xfs_cksum.h"
+#include "xfs_sysfs.h"
 
 kmem_zone_t	*xfs_log_ticket_zone;
 

fs/xfs/xfs_log.c xfs_log_mount( · line 714

Quoted 7 lines

fs/xfs/xfs_log.c +4 -0

@@ -707,6 +708,11 @@ xfs_log_mount(
 		}
 	}
 
+	error = xfs_sysfs_init(&mp->m_log->l_kobject, &xfs_log_ktype,
+			&mp->m_log->l_kobject_complete, &mp->m_kobject, "log");
+	if (error)
+		goto out_destroy_ail;

... that's, ummm, rather verbose. At minimum, a local log variable, but I suspect that if the pattern of "all sysfs dirs have a kobject and a completion" and they are always used together that maybe we should have a:

struct xfs_kobj {
	struct kobject		kobj;
	struct completion	complete;
};

And we pass them around instead. that would make this:

	error = xfs_sysfs_init(&log->l_kobj, &xfs_log_ktype,
			       &mp->m_kobj, "log");

which is much easier to read....

fs/xfs/xfs_log.c xfs_log_unmount( · line 957

Quoted 10 lines

fs/xfs/xfs_log.c +3 -0

+
 	/* Normal transactions can now occur */
 	mp->m_log->l_flags &= ~XLOG_ACTIVE_RECOVERY;
 

@@ -947,6 +953,9 @@ xfs_log_unmount(
 	xfs_log_quiesce(mp);
 
 	xfs_trans_ail_destroy(mp);
+
+	xfs_sysfs_del(&mp->m_log->l_kobject, &mp->m_log->l_kobject_complete);

And that would become:

	xfs_sysfs_del(&log->l_kobj);

Quoted 9 lines

fs/xfs/xfs_log.c +1 -0

+
 	xlog_dealloc_log(mp->m_log);
 }
 

fs/xfs/xfs_log_priv.h +0 -0

index 9bc403a..ce1eee2 100644

fs/xfs/xfs_log_priv.h struct xlog { · line 409

Quoted 5 lines

fs/xfs/xfs_log_priv.h +2 -0

@@ -405,6 +405,9 @@ struct xlog {
 	struct xlog_grant_head	l_reserve_head;
 	struct xlog_grant_head	l_write_head;
 
+	struct kobject		l_kobject;
+	struct completion	l_kobject_complete;

	struct xfs_kobj		l_kobj;

Quoted 21 lines

fs/xfs/xfs_log_priv.h +1 -0

+
 	/* The following field are used for debugging; need to hold icloglock */
 #ifdef DEBUG
 	char			*l_iclog_bak[XLOG_MAX_ICLOGS];

fs/xfs/xfs_sysfs.c +9 -0

index 41365fe..f837527 100644
@@ -54,3 +54,56 @@ xfs_mp_release(struct kobject *kobj)
 struct kobj_type xfs_mp_ktype = {
 	.release = xfs_mp_release,
 };
+
+/* xlog */
+
+static struct attribute *xfs_log_attrs[] = {
+	NULL,
+};
+
+STATIC ssize_t
+xfs_log_show(

fs/xfs/xfs_sysfs.c xfs_mp_release(struct kobject *kobj) · line 74

Quoted 10 lines

fs/xfs/xfs_sysfs.c +9 -0

+	struct kobject		*kobj,
+	struct attribute	*attr,
+	char			*buf)
+{
+	struct xlog *log = container_of(kobj, struct xlog, l_kobject);
 
+	struct xfs_sysfs_attr *xfs_attr = container_of(attr,
+		struct xfs_sysfs_attr, attr);
+
+	return xfs_attr->show ? xfs_attr->show(buf, log) : 0;

I think this could be made much less verbose:

static inline struct xlog *
to_xlog(struct kobject *kobj)
{
return container_of(...)
}

static inline struct xfs_sysfs_attr *
to_attr(struct attribute *attr)
{
return container_of(...)
}

so this becomes:

{
	struct xlog		*log = to_xlog(kobj);
	struct xfs_sysfs_attr	*attr = to_attr(kattr);

	return attr->show ? attr->show(buf, log) : 0;
}

Quoted 25 lines

fs/xfs/xfs_sysfs.c +24 -0

+}
+
+STATIC ssize_t
+xfs_log_store(
+	struct kobject		*kobj,
+	struct attribute	*attr,
+	const char		*buf,
+	size_t			count)
+{
+	struct xlog *log = container_of(kobj, struct xlog, l_kobject);
+	struct xfs_sysfs_attr *xfs_attr = container_of(attr,
+		struct xfs_sysfs_attr, attr);
+
+	return xfs_attr->store ? xfs_attr->store(buf, count, log) : 0;
+}
+
+static struct sysfs_ops xfs_log_ops = {
+	.show = xfs_log_show,
+	.store = xfs_log_store,
+};
+
+STATIC void
+xfs_log_release(struct kobject *kobj)
+{

fs/xfs/xfs_sysfs.c xfs_mp_release(struct kobject *kobj) · line 102

Quoted 4 lines

fs/xfs/xfs_sysfs.c +4 -0

+	struct xlog *log = container_of(kobj, struct xlog, l_kobject);
+
+	complete(&log->l_kobject_complete);
+}

If the release funtion is common with other types, then the xfs_kobj structure is perfect for this use - it will prevent a heap of duplicated release functions...

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Quoted 18 lines · Dave Chinner

Quoted 6 lines · Brian Foster

Embed a kobject into the xfs log data structure (xlog). This creates a 'log' subdirectory for every XFS mount instance in sysfs. The lifecycle of the log kobject is tied to the lifecycle of the log.

Also define a set of generic attribute handlers associated with the log kobject in preparation for the addition of attributes.

The code works fine, but....

Quoted 9 lines

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_log.c      |  9 +++++++++
 fs/xfs/xfs_log_priv.h |  3 +++
 fs/xfs/xfs_sysfs.c    | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysfs.h    |  1 +
 4 files changed, 66 insertions(+)

Quoted 4 lines

Quoted 4 lines

fs/xfs/xfs_sysfs.c +0 -0

index 41365fe..f837527 100644

Quoted 4 lines

Quoted 4 lines

fs/xfs/xfs_sysfs.c +4 -0

+
+STATIC void
+xfs_log_release(struct kobject *kobj)
+{

Quoted 9 lines

fs/xfs/xfs_sysfs.c xfs_mp_release(struct kobject *kobj) · line 102

Quoted 4 lines

fs/xfs/xfs_sysfs.c +4 -0

+	struct xlog *log = container_of(kobj, struct xlog, l_kobject);
+
+	complete(&log->l_kobject_complete);
+}

If the release funtion is common with other types, then the xfs_kobj structure is perfect for this use - it will prevent a heap of duplicated release functions...

It's going to look virtually the same for every kobject. Unfortunately, it needs to go from kobj->xfs_object->xfs_kobj, so each type requires a unique definition. We might be able to just turn it into a macro or something that takes the appropriate info and reduces the clutter. I'll play around with it. Thanks.

Brian

Quoted 6 lines

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Create log attributes to export the current runtime state of the log to sysfs. Note that the filesystem should be frozen for consistency across attributes.

The following per-mount attributes are created: log_head_lsn, log_tail_lsn, reserve_grant_head and write_grant_head. These represent the physical log head, tail and reserve and write grant heads respectively. Attribute values are exported in the following format:

	"cycle:[block,byte]"

... where cycle represents the log cycle and [block,bytes] represents either the basic block or byte offset of the log, depending on the attribute. Log sequence number (LSN) values are encoded in basic blocks and grant heads are encoded in bytes. All values are in decimal format.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_sysfs.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

fs/xfs/xfs_sysfs.c +5 -0

index f837527..fbd82d5 100644
@@ -23,6 +23,9 @@
 #include "xfs_trans_resv.h"
 #include "xfs_ag.h"
 #include "xfs_mount.h"
+#include "xfs_log_format.h"
+#include "xfs_log.h"
+#include "xfs_log_priv.h"
 
 struct xfs_sysfs_attr {
 	struct attribute attr;

@@ -57,7 +60,70 @@ struct kobj_type xfs_mp_ktype = {
 
 /* xlog */
 
+STATIC ssize_t
+log_head_lsn_show(

Quoted 39 lines · Brian Foster

Create log attributes to export the current runtime state of the log to sysfs. Note that the filesystem should be frozen for consistency across attributes.

The following per-mount attributes are created: log_head_lsn, log_tail_lsn, reserve_grant_head and write_grant_head. These represent the physical log head, tail and reserve and write grant heads respectively. Attribute values are exported in the following format:

	"cycle:[block,byte]"

... where cycle represents the log cycle and [block,bytes] represents either the basic block or byte offset of the log, depending on the attribute. Log sequence number (LSN) values are encoded in basic blocks and grant heads are encoded in bytes. All values are in decimal format.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_sysfs.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

fs/xfs/xfs_sysfs.c +3 -0

index f837527..fbd82d5 100644
@@ -23,6 +23,9 @@
 #include "xfs_trans_resv.h"
 #include "xfs_ag.h"
 #include "xfs_mount.h"
+#include "xfs_log_format.h"
+#include "xfs_log.h"
+#include "xfs_log_priv.h"
 
 struct xfs_sysfs_attr {
 	struct attribute attr;

@@ -57,7 +60,70 @@ struct kobj_type xfs_mp_ktype = {
 
 /* xlog */
 

fs/xfs/xfs_sysfs.c struct kobj_type xfs_mp_ktype = { · line 74

Quoted 12 lines

fs/xfs/xfs_sysfs.c +12 -0

+STATIC ssize_t
+log_head_lsn_show(
+	char	*buf,
+	void	*data)
+{
+	struct xlog *log = data;
+	int ret;
+
+	spin_lock(&log->l_icloglock);
+	ret = snprintf(buf, PAGE_SIZE, "%d:%d\n", log->l_curr_cycle,
+			log->l_curr_block);
+	spin_unlock(&log->l_icloglock);

I'd keep the snprintf() outside the iclog lock, just so sampling perturbs behaviour as little as possible. i.e. use cycle/block locals as per all the others.

Otherwise, looks good.

Reviewed-by Dave Chinner <[email protected]>

Signature · 2 lines

Dave Chinner
[email protected]

Create a sysfs-fs-xfs ABI documentation file for newly added sysfs attributes. This is created under the testing section.

Signed-off-by Brian Foster <[email protected]>

Quoted 3 lines · Brian Foster

Create a sysfs-fs-xfs ABI documentation file for newly added sysfs attributes. This is created under the testing section.

Quoted 1 line

Signed-off-by Brian Foster <[email protected]>

Looks fine, but I'm not sure we ever want people to think these are going to form part of the stable XFS ABI - they are encodings of deeply internal functionality of XFS. We're not going to be tied to an implementation because of diagnostic information we expose to to sysfs, so users need to know that this is a volatile interface.

Hence I'd suggest adding a disclaimer indicating that these not ever guaranteed to be stable and could change or be removed at any time without warning. Say, for example, we add another description line like this:

Stability: Volatile

We can then point out the entries that are we'll never guarantee are stable, those that we are stabilising, and those that are considered stable easily?

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]