sysfs root link count broken in 2.6.22-git5

sysfs root link count broken in 2.6.22-git5

[Jean Delvare]

Hi Tejun, hi Greg,

I'm running 2.6.22-git5 and noticed that the link count of the sysfs
root is broken:

$ ls -ld /sys
drwxr-xr-x 2 root root 0 Jul 15 12:27 /sys

sysfs is mounted, the link count should be 11, and is with kernel
2.6.22.1. find(1) complains about the bad link count.

I suppose this is an unexpected side effect of the big sysfs changes
that went in 3 days ago. Do you have any idea what can be wrong and how
to fix it, or do you want me to perform a bisection?

Thanks,
-- 
Jean Delvare

Re: sysfs root link count broken in 2.6.22-git5

[Greg KH]

On Sun, Jul 15, 2007 at 12:42:32PM +0200, Jean Delvare wrote:
> Hi Tejun, hi Greg,
> 
> I'm running 2.6.22-git5 and noticed that the link count of the sysfs
> root is broken:
> 
> $ ls -ld /sys
> drwxr-xr-x 2 root root 0 Jul 15 12:27 /sys
> 
> sysfs is mounted, the link count should be 11, and is with kernel
> 2.6.22.1. find(1) complains about the bad link count.

I suggest updating your version of find(1), I get no such complaint
with:
	$ find --version
	GNU find version 4.3.8
	Built using GNU gnulib version 2007-05-26
	Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION FTS() CBO(level=0)

What are you using?

thanks,

greg k-h

Re: sysfs root link count broken in 2.6.22-git5

[Jean Delvare]

Hi Greg,

On Mon, 16 Jul 2007 20:48:44 -0700, Greg KH wrote:
> On Sun, Jul 15, 2007 at 12:42:32PM +0200, Jean Delvare wrote:
> > I'm running 2.6.22-git5 and noticed that the link count of the sysfs
> > root is broken:
> > 
> > $ ls -ld /sys
> > drwxr-xr-x 2 root root 0 Jul 15 12:27 /sys
> > 
> > sysfs is mounted, the link count should be 11, and is with kernel
> > 2.6.22.1. find(1) complains about the bad link count.
> 
> I suggest updating your version of find(1), I get no such complaint
> with:
> 	$ find --version
> 	GNU find version 4.3.8
> 	Built using GNU gnulib version 2007-05-26
> 	Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION FTS() CBO(level=0)
> 
> What are you using?

$ find --version
GNU find version 4.2.28
Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION

This is the standard version in openSuse 10.2. But how does it matter?
sysfs is broken, not find(1). Don't you see the sysfs root link count
at 2 as I do? This needs to be fixed.

-- 
Jean Delvare

Re: sysfs root link count broken in 2.6.22-git5

[Greg KH]

On Tue, Jul 17, 2007 at 01:12:55PM +0200, Jean Delvare wrote:
> Hi Greg,
> 
> On Mon, 16 Jul 2007 20:48:44 -0700, Greg KH wrote:
> > On Sun, Jul 15, 2007 at 12:42:32PM +0200, Jean Delvare wrote:
> > > I'm running 2.6.22-git5 and noticed that the link count of the sysfs
> > > root is broken:
> > > 
> > > $ ls -ld /sys
> > > drwxr-xr-x 2 root root 0 Jul 15 12:27 /sys
> > > 
> > > sysfs is mounted, the link count should be 11, and is with kernel
> > > 2.6.22.1. find(1) complains about the bad link count.
> > 
> > I suggest updating your version of find(1), I get no such complaint
> > with:
> > 	$ find --version
> > 	GNU find version 4.3.8
> > 	Built using GNU gnulib version 2007-05-26
> > 	Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION FTS() CBO(level=0)
> > 
> > What are you using?
> 
> $ find --version
> GNU find version 4.2.28
> Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION
> 
> This is the standard version in openSuse 10.2. But how does it matter?

Well, some people feel that that message from find is not something that
should be bothering users all the time.  Hence it was fixed in newer
versions.

> sysfs is broken, not find(1). Don't you see the sysfs root link count
> at 2 as I do? This needs to be fixed.

I'm not disagreeing with that, but other than find, what is the downside
of this not being correct?  And what should it be?

thanks,

greg k-h

Re: sysfs root link count broken in 2.6.22-git5

[Jean Delvare]

On Tue, 17 Jul 2007 11:36:52 -0700, Greg KH wrote:
> On Tue, Jul 17, 2007 at 01:12:55PM +0200, Jean Delvare wrote:
> > On Mon, 16 Jul 2007 20:48:44 -0700, Greg KH wrote:
> > > On Sun, Jul 15, 2007 at 12:42:32PM +0200, Jean Delvare wrote:
> > > > I'm running 2.6.22-git5 and noticed that the link count of the sysfs
> > > > root is broken:
> > > > 
> > > > $ ls -ld /sys
> > > > drwxr-xr-x 2 root root 0 Jul 15 12:27 /sys
> > > > 
> > > > sysfs is mounted, the link count should be 11, and is with kernel
> > > > 2.6.22.1. find(1) complains about the bad link count.
> > > 
> > > I suggest updating your version of find(1), I get no such complaint
> > > with:
> > > 	$ find --version
> > > 	GNU find version 4.3.8
> > > 	Built using GNU gnulib version 2007-05-26
> > > 	Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION FTS() CBO(level=0)
> > > 
> > > What are you using?
> > 
> > $ find --version
> > GNU find version 4.2.28
> > Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION
> > 
> > This is the standard version in openSuse 10.2. But how does it matter?
> 
> Well, some people feel that that message from find is not something that
> should be bothering users all the time.  Hence it was fixed in newer
> versions.

My understanding is that find uses the link count to speed up the
search. So even if I admit that printing an error message when it
detects that the count is wrong might confuse or annoy end-users, this
is still a valuable for us developers that we got things wrong. I seem
to remember that it helped us detect bugs in procfs and sysfs several
times already.

> > sysfs is broken, not find(1). Don't you see the sysfs root link count
> > at 2 as I do? This needs to be fixed.
> 
> I'm not disagreeing with that, but other than find, what is the downside
> of this not being correct?  And what should it be?

This breaks libsensors. libsensors uses libsysfs, and libsysfs is not
very smart in that it will initialize successfully even if sysfs is not
mounted. So I added tests after the initialization, to make sure that
sysfs is really there. These tests are:
* The mount point exists.
* The mount point is really mounted.

The code looks like:

       if (sysfs_get_mnt_path(sensors_sysfs_mount, NAME_MAX)
         || stat(sensors_sysfs_mount, &statbuf) < 0
         || statbuf.st_nlink <= 2)      /* Empty directory */
                return 0;		/* Failure */

This works OK with 2.6.22.1, but the last test fails with the current
git kernel even when sysfs is mounted.

You may object that this is not the right way to make sure that sysfs
is mounted, but I don't want to rewrite half of sysfs_get_mnt_path() in
libsensors when a simple stat should does the job.

Thanks,
-- 
Jean Delvare

Re: sysfs root link count broken in 2.6.22-git5

[Tejun Heo]

Hello,

Sorry about late response.  -EWASPUBLICHOLIDAY.  That's something I've
broken apparently.  Will fix soon.

Thanks.

-- 
tejun

Re: sysfs root link count broken in 2.6.22-git5

[Greg KH]

On Tue, Jul 17, 2007 at 11:05:30PM +0200, Jean Delvare wrote:
> On Tue, 17 Jul 2007 11:36:52 -0700, Greg KH wrote:
> > On Tue, Jul 17, 2007 at 01:12:55PM +0200, Jean Delvare wrote:
> > > On Mon, 16 Jul 2007 20:48:44 -0700, Greg KH wrote:
> > > > On Sun, Jul 15, 2007 at 12:42:32PM +0200, Jean Delvare wrote:
> > > > > I'm running 2.6.22-git5 and noticed that the link count of the sysfs
> > > > > root is broken:
> > > > > 
> > > > > $ ls -ld /sys
> > > > > drwxr-xr-x 2 root root 0 Jul 15 12:27 /sys
> > > > > 
> > > > > sysfs is mounted, the link count should be 11, and is with kernel
> > > > > 2.6.22.1. find(1) complains about the bad link count.
> > > > 
> > > > I suggest updating your version of find(1), I get no such complaint
> > > > with:
> > > > 	$ find --version
> > > > 	GNU find version 4.3.8
> > > > 	Built using GNU gnulib version 2007-05-26
> > > > 	Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION FTS() CBO(level=0)
> > > > 
> > > > What are you using?
> > > 
> > > $ find --version
> > > GNU find version 4.2.28
> > > Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION
> > > 
> > > This is the standard version in openSuse 10.2. But how does it matter?
> > 
> > Well, some people feel that that message from find is not something that
> > should be bothering users all the time.  Hence it was fixed in newer
> > versions.
> 
> My understanding is that find uses the link count to speed up the
> search. So even if I admit that printing an error message when it
> detects that the count is wrong might confuse or annoy end-users, this
> is still a valuable for us developers that we got things wrong. I seem
> to remember that it helped us detect bugs in procfs and sysfs several
> times already.

I agree, I'm not trying to say it isn't a bug at all, sorry if it came
across that way.

> > > sysfs is broken, not find(1). Don't you see the sysfs root link count
> > > at 2 as I do? This needs to be fixed.
> > 
> > I'm not disagreeing with that, but other than find, what is the downside
> > of this not being correct?  And what should it be?
> 
> This breaks libsensors. libsensors uses libsysfs, and libsysfs is not
> very smart in that it will initialize successfully even if sysfs is not
> mounted.

libsysfs isn't smart at all, and isn't even supported anymore.  I'd
really suggest droping it entirely, it isn't worth it.

> So I added tests after the initialization, to make sure that
> sysfs is really there. These tests are:
> * The mount point exists.
> * The mount point is really mounted.

Do you know of a 2.6 based distro that does not mount sysfs at /sys?  We
took that check out a long time ago in udev and no one has complained :)

> The code looks like:
> 
>        if (sysfs_get_mnt_path(sensors_sysfs_mount, NAME_MAX)
>          || stat(sensors_sysfs_mount, &statbuf) < 0
>          || statbuf.st_nlink <= 2)      /* Empty directory */
>                 return 0;		/* Failure */
> 
> This works OK with 2.6.22.1, but the last test fails with the current
> git kernel even when sysfs is mounted.

Yeah, but is checking the number of hard links in the directory a safe
way to always verify that it isn't empty?  Isn't there some glibc
function that can detect the mount point of a filesystem or directory?
Something in glibc parses /proc/mounts for something, I can't remember
what it is right now though, sorry.

> You may object that this is not the right way to make sure that sysfs
> is mounted, but I don't want to rewrite half of sysfs_get_mnt_path() in
> libsensors when a simple stat should does the job.

Again, I recommend dropping libsysfs, it's gone from some distros
already :)

And yes, the bug should be fixed, I agree.  Thanks for letting us know.

thanks,

greg k-h

[PATCH] sysfs: fix sysfs root inode nlink accounting

[Tejun Heo]

While making sysfs indoes hashed, sysfs root inode was left out.  Now
that nlink accounting depends on the inode being on the hash, sysfs
root inode nlink isn't adjusted properly.

Put sysfs root inode on the inode hash by allocating it using
sysfs_get_inode() like other sysfs inodes.  While at it, massage
comments a bit.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/mount.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: work/fs/sysfs/mount.c
===================================================================
--- work.orig/fs/sysfs/mount.c
+++ work/fs/sysfs/mount.c
@@ -43,19 +43,19 @@ static int sysfs_fill_super(struct super
 	sb->s_time_gran = 1;
 	sysfs_sb = sb;
 
-	inode = new_inode(sysfs_sb);
+	/* get root inode, initialize and unlock it */
+	inode = sysfs_get_inode(&sysfs_root);
 	if (!inode) {
 		pr_debug("sysfs: could not get root inode\n");
 		return -ENOMEM;
 	}
 
-	sysfs_init_inode(&sysfs_root, inode);
-
 	inode->i_op = &sysfs_dir_inode_operations;
 	inode->i_fop = &sysfs_dir_operations;
-	/* directory inodes start off with i_nlink == 2 (for "." entry) */
-	inc_nlink(inode);
+	inc_nlink(inode); /* directory, account for "." */
+	unlock_new_inode(inode);
 
+	/* instantiate and link root dentry */
 	root = d_alloc_root(inode);
 	if (!root) {
 		pr_debug("%s: could not get root dentry!\n",__FUNCTION__);

[PATCH] sysfs: make sysfs_init_inode() static

[Tejun Heo]

With sysfs_fill_super() converted to use sysfs_get_inode(), there is
no user of sysfs_init_inode() outside of fs/sysfs/inode.c.  Make it
static.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/inode.c |    2 +-
 fs/sysfs/sysfs.h |    1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

Index: work/fs/sysfs/inode.c
===================================================================
--- work.orig/fs/sysfs/inode.c
+++ work/fs/sysfs/inode.c
@@ -133,7 +133,7 @@ static inline void set_inode_attr(struct
  */
 static struct lock_class_key sysfs_inode_imutex_key;
 
-void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
+static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
 {
 	inode->i_blocks = 0;
 	inode->i_mapping->a_ops = &sysfs_aops;
Index: work/fs/sysfs/sysfs.h
===================================================================
--- work.orig/fs/sysfs/sysfs.h
+++ work/fs/sysfs/sysfs.h
@@ -71,7 +71,6 @@ extern void sysfs_remove_one(struct sysf
 extern int sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt);
 
 extern void sysfs_delete_inode(struct inode *inode);
-extern void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode);
 extern struct inode * sysfs_get_inode(struct sysfs_dirent *sd);
 extern void sysfs_instantiate(struct dentry *dentry, struct inode *inode);
 

Re: [PATCH] sysfs: fix sysfs root inode nlink accounting

[Jean Delvare]

Hi Tejun,

On Wed, 18 Jul 2007 14:29:06 +0900, Tejun Heo wrote:
> While making sysfs indoes hashed, sysfs root inode was left out.  Now
> that nlink accounting depends on the inode being on the hash, sysfs
> root inode nlink isn't adjusted properly.
> 
> Put sysfs root inode on the inode hash by allocating it using
> sysfs_get_inode() like other sysfs inodes.  While at it, massage
> comments a bit.

This fixed my problem as expected. Thanks a lot!

> Signed-off-by: Tejun Heo <htejun@gmail.com>

For what it's worth:

Acked-by: Jean Delvare <khali@linux-fr.org>

> ---
>  fs/sysfs/mount.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> Index: work/fs/sysfs/mount.c
> ===================================================================
> --- work.orig/fs/sysfs/mount.c
> +++ work/fs/sysfs/mount.c
> @@ -43,19 +43,19 @@ static int sysfs_fill_super(struct super
>  	sb->s_time_gran = 1;
>  	sysfs_sb = sb;
>  
> -	inode = new_inode(sysfs_sb);
> +	/* get root inode, initialize and unlock it */
> +	inode = sysfs_get_inode(&sysfs_root);
>  	if (!inode) {
>  		pr_debug("sysfs: could not get root inode\n");
>  		return -ENOMEM;
>  	}
>  
> -	sysfs_init_inode(&sysfs_root, inode);
> -
>  	inode->i_op = &sysfs_dir_inode_operations;
>  	inode->i_fop = &sysfs_dir_operations;
> -	/* directory inodes start off with i_nlink == 2 (for "." entry) */
> -	inc_nlink(inode);
> +	inc_nlink(inode); /* directory, account for "." */
> +	unlock_new_inode(inode);
>  
> +	/* instantiate and link root dentry */
>  	root = d_alloc_root(inode);
>  	if (!root) {
>  		pr_debug("%s: could not get root dentry!\n",__FUNCTION__);


-- 
Jean Delvare

Re: [PATCH] sysfs: make sysfs_init_inode() static

[Jean Delvare]

On Wed, 18 Jul 2007 14:30:28 +0900, Tejun Heo wrote:
> With sysfs_fill_super() converted to use sysfs_get_inode(), there is
> no user of sysfs_init_inode() outside of fs/sysfs/inode.c.  Make it
> static.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

Looks good.

Acked-by: Jean Delvare <khali@linux-fr.org>

> ---
>  fs/sysfs/inode.c |    2 +-
>  fs/sysfs/sysfs.h |    1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> Index: work/fs/sysfs/inode.c
> ===================================================================
> --- work.orig/fs/sysfs/inode.c
> +++ work/fs/sysfs/inode.c
> @@ -133,7 +133,7 @@ static inline void set_inode_attr(struct
>   */
>  static struct lock_class_key sysfs_inode_imutex_key;
>  
> -void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
> +static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
>  {
>  	inode->i_blocks = 0;
>  	inode->i_mapping->a_ops = &sysfs_aops;
> Index: work/fs/sysfs/sysfs.h
> ===================================================================
> --- work.orig/fs/sysfs/sysfs.h
> +++ work/fs/sysfs/sysfs.h
> @@ -71,7 +71,6 @@ extern void sysfs_remove_one(struct sysf
>  extern int sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt);
>  
>  extern void sysfs_delete_inode(struct inode *inode);
> -extern void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode);
>  extern struct inode * sysfs_get_inode(struct sysfs_dirent *sd);
>  extern void sysfs_instantiate(struct dentry *dentry, struct inode *inode);
>  


-- 
Jean Delvare

Re: sysfs root link count broken in 2.6.22-git5

[Jean Delvare]

Hi Greg,

On Tue, 17 Jul 2007 20:38:28 -0700, Greg KH wrote:
> On Tue, Jul 17, 2007 at 11:05:30PM +0200, Jean Delvare wrote:
> > This breaks libsensors. libsensors uses libsysfs, and libsysfs is not
> > very smart in that it will initialize successfully even if sysfs is not
> > mounted.
> 
> libsysfs isn't smart at all, and isn't even supported anymore.  I'd
> really suggest droping it entirely, it isn't worth it.

Agreed, except that I do not have the time for this right now. I want
to get lm-sensors-3.0.0 ready for a release candidate first. What
really matters for this is to get the API ready. Implementation details
will come later.

> > So I added tests after the initialization, to make sure that
> > sysfs is really there. These tests are:
> > * The mount point exists.
> > * The mount point is really mounted.
> 
> Do you know of a 2.6 based distro that does not mount sysfs at /sys?  We
> took that check out a long time ago in udev and no one has complained :)

I don't know of any 2.6-based distro not mounting sysfs at /sys, but I
know of 2.4-based distros not mounting sysfs at all ;) libsensors
supports both 2.4 and 2.6 kernels, so being able to tell whether sysfs
is mounted or not, matters.

> > The code looks like:
> > 
> >        if (sysfs_get_mnt_path(sensors_sysfs_mount, NAME_MAX)
> >          || stat(sensors_sysfs_mount, &statbuf) < 0
> >          || statbuf.st_nlink <= 2)      /* Empty directory */
> >                 return 0;		/* Failure */
> > 
> > This works OK with 2.6.22.1, but the last test fails with the current
> > git kernel even when sysfs is mounted.
> 
> Yeah, but is checking the number of hard links in the directory a safe
> way to always verify that it isn't empty?

I think so, yes. To the best of my knowledge, it has worked on all
Unix-like systems for decades. There are other ways, but this is by far
the less expensive.

>                                            Isn't there some glibc
> function that can detect the mount point of a filesystem or directory?
> Something in glibc parses /proc/mounts for something, I can't remember
> what it is right now though, sorry.

Maybe getmntent(3)? Sure I could use this, but how expensive compared
to a single stat(2).

> Again, I recommend dropping libsysfs, it's gone from some distros
> already :)

Really? I'm curious how such distributions support libsensors and the
other tools which still rely on libsysfs. If they have already
converted libsensors for me, that would be good news :)

> And yes, the bug should be fixed, I agree.  Thanks for letting us know.

Tejun already fixed it, that was quick :)

-- 
Jean Delvare

Re: sysfs root link count broken in 2.6.22-git5

[Andreas Schwab]

Jean Delvare <khali@linux-fr.org> writes:

> On Tue, 17 Jul 2007 20:38:28 -0700, Greg KH wrote:
>>                                            Isn't there some glibc
>> function that can detect the mount point of a filesystem or directory?
>> Something in glibc parses /proc/mounts for something, I can't remember
>> what it is right now though, sorry.
>
> Maybe getmntent(3)? Sure I could use this, but how expensive compared
> to a single stat(2).

How about comparing the device numbers of /sys and its parent?  If they
are different then /sys is a mount point.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: sysfs root link count broken in 2.6.22-git5

[Oliver Pinter]

On 7/18/07, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Greg,
>
> On Tue, 17 Jul 2007 20:38:28 -0700, Greg KH wrote:
> > On Tue, Jul 17, 2007 at 11:05:30PM +0200, Jean Delvare wrote:
> > > This breaks libsensors. libsensors uses libsysfs, and libsysfs is not
> > > very smart in that it will initialize successfully even if sysfs is not
> > > mounted.
> >
> > libsysfs isn't smart at all, and isn't even supported anymore.  I'd
> > really suggest droping it entirely, it isn't worth it.
>
> Agreed, except that I do not have the time for this right now. I want
> to get lm-sensors-3.0.0 ready for a release candidate first. What
> really matters for this is to get the API ready. Implementation details
> will come later.
>
> > > So I added tests after the initialization, to make sure that
> > > sysfs is really there. These tests are:
> > > * The mount point exists.
> > > * The mount point is really mounted.
> >
> > Do you know of a 2.6 based distro that does not mount sysfs at /sys?  We
> > took that check out a long time ago in udev and no one has complained :)
>
> I don't know of any 2.6-based distro not mounting sysfs at /sys, but I
> know of 2.4-based distros not mounting sysfs at all ;) libsensors
> supports both 2.4 and 2.6 kernels, so being able to tell whether sysfs
> is mounted or not, matters.

debian has mounted the sysfs

/etc/rcS.d/S02mountkernfs.sh:

#
	# Mount sysfs on /sys
	#
	# Only mount sysfs if it is supported (kernel >= 2.6)
	if grep -E -qs "sysfs\$" /proc/filesystems
	then
		domount sysfs "" /sys sysfs -onodev,noexec,nosuid
	fi

	# Mount /var/run and /var/lock as tmpfs if enabled
	if [ yes = "$RAMRUN" ] ; then
		RUN_OPT=
		[ "${RUN_SIZE:=$TMPFS_SIZE}" ] && RUN_OPT=",size=$RUN_SIZE"
		domount tmpfs "" /var/run varrun -omode=0755,nosuid$RUN_OPT
		touch /var/run/.ramfs
	fi
	if [ yes = "$RAMLOCK" ] ; then
		LOCK_OPT=
		[ "${LOCK_SIZE:=$TMPFS_SIZE}" ] && LOCK_OPT=",size=$LOCK_SIZE"
		domount tmpfs "" /var/lock varlock -omode=1777,nodev,noexec,nosuid$LOCK_OPT
		touch /var/lock/.ramfs
	fi


>
> > > The code looks like:
> > >
> > >        if (sysfs_get_mnt_path(sensors_sysfs_mount, NAME_MAX)
> > >          || stat(sensors_sysfs_mount, &statbuf) < 0
> > >          || statbuf.st_nlink <= 2)      /* Empty directory */
> > >                 return 0;           /* Failure */
> > >
> > > This works OK with 2.6.22.1, but the last test fails with the current
> > > git kernel even when sysfs is mounted.
> >
> > Yeah, but is checking the number of hard links in the directory a safe
> > way to always verify that it isn't empty?
>
> I think so, yes. To the best of my knowledge, it has worked on all
> Unix-like systems for decades. There are other ways, but this is by far
> the less expensive.
>
> >                                            Isn't there some glibc
> > function that can detect the mount point of a filesystem or directory?
> > Something in glibc parses /proc/mounts for something, I can't remember
> > what it is right now though, sorry.
>
> Maybe getmntent(3)? Sure I could use this, but how expensive compared
> to a single stat(2).
>
> > Again, I recommend dropping libsysfs, it's gone from some distros
> > already :)
>
> Really? I'm curious how such distributions support libsensors and the
> other tools which still rely on libsysfs. If they have already
> converted libsensors for me, that would be good news :)
>
> > And yes, the bug should be fixed, I agree.  Thanks for letting us know.
>
> Tejun already fixed it, that was quick :)
>
> --
> Jean Delvare
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: sysfs root link count broken in 2.6.22-git5

[Kay Sievers]

On 7/18/07, Jean Delvare <khali@linux-fr.org> wrote:
> On Tue, 17 Jul 2007 20:38:28 -0700, Greg KH wrote:
> > On Tue, Jul 17, 2007 at 11:05:30PM +0200, Jean Delvare wrote:
> > > This breaks libsensors. libsensors uses libsysfs, and libsysfs is not
> > > very smart in that it will initialize successfully even if sysfs is not
> > > mounted.
> >
> > libsysfs isn't smart at all, and isn't even supported anymore.  I'd
> > really suggest droping it entirely, it isn't worth it.
>
> Agreed, except that I do not have the time for this right now. I want
> to get lm-sensors-3.0.0 ready for a release candidate first. What
> really matters for this is to get the API ready. Implementation details
> will come later.
>
> > > So I added tests after the initialization, to make sure that
> > > sysfs is really there. These tests are:
> > > * The mount point exists.
> > > * The mount point is really mounted.
> >
> > Do you know of a 2.6 based distro that does not mount sysfs at /sys?  We
> > took that check out a long time ago in udev and no one has complained :)
>
> I don't know of any 2.6-based distro not mounting sysfs at /sys, but I
> know of 2.4-based distros not mounting sysfs at all ;) libsensors
> supports both 2.4 and 2.6 kernels, so being able to tell whether sysfs
> is mounted or not, matters.
>
> > > The code looks like:
> > >
> > >        if (sysfs_get_mnt_path(sensors_sysfs_mount, NAME_MAX)
> > >          || stat(sensors_sysfs_mount, &statbuf) < 0
> > >          || statbuf.st_nlink <= 2)      /* Empty directory */
> > >                 return 0;           /* Failure */
> > >
> > > This works OK with 2.6.22.1, but the last test fails with the current
> > > git kernel even when sysfs is mounted.
> >
> > Yeah, but is checking the number of hard links in the directory a safe
> > way to always verify that it isn't empty?
>
> I think so, yes. To the best of my knowledge, it has worked on all
> Unix-like systems for decades. There are other ways, but this is by far
> the less expensive.

Well, just check if /sys/devices/ exists, that should be cheap enough. :)

Kay

Re: sysfs root link count broken in 2.6.22-git5

[Jean Delvare]

Hi Andreas,

On Wed, 18 Jul 2007 22:12:52 +0200, Andreas Schwab wrote:
> Jean Delvare <khali@linux-fr.org> writes:
> > Maybe getmntent(3)? Sure I could use this, but how expensive compared
> > to a single stat(2).
> 
> How about comparing the device numbers of /sys and its parent?  If they
> are different then /sys is a mount point.

Nice trick, thanks. It's slightly more expensive than my current code
(two calls to stat(2) instead of one) but also more reliable. I'll
consider it when we get rid of libsysfs.

-- 
Jean Delvare

Re: sysfs root link count broken in 2.6.22-git5

[Jean Delvare]

Hi Kay,

On Thu, 19 Jul 2007 02:44:54 +0200, Kay Sievers wrote:
> On 7/18/07, Jean Delvare <khali@linux-fr.org> wrote:
> > On Tue, 17 Jul 2007 20:38:28 -0700, Greg KH wrote:
> > > On Tue, Jul 17, 2007 at 11:05:30PM +0200, Jean Delvare wrote:
> > > > The code looks like:
> > > >
> > > >        if (sysfs_get_mnt_path(sensors_sysfs_mount, NAME_MAX)
> > > >          || stat(sensors_sysfs_mount, &statbuf) < 0
> > > >          || statbuf.st_nlink <= 2)      /* Empty directory */
> > > >                 return 0;           /* Failure */
> > > >
> > > > This works OK with 2.6.22.1, but the last test fails with the current
> > > > git kernel even when sysfs is mounted.
> > >
> > > Yeah, but is checking the number of hard links in the directory a safe
> > > way to always verify that it isn't empty?
> >
> > I think so, yes. To the best of my knowledge, it has worked on all
> > Unix-like systems for decades. There are other ways, but this is by far
> > the less expensive.
> 
> Well, just check if /sys/devices/ exists, that should be cheap enough. :)

Yes, this is a possibility, and one I had considered at first. But I
wasn't sure which subdirectory to check. sysfs isn't well known for its
stability, and I didn't know which directories exist since the
early days of sysfs, and which do not. For example, fs, kernel and
module were not present in 2.6.5. I am also not sure if directories
which exist today are guaranteed to exist forever. This is the reason
why I decided to check the link count instead, basically checking that
at least one subdirectory exists, without having to name it.

-- 
Jean Delvare

Re: sysfs root link count broken in 2.6.22-git5

[Jan Engelhardt]

On Jul 19 2007 10:41, Jean Delvare wrote:
>> Well, just check if /sys/devices/ exists, that should be cheap enough. :)
>
>Yes, this is a possibility, and one I had considered at first. But I
>wasn't sure which subdirectory to check. sysfs isn't well known for its
>stability, and I didn't know which directories exist since the
>early days of sysfs, and which do not. For example, fs, kernel and
>module were not present in 2.6.5. I am also not sure if directories
>which exist today are guaranteed to exist forever. This is the reason
>why I decided to check the link count instead, basically checking that
>at least one subdirectory exists, without having to name it.

If anything exists, it is likely to be mounted.
opendir, 3x readdir, exit the loop, done :)


	Jan
--