Linux Hotplug development
 help / color / mirror / Atom feed
* Re: [PATCH] configure: allow usb.ids location to be specified
From: Scott James Remnant @ 2011-05-23 19:37 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <1305926367-3093-1-git-send-email-scott@netsplit.com>

Pushed with the C&P fix.

On Mon, May 23, 2011 at 1:48 AM, Kay Sievers <kay.sievers@vrfy.org> wrote:
>
> On Fri, May 20, 2011 at 23:19, Scott James Remnant <scott@netsplit.com> wrote:
> > We already allow the pci.ids location to be specified, so add a
> > patch doing the same for usb.ids. Please don't make me explain
> > why this is necessary, it will only make you cry.
>
> I'll not ask. :) Sure, go ahead.
>
> Kay

^ permalink raw reply

* Re: [PATCH] configure: allow usb.ids location to be specified
From: Kay Sievers @ 2011-05-23  8:48 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <1305926367-3093-1-git-send-email-scott@netsplit.com>

On Fri, May 20, 2011 at 23:19, Scott James Remnant <scott@netsplit.com> wrote:
> We already allow the pci.ids location to be specified, so add a
> patch doing the same for usb.ids. Please don't make me explain
> why this is necessary, it will only make you cry.

I'll not ask. :) Sure, go ahead.

Kay

^ permalink raw reply

* Re: [PATCH] configure: allow usb.ids location to be specified
From: Greg KH @ 2011-05-20 22:05 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <1305926367-3093-1-git-send-email-scott@netsplit.com>

On Fri, May 20, 2011 at 02:19:27PM -0700, Scott James Remnant wrote:
> We already allow the pci.ids location to be specified, so add a
> patch doing the same for usb.ids. Please don't make me explain
> why this is necessary, it will only make you cry.
> 
> Signed-off-by: Scott James Remnant <scott@netsplit.com>

As the maintainer of the usbutils package, I too cry when I see where
this file ends up :(

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>


^ permalink raw reply

* [PATCH] configure: allow usb.ids location to be specified
From: Scott James Remnant @ 2011-05-20 21:19 UTC (permalink / raw)
  To: linux-hotplug

We already allow the pci.ids location to be specified, so add a
patch doing the same for usb.ids. Please don't make me explain
why this is necessary, it will only make you cry.

Signed-off-by: Scott James Remnant <scott@netsplit.com>
---
 configure.ac |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3646f93..11e5bc5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -93,8 +93,18 @@ if test "x$enable_hwdb" = xyes; then
 		AC_CHECK_FILES([/usr/share/misc/pci.ids], [pciids=/usr/share/misc/pci.ids])
 	fi
 
-	PKG_CHECK_MODULES(USBUTILS, usbutils >= 0.82)
-	AC_SUBST([USB_DATABASE], [$($PKG_CONFIG --variable=usbids usbutils)])
+	AC_ARG_WITH(usb-ids-path,
+		[AS_HELP_STRING([--with-usb-ids-path=DIR], [Path to usb.ids file])],
+		[USB_DATABASE=${withval}],
+		[if test -n "$usbids" ; then
+			PCI_DATABASE="$usbids"
+		else
+			PKG_CHECK_MODULES(USBUTILS, usbutils >= 0.82)
+			AC_SUBST([USB_DATABASE], [$($PKG_CONFIG --variable=usbids usbutils)])
+		fi])
+	AC_MSG_CHECKING([for USB database location])
+	AC_MSG_RESULT([$USB_DATABASE])
+	AC_SUBST(USB_DATABASE)
 
 	AC_ARG_WITH(pci-ids-path,
 		[AS_HELP_STRING([--with-pci-ids-path=DIR], [Path to pci.ids file])],
-- 
1.7.3.1


^ permalink raw reply related

* Re: [PATCH] configure.ac: fixes for rule_generator and modeswitch
From: Kay Sievers @ 2011-05-19 11:39 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <1305804917-7421-1-git-send-email-soltys@ziu.info>

On Thu, May 19, 2011 at 13:35, Michal Soltys <soltys@ziu.info> wrote:
> Signed-off-by: Michal Soltys <soltys@ziu.info>
> ---
>  configure.ac |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

Applied.

Thanks,
Kay

^ permalink raw reply

* [PATCH] configure.ac: fixes for rule_generator and modeswitch
From: Michal Soltys @ 2011-05-19 11:35 UTC (permalink / raw)
  To: linux-hotplug

Signed-off-by: Michal Soltys <soltys@ziu.info>
---
 configure.ac |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d9947ef..3a2d927 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,10 +75,10 @@ AM_CONDITIONAL(WITH_SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_sy
 # ------------------------------------------------------------------------------
 # rule_generator - persistent network and optical device rule generator
 # ------------------------------------------------------------------------------
-AC_ARG_ENABLE([hwdb],
+AC_ARG_ENABLE([rule_generator],
 	AS_HELP_STRING([--disable-rule_generator], [disable persistent network, cdrom support]),
 	[], [enable_rule_generator=yes])
-AM_CONDITIONAL([ENABLE_RULE_GENERATOR], [test "x$enable_rule_genarator" = xyes])
+AM_CONDITIONAL([ENABLE_RULE_GENERATOR], [test "x$enable_rule_generator" = xyes])
 
 # ------------------------------------------------------------------------------
 # usb/pci-db - read vendor/device string database
@@ -190,7 +190,7 @@ AM_CONDITIONAL([ENABLE_EDD], [test "x$enable_edd" = xyes])
 # ------------------------------------------------------------------------------
 # mobile-action-modeswitch - switch Mobile Action cables into serial mode
 # ------------------------------------------------------------------------------
-AC_ARG_ENABLE([ACTION_MODESWITCH],
+AC_ARG_ENABLE([action_modeswitch],
 	AS_HELP_STRING([--enable-action_modeswitch], [enable action modeswitch support]),
 	[], [enable_action_modeswitch=no])
 if test "x$enable_action_modeswitch" = xyes; then
-- 
1.7.4.1


^ permalink raw reply related

* Re: future of sysctls?
From: Lennart Poettering @ 2011-05-18 17:32 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201105121741.27459.ludwig.nussel@suse.de>

On Wed, 18.05.11 09:03, Ludwig Nussel (ludwig.nussel@suse.de) wrote:

> > > > Might be a good idea to just ignore these kinds of settings. Or if this
> > > > is not possible, then set them from NM or whatever controls the network.
> > > 
> > > That's that hack that's currently in place. Network scripts grep
> > > /etc/sysctl.conf for interface specific settings...
> > 
> > Urks. What we could do to make this nicer is add a simple prefix match
> > logic to our sysctl apply tool, so that it is easy to apply a subtree of
> > sysctls when the time comes.
> 
> I've sent a patch to the procps maintainer but he has yet to
> respond. It's not a real solution anyways. It just makes a dirty
> hack a little more efficient.

Note that systemd does not use the procps' implementation of sysctl, but
our own one since the upstream version does not support /etc/sysctl.d/
or anything like this.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

^ permalink raw reply

* Re: future of sysctls?
From: Ludwig Nussel @ 2011-05-18  7:03 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201105121741.27459.ludwig.nussel@suse.de>

Lennart Poettering wrote:
> On Tue, 17.05.11 09:15, Ludwig Nussel (ludwig.nussel@suse.de) wrote:
> > Lennart Poettering wrote:
> > > On Thu, 12.05.11 17:41, Ludwig Nussel (ludwig.nussel@suse.de) wrote:
> > > > I'm currently struggling to find a sane way to set
> > > > net.ipv6.conf.default.use_tempaddr.
> > > > Traditionally at some point during boot "sysctl -e -q -p /etc/sysctl.conf" is
> > > > called. That doesn't really work out anymore. The aforementioned setting needs
> > > > to be applied after the ipv6 module is loaded (could be compiled into the
> > > > kernel too though) otherwise it wouldn't apply. It needs to be set before a
> > > > network driver is loaded though as the default value is copied to
> > > > interfaces specific settings at interface creation time. On top of
> > > > that there are also network interface specific sysctls that need to
> > > > be applied after an interface is created (e.g.
> > > > net.ipv6.conf.eth0.use_tempaddr).
> > > 
> > > Something like this is kinda broken anyway, since it is racy: you can
> > > apply the sysctl only after the interface is already available.
> > 
> > Exactly.
> > 
> > > Might be a good idea to just ignore these kinds of settings. Or if this
> > > is not possible, then set them from NM or whatever controls the network.
> > 
> > That's that hack that's currently in place. Network scripts grep
> > /etc/sysctl.conf for interface specific settings...
> 
> Urks. What we could do to make this nicer is add a simple prefix match
> logic to our sysctl apply tool, so that it is easy to apply a subtree of
> sysctls when the time comes.

I've sent a patch to the procps maintainer but he has yet to
respond. It's not a real solution anyways. It just makes a dirty
hack a little more efficient.

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\
 V_/_  http://www.suse.de/
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 

^ permalink raw reply

* Re: [PULL] Docs: udev.xml: Copyediting
From: Kay Sievers @ 2011-05-17 23:07 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <5cb4f3ce-b6e7-4c2a-b175-2081daad3acb-mfwitten@gmail.com>

On Mon, May 16, 2011 at 04:29, Michael Witten <mfwitten@gmail.com> wrote:
> On Fri, Apr 15, 2011 at 17:41, Michael Witten <mfwitten@gmail.com> wrote:
>> On Mon, 11 Apr 2011 06:31:30 +0000, Michael Witten wrote:
>>
>>> I did some copyediting whilst looking through udev/udev.xml;
>>> this email contains a pull request as well as the inlined
>>> diff of all the changes squashed together.

Applied. Very nice. Thanks a lot.

Kay

^ permalink raw reply

* Re: future of sysctls?
From: Lennart Poettering @ 2011-05-17 10:21 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201105121741.27459.ludwig.nussel@suse.de>

On Tue, 17.05.11 09:15, Ludwig Nussel (ludwig.nussel@suse.de) wrote:

> 
> Lennart Poettering wrote:
> > On Thu, 12.05.11 17:41, Ludwig Nussel (ludwig.nussel@suse.de) wrote:
> > > I'm currently struggling to find a sane way to set
> > > net.ipv6.conf.default.use_tempaddr.
> > > Traditionally at some point during boot "sysctl -e -q -p /etc/sysctl.conf" is
> > > called. That doesn't really work out anymore. The aforementioned setting needs
> > > to be applied after the ipv6 module is loaded (could be compiled into the
> > > kernel too though) otherwise it wouldn't apply. It needs to be set before a
> > > network driver is loaded though as the default value is copied to
> > > interfaces specific settings at interface creation time. On top of
> > > that there are also network interface specific sysctls that need to
> > > be applied after an interface is created (e.g.
> > > net.ipv6.conf.eth0.use_tempaddr).
> > 
> > Something like this is kinda broken anyway, since it is racy: you can
> > apply the sysctl only after the interface is already available.
> 
> Exactly.
> 
> > Might be a good idea to just ignore these kinds of settings. Or if this
> > is not possible, then set them from NM or whatever controls the network.
> 
> That's that hack that's currently in place. Network scripts grep
> /etc/sysctl.conf for interface specific settings...

Urks. What we could do to make this nicer is add a simple prefix match
logic to our sysctl apply tool, so that it is easy to apply a subtree of
sysctls when the time comes.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

^ permalink raw reply

* Re: future of sysctls?
From: Ludwig Nussel @ 2011-05-17  7:15 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201105121741.27459.ludwig.nussel@suse.de>

Lennart Poettering wrote:
> On Thu, 12.05.11 17:41, Ludwig Nussel (ludwig.nussel@suse.de) wrote:
> > I'm currently struggling to find a sane way to set
> > net.ipv6.conf.default.use_tempaddr.
> > Traditionally at some point during boot "sysctl -e -q -p /etc/sysctl.conf" is
> > called. That doesn't really work out anymore. The aforementioned setting needs
> > to be applied after the ipv6 module is loaded (could be compiled into the
> > kernel too though) otherwise it wouldn't apply. It needs to be set before a
> > network driver is loaded though as the default value is copied to
> > interfaces specific settings at interface creation time. On top of
> > that there are also network interface specific sysctls that need to
> > be applied after an interface is created (e.g.
> > net.ipv6.conf.eth0.use_tempaddr).
> 
> Something like this is kinda broken anyway, since it is racy: you can
> apply the sysctl only after the interface is already available.

Exactly.

> Might be a good idea to just ignore these kinds of settings. Or if this
> is not possible, then set them from NM or whatever controls the network.

That's that hack that's currently in place. Network scripts grep
/etc/sysctl.conf for interface specific settings...

> > Are there any plans to better deal with that?
> > Like e.g. emitting events when some part of the kernel registers a sysctl so
> > userspace can override the compiled in default value?
> > Or just offer sysfs attributes instead of sysctls?
> 
> In a systemd world the ipv6 module is loaded very early and hence the
> sysctl should always be available, no special setup needed. If the same
> problem appears in real life with other modules too, then we could order
> sysctl setting after module loading and fix things by this.

Grepping for register_sysctl in the kernel sources shows quite a few modules
that use sysctls. A prominent one is nfs.
If you apply sysctl setttings after module loading, specifically network
drivers, the ipv6 setting won't have any effect anymore though.

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\
 V_/_  http://www.suse.de/
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 

^ permalink raw reply

* Re: [PATCH] Fix crash on termination before udev queue initialization
From: Nix @ 2011-05-16 21:14 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <87wrhqbcr4.fsf_-_@spindle.srvr.nix>

On 16 May 2011, Kay Sievers stated:
>> --- a/udev/udevd.c
>> +++ b/udev/udevd.c
>> @@ -69,7 +69,7 @@ static void log_fn(struct udev *udev, int priority,
>>  }
>>
>>  static struct udev_rules *rules;
>> -static struct udev_queue_export *udev_queue_export;
>> +static struct udev_queue_export *udev_queue_export = NULL;
>
> That's not needed for static vars.

Oh yes. *slaps self*

Obviously I am too tired to be working on Hello World, let alone stuff
running as root...

btw, that patch might have worked. I had a trouble-free boot without
pausing this time... but of course this is an intermittent bug so it
is hard to be sure.

I'll yell if it goes wrong again.

-- 
NULL && (void)

^ permalink raw reply

* Re: [PATCH] Fix crash on termination before udev queue initialization
From: Kay Sievers @ 2011-05-16 20:56 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <87wrhqbcr4.fsf_-_@spindle.srvr.nix>

On Mon, May 16, 2011 at 22:40, Nix <nix@esperi.org.uk> wrote:
> On 16 May 2011, Kay Sievers stated:
>
>> Commited a fix to git, that moves the creation of the queue file above
>> the daemonize. Hopefully that's the race you are seeing.
>
> Hm, not done more than compile it yet, but there's definitely still
> something wrong here: it coredumps if called with invalid arguments.

> --- a/libudev/libudev-queue-private.c
> +++ b/libudev/libudev-queue-private.c
> @@ -104,6 +104,9 @@ void udev_queue_export_cleanup(struct udev_queue_export *udev_queue_export)
>  {
>        char filename[UTIL_PATH_SIZE];
>
> +       if (udev_queue_export = NULL)
> +               return;
> +

Committed.

> --- a/udev/udevd.c
> +++ b/udev/udevd.c
> @@ -69,7 +69,7 @@ static void log_fn(struct udev *udev, int priority,
>  }
>
>  static struct udev_rules *rules;
> -static struct udev_queue_export *udev_queue_export;
> +static struct udev_queue_export *udev_queue_export = NULL;

That's not needed for static vars.

Thanks,
Kay

^ permalink raw reply

* [PATCH] Fix crash on termination before udev queue initialization
From: Nix @ 2011-05-16 20:40 UTC (permalink / raw)
  To: linux-hotplug

On 16 May 2011, Kay Sievers stated:

> Commited a fix to git, that moves the creation of the queue file above
> the daemonize. Hopefully that's the race you are seeing.

Hm, not done more than compile it yet, but there's definitely still
something wrong here: it coredumps if called with invalid arguments.

mutilate:/lib/udev/rules.d# gdb --args /sbin/udevd --no-daemon
[...]
Program received signal SIGSEGV, Segmentation fault.
0x0000000000418a0c in udev_queue_export_cleanup ()
(gdb) bt
#0  udev_queue_export_cleanup (udev_queue_export=0x0) at libudev/libudev-queue-private.c:107
#1  0x00007fffffffc130 in ?? ()
#2  0x00007fffffffc070 in ?? ()
#3  0x0000000000000000 in ?? ()

Fixed by initializing the queue variable to NULL at startup, and making
udev_queue_export_cleanup() just return if called with NULL, like
free().
---
 libudev/libudev-queue-private.c |    3 +++
 udev/udevd.c                    |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/libudev/libudev-queue-private.c b/libudev/libudev-queue-private.c
index 0dcf9b0..747d555 100644
--- a/libudev/libudev-queue-private.c
+++ b/libudev/libudev-queue-private.c
@@ -104,6 +104,9 @@ void udev_queue_export_cleanup(struct udev_queue_export *udev_queue_export)
 {
 	char filename[UTIL_PATH_SIZE];
 
+	if (udev_queue_export = NULL)
+		return;
+
 	util_strscpyl(filename, sizeof(filename), udev_get_run_path(udev_queue_export->udev), "/queue.tmp", NULL);
 	unlink(filename);
 
diff --git a/udev/udevd.c b/udev/udevd.c
index e7384e1..937717a 100644
--- a/udev/udevd.c
+++ b/udev/udevd.c
@@ -69,7 +69,7 @@ static void log_fn(struct udev *udev, int priority,
 }
 
 static struct udev_rules *rules;
-static struct udev_queue_export *udev_queue_export;
+static struct udev_queue_export *udev_queue_export = NULL;
 static struct udev_ctrl *udev_ctrl;
 static struct udev_monitor *monitor;
 static int worker_watch[2] = { -1, -1 };
-- 
1.7.5.1.133.geb78b

^ permalink raw reply related

* Re: udevadm settle persistently failing
From: Nix @ 2011-05-16 12:47 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On 16 May 2011, Kay Sievers stated:
> Commited a fix to git, that moves the creation of the queue file above
> the daemonize. Hopefully that's the race you are seeing.

I'll try it later today, and scatter printf()s through the code to see
what path it's taking out if that doesn't work.

-- 
NULL && (void)

^ permalink raw reply

* Re: udevadm settle persistently failing
From: Kay Sievers @ 2011-05-16 11:28 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On Mon, May 16, 2011 at 12:51, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Mon, May 16, 2011 at 12:01, Nix <nix@esperi.org.uk> wrote:
>> On 16 May 2011, Kay Sievers outgrape:
>>> With devtmpfs you get the "early" /dev for free, and you preserve the
>>> "early" /dev in the real root. Instead of mounting a 'tmpfs' just
>>> mount a 'devtmpfs' at /dev, and it will be magically filled already,
>>> that's the only difference.
>>>
>>> When mounting the real root, just mount --move it over to the real
>>> root before switching to the real root.
>>
>> I'll have to see what busybox's switch_root does. I know it deletes
>> the entire contents of the root directory it switches away from: I'll
>> have to make sure that doesn't somehow include moved-away mountpoints.
>> (I doubt it does.)
>>
>>> Maybe you can run:
>>>   udevadm monitor
>>> in the background writing to a file, and get timestamps of your commands too.
>>
>> They're fine.
>>
>> After a bunch of straces, I think the problem is different. As I
>> suspected from its intermittent nature, it's a race.
>>
>> When udevd starts up, it looks for an old database and converts it into
>> a new-style /run database. This takes nonzero time, and is done *after*
>> daemonization. Unfortunately, in the same time window it is quite
>> possible to fit a pair of 'udevadm trigger's and a 'udevadm settle': and
>> when udevadm settle kicks up and finds that udev hasn't even initialized
>> yet, what does it do? Well, unless I misread the code
>> udev_queue_get_queue_is_empty() returns 1 in this situation, so 'udevadm
>> settle' returns EXIT_SUCCESS, and boom.
>>
>> My evidence for this is very thin: it's no more than a hypothesis
>> really. My only evidence for this so far is that I haven't managed to
>> get the failure to happen unless I mkdir the old udev directories first
>> to force udev to do an old->new db conversion. It would be nice to
>> gather more data, but it's hard to do that without perturbing the race
>> :/ even an ls of /run slows it down enough that it doesn't happen
>> anymore. Hm, perhaps an echo would be fast enough, it's a shell
>> builtin after all...
>>
>> If I'm right (and I'm not sure I am, but it feels plausible), then I
>> suspect the only fix is to move the db conversion so that it happens
>> before daemonization.
>
> Hmm, the trigger should increase the kernel's current uevent seqnum.
> udev_queue_get_queue_is_empty() should see that there are events
> pending, not seen by udevd, and should make settle block until they
> are all handled.
>
> But maybe udev_queue_export_new() needs to move above the daemonize?
> It will create the initial queue file, that will make settle check for
> the udevd vs. kernel number. That could be the reason, for the race.

Commited a fix to git, that moves the creation of the queue file above
the daemonize. Hopefully that's the race you are seeing.

Kay

^ permalink raw reply

* Re: udevadm settle persistently failing
From: Kay Sievers @ 2011-05-16 10:51 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On Mon, May 16, 2011 at 12:01, Nix <nix@esperi.org.uk> wrote:
> On 16 May 2011, Kay Sievers outgrape:
>> With devtmpfs you get the "early" /dev for free, and you preserve the
>> "early" /dev in the real root. Instead of mounting a 'tmpfs' just
>> mount a 'devtmpfs' at /dev, and it will be magically filled already,
>> that's the only difference.
>>
>> When mounting the real root, just mount --move it over to the real
>> root before switching to the real root.
>
> I'll have to see what busybox's switch_root does. I know it deletes
> the entire contents of the root directory it switches away from: I'll
> have to make sure that doesn't somehow include moved-away mountpoints.
> (I doubt it does.)
>
>> Maybe you can run:
>>   udevadm monitor
>> in the background writing to a file, and get timestamps of your commands too.
>
> They're fine.
>
> After a bunch of straces, I think the problem is different. As I
> suspected from its intermittent nature, it's a race.
>
> When udevd starts up, it looks for an old database and converts it into
> a new-style /run database. This takes nonzero time, and is done *after*
> daemonization. Unfortunately, in the same time window it is quite
> possible to fit a pair of 'udevadm trigger's and a 'udevadm settle': and
> when udevadm settle kicks up and finds that udev hasn't even initialized
> yet, what does it do? Well, unless I misread the code
> udev_queue_get_queue_is_empty() returns 1 in this situation, so 'udevadm
> settle' returns EXIT_SUCCESS, and boom.
>
> My evidence for this is very thin: it's no more than a hypothesis
> really. My only evidence for this so far is that I haven't managed to
> get the failure to happen unless I mkdir the old udev directories first
> to force udev to do an old->new db conversion. It would be nice to
> gather more data, but it's hard to do that without perturbing the race
> :/ even an ls of /run slows it down enough that it doesn't happen
> anymore. Hm, perhaps an echo would be fast enough, it's a shell
> builtin after all...
>
> If I'm right (and I'm not sure I am, but it feels plausible), then I
> suspect the only fix is to move the db conversion so that it happens
> before daemonization.

Hmm, the trigger should increase the kernel's current uevent seqnum.
udev_queue_get_queue_is_empty() should see that there are events
pending, not seen by udevd, and should make settle block until they
are all handled.

But maybe udev_queue_export_new() needs to move above the daemonize?
It will create the initial queue file, that will make settle check for
the udevd vs. kernel number. That could be the reason, for the race.

Kay

^ permalink raw reply

* Re: udevadm settle persistently failing
From: Nix @ 2011-05-16 10:36 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On 16 May 2011, Marco d'Itri outgrape:

> On May 16, Nix <nix@esperi.org.uk> wrote:
>
>> My evidence for this is very thin: it's no more than a hypothesis
>> really. My only evidence for this so far is that I haven't managed to
>> get the failure to happen unless I mkdir the old udev directories first
>> to force udev to do an old->new db conversion. It would be nice to
> The Debian initramfs does not do this, so I do not think that this is
> the cause.

Curses. Oh well, back to the research drawing board.

-- 
NULL && (void)

^ permalink raw reply

* Re: udevadm settle persistently failing
From: Marco d'Itri @ 2011-05-16 10:22 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On May 16, Nix <nix@esperi.org.uk> wrote:

> My evidence for this is very thin: it's no more than a hypothesis
> really. My only evidence for this so far is that I haven't managed to
> get the failure to happen unless I mkdir the old udev directories first
> to force udev to do an old->new db conversion. It would be nice to
The Debian initramfs does not do this, so I do not think that this is
the cause.

-- 
ciao,
Marco

^ permalink raw reply

* Re: udevadm settle persistently failing
From: Nix @ 2011-05-16 10:01 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On 16 May 2011, Kay Sievers outgrape:
> With devtmpfs you get the "early" /dev for free, and you preserve the
> "early" /dev in the real root. Instead of mounting a 'tmpfs' just
> mount a 'devtmpfs' at /dev, and it will be magically filled already,
> that's the only difference.
> 
> When mounting the real root, just mount --move it over to the real
> root before switching to the real root.

I'll have to see what busybox's switch_root does. I know it deletes
the entire contents of the root directory it switches away from: I'll
have to make sure that doesn't somehow include moved-away mountpoints.
(I doubt it does.)

> Maybe you can run:
>   udevadm monitor
> in the background writing to a file, and get timestamps of your commands too.

They're fine.

After a bunch of straces, I think the problem is different. As I
suspected from its intermittent nature, it's a race.

When udevd starts up, it looks for an old database and converts it into
a new-style /run database. This takes nonzero time, and is done *after*
daemonization. Unfortunately, in the same time window it is quite
possible to fit a pair of 'udevadm trigger's and a 'udevadm settle': and
when udevadm settle kicks up and finds that udev hasn't even initialized
yet, what does it do? Well, unless I misread the code
udev_queue_get_queue_is_empty() returns 1 in this situation, so 'udevadm
settle' returns EXIT_SUCCESS, and boom.

My evidence for this is very thin: it's no more than a hypothesis
really. My only evidence for this so far is that I haven't managed to
get the failure to happen unless I mkdir the old udev directories first
to force udev to do an old->new db conversion. It would be nice to
gather more data, but it's hard to do that without perturbing the race
:/ even an ls of /run slows it down enough that it doesn't happen
anymore. Hm, perhaps an echo would be fast enough, it's a shell
builtin after all...

If I'm right (and I'm not sure I am, but it feels plausible), then I
suspect the only fix is to move the db conversion so that it happens
before daemonization.

-- 
NULL && (void)

^ permalink raw reply

* Re: udevadm settle persistently failing
From: DJ Lucas @ 2011-05-16  6:23 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On 05/16/2011 12:05 AM, DJ Lucas wrote:
> Probably a Wednesday evening project (CDT).
>

Nevermind, found time. Tracked it down to this commit:

http://git.kernel.org/?p=linux/hotplug/udev.git;a=commit;hÿ2c503df091e6e4e9ab48cdb6df6ec8b7b525d0

-- DJ Lucas


-- 
This message has been scanned for viruses and
dangerous content, and is believed to be clean.


^ permalink raw reply

* Re: udevadm settle persistently failing
From: DJ Lucas @ 2011-05-16  5:05 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On 05/15/2011 01:32 PM, Tom Gundersen wrote:
> On Sun, May 15, 2011 at 8:19 PM, Nix<nix@esperi.org.uk>  wrote:
>> I know that you're not supposed to rely on 'udevadm settle' anymore, but
>> I rely on it across the board for systems with root filesystems that
>> aren't expected to move around (i.e. all of them), because massively
>> reengineering working systems' boot processes is generally considered a
>> bad thing. And it's stopped working. Given how many things expect /dev
>> to be populated, this has fairly serious effects.
>>
>> I can be certain that as of somewhere between udev 164 and 167, 'udevadm
>> settle' has stopped waiting for block devices to appear (though I
>> suspect others have vanished too). I'm booting udev as recommended in
>> the release notes, via
>>
>>   udevd --daemon
>>   udevadm trigger --action­d --type=subsystems
>>   udevadm trigger --action­d --typeÞvices
>>   udevadm settle
>
> We are doing the same on Arch and today I started seeing bug reports
> (after the upgrade to udev 168). So here are my two cents:
>
> Most of the time the problem seems to be related to LVM, but I have
> also seen regular block devices having problems. As would be expected
> using devtmpfs greatly reduces (if not eliminates) the problem. My
> guess was (like Nix said) that "udevadm settle" is somehow broken.
>
> Some related bug reports:
>
> Arch:<https://bugs.archlinux.org/task/24288>,
> Debian:<http://bugs.debian.org/cgi-bin/bugreport.cgi?bugb4010>.
>
> Cheers,
>
> Tom
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Just to add a "me too" regarding regular block devices. First noticed 
with 168, coming from 165. /dev was tmpfs and swap is /dev/sda3 and 
mounted immediately after udevadm --settle. Kernel is 2.6.36.2. In 
addition to the udev change, I added the /run mount point to the mix 
(started very early, directly in rc before any scripts are run). Moving 
to devtmpfs 'fixed' it for me, but that was because devtmpfs has the raw 
sd* nodes, as explained to me by another dev. If I flip to use by-id, it 
breaks again. I plan to go back and see exactly where stuff broke, but 
haven't had the time just yet, more pressing issues. Probably a 
Wednesday evening project (CDT).

-- DJ Lucas


-- 
This message has been scanned for viruses and
dangerous content, and is believed to be clean.


^ permalink raw reply

* Re: [PULL] Docs: udev.xml: Copyediting
From: Michael Witten @ 2011-05-16  2:29 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <5cb4f3ce-b6e7-4c2a-b175-2081daad3acb-mfwitten@gmail.com>

On Fri, Apr 15, 2011 at 17:41, Michael Witten <mfwitten@gmail.com> wrote:
> On Mon, 11 Apr 2011 06:31:30 +0000, Michael Witten wrote:
>
>> I did some copyediting whilst looking through udev/udev.xml;
>> this email contains a pull request as well as the inlined
>> diff of all the changes squashed together.
>>
>> The changes apply cleanly to 960250952193db522b8ced336bf998bd7e8e4a31:
>>
>>   selinux: firmware - do not label files in runtime dir (2011-04-08 03:08:16 +0200)
>>
>> and are available here:
>>
>>   git://github.com/mfwitten/udev.git docs/udev.xml
>>
>> Squash them if desired.
>>
>>       Docs: udev.xml: Offset daemon name with commas
>>       Docs: udev.xml: Remove commas (and unnecessary repetition)
>>       Docs: udev.xml: `are' -> `is'; the subject is `Access'
>>       Docs: udev.xml: Use present tense
>>       Docs: udev.xml: Clarification through proper wording
>>       Docs: udev.xml: `,' -> `;'
>>       Docs: udev.xml: `key value' -> `key-value'
>>       Docs: udev.xml: `,' -> `:'
>>       Docs: udev.xml: Use `assignment' consistently
>>       Docs: udev.xml: `comma-separated' is a better description
>>       Docs: udev.xml: Remove unnecessary repitition
>>       Docs: udev.xml: Add a few more words for context
>>       Docs: udev.xml: Use `unless' for clarity
>>       Docs: udev.xml: Clarify PROGRAM key
>>       Docs: udev.xml: `a shell style' -> `shell-style'
>>       Docs: udev.xml: Clean `*' description
>>       Docs: udev.xml: Clean character range description
>>       Docs: udev.xml: Clean up description of NAME assignment key
>>       Docs: udev.xml: Clean up description of SYMLINK assignment key
>>       Docs: udev.xml: Clean up description of ENV assignment key
>>       Docs: udev.xml: Clean up description of RUN assignment key
>>       Docs: udev.xml: Clean up description of LABEL assignment key
>>       Docs: udev.xml: Add missing `.'
>>       Docs: udev.xml: `which' -> `content of which'
>>       Docs: udev.xml: `commandline' -> `command line'
>>       Docs: udev.xml: Clean up WAIT_FOR description
>>       Docs: udev.xml: `a' -> `the'
>>       Docs: udev.xml: Clean up introduction to substitutions.
>>       Docs: udev.xml: Use normal sentence structure
>>       Docs: udev.xml: Actually make a separate paragraph
>>       Docs: udev.xml: Add comma
>>       Docs: udev.xml: `char' -> `character'
>>       Docs: udev.xml: `comma-separated' is a better description
>>       Docs: udev.xml: Clarify through a change in word ordering
>>       Docs: udev.xml: Improved word order
>>       Docs: udev.xml: Fix dangling modifier
>>
>>  udev/udev.xml |  169 +++++++++++++++++++++++++++++----------------------------
>>  1 files changed, 86 insertions(+), 83 deletions(-)
>>
>> [diff]
>
> Would someone like to commit this?

Good news!

These patches still apply cleanly to the latest master:

  2906cbbae4d97291c1cfc456cf132726fffd8fc0

^ permalink raw reply

* Re: udevadm settle persistently failing
From: Kay Sievers @ 2011-05-15 23:57 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On Mon, May 16, 2011 at 01:47, Nix <nix@esperi.org.uk> wrote:
> On 16 May 2011, Kay Sievers stated:
>> We are sure it's not related to /run? We keep the state there and need
>> a tmpfs there before udevd is started, and it must not be touched, or
>> cleaned by some other stuff, that thinks /var/run (bind mount or
>> symlink) needs to be cleaned during boot.
>
> Just to confirm, I'm not cleaning out /run, or touching it at all except
> to create /run/lock later in boot.

Ok, sounds good.

>> Devtmpfs solves a lot of settle races, yeah. We should run fine on
>> tmpfs, but it's the only config that is really tested these days, so
>> it might be a problem nobody else is really seeing.
>
> I could turn on devtmpfs, I suppose, except that I have an early
> userspace and I'm not sure how I'd need to change it: devtmpfs gets
> mounted on the rootfs, doesn't it?

If you ask the kernel to do that at compile time or with a parameter,
it does that, but only if the rootfs is mounted by the kernel itself,
not in any initramfs case.

> This problem isn't happening on the
> rootfs, it's on the root filesystem that is overmounted over that. (I'm
> using busybox mdev to populate the rootfs /dev because it works for such
> a simple case, and never goes wrong because it's too simple to go wrong.
> However it's also too simple to be much use for a running system.)

With devtmpfs you get the "early" /dev for free, and you preserve the
"early" /dev in the real root. Instead of mounting a 'tmpfs' just
mount a 'devtmpfs' at /dev, and it will be magically filled already,
that's the only difference.

When mounting the real root, just mount --move it over to the real
root before switching to the real root.

>> The current settle wakes up in exactly the moment the last event is
>> gone, instead of it sleep()ing in a loop. It might be a bit earlier,
>> not really before stuff has settled though.
>>
>> Does this work for you?
>>   time (udevadm trigger; udevadm settle)
>>
>> It should not return immediately. You can watch the events with:
>>
>>   udevadm monitor
>> at the same time and check if it really only returns after the last event.
>
> Hm, well, that seems to be working, at least once the system is all the
> way up. But *something* plainly isn't.
>
> On my next boot I'll time the trigger-and-settle pair and see how long
> it takes...

Good.

Maybe you can run:
  udevadm monitor
in the background writing to a file, and get timestamps of your commands too.

For testing, you could also add test rules like:
  KERNEL="null", PROGRAM="/bin/sleep 10"

Which should make settle definitely block for that amount of time.

Kay

^ permalink raw reply

* Re: udevadm settle persistently failing
From: Kay Sievers @ 2011-05-15 23:51 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <8739kfzv1j.fsf@spindle.srvr.nix>

On Mon, May 16, 2011 at 01:38, Nix <nix@esperi.org.uk> wrote:
> On 16 May 2011, Kay Sievers stated:

>> What's 'umount /run' during bootup? That sounds really strange.
>
> Hm, I didn't notice that. Yes, that does seem very odd indeed.
>
> I'll have a look soon, if probably not on that system (it's a headless
> system that does my firewalling so quite hard to reboot). I see the
> symptoms on other systems too.
>
>>> | udevd[339]: failed to execute '/sbin/modprobe' '/sbin/modprobe -bv platform:rtc_cmos': No such file or directory
>>>
>>> I have no clue why udev is trying to run modprobe when this is a
>>> non-modular kernel with all necessary devices built in. But that's not
>>> important.
>>
>> Udev has no idea what the kernel supports, it calls modprobe for all
>> devices with a 'modalias' but no attached driver.
>
> Yeah, it's not terribly important (though why they even have a modalias
> if this kernel does not have modules built in is also unclear. Anyway,
> it can do no harm, since modprobe doesn't even exist on that system.)

Yeah, you could #ifdef the modalias export if the kernel does not
support module loading. This is not optimized.

It' probably just easier to delete the default udev rule that calls
modprobe, if you don't need it. :)

>>> fold:~# ls -l /dev/sda1
>>> brw-rw---- 1 root disk 8, 1 May 15 18:56 /dev/sda1
>>>
>>> it's there. udevadm settle just didn't bother to wait for it to appear.
>>> (This is not a device on a bus for which enumeration takes some time:
>>> it's an SD card on an IDE-lookalike cs5535 bus. I've also seen this
>>> on LVM-atop-RAID-atop-SATA and on ordinary LVM-atop-SATA, so it doesn't
>>> require anything particularly unusual.)
>>
>> Sounds weird. Settle should not return at that point.
>
> Agreed! (This is why I suspect the timeout stuff is simply timing out
> immediately.)

Yeah, that could happen too, if /run/udev/queue.bin would be deleted
by some cleanup script, after udevd is started.

>>                                                       You are not
>> altering the content of /run/udev/ or /dev/.udev/ in any way right?
>
> Gods, no. Recipe for disaster.

Ok, fine.

>> And you provide the /run tmpfs before you start udevd and don't touch
>> it again, right?
>
> ooo! possible bug. Here's my udev startup script:
>
> mount -n /proc
> mount -n /sys
> mount -n /run
> [...]
> mount_tmpfs
> mkdir -p $udev_root/.udev/db $udev_root/.udev/queue $udev_root/.udev/failed

Yeah, never create any private directories of udevd. Most of them do
not even exist anymore in today's udev.

> echo "Creating initial device nodes..."
> udevd --daemon
> udevadm trigger --action­d --type=subsystems
> udevadm trigger --action­d --typeÞvices
> udevadm settle
> sleep 1
>
> *That* is going to cause udev to try to convert from the old to the new
> udev database format every single time it starts, even though there *is*
> no old udev database there, just skeletal directories. I wonder if
> that's causing it?
>
> (I've ripped that mkdir out. Let's see if that fixes things. If it does,
> this suggests that udev needs further bulletproofing, because my udev
> startup script was derived directly from one provided by earlier
> versions of udev: a *lot* of people will have scripts like it.)

Yeah, not a good idea to fiddle around with udev internals. The
existence of the old directory names indicates a need to convert. It
should not really break anything, just waste some time during udevd
startup.

>>> Other things seem to have failed too. I have renaming rules:
>>>
>>> SUBSYSTEM="net", ATTR{address}="00:00:24:cb:c6:a0", NAME="gordianet"
>>> SUBSYSTEM="net", ATTR{address}="00:00:24:cb:c6:a1", NAME="adsl"
>>> SUBSYSTEM="net", ATTR{address}="00:00:24:cb:c6:a3", NAME="wireless"
>>>
>>> yet the devices were not renamed:
>>
>> Hmm, that should be unrelated to the possible settle problem
>
> Yeah. The rename-failure only seems to happen when the settle failure
> happens, so perhaps it's related to parts of the startup script messing
> with the interfaces and causing the kernel to ban renames because
> they're in use. Obviously this doesn't happen if we're still sitting in
> 'udevadm settle' when this takes place.

Yeah, that could explain it. A soon as the netif is up, we can not
rename it anymore.

>>> Put a 'sleep 1' after the udev call, and everything works.
>>
>> Which call? The trigger?
>
> See above: it was right after the settle call.

I see. Hmm, no good idea why this would be.

> Anyway, more tomorrow sometime after more testing. (It's past midnight
> here.)

Sounds good, let me know, if you find something out.

Kay

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox