* Re: udev 182 not responding to the DISK_EJECT_REQUEST
From: Kay Sievers @ 2012-03-24 22:10 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <20120324214648.GA420@linux1>
On Sat, Mar 24, 2012 at 22:46, William Hubbs <w.d.hubbs@gmail.com> wrote:
> we have the following issue reported by a gentoo user.
>
> https://bugs.gentoo.org/show_bug.cgi?id@9563
>
> I realize that we no longer create /dev/dvd, but what about responding
> to EJECT_REQUEST?
It should all work. DISK_EJECT_REQUEST=1 handling work fine for me here.
Kay
^ permalink raw reply
* udev 182 not responding to the DISK_EJECT_REQUEST
From: William Hubbs @ 2012-03-24 21:46 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 218 bytes --]
All,
we have the following issue reported by a gentoo user.
https://bugs.gentoo.org/show_bug.cgi?id=409563
I realize that we no longer create /dev/dvd, but what about responding
to EJECT_REQUEST?
Thanks,
William
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [ANNOUNCE] kmod 7
From: Lucas De Marchi @ 2012-03-19 11:46 UTC (permalink / raw)
To: linux-modules, LKML, linux-hotplug
kmod 7 is out:
ftp://ftp.kernel.org/pub/linux/utils/kernel/kmod/kmod-7.tar.xz
ftp://ftp.kernel.org/pub/linux/utils/kernel/kmod/kmod-7.tar.sign
Just a bug-fix release with some bugs found mainly by Debian guys.
Check the NEWS file for more information.
Thanks a lot for all of you helping with debugging, testing and patches.
Shortlog is below.
Lucas De Marchi
--
Dave Reisner (1):
modprobe: fix error path in removing modules
Lucas De Marchi (7):
modprobe: don't check if module builtin to decide if it's builtin
modprobe: always try to remove all modules in command line
modprobe: set log prio to 0 if user passed -q arg
config: use order /etc, /run, /lib
build-sys: re-organize configure.ac
build-sys: don't set CFLAGS and LDFLAGS
kmod 7
Randy Witt (2):
Add CC_CHECK_LDFLAGS_APPEND m4 macro.
configure.ac: Move link only flags out of CFLAGS and into LDFLAGS.
^ permalink raw reply
* [ANNOUNCE] udev 182
From: Kay Sievers @ 2012-03-18 22:21 UTC (permalink / raw)
To: linux-hotplug
Here comes a new udev version. Thanks to all who have contributed to
this release.
The tarball can be found here:
ftp://ftp.kernel.org/pub/linux/utils/kernel/hotplug/
The development repository can be found here:
http://www.kernel.org/git/?p=linux/hotplug/udev.git;a=summary
The ChangeLog can be found here:
http://www.kernel.org/git/?p=linux/hotplug/udev.git;a=blob;hb=HEAD;f=ChangeLog
udev 182
====
Rules files in /etc/udev/rules.s/ with the same name as rules files in
/run/udev/rules.d/ now always have precedence. The stack of files is now:
/usr/lib (package), /run (runtime, auto-generated), /etc (admin), while
the later ones override the earlier ones. In other words: the admin has
always the last say.
USB auto-suspend is now enabled by default for some built-in USB HID
devices.
/dev/disk/by-path/ links are no longer created for ATA devices behind
an 'ATA transport class', the logic to extract predictable numbers does
not exist in the kernel at this moment.
/dev/disk/by-id/scsi-* compatibility links are no longer created for
ATA devices, they have their own ata-* prefix.
The s390 rule to set mode = 0666 for /dev/z90crypt is is removed from
the udev tree and will be part of s390utils (or alternatively could be
done by the kernel driver itself).
The udev-acl tool is no longer provided, it will be part of a future
ConsoleKit release. On systemd systems, advanced ConsoleKit and udev-acl
functionality are provided by systemd.
^ permalink raw reply
* [PATCH] reinstate TIMEOUT= handling
From: Tom Gundersen @ 2012-03-15 16:41 UTC (permalink / raw)
To: linux-hotplug
Without treating events with timeouts specially some drivers would
cause a 30 seconds stall on boot: .
I also received reports of some drivers not working at all, even
after the timeout.
We will remove this patch when more drivers have been fixed in
the kernel (3.4?).
This reverts 43d5c5f03645c4b842659f9b5bd0ae465e885e92 and
57c6f8ae5f52a6e8ffc66a54966346f733dded39.
---
Note: this is mostly a FYI, and whether or not it makes sense
to apply this upstream depends on how big problems other report
regarding this issue.
TODO | 2 ++
src/libudev-device.c | 19 +++++++++++++++++++
src/libudev-private.h | 1 +
src/udevd.c | 13 ++++++++++---
4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/TODO b/TODO
index 36e8440..c2e59b6 100644
--- a/TODO
+++ b/TODO
@@ -1,6 +1,8 @@
- find a way to tell udev to not cancel firmware
requests in initramfs
+ - remove TIMEOUT= handling
+
- move /lib/udev/devices/ to tmpfiles
- trigger --subsystem-match=usb/usb_device
diff --git a/src/libudev-device.c b/src/libudev-device.c
index 10f28b8..639c367 100644
--- a/src/libudev-device.c
+++ b/src/libudev-device.c
@@ -68,6 +68,7 @@ struct udev_device {
struct udev_list tags_list;
unsigned long long int seqnum;
unsigned long long int usec_initialized;
+ int timeout;
int devlink_priority;
int refcount;
dev_t devnum;
@@ -160,6 +161,21 @@ static int udev_device_set_devnum(struct udev_device *udev_device, dev_t devnum)
return 0;
}
+int udev_device_get_timeout(struct udev_device *udev_device)
+{
+ return udev_device->timeout;
+}
+
+static int udev_device_set_timeout(struct udev_device *udev_device, int timeout)
+{
+ char num[32];
+
+ udev_device->timeout = timeout;
+ snprintf(num, sizeof(num), "%u", timeout);
+ udev_device_add_property(udev_device, "TIMEOUT", num);
+ return 0;
+}
+
const char *udev_device_get_devpath_old(struct udev_device *udev_device)
{
return udev_device->devpath_old;
@@ -414,6 +430,8 @@ void udev_device_add_property_from_string_parse(struct udev_device *udev_device,
udev_device_set_devpath_old(udev_device, &property[12]);
} else if (strncmp(property, "SEQNUM=", 7) = 0) {
udev_device_set_seqnum(udev_device, strtoull(&property[7], NULL, 10));
+ } else if (strncmp(property, "TIMEOUT=", 8) = 0) {
+ udev_device_set_timeout(udev_device, strtoull(&property[8], NULL, 10));
} else if (strncmp(property, "IFINDEX=", 8) = 0) {
udev_device_set_ifindex(udev_device, strtoull(&property[8], NULL, 10));
} else if (strncmp(property, "DEVMODE=", 8) = 0) {
@@ -599,6 +617,7 @@ struct udev_device *udev_device_new(struct udev *udev)
udev_list_init(udev, &udev_device->sysattr_value_list, true);
udev_list_init(udev, &udev_device->sysattr_list, false);
udev_list_init(udev, &udev_device->tags_list, true);
+ udev_device->timeout = -1;
udev_device->watch_handle = -1;
/* copy global properties */
udev_list_entry_foreach(list_entry, udev_get_properties_list_entry(udev))
diff --git a/src/libudev-private.h b/src/libudev-private.h
index 5f5c64a..ec63b67 100644
--- a/src/libudev-private.h
+++ b/src/libudev-private.h
@@ -87,6 +87,7 @@ const char *udev_device_get_id_filename(struct udev_device *udev_device);
void udev_device_set_is_initialized(struct udev_device *udev_device);
int udev_device_add_tag(struct udev_device *udev_device, const char *tag);
void udev_device_cleanup_tags_list(struct udev_device *udev_device);
+int udev_device_get_timeout(struct udev_device *udev_device);
unsigned long long udev_device_get_usec_initialized(struct udev_device *udev_device);
void udev_device_set_usec_initialized(struct udev_device *udev_device, unsigned long long usec_initialized);
int udev_device_get_devlink_priority(struct udev_device *udev_device);
diff --git a/src/udevd.c b/src/udevd.c
index 1702217..88e9272 100644
--- a/src/udevd.c
+++ b/src/udevd.c
@@ -401,7 +401,7 @@ out:
}
}
-static void event_run(struct event *event)
+static void event_run(struct event *event, bool force)
{
struct udev_list_node *loop;
@@ -427,7 +427,7 @@ static void event_run(struct event *event)
return;
}
- if (children >= children_max) {
+ if (!force && children >= children_max) {
if (children_max > 1)
info(event->udev, "maximum number (%i) of children reached\n", children);
return;
@@ -461,6 +461,13 @@ static int event_queue_insert(struct udev_device *dev)
event->state = EVENT_QUEUED;
udev_list_node_append(&event->node, &event_list);
+
+ /* run all events with a timeout set immediately */
+ if (udev_device_get_timeout(dev) > 0) {
+ event_run(event, true);
+ return 0;
+ }
+
return 0;
}
@@ -577,7 +584,7 @@ static void event_queue_start(struct udev *udev)
continue;
}
- event_run(event);
+ event_run(event, false);
}
}
--
1.7.9.4
^ permalink raw reply related
* Re: [PATCH] rules: Enable USB autosuspend on more USB HID devices
From: Kay Sievers @ 2012-03-12 21:43 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <1331581526-14865-1-git-send-email-mjg@redhat.com>
On Mon, Mar 12, 2012 at 20:45, Matthew Garrett <mjg@redhat.com> wrote:
> Many servers will be connected to KVMs or include iLO support, and this
> is often presented as a set of USB input devices. Enabling autosuspend on
> these allows the USB hardware to be powered down, avoiding unnecessary
> wakeups and power consumption. The input devices will be self powered, so
> there's no risk of losing input events as there would be for real input
> devices. The same is true of USB input devices that are built into the
> system.
Applied.
Thanks,
Kay
^ permalink raw reply
* [PATCH] rules: Enable USB autosuspend on more USB HID devices
From: Matthew Garrett @ 2012-03-12 19:45 UTC (permalink / raw)
To: linux-hotplug
Many servers will be connected to KVMs or include iLO support, and this
is often presented as a set of USB input devices. Enabling autosuspend on
these allows the USB hardware to be powered down, avoiding unnecessary
wakeups and power consumption. The input devices will be self powered, so
there's no risk of losing input events as there would be for real input
devices. The same is true of USB input devices that are built into the
system.
---
Makefile.am | 2 +-
rules/42-qemu-usb.rules | 13 ------------
rules/42-usb-hid-pm.rules | 49 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 50 insertions(+), 14 deletions(-)
delete mode 100644 rules/42-qemu-usb.rules
create mode 100644 rules/42-usb-hid-pm.rules
diff --git a/Makefile.am b/Makefile.am
index 7b84c1c..5fb2c13 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -150,7 +150,7 @@ INSTALL_DATA_HOOKS += udev-confdirs
udevrulesdir = $(libexecdir)/udev/rules.d
dist_udevrules_DATA = \
- rules/42-qemu-usb.rules \
+ rules/42-usb-hid-pm.rules \
rules/50-udev-default.rules \
rules/60-persistent-storage-tape.rules \
rules/60-persistent-serial.rules \
diff --git a/rules/42-qemu-usb.rules b/rules/42-qemu-usb.rules
deleted file mode 100644
index a4e3864..0000000
--- a/rules/42-qemu-usb.rules
+++ /dev/null
@@ -1,13 +0,0 @@
-#
-# Enable autosuspend for qemu emulated usb hid devices.
-#
-# Note that there are buggy qemu versions which advertise remote
-# wakeup support but don't actually implement it correctly. This
-# is the reason why we need a match for the serial number here.
-# The serial number "42" is used to tag the implementations where
-# remote wakeup is working.
-#
-
-ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Mouse", ATTR{serial}="42", TEST="power/control", ATTR{power/control}="auto"
-ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Tablet", ATTR{serial}="42", TEST="power/control", ATTR{power/control}="auto"
-ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Keyboard", ATTR{serial}="42", TEST="power/control", ATTR{power/control}="auto"
diff --git a/rules/42-usb-hid-pm.rules b/rules/42-usb-hid-pm.rules
new file mode 100644
index 0000000..d5d5897
--- /dev/null
+++ b/rules/42-usb-hid-pm.rules
@@ -0,0 +1,49 @@
+#
+# Enable autosuspend for qemu emulated usb hid devices.
+#
+# Note that there are buggy qemu versions which advertise remote
+# wakeup support but don't actually implement it correctly. This
+# is the reason why we need a match for the serial number here.
+# The serial number "42" is used to tag the implementations where
+# remote wakeup is working.
+#
+
+ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Mouse", ATTR{serial}="42", TEST="power/control", ATTR{power/control}="auto"
+ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Tablet", ATTR{serial}="42", TEST="power/control", ATTR{power/control}="auto"
+ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Keyboard", ATTR{serial}="42", TEST="power/control", ATTR{power/control}="auto"
+
+#
+# Enable autosuspend for KVM and iLO usb hid devices. These are
+# effectively self-powered (despite what some claim in their USB
+# profiles) and so it's safe to do so.
+#
+
+# AMI 046b:ff10
+ACTION="add", SUBSYSTEM="usb", ATTR{idVendor}="046b", ATTR{idProduct}="ff10", TEST="power/control", ATTR{power/control}="auto"
+
+#
+# Catch-all for Avocent HID devices. Keyed off interface in order to only
+# trigger on HID class devices.
+#
+ACTION="add", SUBSYSTEM="usb", ATTRS{idVendor}="0624", ATTR{bInterfaceClass}="03", TEST="../power/control", ATTR{../power/control}="auto"
+
+# Dell DRAC 4
+ACTION="add", SUBSYSTEM="usb", ATTR{idVendor}="413c", ATTR{idProduct}="2500", TEST="power/control", ATTR{power/control}="auto"
+
+# Dell DRAC 5
+ACTION="add", SUBSYSTEM="usb", ATTR{idVendor}="413c", ATTR{idProduct}="0000", TEST="power/control", ATTR{power/control}="auto"
+
+# Hewlett Packard iLO
+ACTION="add", SUBSYSTEM="usb", ATTR{idVendor}="03f0", ATTR{idProduct}="7029", TEST="power/control", ATTR{power/control}="auto"
+ACTION="add", SUBSYSTEM="usb", ATTR{idVendor}="03f0", ATTR{idProduct}="1027", TEST="power/control", ATTR{power/control}="auto"
+
+# IBM remote access
+ACTION="add", SUBSYSTEM="usb", ATTR{idVendor}="04b3", ATTR{idProduct}="4001", TEST="power/control", ATTR{power/control}="auto"
+ACTION="add", SUBSYSTEM="usb", ATTR{idVendor}="04b3", ATTR{idProduct}="4002", TEST="power/control", ATTR{power/control}="auto"
+ACTION="add", SUBSYSTEM="usb", ATTRS{idVendor}="04b3", ATTR{idProduct}="4012", TEST="power/control", ATTR{power/control}="auto"
+
+# Raritan Computer, Inc KVM.
+ACTION="add", SUBSYSTEM="usb", ATTR{idVendor}="14dd", ATTR{idProduct}="0002", TEST="power/control", ATTR{power/control}="auto"
+
+# USB HID devices that are internal to the machine should also be safe to autosuspend
+ACTION="add", SUBSYSTEM="usb", ATTR{bInterfaceClass}="03", ATTRS{removable}="fixed", TEST="../power/control", ATTR{../power/control}="auto"
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH] uevent: send events in correct order according to seqnum (v3)
From: Greg Kroah-Hartman @ 2012-03-07 17:47 UTC (permalink / raw)
To: Andrew Wagin; +Cc: Kay Sievers, linux-hotplug, linux-kernel, Andrey Vagin
In-Reply-To: <CANaxB-yUcg64UxjUSdbNEicnZhBBh_Xx7hPY6DAdb+kA4+AR9w@mail.gmail.com>
On Wed, Mar 07, 2012 at 09:34:19PM +0400, Andrew Wagin wrote:
> > Is this something that you have seen "in the wild"? If so, we should
> > backport it to older kernels as well, right?
>
> Yes, we should. I found this bug in RHEL6, it uses 2.6.32 kernel.
>
> https://bugzilla.redhat.com/show_bug.cgi?idy9834
Great, thanks for letting me know, I'll queue it up for the older
kernels as well.
greg k-h
^ permalink raw reply
* Re: [PATCH] uevent: send events in correct order according to seqnum (v3)
From: Andrew Wagin @ 2012-03-07 17:34 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Kay Sievers, linux-hotplug, linux-kernel, Andrey Vagin
In-Reply-To: <20120307154517.GA16080@kroah.com>
> Is this something that you have seen "in the wild"? If so, we should
> backport it to older kernels as well, right?
Yes, we should. I found this bug in RHEL6, it uses 2.6.32 kernel.
https://bugzilla.redhat.com/show_bug.cgi?idy9834
^ permalink raw reply
* Re: [PATCH] uevent: send events in correct order according to seqnum (v3)
From: Greg Kroah-Hartman @ 2012-03-07 15:45 UTC (permalink / raw)
To: Andrew Vagin; +Cc: Kay Sievers, linux-hotplug, linux-kernel
In-Reply-To: <1331117396-11600-1-git-send-email-avagin@openvz.org>
On Wed, Mar 07, 2012 at 02:49:56PM +0400, Andrew Vagin wrote:
> The queue handling in the udev daemon assumes that the events are
> ordered.
>
> Before this patch uevent_seqnum is incremented under sequence_lock,
> than an event is send uner uevent_sock_mutex. I want to say that code
> contained a window between incrementing seqnum and sending an event.
Is this something that you have seen "in the wild"? If so, we should
backport it to older kernels as well, right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] uevent: send events in correct order according to seqnum (v3)
From: Kay Sievers @ 2012-03-07 11:03 UTC (permalink / raw)
To: Andrew Vagin; +Cc: Greg Kroah-Hartman, linux-hotplug, linux-kernel
In-Reply-To: <1331117396-11600-1-git-send-email-avagin@openvz.org>
On Wed, Mar 7, 2012 at 11:49, Andrew Vagin <avagin@openvz.org> wrote:
> The queue handling in the udev daemon assumes that the events are
> ordered.
>
> Before this patch uevent_seqnum is incremented under sequence_lock,
> than an event is send uner uevent_sock_mutex. I want to say that code
> contained a window between incrementing seqnum and sending an event.
>
> This patch locks uevent_sock_mutex before incrementing uevent_seqnum.
>
> v2: delete sequence_lock, uevent_seqnum is protected by uevent_sock_mutex
> v3: unlock the mutex before the goto exit
>
> Thanks for Kay for the comments.
>
> Signed-off-by: Andrew Vagin <avagin@openvz.org>
Looks good to me. Works fine in a kvm installation here.
Feel free to add:
Tested-By: Kay Sievers <kay.sievers@vrfy.org>
Thanks lot for the udev debugging and taking care of it,
Kay
^ permalink raw reply
* [PATCH] uevent: send events in correct order according to seqnum (v3)
From: Andrew Vagin @ 2012-03-07 10:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Kay Sievers; +Cc: avagin, linux-hotplug, linux-kernel
In-Reply-To: <1331064368-55675-1-git-send-email-avagin@openvz.org>
The queue handling in the udev daemon assumes that the events are
ordered.
Before this patch uevent_seqnum is incremented under sequence_lock,
than an event is send uner uevent_sock_mutex. I want to say that code
contained a window between incrementing seqnum and sending an event.
This patch locks uevent_sock_mutex before incrementing uevent_seqnum.
v2: delete sequence_lock, uevent_seqnum is protected by uevent_sock_mutex
v3: unlock the mutex before the goto exit
Thanks for Kay for the comments.
Signed-off-by: Andrew Vagin <avagin@openvz.org>
---
lib/kobject_uevent.c | 19 +++++++++----------
1 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index e66e9b6..75cbdb5 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -29,16 +29,17 @@
u64 uevent_seqnum;
char uevent_helper[UEVENT_HELPER_PATH_LEN] = CONFIG_UEVENT_HELPER_PATH;
-static DEFINE_SPINLOCK(sequence_lock);
#ifdef CONFIG_NET
struct uevent_sock {
struct list_head list;
struct sock *sk;
};
static LIST_HEAD(uevent_sock_list);
-static DEFINE_MUTEX(uevent_sock_mutex);
#endif
+/* This lock protects uevent_seqnum and uevent_sock_list */
+static DEFINE_MUTEX(uevent_sock_mutex);
+
/* the strings here must match the enum in include/linux/kobject.h */
static const char *kobject_actions[] = {
[KOBJ_ADD] = "add",
@@ -136,7 +137,6 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
struct kobject *top_kobj;
struct kset *kset;
const struct kset_uevent_ops *uevent_ops;
- u64 seq;
int i = 0;
int retval = 0;
#ifdef CONFIG_NET
@@ -243,17 +243,16 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
else if (action = KOBJ_REMOVE)
kobj->state_remove_uevent_sent = 1;
+ mutex_lock(&uevent_sock_mutex);
/* we will send an event, so request a new sequence number */
- spin_lock(&sequence_lock);
- seq = ++uevent_seqnum;
- spin_unlock(&sequence_lock);
- retval = add_uevent_var(env, "SEQNUM=%llu", (unsigned long long)seq);
- if (retval)
+ retval = add_uevent_var(env, "SEQNUM=%llu", (unsigned long long)++uevent_seqnum);
+ if (retval) {
+ mutex_unlock(&uevent_sock_mutex);
goto exit;
+ }
#if defined(CONFIG_NET)
/* send netlink message */
- mutex_lock(&uevent_sock_mutex);
list_for_each_entry(ue_sk, &uevent_sock_list, list) {
struct sock *uevent_sock = ue_sk->sk;
struct sk_buff *skb;
@@ -290,8 +289,8 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
} else
retval = -ENOMEM;
}
- mutex_unlock(&uevent_sock_mutex);
#endif
+ mutex_unlock(&uevent_sock_mutex);
/* call uevent_helper, usually only enabled during early boot */
if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] uevent: send events in correct order according to seqnum (v2)
From: Kay Sievers @ 2012-03-07 10:18 UTC (permalink / raw)
To: Andrew Vagin; +Cc: Greg Kroah-Hartman, linux-hotplug, linux-kernel
In-Reply-To: <1331114342-6187-1-git-send-email-avagin@openvz.org>
On Wed, Mar 7, 2012 at 10:59, Andrew Vagin <avagin@openvz.org> wrote:
> The queue handling in the udev daemon assumes that the events are
> ordered.
> - retval = add_uevent_var(env, "SEQNUM=%llu", (unsigned long long)seq);
> + retval = add_uevent_var(env, "SEQNUM=%llu", (unsigned long long)++uevent_seqnum);
> if (retval)
> goto exit;
We need to unlock the mutex before the goto exit?
Other than that, looks good to me.
Thanks,
Kay
^ permalink raw reply
* [PATCH] uevent: send events in correct order according to seqnum (v2)
From: Andrew Vagin @ 2012-03-07 9:59 UTC (permalink / raw)
To: Greg Kroah-Hartman, Kay Sievers; +Cc: avagin, linux-hotplug, linux-kernel
In-Reply-To: <1331064368-55675-1-git-send-email-avagin@openvz.org>
The queue handling in the udev daemon assumes that the events are
ordered.
Before this patch uevent_seqnum is incremented under sequence_lock,
than an event is send uner uevent_sock_mutex. I want to say that code
contained a window between incrementing seqnum and sending an event.
This patch locks uevent_sock_mutex before incrementing uevent_seqnum.
v2: delete sequence_lock, uevent_seqnum is protected by uevent_sock_mutex
Signed-off-by: Andrew Vagin <avagin@openvz.org>
---
lib/kobject_uevent.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index e66e9b6..15a7ab9 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -29,16 +29,17 @@
u64 uevent_seqnum;
char uevent_helper[UEVENT_HELPER_PATH_LEN] = CONFIG_UEVENT_HELPER_PATH;
-static DEFINE_SPINLOCK(sequence_lock);
#ifdef CONFIG_NET
struct uevent_sock {
struct list_head list;
struct sock *sk;
};
static LIST_HEAD(uevent_sock_list);
-static DEFINE_MUTEX(uevent_sock_mutex);
#endif
+/* This lock protects uevent_seqnum and uevent_sock_list */
+static DEFINE_MUTEX(uevent_sock_mutex);
+
/* the strings here must match the enum in include/linux/kobject.h */
static const char *kobject_actions[] = {
[KOBJ_ADD] = "add",
@@ -136,7 +137,6 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
struct kobject *top_kobj;
struct kset *kset;
const struct kset_uevent_ops *uevent_ops;
- u64 seq;
int i = 0;
int retval = 0;
#ifdef CONFIG_NET
@@ -243,17 +243,14 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
else if (action = KOBJ_REMOVE)
kobj->state_remove_uevent_sent = 1;
+ mutex_lock(&uevent_sock_mutex);
/* we will send an event, so request a new sequence number */
- spin_lock(&sequence_lock);
- seq = ++uevent_seqnum;
- spin_unlock(&sequence_lock);
- retval = add_uevent_var(env, "SEQNUM=%llu", (unsigned long long)seq);
+ retval = add_uevent_var(env, "SEQNUM=%llu", (unsigned long long)++uevent_seqnum);
if (retval)
goto exit;
#if defined(CONFIG_NET)
/* send netlink message */
- mutex_lock(&uevent_sock_mutex);
list_for_each_entry(ue_sk, &uevent_sock_list, list) {
struct sock *uevent_sock = ue_sk->sk;
struct sk_buff *skb;
@@ -290,8 +287,8 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
} else
retval = -ENOMEM;
}
- mutex_unlock(&uevent_sock_mutex);
#endif
+ mutex_unlock(&uevent_sock_mutex);
/* call uevent_helper, usually only enabled during early boot */
if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] uevent: send events in correct order according to seqnum
From: Greg Kroah-Hartman @ 2012-03-07 5:52 UTC (permalink / raw)
To: avagin@gmail.com; +Cc: Kay Sievers, Andrew Vagin, linux-hotplug, linux-kernel
In-Reply-To: <4F567E1C.80003@gmail.com>
On Wed, Mar 07, 2012 at 01:14:04AM +0400, avagin@gmail.com wrote:
> On 03/07/2012 01:03 AM, Kay Sievers wrote:
> >On Tue, Mar 6, 2012 at 21:06, Andrew Vagin<avagin@openvz.org> wrote:
> >>
> >>The queue handling in the udev daemon assumes that the events are
> >>ordered.
> >>
> >>Before this patch uevent_seqnum is incremented under sequence_lock,
> >>than an event is send uner uevent_sock_mutex. I want to say that code
> >>contained a window between incrementing seqnum and sending an event.
> >>
> >>This patch locks uevent_sock_mutex before incrementing uevent_seqnum.
> >
> >I think we can remove the spin_lock(&sequence_lock); entirely now, right?
>
> I thought about that too. sequence_lock is used when CONFIG_NET
> isn't defined. I've looked on this code one more time and we may
> leave only uevent_sock_mutex and use it even when CONFIG_NET isn't
> defined.
> Thanks for the comment.
>
> Greg, do you have other objections about this patch?
Let's see the one based on Kay's comments first please.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] uevent: send events in correct order according to seqnum
From: avagin @ 2012-03-06 21:14 UTC (permalink / raw)
To: Kay Sievers; +Cc: Andrew Vagin, Greg Kroah-Hartman, linux-hotplug, linux-kernel
In-Reply-To: <CAPXgP10qwPn2-QeyPpDy3GiEqvU6jB-wVuUOUXwcGBvOWOUA1Q@mail.gmail.com>
On 03/07/2012 01:03 AM, Kay Sievers wrote:
> On Tue, Mar 6, 2012 at 21:06, Andrew Vagin<avagin@openvz.org> wrote:
>>
>> The queue handling in the udev daemon assumes that the events are
>> ordered.
>>
>> Before this patch uevent_seqnum is incremented under sequence_lock,
>> than an event is send uner uevent_sock_mutex. I want to say that code
>> contained a window between incrementing seqnum and sending an event.
>>
>> This patch locks uevent_sock_mutex before incrementing uevent_seqnum.
>
> I think we can remove the spin_lock(&sequence_lock); entirely now, right?
I thought about that too. sequence_lock is used when CONFIG_NET isn't
defined. I've looked on this code one more time and we may leave only
uevent_sock_mutex and use it even when CONFIG_NET isn't defined.
Thanks for the comment.
Greg, do you have other objections about this patch?
>
> Also the section with:
> seq = ++uevent_seqnum;
> can just be:
> add_uevent_var(env, "SEQNUM=%llu", (unsigned long long) ++uevent_seqnum);
> right?
>
> And the:
> mutex_lock(&uevent_sock_mutex);
> can just move outside of the _NET ifdef and we always use the mutex
> instead of the spinlock?
>
> That could look much simpler than the current code, I think.
>
> Thanks,
> Kay
^ permalink raw reply
* Re: [PATCH] uevent: send events in correct order according to seqnum
From: Kay Sievers @ 2012-03-06 21:03 UTC (permalink / raw)
To: Andrew Vagin; +Cc: Greg Kroah-Hartman, linux-hotplug, linux-kernel
In-Reply-To: <1331064368-55675-1-git-send-email-avagin@openvz.org>
On Tue, Mar 6, 2012 at 21:06, Andrew Vagin <avagin@openvz.org> wrote:
>
> The queue handling in the udev daemon assumes that the events are
> ordered.
>
> Before this patch uevent_seqnum is incremented under sequence_lock,
> than an event is send uner uevent_sock_mutex. I want to say that code
> contained a window between incrementing seqnum and sending an event.
>
> This patch locks uevent_sock_mutex before incrementing uevent_seqnum.
I think we can remove the spin_lock(&sequence_lock); entirely now, right?
Also the section with:
seq = ++uevent_seqnum;
can just be:
add_uevent_var(env, "SEQNUM=%llu", (unsigned long long) ++uevent_seqnum);
right?
And the:
mutex_lock(&uevent_sock_mutex);
can just move outside of the _NET ifdef and we always use the mutex
instead of the spinlock?
That could look much simpler than the current code, I think.
Thanks,
Kay
^ permalink raw reply
* [PATCH] uevent: send events in correct order according to seqnum
From: Andrew Vagin @ 2012-03-06 20:06 UTC (permalink / raw)
To: Greg Kroah-Hartman, Kay Sievers; +Cc: avagin, linux-hotplug, linux-kernel
In-Reply-To: <CAPXgP10GRZ6Y3+dtNSw5zQMaM9FnSBBfLrCJmQ948gA3tALtfw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 493 bytes --]
The queue handling in the udev daemon assumes that the events are
ordered.
Before this patch uevent_seqnum is incremented under sequence_lock,
than an event is send uner uevent_sock_mutex. I want to say that code
contained a window between incrementing seqnum and sending an event.
This patch locks uevent_sock_mutex before incrementing uevent_seqnum.
Signed-off-by: Andrew Vagin <avagin@openvz.org>
---
lib/kobject_uevent.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-uevent-send-events-in-correct-order-according-to-seq.patch --]
[-- Type: text/x-patch; name="0001-uevent-send-events-in-correct-order-according-to-seq.patch", Size: 798 bytes --]
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index e66e9b6..596c40d 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -243,6 +243,9 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
else if (action == KOBJ_REMOVE)
kobj->state_remove_uevent_sent = 1;
+#if defined(CONFIG_NET)
+ mutex_lock(&uevent_sock_mutex);
+#endif
/* we will send an event, so request a new sequence number */
spin_lock(&sequence_lock);
seq = ++uevent_seqnum;
@@ -253,7 +256,6 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
#if defined(CONFIG_NET)
/* send netlink message */
- mutex_lock(&uevent_sock_mutex);
list_for_each_entry(ue_sk, &uevent_sock_list, list) {
struct sock *uevent_sock = ue_sk->sk;
struct sk_buff *skb;
^ permalink raw reply related
* Re: [PATCH] udev: fix problem due to unsorted events
From: Kay Sievers @ 2012-03-06 12:43 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <1331023681-27958-1-git-send-email-avagin@gmail.com>
On Tue, Mar 6, 2012 at 13:13, Andrew Wagin <avagin@gmail.com> wrote:
> 2012/3/6 Kay Sievers <kay.sievers@vrfy.org>:
>> On Tue, Mar 6, 2012 at 09:48, Andrey Vagin <avagin@gmail.com> wrote:
>>> From: Andrey Vagin <avagin@openvz.org>
>>>
>>> A kernel gives events, but this events can be unsorted.
>>> For example, here is log from system:
>>> udevd[77]: seq 924 queued, 'add' 'bdi'
>>> udevd[77]: seq 926 queued, 'add' 'block'
>>> udevd[77]: seq 927 queued, 'add' 'block'
>>> udevd[77]: seq 928 queued, 'add' 'block
>>> udevd[77]: seq 925 queued, 'add' 'drivers'
>>
>> The kernel cannot send un-ordered events. Please try to reproduce that
>> with 'udevadm monitor'.
>
> I can't reproduce this bug, because I have not this host already.
> I've attached the console.log. I have not other logs.
> I showed you messages from event_queue_insert(). Actually it's what you ask.
> I suppose that a kernel should not send un-ordered events, but it does.
>
> Let's look at kobject_uevent_env() in kernel source:
>
> spin_lock(&sequence_lock);
> seq = ++uevent_seqnum;
> spin_unlock(&sequence_lock);
>
> .... <-- here someone can send its event
>
> /* send netlink message */
> mutex_lock(&uevent_sock_mutex);
> .....
>
> I want to say that code contains a window between incrementing seqnum
> and sending an event.
Yeah, that looks suspicious. We need to find out what and how to fix
that. The queue handling in the daemon assumes that the events are
ordered, so there might not only a problem with 'settle'.
Kay
^ permalink raw reply
* Re: [PATCH] udev: fix problem due to unsorted events
From: Kay Sievers @ 2012-03-06 11:06 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <1331023681-27958-1-git-send-email-avagin@gmail.com>
On Tue, Mar 6, 2012 at 09:48, Andrey Vagin <avagin@gmail.com> wrote:
> From: Andrey Vagin <avagin@openvz.org>
>
> A kernel gives events, but this events can be unsorted.
> For example, here is log from system:
> udevd[77]: seq 924 queued, 'add' 'bdi'
> udevd[77]: seq 926 queued, 'add' 'block'
> udevd[77]: seq 927 queued, 'add' 'block'
> udevd[77]: seq 928 queued, 'add' 'block
> udevd[77]: seq 925 queued, 'add' 'drivers'
The kernel cannot send un-ordered events. Please try to reproduce that
with 'udevadm monitor'.
I doubt that this is a problem udev should try to work around, its
logic is entirely based on the assumption that kernel events are
always in order.
Kay
^ permalink raw reply
* [PATCH] udev: fix problem due to unsorted events
From: Andrey Vagin @ 2012-03-06 8:48 UTC (permalink / raw)
To: linux-hotplug
From: Andrey Vagin <avagin@openvz.org>
A kernel gives events, but this events can be unsorted.
For example, here is log from system:
udevd[77]: seq 924 queued, 'add' 'bdi'
udevd[77]: seq 926 queued, 'add' 'block'
udevd[77]: seq 927 queued, 'add' 'block'
udevd[77]: seq 928 queued, 'add' 'block
udevd[77]: seq 925 queued, 'add' 'drivers'
In this case "udevadm settle" returns an error:
queue is empty but kernel events still pending [928]<->[925]
Let's look at update_queue(). It contains the follow code:
/* now write to the queue */
if (state = DEVICE_QUEUED) {
udev_queue_export->queued_count++;
udev_queue_export->seqnum_min = seqnum;
}
where seqnum_min is latest sequence number in queue file. Probably we
should check that seqnum is not less than seqnum_min and update it only
in this case.
https://bugzilla.redhat.com/show_bug.cgi?idy9834
Signed-off-by: Andrey Vagin <avagin@gmail.com>
---
src/libudev-queue-private.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/libudev-queue-private.c b/src/libudev-queue-private.c
index 7177195..6a689c4 100644
--- a/src/libudev-queue-private.c
+++ b/src/libudev-queue-private.c
@@ -376,7 +376,8 @@ static int update_queue(struct udev_queue_export *udev_queue_export,
/* now write to the queue */
if (state = DEVICE_QUEUED) {
udev_queue_export->queued_count++;
- udev_queue_export->seqnum_min = seqnum;
+ if (udev_queue_export->seqnum_min < seqnum)
+ udev_queue_export->seqnum_min = seqnum;
} else {
udev_queue_export->waste_bytes += queue_record_size(devpath_len) + queue_record_size(0);
udev_queue_export->queued_count--;
--
1.7.7.6
^ permalink raw reply related
* RE: persistent minor ids and multipath devices
From: Janec, Jozef @ 2012-03-05 19:28 UTC (permalink / raw)
To: linux-hotplug
Hello All,
I looking for way how to configure persistent minor ids for multipath devices. There is way how to set persistent minor ids for lvols. This very useful when we are tracking performance information for devices. Almost all performance on linux is using devices dm253-id for tracking the information for the devices. Once the server is rebooted the minor ids for the devices are changed. Is there possible somehow set minor id for the device in device mapper via udev rules?
Thanks for every info
Best regards
Jozef
^ permalink raw reply
* Re: A rule gets applied only after running `udevadm test`
From: Kay Sievers @ 2012-03-05 16:38 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <jj0l5p$kl$1@dough.gmane.org>
On Mon, Mar 5, 2012 at 12:08, RogutÄ—s Sparnuotos <rogutes@googlemail.com> wrote:
> IIRC, a mere `udevadm trigger` used to work and now I see that Archlinux's
> initscripts have
> Â udevadm trigger --actiond --typeÞvices
That type of trigger should work fine.
Kay
^ permalink raw reply
* Re: A rule gets applied only after running `udevadm test`
From: Rogutės Sparnuotos @ 2012-03-05 11:08 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <jj0l5p$kl$1@dough.gmane.org>
On 2012.03.05 01:48, Kay Sievers wrote:
> On Sun, Mar 4, 2012 at 22:00, RogutÄ—s Sparnuotos<rogutes@googlemail.com> wrote:
>> I have 2 custom rules to rename network interfaces:
>>
>> SUBSYSTEM="net", ACTION="add", ATTR{address}="00:1f:d0:5a:7d:48",
>> NAME="eth_int"
>> SUBSYSTEM="net", ACTION="add", ATTR{address}="00:50:22:e9:7d:09",
>> NAME="eth1"
>>
>> But they aren't triggered on boot (although another rule from the same
>> file is applied). Now if I run
>>
>> $ udevadm test --actiond \
>> /sys/devices/pci0000:00/0000:00:1c.4/0000:04:00.0/net/eth1
>>
>> $ udevadm test --actiond \
>> /sys/devices/pci0000:00/0000:00:1e.0/0000:05:01.0/net/eth0
>>
>> the interfaces get renamed. What could I do to make these rules work on
>> boot? Could this be an udev bug caused by a module-less kernel?
>
> Does:
> udevadm trigger --actiond
> make it work the same way as running 'udevadm test'? Then it's more
> likely an issue with your init system/bootup logic and not with udev.
>
> Kay
Thank you for the hint, you were right.
IIRC, a mere `udevadm trigger` used to work and now I see that
Archlinux's initscripts have
udevadm trigger --actiond --type=subsystems
udevadm trigger --actiond --typeÞvices
^ permalink raw reply
* Re: A rule gets applied only after running `udevadm test`
From: Kay Sievers @ 2012-03-04 23:48 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <jj0l5p$kl$1@dough.gmane.org>
On Sun, Mar 4, 2012 at 22:00, RogutÄ—s Sparnuotos <rogutes@googlemail.com> wrote:
> I have 2 custom rules to rename network interfaces:
>
> SUBSYSTEM="net", ACTION="add", ATTR{address}="00:1f:d0:5a:7d:48",
> NAME="eth_int"
> SUBSYSTEM="net", ACTION="add", ATTR{address}="00:50:22:e9:7d:09",
> NAME="eth1"
>
> But they aren't triggered on boot (although another rule from the same
> file is applied). Now if I run
>
> $ udevadm test --actiond \
> /sys/devices/pci0000:00/0000:00:1c.4/0000:04:00.0/net/eth1
>
> $ udevadm test --actiond \
> /sys/devices/pci0000:00/0000:00:1e.0/0000:05:01.0/net/eth0
>
> the interfaces get renamed. What could I do to make these rules work on
> boot? Could this be an udev bug caused by a module-less kernel?
Does:
udevadm trigger --actiond
make it work the same way as running 'udevadm test'? Then it's more
likely an issue with your init system/bootup logic and not with udev.
Kay
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox