this is especially useful after /sys/slab introduced, for example:
$ ls -l /sys/slab/mm_struct
lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> :0000448
instead of:
$ ls -l /sys/slab/mm_struct
lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> ../slab/:0000448
Signed-off-by: Denis Cheng <[email protected]>
---
fs/sysfs/symlink.c | 38 +++++++++++++++++++++++++++-----------
1 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 3eac20c..230a925 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -19,30 +19,41 @@
#include "sysfs.h"
-static int object_depth(struct sysfs_dirent *sd)
+static struct sysfs_dirent *object_base(struct sysfs_dirent *parent,
+ struct sysfs_dirent *sd)
+{
+ for (; sd->s_parent; sd = sd->s_parent)
+ if (sd->s_parent == parent)
+ return parent;
+ return NULL;
+}
+
+static int object_depth(struct sysfs_dirent *base, struct sysfs_dirent *sd)
{
int depth = 0;
- for (; sd->s_parent; sd = sd->s_parent)
+ for (; sd->s_parent && sd != base; sd = sd->s_parent)
depth++;
return depth;
}
-static int object_path_length(struct sysfs_dirent * sd)
+static int object_path_length(struct sysfs_dirent *base,
+ struct sysfs_dirent *sd)
{
int length = 1;
- for (; sd->s_parent; sd = sd->s_parent)
+ for (; sd->s_parent && sd != base; sd = sd->s_parent)
length += strlen(sd->s_name) + 1;
return length;
}
-static void fill_object_path(struct sysfs_dirent *sd, char *buffer, int length)
+static void fill_object_path(struct sysfs_dirent *base,
+ struct sysfs_dirent *sd, char *buffer, int length)
{
--length;
- for (; sd->s_parent; sd = sd->s_parent) {
+ for (; sd->s_parent && sd != base; sd = sd->s_parent) {
int cur = strlen(sd->s_name);
/* back up enough to print this bus id with '/' */
@@ -129,18 +140,23 @@ static int sysfs_get_target_path(struct sysfs_dirent * parent_sd,
{
char * s;
int depth, size;
+ struct sysfs_dirent *base;
- depth = object_depth(parent_sd);
- size = object_path_length(target_sd) + depth * 3 - 1;
+ base = object_base(parent_sd, target_sd);
+ depth = object_depth(base, parent_sd);
+ size = object_path_length(base, target_sd) + depth * 3 - 1;
if (size > PATH_MAX)
return -ENAMETOOLONG;
- pr_debug("%s: depth = %d, size = %d\n", __FUNCTION__, depth, size);
+ pr_debug("%s: base = %s, depth = %d, size = %d\n",
+ __FUNCTION__,
+ base ? base->s_name : "/sys",
+ depth, size);
for (s = path; depth--; s += 3)
- strcpy(s,"../");
+ strcpy(s, "../");
- fill_object_path(target_sd, path, size);
+ fill_object_path(base, target_sd, path, size);
pr_debug("%s: path = '%s'\n", __FUNCTION__, path);
return 0;
--
1.5.3.4
On Wed, Oct 31, 2007 at 06:34:20PM +0800, Denis Cheng wrote:
> this is especially useful after /sys/slab introduced, for example:
>
> $ ls -l /sys/slab/mm_struct
> lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> :0000448
>
> instead of:
>
> $ ls -l /sys/slab/mm_struct
> lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> ../slab/:0000448
>
> Signed-off-by: Denis Cheng <[email protected]>
As pretty as this change is, it's not really necessary, right?
Is there any other place in /sys that would benefit from this?
thanks,
greg k-h
On 10/31/07, Greg KH <[email protected]> wrote:
> On Wed, Oct 31, 2007 at 06:34:20PM +0800, Denis Cheng wrote:
> > this is especially useful after /sys/slab introduced, for example:
> >
> > $ ls -l /sys/slab/mm_struct
> > lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> :0000448
> >
> > instead of:
> >
> > $ ls -l /sys/slab/mm_struct
> > lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> ../slab/:0000448
> >
> > Signed-off-by: Denis Cheng <[email protected]>
>
> As pretty as this change is, it's not really necessary, right?
I don't think so.
Suppose to create a symlink on the disk, say /usr/src/linux, that
points to /usr/src/linux-2.6.23, the best way is:
# cd /usr/src/
# ln -s linux-2.6.23 linux
# ls -l linux
# ls -l linux
lrwxrwxrwx 1 root root 14 2007-10-16 19:21 linux -> linux-2.6.23
other than:
# cd /usr/src/
# ln -s /usr/src/linux-2.6.23 linux
or
# ln -s ../../usr/src/linux-2.6.23 linux
# ls -l linux
lrwxrwxrwx 1 root root 14 2007-10-16 19:21 linux -> ../../usr/src/linux-2.6.23
Anyone know this, since sysfs is also a filesystem, it should conform
the perfect way.
For another point, consider the code in fs/sysfs/symlink.c:
static int sysfs_get_target_path(struct sysfs_dirent * parent_sd,
struct sysfs_dirent * target_sd, char *path)
{
...
size = object_path_length(target_sd) + depth * 3 - 1;
if (size > PATH_MAX)
return -ENAMETOOLONG;
Since having longer readlink result would consume more memory on the
output parameter path, that is error prone to return -ENAMETOOLONG; we
just need the shorter readlink result.
>
> Is there any other place in /sys that would benefit from this?
Yes. there are already some other symlinks those are also not crossing
top subdirectory of /sys, they would benefit from this patch:
I have found all of them by this little shell:
$ find /sys -type l -printf '%p -> %l -> ' -exec readlink -f '{}' \; | gawk '{
split($1, a, "/");
split($5, b, "/");
if (a[3] == b[3])
print;
}'
that will print many lines like:
...
/sys/block/hdd/subsystem -> ../../block -> /sys/block
/sys/module/snd_mixer_oss/holders/snd_pcm_oss ->
../../../module/snd_pcm_oss -> /sys/module/snd_pcm_oss
/sys/class/sound/audio/subsystem -> ../../../class/sound -> /sys/class/sound
/sys/class/pci_bus/0000:00/subsystem -> ../../../class/pci_bus ->
/sys/class/pci_bus
...
>
> thanks,
>
> greg k-h
>
--
Denis Cheng
Linux Application Developer
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
On Oct 31, 2007 3:34 PM, Greg KH <[email protected]> wrote:
> On Wed, Oct 31, 2007 at 06:34:20PM +0800, Denis Cheng wrote:
> > this is especially useful after /sys/slab introduced, for example:
> >
> > $ ls -l /sys/slab/mm_struct
> > lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> :0000448
> >
> > instead of:
> >
> > $ ls -l /sys/slab/mm_struct
> > lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> ../slab/:0000448
> >
> > Signed-off-by: Denis Cheng <[email protected]>
>
> As pretty as this change is, it's not really necessary, right?
>
> Is there any other place in /sys that would benefit from this?
Yeah:
ep_81 -> ../../../../../devices/pci0000:00/0000:00:1d.0/usb2/2-0:1.0/usb_endpoint/usbdev2.1_ep81
just becomes:
ep_81 -> usb_endpoint/usbdev1.1_ep81
which is nice. The current logic always goes back down to the root of sysfs
and moves forward to the target, which in this case prepends the pointless:
"../../../../../devices/pci0000:00/0000:00:1d.0/usb2/2-0:1.0/" to
the target string.
Kay
On Thu, 2007-11-01 at 19:59 +0100, Kay Sievers wrote:
> On Oct 31, 2007 3:34 PM, Greg KH <[email protected]> wrote:
> > On Wed, Oct 31, 2007 at 06:34:20PM +0800, Denis Cheng wrote:
> > > this is especially useful after /sys/slab introduced, for example:
> > >
> > > $ ls -l /sys/slab/mm_struct
> > > lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> :0000448
> > >
> > > instead of:
> > >
> > > $ ls -l /sys/slab/mm_struct
> > > lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> ../slab/:0000448
> > >
> > > Signed-off-by: Denis Cheng <[email protected]>
> >
> > As pretty as this change is, it's not really necessary, right?
> >
> > Is there any other place in /sys that would benefit from this?
>
> Yeah:
> ep_81 -> ../../../../../devices/pci0000:00/0000:00:1d.0/usb2/2-0:1.0/usb_endpoint/usbdev2.1_ep81
>
> just becomes:
> ep_81 -> usb_endpoint/usbdev1.1_ep81
>
> which is nice. The current logic always goes back down to the root of sysfs
> and moves forward to the target, which in this case prepends the pointless:
> "../../../../../devices/pci0000:00/0000:00:1d.0/usb2/2-0:1.0/" to
> the target string.
Here is what I get when reading all symlinks in the current kernel:
$ for i in $(find /sys/ -type l); do readlink $i; done | wc
1044 1047 37826
And with the optimized relative link targets:
$ for i in $(find /sys/ -type l); do readlink $i; done | wc
1044 1047 33584
It 's more than 4kb less of "../" + redundant directory names. :)
Here is a bit simpler patch, that also removes more code from the kernel
than it adds, and does the optimized link targets. Maybe it adds more
bugs than it removes too. :)
Thanks,
Kay
From: Kay Sievers <[email protected]>
Subject: sysfs: create optimal relative symlink targets
Instead of walking from the source down to the root of sysfs, and back
to the target, we stop at the first directory the source and the target
share.
This link:
/devices/pci0000:00/0000:00:1d.7/usb1/1-0:1.0/ep_81
pointed to:
../../../../../devices/pci0000:00/0000:00:1d.0/usb2/2-0:1.0/usb_endpoint/usbdev2.1_ep81
now it just points to:
usb_endpoint/usbdev1.1_ep81
Thanks to Denis Cheng for bringing this up, and sending the initial patch.
CC: Denis Cheng <[email protected]>
Signed-off-by: Kay Sievers <[email protected]>
---
symlink.c | 88 +++++++++++++++++++++++++++++---------------------------------
1 file changed, 42 insertions(+), 46 deletions(-)
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 3eac20c..5f66c44 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -19,39 +19,6 @@
#include "sysfs.h"
-static int object_depth(struct sysfs_dirent *sd)
-{
- int depth = 0;
-
- for (; sd->s_parent; sd = sd->s_parent)
- depth++;
-
- return depth;
-}
-
-static int object_path_length(struct sysfs_dirent * sd)
-{
- int length = 1;
-
- for (; sd->s_parent; sd = sd->s_parent)
- length += strlen(sd->s_name) + 1;
-
- return length;
-}
-
-static void fill_object_path(struct sysfs_dirent *sd, char *buffer, int length)
-{
- --length;
- for (; sd->s_parent; sd = sd->s_parent) {
- int cur = strlen(sd->s_name);
-
- /* back up enough to print this bus id with '/' */
- length -= cur;
- strncpy(buffer + length, sd->s_name, cur);
- *(buffer + --length) = '/';
- }
-}
-
/**
* sysfs_create_link - create symlink between two objects.
* @kobj: object whose directory we're creating the link in.
@@ -112,7 +79,6 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
return error;
}
-
/**
* sysfs_remove_link - remove symlink in object's directory.
* @kobj: object we're acting for.
@@ -124,24 +90,54 @@ void sysfs_remove_link(struct kobject * kobj, const char * name)
sysfs_hash_and_remove(kobj->sd, name);
}
-static int sysfs_get_target_path(struct sysfs_dirent * parent_sd,
- struct sysfs_dirent * target_sd, char *path)
+static int sysfs_get_target_path(struct sysfs_dirent *parent_sd,
+ struct sysfs_dirent *target_sd, char *path)
{
- char * s;
- int depth, size;
+ struct sysfs_dirent *base, *sd;
+ char *s = path;
+ int len = 0;
+
+ /* go up to the root, stop at the base */
+ base = parent_sd;
+ while (base->s_parent) {
+ sd = target_sd->s_parent;
+ while (sd->s_parent && base != sd)
+ sd = sd->s_parent;
+
+ if (base == sd)
+ break;
+
+ strcpy(s, "../");
+ s += 3;
+ base = base->s_parent;
+ }
+
+ /* determine end of target string for reverse fillup */
+ sd = target_sd;
+ while (sd->s_parent && sd != base) {
+ len += strlen(sd->s_name) + 1;
+ sd = sd->s_parent;
+ }
- depth = object_depth(parent_sd);
- size = object_path_length(target_sd) + depth * 3 - 1;
- if (size > PATH_MAX)
+ /* check limits */
+ if (len < 2)
+ return -EINVAL;
+ len--;
+ if ((s - path) + len > PATH_MAX)
return -ENAMETOOLONG;
- pr_debug("%s: depth = %d, size = %d\n", __FUNCTION__, depth, size);
+ /* reverse fillup of target string from target to base */
+ sd = target_sd;
+ while (sd->s_parent && sd != base) {
+ int slen = strlen(sd->s_name);
- for (s = path; depth--; s += 3)
- strcpy(s,"../");
+ len -= slen;
+ strncpy(s + len, sd->s_name, slen);
+ if (len)
+ s[--len] = '/';
- fill_object_path(target_sd, path, size);
- pr_debug("%s: path = '%s'\n", __FUNCTION__, path);
+ sd = sd->s_parent;
+ }
return 0;
}