All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jean Delvare <khali@linux-fr.org>
To: Greg KH <gregkh@suse.de>
Cc: Tejun Heo <htejun@gmail.com>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: sysfs root link count broken in 2.6.22-git5
Date: Wed, 18 Jul 2007 22:06:52 +0200	[thread overview]
Message-ID: <20070718220652.15ad2c4b@hyperion.delvare> (raw)
In-Reply-To: <20070718033828.GA8582@suse.de>

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

  reply	other threads:[~2007-07-18 20:05 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-15 10:42 sysfs root link count broken in 2.6.22-git5 Jean Delvare
2007-07-17  3:48 ` Greg KH
2007-07-17 11:12   ` Jean Delvare
2007-07-17 18:36     ` Greg KH
2007-07-17 21:05       ` Jean Delvare
2007-07-18  3:05         ` Tejun Heo
2007-07-18  3:38         ` Greg KH
2007-07-18 20:06           ` Jean Delvare [this message]
2007-07-18 20:12             ` Andreas Schwab
2007-07-19  7:42               ` Jean Delvare
2007-07-18 21:21             ` Oliver Pinter
2007-07-19  0:44             ` Kay Sievers
2007-07-19  8:41               ` Jean Delvare
2007-07-19 16:02                 ` Jan Engelhardt
2007-07-18  5:29         ` [PATCH] sysfs: fix sysfs root inode nlink accounting Tejun Heo
2007-07-18  5:30           ` [PATCH] sysfs: make sysfs_init_inode() static Tejun Heo
2007-07-18 14:04             ` Jean Delvare
2007-07-18 14:02           ` [PATCH] sysfs: fix sysfs root inode nlink accounting Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070718220652.15ad2c4b@hyperion.delvare \
    --to=khali@linux-fr.org \
    --cc=gregkh@suse.de \
    --cc=htejun@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.