mdev -d can (silently) die with "read: no buffer space available"

[Alexander Zangerl]

On Sat, 14 Dec 2019 14:49:20 +0100, Jan Klötzke writes:
>Just to double check: did you increase BUFFER_SIZE or RCVBUF? I'm
>asking because the error that you observed (ENOBUFS) is returned by the
>kernel if the socket ran out of buffer space, that is mdev did not read
>the socket fast enough. OTOH if BUFFER_SIZE would be too small then the
>event would just be silently truncated.

i only increased BUFFER_SIZE.

--- a/util-linux/mdev.c
+++ b/util-linux/mdev.c
@@ -1150,7 +1150,7 @@ static void initial_scan(char *temp)
 #if ENABLE_FEATURE_MDEV_DAEMON
 
 /* uevent applet uses 16k buffer, and mmaps it before every read */
-# define BUFFER_SIZE (2 * 1024)
+# define BUFFER_SIZE (64 * 1024)
 # define RCVBUF (2 * 1024 * 1024)
 # define MAX_ENV 32

2 mb for the socket buffer seems ample, but
reading possibly that much in bites of only 2 kb feels a bit disproportional.
maybe reading only one such nibble per cycle is what causes the
buffer to grow faster than mdev can consume it? i'm not sure.

>This has always been this way. I think it makes sense for a plain "mdev
>-s" which is a synchronous invocation. But for "mdev -d" it might indeed
>be better to write everything to mdev.log, including the initial scan.

i concur.

>> i think open_mdev_log() should be called in daemon_loop, not just
>> after initial_scan() and bb_daemonize_or_rexec().
>
>That would incur a considerable syscall overhead for the regular case
>where mdev.log does not exist. Then mdev would try to open mdev.log for
>every event it receives.

i meant 'guarded by a stat() and inode comparison'.
that shouldn't be too expensive i think. how about this? the patch works
for any number of instances of mdev.log being created and removed later on,
and costs one stat().

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dynamic_mdev_log
Type: text/x-diff
Size: 671 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20191215/43f37ece/attachment.diff>
-------------- next part --------------

>Maybe adding SIGHUP handler that
>re-reads mdev.conf and SIGUSR1 to (re-)open mdev.log is a good idea...

that sounds great but maybe a bit more complex than necessary?

having to restart mdev to activate a new mdev.conf feels reasonable to me,
as the daemon doesn't have any internal state to keep long-term.

also, (re)opening the log only via signal would mean that there's a
near-zero chance of getting debug output for the initial scan.

regards
az


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20191215/43f37ece/attachment.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital Signature
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20191215/43f37ece/attachment.asc>