[PATCH 0/6] xfs sysfs support

[PATCH 0/6] xfs sysfs support

[Brian Foster]

Hi all,

Here's a first version of sysfs support for XFS. This is based on and
incorporates feedback from the rfc, available here:

	http://oss.sgi.com/archives/xfs/2014-05/msg00425.html

The idea of using debugfs instead was brought up, but not much
discussion around it, so I've left things as is with respect to using
sysfs.

This series creates new xfs_sysfs.[c,h] source files to contain the
general infrastructure for sysfs attribute files in XFS, creates a
global kset for the module (represented as /sys/fs/xfs) and embeds a
couple kobjects to start creating an attribute heirarchy.

Patch 1 fixes what looks like a couple minor error handling errors in
xfs_mountfs(). Patches 2-5 add the sysfs infrastructure, objects and
attributes. Patch 6 adds documentation for the newly defined attributes.

Thoughts, reviews, flames appreciated.

Brian

v1:
- Move sysfs infrastructure code to new source file, add helpers for
  object initialization, etc.
- Created an xfs_mount->xlog object heirarchy for attributes associated
  with the log.
- Renamed the reserve/write grant head attributes to
  '[reserve,write]_grant_head.'
- Use the 'cycle:block' or 'cycle:bytes' decimal format for attributes
  (rather than export encoded values).
- Included generic mountfs fix and doc.

Brian Foster (6):
  xfs: fix a couple error sequence jumps in xfs_mountfs()
  xfs: add a sysfs kset
  xfs: add xfs_mount sysfs kobject
  xfs: add xlog sysfs kobject and attribute handlers
  xfs: add log attributes for log lsn and grant head data
  xfs: document log sysfs attributes in testing ABI

 Documentation/ABI/testing/sysfs-fs-xfs |  39 ++++++++
 fs/xfs/Makefile                        |   1 +
 fs/xfs/xfs_log.c                       |   9 ++
 fs/xfs/xfs_log_priv.h                  |   3 +
 fs/xfs/xfs_mount.c                     |  19 +++-
 fs/xfs/xfs_mount.h                     |   2 +
 fs/xfs/xfs_super.c                     |  12 ++-
 fs/xfs/xfs_sysfs.c                     | 175 +++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysfs.h                     |  47 +++++++++
 9 files changed, 303 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-fs-xfs
 create mode 100644 fs/xfs/xfs_sysfs.c
 create mode 100644 fs/xfs/xfs_sysfs.h

-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/6] xfs: fix a couple error sequence jumps in xfs_mountfs()

[Brian Foster]

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 <bfoster@redhat.com>
---
 fs/xfs/xfs_mount.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 3507cd0..8a41aba 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -855,7 +855,7 @@ xfs_mountfs(
 	     !mp->m_sb.sb_inprogress) {
 		error = xfs_initialize_perag_data(mp, sbp->sb_agcount);
 		if (error)
-			goto out_fail_wait;
+			goto out_log_dealloc;;
 	}
 
 	/*
@@ -927,7 +927,7 @@ xfs_mountfs(
 			xfs_notice(mp, "resetting quota flags");
 			error = xfs_mount_reset_sbqflags(mp);
 			if (error)
-				return error;
+				goto out_rtunmount;
 		}
 	}
 
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/6] xfs: add a sysfs kset

[Brian Foster]

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 <bfoster@redhat.com>
---
 fs/xfs/xfs_super.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 8f0333b..1766214 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -61,6 +61,7 @@
 static const struct super_operations xfs_super_operations;
 static kmem_zone_t *xfs_ioend_zone;
 mempool_t *xfs_ioend_pool;
+struct kset *xfs_kset;
 
 #define MNTOPT_LOGBUFS	"logbufs"	/* number of XFS log buffers */
 #define MNTOPT_LOGBSIZE	"logbsize"	/* size of XFS log buffers */
@@ -1761,9 +1762,15 @@ init_xfs_fs(void)
 	if (error)
 		goto out_cleanup_procfs;
 
+	xfs_kset = kset_create_and_add("xfs", NULL, fs_kobj);
+	if (!xfs_kset) {
+		error = -ENOMEM;
+		goto out_sysctl_unregister;;
+	}
+
 	error = xfs_qm_init();
 	if (error)
-		goto out_sysctl_unregister;
+		goto out_kset_unregister;
 
 	error = register_filesystem(&xfs_fs_type);
 	if (error)
@@ -1772,6 +1779,8 @@ init_xfs_fs(void)
 
  out_qm_exit:
 	xfs_qm_exit();
+ out_kset_unregister:
+	kset_unregister(xfs_kset);
  out_sysctl_unregister:
 	xfs_sysctl_unregister();
  out_cleanup_procfs:
@@ -1793,6 +1802,7 @@ exit_xfs_fs(void)
 {
 	xfs_qm_exit();
 	unregister_filesystem(&xfs_fs_type);
+	kset_unregister(xfs_kset);
 	xfs_sysctl_unregister();
 	xfs_cleanup_procfs();
 	xfs_buf_terminate();
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/6] xfs: add xfs_mount sysfs kobject

[Brian Foster]

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 <bfoster@redhat.com>
---
 fs/xfs/Makefile    |  1 +
 fs/xfs/xfs_mount.c | 15 ++++++++++++++-
 fs/xfs/xfs_mount.h |  2 ++
 fs/xfs/xfs_sysfs.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysfs.h | 46 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 119 insertions(+), 1 deletion(-)
 create mode 100644 fs/xfs/xfs_sysfs.c
 create mode 100644 fs/xfs/xfs_sysfs.h

diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index c21f435..d8da236 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -51,6 +51,7 @@ xfs-y				+= xfs_aops.o \
 				   xfs_mru_cache.o \
 				   xfs_super.o \
 				   xfs_symlink.o \
+				   xfs_sysfs.o \
 				   xfs_trans.o \
 				   xfs_xattr.o \
 				   kmem.o \
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 8a41aba..656614b 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -42,6 +42,7 @@
 #include "xfs_trace.h"
 #include "xfs_icache.h"
 #include "xfs_dinode.h"
+#include "xfs_sysfs.h"
 
 
 #ifdef HAVE_PERCPU_SB
@@ -60,6 +61,8 @@ static DEFINE_MUTEX(xfs_uuid_table_mutex);
 static int xfs_uuid_table_size;
 static uuid_t *xfs_uuid_table;
 
+extern struct kset *xfs_kset;
+
 /*
  * See if the UUID is unique among mounted XFS filesystems.
  * Mount fails if UUID is nil or a FS with the same UUID is already mounted.
@@ -731,10 +734,16 @@ xfs_mountfs(
 
 	xfs_set_maxicount(mp);
 
-	error = xfs_uuid_mount(mp);
+	mp->m_kobject.kset = xfs_kset;
+	error = xfs_sysfs_init(&mp->m_kobject, &xfs_mp_ktype,
+			&mp->m_kobject_complete, NULL, mp->m_fsname);
 	if (error)
 		goto out;
 
+	error = xfs_uuid_mount(mp);
+	if (error)
+		goto out_remove_sysfs;
+
 	/*
 	 * Set the minimum read and write sizes
 	 */
@@ -989,6 +998,8 @@ xfs_mountfs(
 	xfs_da_unmount(mp);
  out_remove_uuid:
 	xfs_uuid_unmount(mp);
+ out_remove_sysfs:
+	xfs_sysfs_del(&mp->m_kobject, &mp->m_kobject_complete);
  out:
 	return error;
 }
@@ -1071,6 +1082,8 @@ xfs_unmountfs(
 	xfs_errortag_clearall(mp, 0);
 #endif
 	xfs_free_perag(mp);
+
+	xfs_sysfs_del(&mp->m_kobject, &mp->m_kobject_complete);
 }
 
 int
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 7295a0b..6d7e5d7 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -166,6 +166,8 @@ typedef struct xfs_mount {
 						   on the next remount,rw */
 	int64_t			m_low_space[XFS_LOWSP_MAX];
 						/* low free space thresholds */
+	struct kobject		m_kobject;
+	struct completion	m_kobject_complete;
 
 	struct workqueue_struct	*m_data_workqueue;
 	struct workqueue_struct	*m_unwritten_workqueue;
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
new file mode 100644
index 0000000..41365fe
--- /dev/null
+++ b/fs/xfs/xfs_sysfs.c
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#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;
+	ssize_t (*show)(char *buf, void *data);
+	ssize_t (*store)(const char *buf, size_t count, void *data);
+};
+
+#define XFS_SYSFS_ATTR_RW(name) \
+	static struct xfs_sysfs_attr xfs_sysfs_attr_##name = __ATTR_RW(name)
+#define XFS_SYSFS_ATTR_RO(name) \
+	static struct xfs_sysfs_attr xfs_sysfs_attr_##name = __ATTR_RO(name)
+
+#define ATTR_LIST(name) &xfs_sysfs_attr_##name.attr
+
+/*
+ * xfs_mount kobject. This currently has no attributes and thus no need for show
+ * and store helpers. The mp kobject serves as the per-mount parent object that
+ * is identified by the fsname under sysfs.
+ */
+
+STATIC void
+xfs_mp_release(struct kobject *kobj)
+{
+	struct xfs_mount *mp = container_of(kobj, struct xfs_mount, m_kobject);
+
+	complete(&mp->m_kobject_complete);
+}
+
+struct kobj_type xfs_mp_ktype = {
+	.release = xfs_mp_release,
+};
diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h
new file mode 100644
index 0000000..c885acf
--- /dev/null
+++ b/fs/xfs/xfs_sysfs.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef __XFS_SYSFS_H__
+#define __XFS_SYSFS_H__
+
+extern struct kobj_type xfs_mp_ktype;	/* xfs_mount */
+
+static inline int
+xfs_sysfs_init(
+	struct kobject		*kobj,
+	struct kobj_type	*ktype,
+	struct completion	*complete,
+	struct kobject		*parent,
+	const char		*name)
+{
+	init_completion(complete);
+	return kobject_init_and_add(kobj, ktype, parent, "%s", name);
+}
+
+static inline void
+xfs_sysfs_del(
+	struct kobject		*kobj,
+	struct completion	*complete)
+{
+	kobject_del(kobj);
+	kobject_put(kobj);
+	wait_for_completion(complete);
+}
+
+#endif	/* __XFS_SYSFS_H__ */
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/6] xfs: add xlog sysfs kobject and attribute handlers

[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.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 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(+)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 292308d..8eb10d5 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -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;
 
@@ -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;
+
 	/* 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);
+
 	xlog_dealloc_log(mp->m_log);
 }
 
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 9bc403a..ce1eee2 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -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;
+
 	/* The following field are used for debugging; need to hold icloglock */
 #ifdef DEBUG
 	char			*l_iclog_bak[XLOG_MAX_ICLOGS];
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 41365fe..f837527 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -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(
+	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;
+}
+
+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)
+{
+	struct xlog *log = container_of(kobj, struct xlog, l_kobject);
+
+	complete(&log->l_kobject_complete);
+}
+
+struct kobj_type xfs_log_ktype = {
+	.release = xfs_log_release,
+	.sysfs_ops = &xfs_log_ops,
+	.default_attrs = xfs_log_attrs,
+};
+
diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h
index c885acf..469f218 100644
--- a/fs/xfs/xfs_sysfs.h
+++ b/fs/xfs/xfs_sysfs.h
@@ -20,6 +20,7 @@
 #define __XFS_SYSFS_H__
 
 extern struct kobj_type xfs_mp_ktype;	/* xfs_mount */
+extern struct kobj_type xfs_log_ktype;	/* xlog */
 
 static inline int
 xfs_sysfs_init(
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 5/6] xfs: add log attributes for log lsn and grant head data

[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 <bfoster@redhat.com>
---
 fs/xfs/xfs_sysfs.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index f837527..fbd82d5 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -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(
+	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);
+
+	return ret;
+}
+XFS_SYSFS_ATTR_RO(log_head_lsn);
+
+STATIC ssize_t
+log_tail_lsn_show(
+	char	*buf,
+	void	*data)
+{
+	struct xlog *log = data;
+	int cycle;
+	int block;
+
+	xlog_crack_atomic_lsn(&log->l_tail_lsn, &cycle, &block);
+	return snprintf(buf, PAGE_SIZE, "%d:%d\n", cycle, block);
+}
+XFS_SYSFS_ATTR_RO(log_tail_lsn);
+
+STATIC ssize_t
+reserve_grant_head_show(
+	char	*buf,
+	void	*data)
+{
+	struct xlog *log = data;
+	int cycle;
+	int bytes;
+
+	xlog_crack_grant_head(&log->l_reserve_head.grant, &cycle, &bytes);
+	return snprintf(buf, PAGE_SIZE, "%d:%d\n", cycle, bytes);
+}
+XFS_SYSFS_ATTR_RO(reserve_grant_head);
+
+STATIC ssize_t
+write_grant_head_show(
+	char	*buf,
+	void	*data)
+{
+	struct xlog *log = data;
+	int cycle;
+	int bytes;
+
+	xlog_crack_grant_head(&log->l_write_head.grant, &cycle, &bytes);
+	return snprintf(buf, PAGE_SIZE, "%d:%d\n", cycle, bytes);
+}
+XFS_SYSFS_ATTR_RO(write_grant_head);
+
 static struct attribute *xfs_log_attrs[] = {
+	ATTR_LIST(log_head_lsn),
+	ATTR_LIST(log_tail_lsn),
+	ATTR_LIST(reserve_grant_head),
+	ATTR_LIST(write_grant_head),
 	NULL,
 };
 
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 6/6] xfs: document log sysfs attributes in testing ABI

[Brian Foster]

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 <bfoster@redhat.com>
---
 Documentation/ABI/testing/sysfs-fs-xfs | 39 ++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-fs-xfs

diff --git a/Documentation/ABI/testing/sysfs-fs-xfs b/Documentation/ABI/testing/sysfs-fs-xfs
new file mode 100644
index 0000000..b81aa08
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-fs-xfs
@@ -0,0 +1,39 @@
+What:		/sys/fs/xfs/<disk>/log/log_head_lsn
+Date:		June 2014
+KernelVersion:	3.16
+Contact:	xfs@oss.sgi.com
+Description:
+		The log sequence number (LSN) of the current head of the
+		log. The LSN is exported in "cycle:basic block" format.
+Users:		xfstests
+
+What:		/sys/fs/xfs/<disk>/log/log_tail_lsn
+Date:		June 2014
+KernelVersion:	3.16
+Contact:	xfs@oss.sgi.com
+Description:
+		The log sequence number (LSN) of the current tail of the
+		log. The LSN is exported in "cycle:basic block" format.
+
+What:		/sys/fs/xfs/<disk>/log/reserve_grant_head
+Date:		June 2014
+KernelVersion:	3.16
+Contact:	xfs@oss.sgi.com
+Description:
+		The current state of the log reserve grant head. It
+		represents the total log reservation of all currently
+		outstanding transactions. The grant head is exported in
+		"cycle:bytes" format.
+Users:		xfstests
+
+What:		/sys/fs/xfs/<disk>/log/write_grant_head
+Date:		June 2014
+KernelVersion:	3.16
+Contact:	xfs@oss.sgi.com
+Description:
+		The current state of the log write grant head. It
+		represents the total log reservation of all currently
+		oustanding transactions, including regrants due to
+		rolling transactions. The grant head is exported in
+		"cycle:bytes" format.
+Users:		xfstests
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/6] xfs: fix a couple error sequence jumps in xfs_mountfs()

[Dave Chinner]

On Fri, Jun 06, 2014 at 09:13:29AM -0400, Brian Foster wrote:
> 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 <bfoster@redhat.com>

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/6] xfs: add a sysfs kset

[Dave Chinner]

On Fri, Jun 06, 2014 at 09:13:30AM -0400, Brian Foster wrote:
> 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 <bfoster@redhat.com>

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/6] xfs: add xfs_mount sysfs kobject

[Dave Chinner]

On Fri, Jun 06, 2014 at 09:13:31AM -0400, Brian Foster wrote:
> 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 <bfoster@redhat.com>
....
> +#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;
> +	ssize_t (*show)(char *buf, void *data);
> +	ssize_t (*store)(const char *buf, size_t count, void *data);
> +};
> +
> +#define XFS_SYSFS_ATTR_RW(name) \
> +	static struct xfs_sysfs_attr xfs_sysfs_attr_##name = __ATTR_RW(name)
> +#define XFS_SYSFS_ATTR_RO(name) \
> +	static struct xfs_sysfs_attr xfs_sysfs_attr_##name = __ATTR_RO(name)
> +
> +#define ATTR_LIST(name) &xfs_sysfs_attr_##name.attr

Not a fan of the sysfs attribute stuff, but it's a generic pattern
so at least the pain is shared by everyone...

Reviewed-by: Dave Chinner <dchinner@redhat.com>


-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/6] xfs: add xlog sysfs kobject and attribute handlers

[Dave Chinner]

On Fri, Jun 06, 2014 at 09:13:32AM -0400, Brian Foster wrote:
> 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....
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  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(+)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 292308d..8eb10d5 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -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;
>  
> @@ -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....

> +
>  	/* 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);

> +
>  	xlog_dealloc_log(mp->m_log);
>  }
>  
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 9bc403a..ce1eee2 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -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;

> +
>  	/* The following field are used for debugging; need to hold icloglock */
>  #ifdef DEBUG
>  	char			*l_iclog_bak[XLOG_MAX_ICLOGS];
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index 41365fe..f837527 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -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(
> +	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;
}


> +}
> +
> +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)
> +{
> +	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.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 5/6] xfs: add log attributes for log lsn and grant head data

[Dave Chinner]

On Fri, Jun 06, 2014 at 09:13:33AM -0400, Brian Foster wrote:
> 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 <bfoster@redhat.com>
> ---
>  fs/xfs/xfs_sysfs.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index f837527..fbd82d5 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -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(
> +	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 <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 6/6] xfs: document log sysfs attributes in testing ABI

[Dave Chinner]

On Fri, Jun 06, 2014 at 09:13:34AM -0400, Brian Foster wrote:
> 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 <bfoster@redhat.com>

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.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/6] xfs: add xlog sysfs kobject and attribute handlers

[Brian Foster]

On Thu, Jun 26, 2014 at 02:47:19PM +1000, Dave Chinner wrote:
> On Fri, Jun 06, 2014 at 09:13:32AM -0400, Brian Foster wrote:
> > 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....
> > 
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > ---
> >  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(+)
> > 
> > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > index 292308d..8eb10d5 100644
> > --- a/fs/xfs/xfs_log.c
> > +++ b/fs/xfs/xfs_log.c
> > @@ -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;
> >  
> > @@ -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....
> 

Indeed, that's a nice cleanup.

> > +
> >  	/* 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);
> 
> > +
> >  	xlog_dealloc_log(mp->m_log);
> >  }
> >  
> > diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> > index 9bc403a..ce1eee2 100644
> > --- a/fs/xfs/xfs_log_priv.h
> > +++ b/fs/xfs/xfs_log_priv.h
> > @@ -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;
> 
> > +
> >  	/* The following field are used for debugging; need to hold icloglock */
> >  #ifdef DEBUG
> >  	char			*l_iclog_bak[XLOG_MAX_ICLOGS];
> > diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> > index 41365fe..f837527 100644
> > --- a/fs/xfs/xfs_sysfs.c
> > +++ b/fs/xfs/xfs_sysfs.c
> > @@ -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(
> > +	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;
> }
> 

OK.

> 
> > +}
> > +
> > +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)
> > +{
> > +	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

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 5/6] xfs: add log attributes for log lsn and grant head data

[Brian Foster]

On Thu, Jun 26, 2014 at 02:52:56PM +1000, Dave Chinner wrote:
> On Fri, Jun 06, 2014 at 09:13:33AM -0400, Brian Foster wrote:
> > 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 <bfoster@redhat.com>
> > ---
> >  fs/xfs/xfs_sysfs.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 66 insertions(+)
> > 
> > diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> > index f837527..fbd82d5 100644
> > --- a/fs/xfs/xfs_sysfs.c
> > +++ b/fs/xfs/xfs_sysfs.c
> > @@ -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(
> > +	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.
> 

Ok, will fix...

> Otherwise, looks good.
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> 

Thanks!

Brian

> -- 
> Dave Chinner
> david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 6/6] xfs: document log sysfs attributes in testing ABI

[Brian Foster]

On Thu, Jun 26, 2014 at 03:03:45PM +1000, Dave Chinner wrote:
> On Fri, Jun 06, 2014 at 09:13:34AM -0400, Brian Foster wrote:
> > 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 <bfoster@redhat.com>
> 
> 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.
> 

I was under the impression that sysfs in general was considered a stable
ABI. Perhaps not to the degree of things like system call interfaces,
but the Documentation/ABI/README describes the expected lifecycle for
these attributes (i.e., testing->stable->obsolete->removed).

> 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?
> 

I'm not against adding another keyword, though I'm curious how this is
different from the above. Wouldn't we use the location of the
documentation to identify how stable the interface is expected to be? To
put it another way, this seems fine under the testing section (but that
suggests volatility already), but I wouldn't ever place a "Stability:
volatile" attribute under the stable section.

Taking a look at the ABI/testing/sysfs-fs-ext4 file, it goes back to
2009 with the most recent update in 2012 and it's still under the
testing section. It's not clear if that's by design or just stale
documentation, but perhaps that is technically the right approach. E.g.,
while clearly those attributes aren't still under "testing," they aren't
ever going to be considered "stable ABI." That means use them for
informational and tuning purposes, don't ever build userspace programs
with significant dependence on them, etc. Anything that does grow unique
importance (i.e., a future spaceman dependency) can get moved over to
stable as we'll expect to provide some kind of deprecation warning and
period of backwards compatibility for supported tools. Thoughts?

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/6] xfs: add xlog sysfs kobject and attribute handlers

[Brian Foster]

On Thu, Jun 26, 2014 at 08:28:55AM -0400, Brian Foster wrote:
> On Thu, Jun 26, 2014 at 02:47:19PM +1000, Dave Chinner wrote:
> > On Fri, Jun 06, 2014 at 09:13:32AM -0400, Brian Foster wrote:
> > > 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....
> > > 
> > > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > > ---
> > >  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(+)
> > > 
...
> > > diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> > > index 41365fe..f837527 100644
> > > --- a/fs/xfs/xfs_sysfs.c
> > > +++ b/fs/xfs/xfs_sysfs.c
...
> > > +
> > > +STATIC void
> > > +xfs_log_release(struct kobject *kobj)
> > > +{
> > > +	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.
> 

Oops, never mind. I didn't grok what we'd get handed back here,
apparently. We get the kobject, contained by the new xfs_kobj and
already carrying the completion. There's no need to reference the
outermost structure so this can be made generic.

Brian

> Brian
> 
> > Cheers,
> > 
> > Dave.
> > -- 
> > Dave Chinner
> > david@fromorbit.com
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs