Discussion:
udev: Why non-blocking poll() with blocking recvmsg()?
Tetsuo Handa
2013-08-19 11:39:44 UTC
Permalink
Hello.

I'm experiencing a boot failure problem with wait-for-root utility in Ubuntu 12.04.
Although wait-for-root uses blocking socket, udev_monitor_receive_device() sometimes
immediately returns NULL.

I haven't identified which NULL in udev_monitor_receive_device() is returned.
But I came to wonder why poll() in udev_monitor_receive_device() uses timeout == 0.

---------- Quoting from http://git.kernel.org/cgit/linux/hotplug/udev.git/tree/src/libudev-monitor.c start ----------
UDEV_EXPORT struct udev_device *udev_monitor_receive_device(struct udev_monitor *udev_monitor)
{
(...snipped...)
retry:
(...snipped...)
buflen = recvmsg(udev_monitor->sock, &smsg, 0);
if (buflen < 0) {
if (errno != EINTR)
info(udev_monitor->udev, "unable to receive message\n");
return NULL;
}
(...snipped...)
/* skip device, if it does not pass the current filter */
if (!passes_filter(udev_monitor, udev_device)) {
struct pollfd pfd[1];
int rc;

udev_device_unref(udev_device);

/* if something is queued, get next device */
pfd[0].fd = udev_monitor->sock;
pfd[0].events = POLLIN;
rc = poll(pfd, 1, 0);
if (rc > 0)
goto retry;
return NULL;
}
(...snipped...)
}
---------- Quoting from http://git.kernel.org/cgit/linux/hotplug/udev.git/tree/src/libudev-monitor.c end ----------

recvmsg() waits until something is queued if udev_monitor->sock is blocking,
but poll() does not wait until something is queued even if udev_monitor->sock
is blocking. I think that this inconsistency makes udev_monitor_receive_device()
to immediately return NULL when an event where passes_filter() returns 0 was
already queued but another event where passes_filter() returns 1 is not yet
queued, making wait-for-root utility which is waiting for only "block" subsystem
events fail.

Why don't we unconditionally go to retry rather than poll(pfd, 1, 0) so that
"blocking socket can wait for next event" and "non-blocking socket can return
immediately"?

Regards.
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Martin Pitt
2013-08-19 12:15:59 UTC
Permalink
Hello Tetsuo,
Post by Tetsuo Handa
I'm experiencing a boot failure problem with wait-for-root utility in Ubuntu 12.04.
Note that this is an Ubuntu specific program.
Post by Tetsuo Handa
Although wait-for-root uses blocking socket, udev_monitor_receive_device() sometimes
immediately returns NULL.
It *assumed* a blocking socket, but didn't make sure that it actually
was. Originally libudev used blocking sockets, but at some point [1]
this was switched to non-blocking by default, and wait-for-root needed
to be adjusted to that. Please see https://launchpad.net/bugs/1154813,
it's fixed in newer Ubuntu releases.

[1] http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=13d83b8

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tetsuo Handa
2013-08-19 12:37:32 UTC
Permalink
Hello Martin.
Post by Martin Pitt
Post by Tetsuo Handa
Although wait-for-root uses blocking socket, udev_monitor_receive_device() sometimes
immediately returns NULL.
It *assumed* a blocking socket, but didn't make sure that it actually
was. Originally libudev used blocking sockets, but at some point [1]
this was switched to non-blocking by default, and wait-for-root needed
to be adjusted to that. Please see https://launchpad.net/bugs/1154813,
it's fixed in newer Ubuntu releases.
Yes, I have checked LP #1154813. But I think that my case is different.
I'm using Ubuntu 12.04 (Precise Pangolin), not Ubuntu 13.04 (Raring Ringtail);
I think [1] is not yet applied as of Ubuntu 12.04.

What #1154813 solves is that "make sure that udev_monitor_receive_device()
called by wait-for-root waits for event at recvmsg()".

What I'm talking is that "udev_monitor_receive_device() called by wait-for-root
does not wait for next event at poll() whereas it waits for first event at
recvmsg()".
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tetsuo Handa
2013-08-23 12:48:16 UTC
Permalink
Hello.
Post by Tetsuo Handa
I haven't identified which NULL in udev_monitor_receive_device() is returned.
I identified that "recvmsg() in udev_monitor_receive_device() returning ENOBUFS
error" is the cause of "udev_monitor_receive_device() returning NULL".

Therefore, I moved the discussion to LP #1215911.
https://launchpad.net/bugs/1215911

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Kay Sievers
2013-08-19 12:45:52 UTC
Permalink
On Mon, Aug 19, 2013 at 1:39 PM, Tetsuo Handa
Post by Tetsuo Handa
recvmsg() waits until something is queued if udev_monitor->sock is blocking,
Only on a blocking socket, which it isn't by default:
http://www.freedesktop.org/software/systemd/libudev/libudev-udev-monitor.html#udev-monitor-receive-device
Post by Tetsuo Handa
but poll() does not wait until something is queued even if udev_monitor->sock
is blocking.
I think that this inconsistency makes udev_monitor_receive_device()
to immediately return NULL when an event where passes_filter() returns 0 was
already queued but another event where passes_filter() returns 1 is not yet
queued, making wait-for-root utility which is waiting for only "block" subsystem
events fail.
Why don't we unconditionally go to retry rather than poll(pfd, 1, 0) so that
"blocking socket can wait for next event" and "non-blocking socket can return
immediately"?
It's from a time where the socket was still blocking, but we still
used poll() in calling code. We didn't want to block the caller, hence
the poll() inside the library.

The poll() inside of libudev-monitor.c can probably just be removed
today. Or even the whole retry be removed, it might not be a useful
optimization at all.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tetsuo Handa
2013-08-19 13:24:47 UTC
Permalink
Hello Kay.
Post by Kay Sievers
Post by Tetsuo Handa
recvmsg() waits until something is queued if udev_monitor->sock is blocking,
I confirmed using strace utility that Ubuntu 12.04's wait-for-root
(source code shown below) calls recvmsg() using blocking mode.

# strace /usr/lib/initramfs-tools/bin/wait-for-root /dev/sdaX 5
Post by Kay Sievers
It's from a time where the socket was still blocking, but we still
used poll() in calling code. We didn't want to block the caller, hence
the poll() inside the library.
So, "poll() in udev_monitor_receive_device() does not wait" is a feature?

Then, wait-for-root is responsible for retrying the loop (line 91 to 104) when
udev_monitor_receive_device() returned NULL (without waiting for a "block"
subsystem event) caused by udev_monitor_receive_device() getting a non-"block"
subsystem event at recvmsg(), isn't it?

---------- wait-for-root.c start ----------
1:#include <libudev.h>
2:
3:#include <sys/types.h>
4:#include <sys/stat.h>
5:
6:#include <time.h>
7:#include <stdio.h>
8:#include <limits.h>
9:#include <signal.h>
10:#include <stdlib.h>
11:#include <string.h>
12:#include <unistd.h>
13:
14:
15:static int device_queued (struct udev *udev, const char *path);
16:static int matching_device (struct udev_device *device, const char *path);
17:
18:static void alarm_handler (int signum);
19:
20:
21:int
22:main (int argc,
23: char *argv[])
24:{
25: const char * devpath;
26: char path[PATH_MAX];
27: int timeout;
28: struct udev * udev;
29: struct udev_monitor *udev_monitor;
30: struct stat devstat;
31: struct udev_device * udev_device;
32: const char * type;
33:
34: if (argc != 3) {
35: fprintf (stderr, "Usage: %s DEVICE TIMEOUT\n", argv[0]);
36: exit (2);
37: }
38:
39: devpath = argv[1];
40: if (! strncmp (devpath, "UUID=", 5)) {
41: strcpy (path, "/dev/disk/by-uuid/");
42: strcat (path, devpath + 5);
43: } else if (! strncmp (devpath, "LABEL=", 6)) {
44: strcpy (path, "/dev/disk/by-label/");
45: strcat (path, devpath + 6);
46: } else {
47: strcpy (path, devpath);
48: }
49:
50: timeout = atoi (argv[2]);
51:
52: signal (SIGALRM, alarm_handler);
53: alarm (timeout);
54:
55: /* Connect to the udev monitor first; if we stat() first, the
56: * event might happen between the stat() and the time we actually
57: * get hooked up.
58: */
59: udev = udev_new ();
60: udev_monitor = udev_monitor_new_from_netlink (udev, "udev");
61:
62: udev_monitor_filter_add_match_subsystem_devtype (udev_monitor, "block", NULL);
63: udev_monitor_enable_receiving (udev_monitor);
64:
65: /* If the device is not being processed, check to see whether it
66: * exists already on the filesystem. If this is true, we don't need
67: * to wait for it can obtain the filesystem type by looking up the
68: * udevdb record by major/minor.
69: */
70: if ((! device_queued (udev, devpath))
71: && (stat (path, &devstat) == 0)
72: && S_ISBLK (devstat.st_mode))
73: {
74: udev_device = udev_device_new_from_devnum (udev, 'b', devstat.st_rdev);
75: if (udev_device) {
76: type = udev_device_get_property_value (udev_device, "ID_FS_TYPE");
77: if (type) {
78: printf ("%s\n", type);
79:
80: udev_device_unref (udev_device);
81: goto exit;
82: }
83:
84: udev_device_unref (udev_device);
85: }
86: }
87:
88: /* When the device doesn't exist yet, or is still being processed
89: * by udev, use the monitor socket to wait it to be done.
90: */
91: while ((udev_device = udev_monitor_receive_device (udev_monitor)) != NULL) {
92: if (matching_device (udev_device, devpath)) {
93: type = udev_device_get_property_value (udev_device, "ID_FS_TYPE");
94: if (type) {
95: printf ("%s\n", type);
96:
97: udev_device_unref (udev_device);
98: goto exit;
99: }
100:
101: }
102:
103: udev_device_unref (udev_device);
104: }
105:
106:exit:
107: udev_monitor_unref (udev_monitor);
108: udev_unref (udev);
109:
110: exit (0);
111:}
112:
113:
114:static int
115:device_queued (struct udev *udev,
116: const char * devpath)
117:{
118: struct udev_queue * udev_queue;
119: struct udev_list_entry *queue_entry;
120: int found = 0;
121:
122: udev_queue = udev_queue_new (udev);
123:
124: for (queue_entry = udev_queue_get_queued_list_entry (udev_queue);
125: queue_entry != NULL;
126: queue_entry = udev_list_entry_get_next (queue_entry)) {
127: const char * syspath;
128: struct udev_device *udev_device;
129:
130: syspath = udev_list_entry_get_name (queue_entry);
131: udev_device = udev_device_new_from_syspath (udev, syspath);
132: if (udev_device) {
133: if (matching_device (udev_device, devpath))
134: found = 1;
135:
136: udev_device_unref (udev_device);
137: }
138: }
139:
140: udev_queue_unref (udev_queue);
141:
142: return found;
143:}
144:
145:static int
146:matching_device (struct udev_device *device,
147: const char * path)
148:{
149: const char * devnode;
150: struct udev_list_entry *devlinks_entry;
151:
152: /* Match by name */
153: devnode = udev_device_get_devnode (device);
154: if (devnode && (! strcmp (path, devnode)))
155: return 1;
156:
157: /* Match by UUID */
158: if (! strncmp (path, "UUID=", 5)) {
159: const char *uuid;
160:
161: uuid = udev_device_get_property_value (device, "ID_FS_UUID");
162: if (uuid && (! strcmp (path + 5, uuid)))
163: return 1;
164: }
165:
166: /* Match by LABEL */
167: if (! strncmp (path, "LABEL=", 6)) {
168: const char *label;
169:
170: label = udev_device_get_property_value (device, "ID_FS_LABEL");
171: if (label && (! strcmp (path + 6, label)))
172: return 1;
173: }
174:
175: /* Match by symlink */
176: for (devlinks_entry = udev_device_get_devlinks_list_entry (device);
177: devlinks_entry != NULL;
178: devlinks_entry = udev_list_entry_get_next (devlinks_entry))
179: if (! strcmp (path, udev_list_entry_get_name (devlinks_entry)))
180: return 1;
181:
182: return 0;
183:}
184:
185:
186:static void
187:alarm_handler (int signum)
188:{
189: exit (1);
190:}
---------- wait-for-root.c end ----------
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...