Re: Letting mdev + devtmpfs play together?
Hi, On 04/19/2015 06:57 PM, Denys Vlasenko wrote: On Sat, Apr 18, 2015 at 5:05 PM, Floris Bos wrote: Hi, I was wondering if it is possible to configure mdev NOT to add and remove device nodes, and let devtmpfs handle that part instead? examples/mdev.conf says: # Syntax: # [-]devicename_regex user:group mode [=path]|[>path]|[!] [@|$|*cmd args...] # [-]$ENVVAR=regexuser:group mode [=path]|[>path]|[!] [@|$|*cmd args...] # [-]@maj,min[-min2] user:group mode [=path]|[>path]|[!] [@|$|*cmd args...] # # [-]: do not stop on this match, continue reading mdev.conf # =: move, >: move and create a symlink # !: do not create device node ^ Thanks, missed that. Now have as first rule: -.*root:root 600 ! and that seems to work fine to let devtmpfs do the creation and removal, while still being able to use mdev for responding to certain devices. BTW did notice ! only works when FEATURE_MDEV_RENAME is enabled, which is a bit counter-intuitive. Race condition while writing the first sequence number? Meanwhile 527 and later process normally. However eventually 526 times out, writes 527 to .seq and the mess is complete Yes, this is all wrong. 526 should have just go ahead and do its thing, but not write a new seqfile. This is not really surprising, the sequence handling code was even more buggy before. When I tried to use it, I had to fix several bugs. I have committed two fixes just now, can you try current git? Thanks. Can confirm that at least the lower number no longer overwrites the seq: == mdev[524]: first seq written mdev[524]: 00:00:02.166766 ACTION:add SUBSYSTEM:block DEVNAME:mmcblk0p2 DEVPATH:/devices/platform/soc/3f30.mmc/mmc_host/mmc0/mmc0:1234/block/mmcblk0/mmcblk0p2 mdev[524]: dev 179,2 mdev[524]: rule matched, line 2 mdev[522]: 00:00:02.169764 ACTION:add SUBSYSTEM:block DEVNAME:mmcblk0 DEVPATH:/devices/platform/soc/3f30.mmc/mmc_host/mmc0/mmc0:1234/block/mmcblk0 mdev[522]: dev 179,0 mdev[522]: rule matched, line 2 mdev[524]: rule matched, line -1 mdev[525]: 00:00:02.170720 mdev.seq='524', need '525' mdev[524]: 00:00:02.170911 exiting mdev[523]: 00:00:02.172455 ACTION:add SUBSYSTEM:block DEVNAME:mmcblk0p1 DEVPATH:/devices/platform/soc/3f30.mmc/mmc_host/mmc0/mmc0:1234/block/mmcblk0/mmcblk0p1 mdev[523]: dev 179,1 mdev[523]: rule matched, line 2 mdev[522]: rule matched, line -1 mdev[522]: 00:00:02.173844 exiting mdev[525]: 00:00:02.174127 ACTION:add SUBSYSTEM:block DEVNAME:mmcblk0p5 DEVPATH:/devices/platform/soc/3f30.mmc/mmc_host/mmc0/mmc0:1234/block/mmcblk0/mmcblk0p5 == Yours sincerely, Floris Bos ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Letting mdev + devtmpfs play together?
On 19/04/2015 18:57, Denys Vlasenko wrote: # !: do not create device node Ah, I missed the "major = -2" trick. Sorry for giving out wrong information... I should check for all the available documentation before diving into the source (and verifying that the documentation is correct) >.> -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Letting mdev + devtmpfs play together?
On Sat, Apr 18, 2015 at 5:05 PM, Floris Bos wrote: > Hi, > > I was wondering if it is possible to configure mdev NOT to add and remove > device nodes, and let devtmpfs handle that part instead? examples/mdev.conf says: # Syntax: # [-]devicename_regex user:group mode [=path]|[>path]|[!] [@|$|*cmd args...] # [-]$ENVVAR=regexuser:group mode [=path]|[>path]|[!] [@|$|*cmd args...] # [-]@maj,min[-min2] user:group mode [=path]|[>path]|[!] [@|$|*cmd args...] # # [-]: do not stop on this match, continue reading mdev.conf # =: move, >: move and create a symlink # !: do not create device node ^ > I tried enabling waiting for sequence numbers by echoing a newline to > /dev/mdev.seq, however I noticed there are situations where that does not > work properly either. > > == > mdev[527]: first seq written > mdev[527]: 2.902914 ACTION:add SUBSYSTEM:usb DEVNAME:(null) > DEVPATH:/devices/platform/bcm2708_usb/usb1/1-1/1-1:1.0 > mdev[526]: 2.902980 waiting for '527' > == > > 526 waiting for 527? It says "I see '527' and that's not what it needs to be, I'm waiting for a correct one". The correct one would be '526' in this case. > Race condition while writing the first sequence number? > > Meanwhile 527 and later process normally. > However eventually 526 times out, writes 527 to .seq and the mess is > complete Yes, this is all wrong. 526 should have just go ahead and do its thing, but not write a new seqfile. This is not really surprising, the sequence handling code was even more buggy before. When I tried to use it, I had to fix several bugs. I have committed two fixes just now, can you try current git? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] libbb: remove unnecessary argument to nonblock_immune_read
On Sun, Apr 19, 2015 at 10:50:25AM +0100, Ron Yorston wrote: > The loop_on_EINTR argument to nonblock_immune_read is always set to 1. > > function old new delta > xmalloc_reads200 195 -5 > pgetc488 483 -5 > argstr 13131308 -5 > nonblock_immune_read 123 86 -37 > -- > (add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-52) Total: -52 bytes An even better question is whether the looping code could be eliminated entirely. Are any of the bb commands that use this code installing interurpting signal handlers? If not then the code for supporting EINTR is useless. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] libbb: remove unnecessary argument to nonblock_immune_read
The loop_on_EINTR argument to nonblock_immune_read is always set to 1. function old new delta xmalloc_reads200 195 -5 pgetc488 483 -5 argstr 13131308 -5 nonblock_immune_read 123 86 -37 -- (add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-52) Total: -52 bytes Signed-off-by: Ron Yorston --- include/libbb.h | 2 +- libbb/read_printf.c | 8 shell/ash.c | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/libbb.h b/include/libbb.h index 0f8363b..21da5f1 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -713,7 +713,7 @@ void* xrealloc_vector_helper(void *vector, unsigned sizeof_and_shift, int idx) F extern ssize_t safe_read(int fd, void *buf, size_t count) FAST_FUNC; -extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count, int loop_on_EINTR) FAST_FUNC; +extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count) FAST_FUNC; // NB: will return short read on error, not -1, // if some data was read before error occurred extern ssize_t full_read(int fd, void *buf, size_t count) FAST_FUNC; diff --git a/libbb/read_printf.c b/libbb/read_printf.c index 5ed6e36..b6a17cc 100644 --- a/libbb/read_printf.c +++ b/libbb/read_printf.c @@ -45,20 +45,20 @@ * which detects EAGAIN and uses poll() to wait on the fd. * Thankfully, poll() doesn't care about O_NONBLOCK flag. */ -ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count, int loop_on_EINTR) +ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count) { struct pollfd pfd[1]; ssize_t n; while (1) { - n = loop_on_EINTR ? safe_read(fd, buf, count) : read(fd, buf, count); + n = safe_read(fd, buf, count); if (n >= 0 || errno != EAGAIN) return n; /* fd is in O_NONBLOCK mode. Wait using poll and repeat */ pfd[0].fd = fd; pfd[0].events = POLLIN; /* note: safe_poll pulls in printf */ - loop_on_EINTR ? safe_poll(pfd, 1, -1) : poll(pfd, 1, -1); + safe_poll(pfd, 1, -1); } } @@ -81,7 +81,7 @@ char* FAST_FUNC xmalloc_reads(int fd, size_t *maxsz_p) p = buf + sz; sz += 128; } - if (nonblock_immune_read(fd, p, 1, /*loop_on_EINTR:*/ 1) != 1) { + if (nonblock_immune_read(fd, p, 1) != 1) { /* EOF/error */ if (p == buf) { /* we read nothing */ free(buf); diff --git a/shell/ash.c b/shell/ash.c index b568013..6718901 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5923,7 +5923,7 @@ expbackq(union node *cmd, int quoted, int quotes) read: if (in.fd < 0) break; - i = nonblock_immune_read(in.fd, buf, sizeof(buf), /*loop_on_EINTR:*/ 1); + i = nonblock_immune_read(in.fd, buf, sizeof(buf)); TRACE(("expbackq: read returns %d\n", i)); if (i <= 0) break; @@ -9677,7 +9677,7 @@ preadfd(void) #if ENABLE_FEATURE_EDITING retry: if (!iflag || g_parsefile->pf_fd != STDIN_FILENO) - nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1, /*loop_on_EINTR:*/ 1); + nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1); else { int timeout = -1; # if ENABLE_ASH_IDLE_TIMEOUT @@ -9719,7 +9719,7 @@ preadfd(void) } } #else - nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1, /*loop_on_EINTR:*/ 1); + nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1); #endif #if 0 /* disabled: nonblock_immune_read() handles this problem */ -- 2.1.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox