Backing Store for sysfs

fs/super.c do_kern_mount(const char *fstype, int fl · line 702

Quoted 11 lines · Maneesh Soni

fs/super.c renamed from fs/super.c~sysfs-mount +4 -0

@@ -692,6 +693,10 @@ do_kern_mount(const char *fstype, int fl
 	mnt->mnt_mountpoint = sb->s_root;
 	mnt->mnt_parent = mnt;
 	up_write(&sb->s_umount);
+
+	if (!strcmp(fstype, "sysfs"))
+		sysfs_mount = mnt;
+
 	put_filesystem(type);
 	return mnt;
 out_sb:

That's too ugly for words. Vetoed. Sorry, but *that* will not fly.

diff -puN include/linux/kobject.h~sysfs-kobject include/linux/kobject.h

Quoted 4 lines · Maneesh Soni

diff -puN include/linux/kobject.h~sysfs-kobject include/linux/kobject.h

include/linux/kobject.h struct kobject { · line 41

Quoted 10 lines

include/linux/kobject.h renamed from include/linux/kobject.h~sysfs-kobject +6 -0

@@ -32,6 +32,12 @@ struct kobject {
 	struct kset		* kset;
 	struct kobj_type	* ktype;
 	struct dentry		* dentry;
+ 	struct list_head	k_sibling;
+ 	struct list_head	k_children;
+	struct list_head	attr;
+	struct list_head	attr_group;
+	struct rw_semaphore	k_rwsem;
+	char 			*k_symlink;
 };

Ouch. Like Al said, this is too bloated. Remember, not all kobjects

are registered for use in sysfs. This makes the overhead for such usages pretty high :(

Quoted 17 lines · Greg KH

Quoted 4 lines · Maneesh Soni

diff -puN include/linux/kobject.h~sysfs-kobject include/linux/kobject.h

include/linux/kobject.h struct kobject { · line 41

Quoted 10 lines

include/linux/kobject.h renamed from include/linux/kobject.h~sysfs-kobject +6 -0

@@ -32,6 +32,12 @@ struct kobject {
 	struct kset		* kset;
 	struct kobj_type	* ktype;
 	struct dentry		* dentry;
+ 	struct list_head	k_sibling;
+ 	struct list_head	k_children;
+	struct list_head	attr;
+	struct list_head	attr_group;
+	struct rw_semaphore	k_rwsem;
+	char 			*k_symlink;
 };

Ouch. Like Al said, this is too bloated. Remember, not all kobjects

That is not what LowFree numbers after mounting sysfs says. Sure you add some 48 bytes to kobject, but you are no longer pinning 256-byte(??) dentries and possibly bigger inodes for kobjects. Doesn't that count ?

				2.6.0-test6		With patches.

-----------------
dentry_cache (active) 2520 2544
inode_cache (active) 1058 1050
LowFree 875032 KB 874748 KB

Quoted 6 lines · Maneesh Soni

				2.6.0-test6		With patches.

-----------------
dentry_cache (active) 2520 2544
inode_cache (active) 1058 1050
LowFree 875032 KB 874748 KB

So with these patches we actually eat up more LowFree if all sysfs entries are searched, and make the dentry_cache bigger? That's not good :(

Remember, every kobject that's created will cause a call to /sbin/hotplug which will cause udev to walk the sysfs tree to get the information for that kobject. So I don't see any savings in these patches, do you?

Quoted 8 lines · Greg KH

Quoted 6 lines · Maneesh Soni

				2.6.0-test6		With patches.

-----------------
dentry_cache (active) 2520 2544
inode_cache (active) 1058 1050
LowFree 875032 KB 874748 KB

Quoted 2 lines

So with these patches we actually eat up more LowFree if all sysfs entries are searched, and make the dentry_cache bigger? That's not good :(

My guess is that those 24 dentries are just noise. What we should do is verify with a large number of devices if the numbers are all that different after a walk of the sysfs tree.

Quoted 4 lines

Remember, every kobject that's created will cause a call to /sbin/hotplug which will cause udev to walk the sysfs tree to get the information for that kobject. So I don't see any savings in these patches, do you?

Assuming that unused files/dirs are aged out of dentry and inode cache, it should benefit. The numbers you should look at are -

--------------------------------------------------------
After mounting sysfs
-------------------

dentry_cache (active)           2350                    1321
inode_cache (active)            1058                    31
LowFree                         875096 KB               875836 KB

--------------------------------------------------------

That saves ~800KB. If you just mount sysfs and use a few files, you aren't eating up dentries and inodes for every file in sysfs. How often do you expect hotplug events to happen in a system ?

Some time after a hotplug event, dentries/inodes will get aged out and then you should see savings. It should greatly benefit in a normal system.

Now if the additional kobjects cause problems with userland hotplug, then that needs to be resolved. However that seems to be a different problem altogether. Could you please elaborate on that ?

Quoted 3 lines · Dipankar Sarma

My guess is that those 24 dentries are just noise. What we should do is verify with a large number of devices if the numbers are all that different after a walk of the sysfs tree.

Ok, a better test would be with a _lot_ of devices. Care to test with a lot of scsi debug devices?

Quoted 8 lines

--------------------------------------------------------
After mounting sysfs
-------------------

dentry_cache (active)           2350                    1321
inode_cache (active)            1058                    31
LowFree                         875096 KB               875836 KB

--------------------------------------------------------

Quoted 3 lines

That saves ~800KB. If you just mount sysfs and use a few files, you aren't eating up dentries and inodes for every file in sysfs. How often do you expect hotplug events to happen in a system ?

Every kobject that is created and is associated with a subsystem generates a hotplug call. So that's about every kobject that we care about here :)

Quoted 4 lines

Some time after a hotplug event, dentries/inodes will get aged out and then you should see savings. It should greatly benefit in a normal system.

Can you show this happening?

Quoted 4 lines

Now if the additional kobjects cause problems with userland hotplug, then that needs to be resolved. However that seems to be a different problem altogether. Could you please elaborate on that ?

No, I don't think the additional ones you have added will cause problems, but can you verify this? Just log all hotplug events happening in your system (point /proc/sys/kernel/hotplug to a simple logging program).

But again, I don't think the added overhead you have added to a kobject is acceptable for not much gain for the normal case (systems without a zillion devices.)

Quoted 13 lines · Greg KH

Quoted 11 lines · Dipankar Sarma

--------------------------------------------------------
After mounting sysfs
-------------------

dentry_cache (active)           2350                    1321
inode_cache (active)            1058                    31
LowFree                         875096 KB               875836 KB

--------------------------------------------------------

That saves ~800KB. If you just mount sysfs and use a few files, you aren't eating up dentries and inodes for every file in sysfs. How often do you expect hotplug events to happen in a system ?

Quoted 3 lines

Every kobject that is created and is associated with a subsystem generates a hotplug call. So that's about every kobject that we care about here :)

That would not happen in a normal running system often, right ? So, I don't see the point looking at mem usage after hotplug events.

Quoted 5 lines

Quoted 3 lines

Some time after a hotplug event, dentries/inodes will get aged out and then you should see savings. It should greatly benefit in a normal system.

Can you show this happening?

It should be easy to demonstrate. That is how dentries/inodes work for on-disk filesystems. If Maneesh's patch didn't work that way, then the whole point is lost. I hope that is not the case.

Quoted 9 lines

Quoted 3 lines

Now if the additional kobjects cause problems with userland hotplug, then that needs to be resolved. However that seems to be a different problem altogether. Could you please elaborate on that ?

No, I don't think the additional ones you have added will cause problems, but can you verify this? Just log all hotplug events happening in your system (point /proc/sys/kernel/hotplug to a simple logging program).

Quoted 3 lines

But again, I don't think the added overhead you have added to a kobject is acceptable for not much gain for the normal case (systems without a zillion devices.)

IIRC, Maneesh test machine is a 2-way P4 xeon with six scsi disks and savings are of about 800KB. That is as normal a case as it gets, I think. It only gets better as you have more devices in your system.

On Mon, Oct 06, 2003 at 11:31:19PM +0530, Dipankar Sarma wrote:
No. My main point is that for every hotplug event (which is caused by a kobject being created or destroyed), udev will run and look at the sysfs entry for the kobject (by using libsysfs which reads in all of the kobject information, including attributes). This is a normal event, so we have to care about what happens after running 'find' on the sysfs tree as that is basically what will always happen.

Does that make more sense? We can't just look at what happens with this patch without actually accessing all of the sysfs tree, as that will be the "normal" case.

Quoted 5 lines

Quoted 1 line

Can you show this happening?

It should be easy to demonstrate. That is how dentries/inodes work for on-disk filesystems. If Maneesh's patch didn't work that way, then the whole point is lost. I hope that is not the case.

Me too. It's just that the free memory numbers didn't show much gain with this patch on his system. That worries me.

Quoted 7 lines

Quoted 3 lines

But again, I don't think the added overhead you have added to a kobject is acceptable for not much gain for the normal case (systems without a zillion devices.)

IIRC, Maneesh test machine is a 2-way P4 xeon with six scsi disks and savings are of about 800KB. That is as normal a case as it gets, I think. It only gets better as you have more devices in your system.

800Kb after running find? I don't see that :)

Quoted 8 lines · Greg KH

On Mon, Oct 06, 2003 at 11:31:19PM +0530, Dipankar Sarma wrote:
No. My main point is that for every hotplug event (which is caused by a kobject being created or destroyed), udev will run and look at the sysfs entry for the kobject (by using libsysfs which reads in all of the kobject information, including attributes). This is a normal event, so we have to care about what happens after running 'find' on the sysfs tree as that is basically what will always happen.

Quoted 3 lines

Does that make more sense? We can't just look at what happens with this patch without actually accessing all of the sysfs tree, as that will be the "normal" case.

That sounds odd. So, udev essentially results in a frequent and continuous "find /sys" ? That doesn't sound good. You are unnecessarily adding pressure on vfs (dcache specially). We will discuss this offline then and see what needs to be done.

No, not a 'find', we look up the kobject that was added, and its attributes. Doing a 'find' will emulate this for your tests, that's all.

The following patch set(mailed separately) provides a prototype for a backing store mechanism for sysfs. Currently sysfs pins all its dentries and inodes in memory there by wasting kernel lowmem even when it is not mounted.

With this patch set we create sysfs dentry whenever it is required like other real filesystems and, age and free it as per the dcache rules. We now save significant amount of Lowmem by avoiding un-necessary pinning. The following numbers were on a 2-way system with 6 disks and 2 NICs with about 1028 dentries. The numbers are just indicative as they are system wide collected from /proc and are not exclusively for sysfs.

First off, I'm not philosophically opposed to the concept of reducing sysfs and kobject memory usage. I think it can be gracefully done, though I don't think this is quite the solution, and I don't have one myself..

Now, you would really only problems when you have a large number of devices and a limited amount of a Lowmem. I.e. it's only a problem on large systems with 32-bit processors. And, the traditional arguments against this situation is to a) use and promote 64-bit platforms and b) that if you have that many devices, you (or your customers) can surely afford enough memory to make the sysfs footprint a non-issue.

Concerning the patch, I really don't like it. I look at the kobject and sysfs code with the assumption in my mind that the objects are already too large and the code more complex than it should be. Adding to this is not the right approach, just as a general rule of thumb.

Also, I don't think that increasing the co-dependency bewteen the kobject and sysfs hierarchies is the right thing to do. They each have one pointer back to the corresponding location in the other tree, which is about as lightweight as you can get. Adding more only increases bloat for kobjects that are not represented in sysfs, and increases the total overhead of the entire system.

You can also use the assumption that an attribute group exists for all the kobjects in a kset, and that a kobject knows what kset it belongs to. And that eventually, all attributes should be added as part of an attribute group..

Let's look at it this way - unless you find a way to save sizeof(struct dentry)
+ sizeof(struct inode) in every kobject/attr etc., there is no
way you can beat ageing of dentries/inodes.

sysfs currently uses dentries to represent filesystem hierarchy. We want to create the dentries on the fly and age them out. So, we can no longer use dentries to represent filesystem hierarchy. Now, *something* has to represent the actual filesystem hierarchy, so that dentries/inodes can be created on a lookup miss based on that. So, what do you do here ? kobject and its associates already represent most of the information necessary for a backing store. Maneesh just added a little more to complete what is equivalent of a on-disk filesystem. This allows vfs to create dentries and inodes on the fly and age later on. Granted that there are probably ugliness in the design to be sorted out and it may have taken kobjects in a slightly different direction than earlier, but it is not that odd when you look at it from the VFS point of view.

Quoted 6 lines · Dipankar Sarma

sysfs currently uses dentries to represent filesystem hierarchy. We want to create the dentries on the fly and age them out. So, we can no longer use dentries to represent filesystem hierarchy. Now, *something* has to represent the actual filesystem hierarchy, so that dentries/inodes can be created on a lookup miss based on that. So, what do you do here ? kobject and

Quoted 8 lines

its associates already represent most of the information necessary for a backing store. Maneesh just added a little more to complete what is equivalent of a on-disk filesystem. This allows vfs to create dentries and inodes on the fly and age later on. Granted that there are probably ugliness in the design to be sorted out and it may have taken kobjects in a slightly different direction than earlier, but it is not that odd when you look at it from the VFS point of view.

Rot. First of all, *not* *all* *kobjects* *are* *in* *sysfs*. And these are pure loss in your case.

What's more important, for leaves of the sysfs tree your overhead is also a loss - we don't need to pin dentry down for them even with current sysfs
design. And that can be done with minimal code changes and no data changes
at all. Your patch will have to be more attractive than that. What's the expected ratio of directories to non-directories in sysfs?

Quoted 5 lines

What's more important, for leaves of the sysfs tree your overhead is also a loss - we don't need to pin dentry down for them even with current sysfs
design. And that can be done with minimal code changes and no data changes
at all. Your patch will have to be more attractive than that. What's the expected ratio of directories to non-directories in sysfs?

ISTR, a large number of files in sysfs are attributes which are leaves. So, keeping a kobject tree partially connected using dentries as backing store as opposed to having everything connected might just be enough. It will be looked into.

What's more important, for leaves of the sysfs tree your overhead is also a loss - we don't need to pin dentry down for them even with current sysfs
design. And that can be done with minimal code changes and no data changes
at all. Your patch will have to be more attractive than that. What's the expected ratio of directories to non-directories in sysfs?

That is not a very realistic argument, ia32 customers will likely run systems with large number of disks and will have lowmem problem.

Besides that think about the added complexity of lookups due to all those pinned dentries forever residing in dentry hash table.

I don't see how you can claim that the total overhead of the entire system is high. See Christian's numbers. The point here is pretty straightforward -

sysfs currently uses dentries to represent filesystem hierarchy. We want to create the dentries on the fly and age them out. So, we can no longer use dentries to represent filesystem hierarchy. Now, *something* has to represent the actual filesystem hierarchy, so that dentries/inodes can be created on a lookup miss based on that. So, what do you do here ? kobject and its associates already represent most of the information necessary for a backing store.

Quoted 4 lines

You can also use the assumption that an attribute group exists for all the kobjects in a kset, and that a kobject knows what kset it belongs to. And that eventually, all attributes should be added as part of an attribute group..

As I said before, no matter how much you save on kobjects and attrs, I can't see how you can account for ageing of dentries and inodes. Please look at it from the VFS angle and see if there is a better way to represent kobjects/attrs in order to create dentries/inodes on demand and age later.

It's not a realistic requirement for me to solve your customer problems. :) I've been involved in this argument before, and the arguments have been the same, pretty much along party lines of IBM vs. Everyone else. I'm not here to point fingers, but you must heed the fact that we've been here before.

Quoted 2 lines

Besides that think about the added complexity of lookups due to all those pinned dentries forever residing in dentry hash table.

Well, along with more memory and more devices, I would expect your customers to also be paying for the fastest processors. :)

Quoted 8 lines

sysfs currently uses dentries to represent filesystem hierarchy. We want to create the dentries on the fly and age them out. So, we can no longer use dentries to represent filesystem hierarchy. Now, *something* has to represent the actual filesystem hierarchy, so that dentries/inodes can be created on a lookup miss based on that. So, what do you do here ? kobject and its associates already represent most of the information necessary for a backing store.

I understand what you're trying to do, and I say it's the wrong approach. You're overloading kobjects in a manner unintended, and in a way that is not welcome. I do not have an alternative solution, but my last email gave some hints of where to look. Don't get bitter because I disagree.

Quoted 5 lines

Quoted 4 lines

You can also use the assumption that an attribute group exists for all the kobjects in a kset, and that a kobject knows what kset it belongs to. And that eventually, all attributes should be added as part of an attribute group..

Quoted 5 lines

As I said before, no matter how much you save on kobjects and attrs, I can't see how you can account for ageing of dentries and inodes. Please look at it from the VFS angle and see if there is a better way to represent kobjects/attrs in order to create dentries/inodes on demand and age later.

That's what I told you, only reversed - try again. The patch posted in unacceptable, though I'm willing to look at alternatives. I don't have or

see a problem with the current situation, so your arguments are going to have to be a bit stronger.

Quoted 3 lines

That's what I told you, only reversed - try again. The patch posted in unacceptable, though I'm willing to look at alternatives. I don't have or

Viro's suggestion of pinning the non-leaf dentries only seems like a very good first alternative to try out.

Quoted 2 lines

see a problem with the current situation, so your arguments are going to have to be a bit stronger.

By not pinning dentries, you save several hundreds of KBs of lowmem in a common case low-end system with six disks, much reduced number of dentries in the hash table and huge savings in large systems. I would hope that is a good argument. Granted you don't like Maneesh's patch as it is now, but those things will change as more feedbacks come in.

Uh, that's about the same thing I suggested, though probably not as concisely:

"As I said before, I don't know the right solution, but the directions to look in are related to attribute groups. Attributes definitely consume the most amount of memory (as opposed to the kobject hierachy), so delaying their creation would help, hopefully without making the interface too awkward.

You can also use the assumption that an attribute group exists for all the kobjects in a kset, and that a kobject knows what kset it belongs to. And

that eventually, all attributes should be added as part of an attribute group.."

Attributes are the leaf entries, and they don't need to always exist. But, you have easy access to them via the attribute groups of the ksets the kobjects belong to.

Quoted 4 lines · Patrick Mochel

Uh, that's about the same thing I suggested, though probably not as concisely:

"As I said before, I don't know the right solution, but the directions to look in are related to attribute groups. Attributes definitely consume the most amount of memory (as opposed to the kobject hierachy), so delaying their creation would help, hopefully without making the interface too awkward.

Ok.. attributes do consume maximum in sysfs. In the system I mentioned leaf dentries are about 65% of the total.

Quoted 2 lines

You can also use the assumption that an attribute group exists for all the kobjects in a kset, and that a kobject knows what kset it belongs to. And

That's not correct... kobject corresponding to /sys/block/hda/queue doesnot know which kset it belongs to and what are its attributes. Same for /sys/block/hda/queue/iosched.

Quoted 2 lines

that eventually, all attributes should be added as part of an attribute group.."

Quoted 3 lines

Attributes are the leaf entries, and they don't need to always exist. But, you have easy access to them via the attribute groups of the ksets the kobjects belong to.

Having backing store just for leaf dentries should be fine. But there is _no_ easy access for attributes. For this also I see some data change required as of now. The reasons are
- not all kobjects belong to a kset. For example, /sys/block/hda/queue
- not all ksets have attribute groups

Quoted 10 lines

Having backing store just for leaf dentries should be fine. But there is _no_ easy access for attributes. For this also I see some data change required as of now. The reasons are
- not all kobjects belong to a kset. For example, /sys/block/hda/queue
- not all ksets have attribute groups

queue and iosched might not be good examples as they are somewhat broken wrt the block device scheme. Possibly they will be put in their own kset, with /sys/block/hda/queue symlinked to them.