[RFC PATCH 0/2] xfs: sysfs attribute support

[RFC PATCH 0/2] xfs: sysfs attribute support

[Brian Foster]

Hi all,

This is an rfc to get some discussion rolling on sysfs support for XFS.
The motivation for this work is here:

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

... and also illustrated by an xfstests test I'll send in conjunction
that demonstrates use of the values exported here. This functionality
creates a /sys/fs/xfs directory at module initialization time and a
per-mount subdirectory for each mounted device.

The basic sysfs support code is pretty much borrowed from ext4. I've
dumped it in xfs_mount.c for lack of a better place. While the currently
exported values are more associated with the log, I'm starting with a
flat directory structure here that seemingly aligns with struct
xfs_mount. That said, I could drop the code into xfs_stats.c or create
an entirely new file if that is preferred.

I'm also interested in thoughts about what else might be exported here,
if anything.  I'm pretty sure we don't want to go and add all kinds of
tunables and knobs just for fun. ;) I suppose things like the current
AIL target or CIL push sequence could be relevant. There's also
opportunity for per-AG data, though that would probably warrant a new
subdirectory for AGs and require another child kobject, perhaps embedded
in xfs_perag.

Dave, I believe you had some ideas along the lines of not just dumping
perag data, but having some element of control over an allocation
group. Care to elaborate a bit more on that?

Thoughts/flames on any or all of this appreciated.

Brian

P.S., From the looks of it, the attributes we end up creating need to be
documented under /Documentation/ABI/testing/, so it's worth thinking
about the desired directory structure a bit up front as well.

Brian Foster (2):
  xfs: add basic per-mount sysfs attribute support
  xfs: sysfs attributes for the current log state

 fs/xfs/xfs_mount.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_mount.h |   2 +
 fs/xfs/xfs_super.c |  12 ++++-
 3 files changed, 146 insertions(+), 1 deletion(-)

-- 
1.8.3.1

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

[RFC PATCH 1/2] xfs: add basic per-mount sysfs attribute support

[Brian Foster]

Initialize/destroy a kset on module init/fini for XFS. The XFS attribute
directory is represented as /sys/fs/xfs. Create a subdirectory per-mount
based on the system device name.

Also add some basic macros to aid in the addition of sysfs attributes.
This code is modeled after the equivalent mechanism for ext4.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_mount.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_mount.h |  2 ++
 fs/xfs/xfs_super.c | 12 ++++++++-
 3 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 944f3d9..9ed9dd0 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -60,6 +60,9 @@ static DEFINE_MUTEX(xfs_uuid_table_mutex);
 static int xfs_uuid_table_size;
 static uuid_t *xfs_uuid_table;
 
+extern struct kset *xfs_kset;
+static struct kobj_type xfs_ktype;
+
 /*
  * 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.
@@ -955,6 +958,13 @@ xfs_mountfs(
 	"Unable to allocate reserve blocks. Continuing without reserve pool.");
 	}
 
+	mp->m_kobject.kset = xfs_kset;
+	init_completion(&mp->m_kobject_complete);
+	error = kobject_init_and_add(&mp->m_kobject, &xfs_ktype, NULL,
+				     "%s", mp->m_fsname);
+	if (error)
+		goto out_rtunmount;
+
 	return 0;
 
  out_rtunmount:
@@ -986,6 +996,10 @@ xfs_unmountfs(
 	__uint64_t		resblks;
 	int			error;
 
+	kobject_del(&mp->m_kobject);
+	kobject_put(&mp->m_kobject);
+	wait_for_completion(&mp->m_kobject_complete);
+
 	cancel_delayed_work_sync(&mp->m_eofblocks_work);
 
 	xfs_qm_unmount_quotas(mp);
@@ -2007,3 +2021,68 @@ balance_counter:
 }
 
 #endif
+
+/* sysfs support */
+
+struct xfs_sysfs_attr {
+	struct attribute attr;
+	ssize_t (*show)(struct xfs_mount *mp, char *buf);
+	ssize_t (*store)(struct xfs_mount *mp, const char *buf, size_t count);
+};
+
+#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
+
+static struct attribute *xfs_sysfs_attrs[] = {
+	NULL,
+};
+
+STATIC ssize_t
+xfs_sysfs_show(
+	struct kobject *kobj,
+	struct attribute *attr,
+	char *buf)
+{
+	struct xfs_mount *mp = container_of(kobj, struct xfs_mount, m_kobject);
+	struct xfs_sysfs_attr *xfs_attr = container_of(attr,
+		struct xfs_sysfs_attr, attr);
+
+	return xfs_attr->show ? xfs_attr->show(mp, buf) : 0;
+}
+
+STATIC ssize_t
+xfs_sysfs_store(
+	struct kobject *kobj,
+	struct attribute *attr,
+	const char *buf,
+	size_t count)
+{
+	struct xfs_mount *mp = container_of(kobj, struct xfs_mount, m_kobject);
+	struct xfs_sysfs_attr *xfs_attr = container_of(attr,
+		struct xfs_sysfs_attr, attr);
+
+	return xfs_attr->store ? xfs_attr->store(mp, buf, count) : 0;
+}
+
+static struct sysfs_ops xfs_sysfs_ops = {
+	.show = xfs_sysfs_show,
+	.store = xfs_sysfs_store,
+};
+
+STATIC void
+xfs_kobj_release(struct kobject *kobj)
+{
+	struct xfs_mount *mp = container_of(kobj, struct xfs_mount, m_kobject);
+
+	complete(&mp->m_kobject_complete);
+}
+
+static struct kobj_type xfs_ktype = {
+	.release = xfs_kobj_release,
+	.sysfs_ops = &xfs_sysfs_ops,
+	.default_attrs = xfs_sysfs_attrs,
+};
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index a466c5e..b7f1cfb 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -172,6 +172,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_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

Re: [RFC PATCH 1/2] xfs: add basic per-mount sysfs attribute support

[Dave Chinner]

On Fri, May 16, 2014 at 02:12:45PM -0400, Brian Foster wrote:
> Initialize/destroy a kset on module init/fini for XFS. The XFS attribute
> directory is represented as /sys/fs/xfs. Create a subdirectory per-mount
> based on the system device name.
> 
> Also add some basic macros to aid in the addition of sysfs attributes.
> This code is modeled after the equivalent mechanism for ext4.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/xfs_mount.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_mount.h |  2 ++
>  fs/xfs/xfs_super.c | 12 ++++++++-
>  3 files changed, 92 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 944f3d9..9ed9dd0 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -60,6 +60,9 @@ static DEFINE_MUTEX(xfs_uuid_table_mutex);
>  static int xfs_uuid_table_size;
>  static uuid_t *xfs_uuid_table;
>  
> +extern struct kset *xfs_kset;
> +static struct kobj_type xfs_ktype;
> +
>  /*
>   * 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.
> @@ -955,6 +958,13 @@ xfs_mountfs(
>  	"Unable to allocate reserve blocks. Continuing without reserve pool.");
>  	}
>  
> +	mp->m_kobject.kset = xfs_kset;
> +	init_completion(&mp->m_kobject_complete);
> +	error = kobject_init_and_add(&mp->m_kobject, &xfs_ktype, NULL,
> +				     "%s", mp->m_fsname);
> +	if (error)
> +		goto out_rtunmount;
> +

I think this should be wrapped in a helper, same for the destruction
code. It shoul dalso be called earlier rather than later, because we
are going to want to add children to this object (e.g. per-ag sysfs
attribute directories), and we probably want to isolate information
from different subsystems to different subdirectories under this.
e.g;

	/sysfs/fs/xfs/vda/....
	             /vdb/log/...
		         /trans/...
		         /ag/0/sb/....
			      /agf/....
			      /agi/....
			      /agfl/....
			    /1/sb/...
			    ...
			    /n/agfl/....
			 /quota/....

Dumping everything in the root directory is probably not a good
idea, and hence we shoul dbe creating the heirarchy as we initialise
subsystems during mount, and freeing them as we tear them down from
unmount...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

[RFC PATCH 2/2] xfs: sysfs attributes for the current log state

[Brian Foster]

Create sysfs attributes to export the current runtime state of the log
to userspace. Note that the filesystem should be frozen for best
accuracy/consistency when reading these values, but is not required.
This is for testing and debug purposes only.

Create the following per-mount attributes: log_head_lsn, log_tail_lsn,
reserve_head_lsn and write_head_lsn. These represent the physical log
head, tail and reserve and write grant heads respectively. All values
are exported as raw log sequence numbers (LSN). Note that the grant
heads are in units of bytes while other LSNs are in units of basic
blocks.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_mount.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 9ed9dd0..d0d0617 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_log_priv.h"
 
 
 #ifdef HAVE_PERCPU_SB
@@ -2037,7 +2038,60 @@ struct xfs_sysfs_attr {
 
 #define ATTR_LIST(name) &xfs_sysfs_attr_##name.attr
 
+/* sysfs attributes */
+
+STATIC ssize_t
+log_head_lsn_show(
+	struct xfs_mount	*mp,
+	char			*buf)
+{
+	struct xlog *log = mp->m_log;
+	int ret;
+
+	spin_lock(&log->l_icloglock);
+	ret = snprintf(buf, PAGE_SIZE, "0x%llx\n",
+		xlog_assign_lsn(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(
+	struct xfs_mount	*mp,
+	char			*buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%lx\n",
+		atomic64_read(&mp->m_log->l_tail_lsn));
+}
+XFS_SYSFS_ATTR_RO(log_tail_lsn);
+
+STATIC ssize_t
+reserve_head_lsn_show(
+	struct xfs_mount	*mp,
+	char			*buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%lx\n",
+		atomic64_read(&mp->m_log->l_reserve_head.grant));
+}
+XFS_SYSFS_ATTR_RO(reserve_head_lsn);
+
+STATIC ssize_t
+write_head_lsn_show(
+	struct xfs_mount	*mp,
+	char			*buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%lx\n",
+		atomic64_read(&mp->m_log->l_write_head.grant));
+}
+XFS_SYSFS_ATTR_RO(write_head_lsn);
+
 static struct attribute *xfs_sysfs_attrs[] = {
+	ATTR_LIST(log_head_lsn),
+	ATTR_LIST(log_tail_lsn),
+	ATTR_LIST(reserve_head_lsn),
+	ATTR_LIST(write_head_lsn),
 	NULL,
 };
 
-- 
1.8.3.1

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

Re: [RFC PATCH 2/2] xfs: sysfs attributes for the current log state

[Dave Chinner]

On Fri, May 16, 2014 at 02:12:46PM -0400, Brian Foster wrote:
> Create sysfs attributes to export the current runtime state of the log
> to userspace. Note that the filesystem should be frozen for best
> accuracy/consistency when reading these values, but is not required.
> This is for testing and debug purposes only.
> 
> Create the following per-mount attributes: log_head_lsn, log_tail_lsn,
> reserve_head_lsn and write_head_lsn. These represent the physical log

Reserve and write heads are not log sequence numbers (LSNs). A LSN
is a cycle:block count tuple, while a grant head is a cycle:byte
count tuple....

Calling the reserve_grant_head/write_grant_head would make more
sense, I think, as would splitting them into cycle/byte output
pairs. Splitting them make sense because if we increase the log size
beyond 2GB we're going to need a different in-memory representation
for the grant heads (i.e. need more than 32 bits for byte count), so
we should probably handle that up front in the sysfs API...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [RFC PATCH 2/2] xfs: sysfs attributes for the current log state

[Brian Foster]

On Tue, May 20, 2014 at 07:56:58AM +1000, Dave Chinner wrote:
> On Fri, May 16, 2014 at 02:12:46PM -0400, Brian Foster wrote:
> > Create sysfs attributes to export the current runtime state of the log
> > to userspace. Note that the filesystem should be frozen for best
> > accuracy/consistency when reading these values, but is not required.
> > This is for testing and debug purposes only.
> > 
> > Create the following per-mount attributes: log_head_lsn, log_tail_lsn,
> > reserve_head_lsn and write_head_lsn. These represent the physical log
> 
> Reserve and write heads are not log sequence numbers (LSNs). A LSN
> is a cycle:block count tuple, while a grant head is a cycle:byte
> count tuple....
> 

Yeah, I suppose that's some terminology abuse... ;)

> Calling the reserve_grant_head/write_grant_head would make more
> sense, I think, as would splitting them into cycle/byte output
> pairs. Splitting them make sense because if we increase the log size
> beyond 2GB we're going to need a different in-memory representation
> for the grant heads (i.e. need more than 32 bits for byte count), so
> we should probably handle that up front in the sysfs API...
> 

Ok. reserve_grant_head and write_grant_head in the decimal format of
"cycle:bytes" it is. I'll leave the names of the others and convert them
to a similar "cycle:blocks" format. Thanks!

Brian

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

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

Re: [RFC PATCH 2/2] xfs: sysfs attributes for the current log state

[Dave Chinner]

On Tue, May 20, 2014 at 07:17:06AM -0400, Brian Foster wrote:
> On Tue, May 20, 2014 at 07:56:58AM +1000, Dave Chinner wrote:
> > On Fri, May 16, 2014 at 02:12:46PM -0400, Brian Foster wrote:
> > > Create sysfs attributes to export the current runtime state of the log
> > > to userspace. Note that the filesystem should be frozen for best
> > > accuracy/consistency when reading these values, but is not required.
> > > This is for testing and debug purposes only.
> > > 
> > > Create the following per-mount attributes: log_head_lsn, log_tail_lsn,
> > > reserve_head_lsn and write_head_lsn. These represent the physical log
> > 
> > Reserve and write heads are not log sequence numbers (LSNs). A LSN
> > is a cycle:block count tuple, while a grant head is a cycle:byte
> > count tuple....
> > 
> 
> Yeah, I suppose that's some terminology abuse... ;)
> 
> > Calling the reserve_grant_head/write_grant_head would make more
> > sense, I think, as would splitting them into cycle/byte output
> > pairs. Splitting them make sense because if we increase the log size
> > beyond 2GB we're going to need a different in-memory representation
> > for the grant heads (i.e. need more than 32 bits for byte count), so
> > we should probably handle that up front in the sysfs API...
> > 
> 
> Ok. reserve_grant_head and write_grant_head in the decimal format of
> "cycle:bytes" it is. I'll leave the names of the others and convert them
> to a similar "cycle:blocks" format. Thanks!

FWIW, I think all the sysfs code should be in it's own file, not in
xfs_mount.c...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [RFC PATCH 0/2] xfs: sysfs attribute support

[Christoph Hellwig]

On Fri, May 16, 2014 at 02:12:44PM -0400, Brian Foster wrote:
> Hi all,
> 
> This is an rfc to get some discussion rolling on sysfs support for XFS.
> The motivation for this work is here:
> 
> http://oss.sgi.com/archives/xfs/2014-05/msg00381.html

All the sysfs magic for this looks pretty ugly.  Why can't we just
dump it into debugfs given that it's very much a debug only feature
anyway?

It would also be really nitfy to have a tool to fronted it in the style
of xfs_db and xfs_io..

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

Re: [RFC PATCH 0/2] xfs: sysfs attribute support

[Brian Foster]

On Wed, May 21, 2014 at 07:48:59AM -0700, Christoph Hellwig wrote:
> On Fri, May 16, 2014 at 02:12:44PM -0400, Brian Foster wrote:
> > Hi all,
> > 
> > This is an rfc to get some discussion rolling on sysfs support for XFS.
> > The motivation for this work is here:
> > 
> > http://oss.sgi.com/archives/xfs/2014-05/msg00381.html
> 
> All the sysfs magic for this looks pretty ugly.  Why can't we just
> dump it into debugfs given that it's very much a debug only feature
> anyway?
> 

Yeah, it kind of is. I think it also could get a bit worse as the
dataset expands and we have to embed more kobjects in the various data
structures. i.e., I think creating a full on directory structure in
sysfs will involve a heirarchy of ksets/kobjects that would have to be
instantiated and reclaimed as the associated subsystems are.

debugfs seems like a reasonable suggestion. I think it's suitable for my
initial purpose here (log leak detection), at least. Taking a first look
through the debugfs code and some examples, it looks like there are some
templates for simple/basic types, and then more customized items/output
or things that require locking would require custom handlers. It appears
we can associate a file with an object in memory. The desired directory
structure is created more explicitly (debugfs_create_dir()), so we can
still create an arbitrary directory structure. That and the fact that
debugfs probably isn't considered ABI are positives to me.

I suspect the debugfs interface means we'd probably want to combine the
setup and teardown into a single path (i.e., xfs_init/fini_debugfs()),
otherwise this would probably get just as ugly by carrying around
debugfs directories in in-memory objects such that the infrastructure is
available per-subsystem. We'd also have to consider that we might
require active modification of the directory structure
post-initialization (i.e., Dave's example included per AG attributes, so
consider growfs). That could perhaps get ugly with this model vs. one
that naturally updates as objects are instantiated/freed.

So to me, the right approach seems like it depends on the grand scheme
of things here. The wide and encompassing model with per-AG data and
controls probably favors the sysfs model where individual files are
managed via the individual objects. The current/initial use case
probably favors using debugfs. I'd also be happy to start this off with
debugfs and we can consider converting it to sysfs when the expanded use
case is more defined and justified (though if use cases are clear to
others, I'd prefer to do it correctly now rather than waste time
converting back and forth). Thoughts?

> It would also be really nitfy to have a tool to fronted it in the style
> of xfs_db and xfs_io..

Perhaps, but what's the purpose of having a tool that just reads/writes
these kind of attribute files? Something like that would be great were
these controls exposed via ioctl(), of course, but the existence of
sysfs/debugfs with per-file controls eliminates the need for that kind
of custom tool IMO. Did you have additional functionality in mind?

Thanks for looking at this...

Brian

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