| Message ID | mailman.7032.1544175957.2376.openwrt-devel@lists.openwrt.org |
|---|---|
| State | Changes Requested |
| Delegated to: | John Crispin |
| Headers | show |
| Series | [OpenWrt-Devel] procd: remove /dev filter on uevents | expand |
Hi, I had a brief discussion with John on this matter and was being told that the reason for this filter was to optimize boot time. When we remove the /dev filter, boot time will increase considerably on lower end devices due to the resulting hotplug-call overhead of the huge volume of additional uevents. A better approach here would be to selectively whitelist uevents based on subsystem or similar attributes, e.g. `DEVTYPE=usb_device`. ~ Jo
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software. > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Jo-Philipp Wich > Sent: Friday, December 7, 2018 10:51 > To: openwrt-devel@lists.openwrt.org > Subject: Re: [OpenWrt-Devel] [PATCH] procd: remove /dev filter on uevents > > Hi, > > I had a brief discussion with John on this matter and was being told that the > reason for this filter was to optimize boot time. > > When we remove the /dev filter, boot time will increase considerably on > lower end devices due to the resulting hotplug-call overhead of the huge > volume of additional uevents. > > A better approach here would be to selectively whitelist uevents based on > subsystem or similar attributes, e.g. `DEVTYPE=usb_device`. > > ~ Jo I can imagine that this would increase boot times on low end devices. Looking at the commit message introducing the filter it seems to cut down the amount of events by half. How about adding a compile option to procd that enables/disables this filter. So by default this filter is enabled, but using a makemenu option in the procd configuration (similar as "Mount /tmp using zram" option) you would be able to disable the filter for high-end boards that require it. This would be fairly easy to implement. Best regards, Tjaling Hattink
On 07/12/2018 11:11, Hattink, Tjalling wrote: >> -----Original Message----- >> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] >> On Behalf Of Jo-Philipp Wich >> Sent: Friday, December 7, 2018 10:51 >> To: openwrt-devel@lists.openwrt.org >> Subject: Re: [OpenWrt-Devel] [PATCH] procd: remove /dev filter on uevents >> >> Hi, >> >> I had a brief discussion with John on this matter and was being told that the >> reason for this filter was to optimize boot time. >> >> When we remove the /dev filter, boot time will increase considerably on >> lower end devices due to the resulting hotplug-call overhead of the huge >> volume of additional uevents. >> >> A better approach here would be to selectively whitelist uevents based on >> subsystem or similar attributes, e.g. `DEVTYPE=usb_device`. >> >> ~ Jo > I can imagine that this would increase boot times on low end devices. > Looking at the commit message introducing the filter it seems to > cut down the amount of events by half. > > How about adding a compile option to procd that enables/disables this > filter. So by default this filter is enabled, but using a makemenu option > in the procd configuration (similar as "Mount /tmp using zram" option) > you would be able to disable the filter for high-end boards that require > it. This would be fairly easy to implement. > > Best regards, > Tjaling Hattink Hi, I actually have a rather strong opinion on this one and would prefer to hardcode uevents that we want to opt-in as Jo suggested. compile time options do look nice, but we have a trizillion of them already and they per default are not enabled in binary releases making them virtually useless to anyone that was not involved in adding them to the tree. John
Jo-Philipp Wich <jo@mein.io> writes: > A better approach here would be to selectively whitelist uevents based > on subsystem or similar attributes, e.g. `DEVTYPE=usb_device`. Just for the record: "DEVTYPE=usb_device" devices *do* have a "dev" attribute. Bjørn
diff --git a/plug/udevtrigger.c b/plug/udevtrigger.c index f87a95e..db0c29a 100644 --- a/plug/udevtrigger.c +++ b/plug/udevtrigger.c @@ -161,9 +161,8 @@ static int device_list_insert(const char *path) dbg("add '%s'" , path); - /* we only have a device, if we have a dev and an uevent file */ - if (!device_has_attribute(path, "/dev", S_IRUSR) || - !device_has_attribute(path, "/uevent", S_IWUSR)) + /* we can only trigger a hotplug event, if we have an uevent file */ + if (!device_has_attribute(path, "/uevent", S_IWUSR)) return -1; strlcpy(devpath, &path[4], sizeof(devpath));
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software. The udevtrigger tool is responsible for firing hotplug events for all devices that have been enumerated by the kernel before the hotplug daemon is running. This happens during the 'early' (coldplug) stage of procd. A filter is in place during the scan of devices that requires a dev attribute file to be present in the sysfs. The argument is that without this attribute you are not able to create a device node under '/dev'. But there might be other hotplug scripts that are for example do detection of certain connected devices that do not have a dev attribute file, for example USB devices and their siblings. To make sure these hotplug scripts are also called during coldplug the filter is removed. Signed-off-by: Tjalling Hattink <t.hattink@fugro.com> --- plug/udevtrigger.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)