mirror of https://lore.kernel.org/lkml/
 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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome