Re: Letting mdev + devtmpfs play together?

2015-04-19 Thread Floris Bos

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?

2015-04-19 Thread Laurent Bercot

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?

2015-04-19 Thread Denys Vlasenko
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

2015-04-19 Thread Rich Felker
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

2015-04-19 Thread Ron Yorston
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