Re: [OpenWrt-Devel] [PATCH] procd: remove /dev filter on uevents
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.--- Begin Message --- > -Original Message- > From: John Crispin [mailto:j...@phrozen.org] > Sent: Friday, December 7, 2018 19:39 > To: Hattink, Tjalling ; openwrt- > de...@lists.openwrt.org > Subject: Re: RE: [OpenWrt-Devel] [PATCH] procd: remove /dev filter on > uevents > > > On 07/12/2018 11:11, Hattink, Tjalling wrote: > >> -Original Message- > >> From: openwrt-devel [mailto:openwrt-devel- > boun...@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 Hi, Very valid point. This would be the most clean solution when the hotplug scripts themselves would indicate which uevents they want to trigger on. This could also make hotplug itself more efficient if it has lists of uevents and scripts it should fire, instead of firing all scripts and see if one of them acts on it. Unfortunately I will not get the time from my employer to work on such a system, so I cannot provide follow-up patches. I hope someone else can eventually pick this up. Until then we will rework our hotplug scripts to somehow get triggered on this limited set of uevents. Best regards, Tjalling Hattink --- End Message --- ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] procd: remove /dev filter on uevents
Jo-Philipp Wich 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 ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] procd: remove /dev filter on uevents
On 07/12/2018 11:11, Hattink, Tjalling wrote: -Original Message- From: openwrt-devel [mailto:openwrt-devel-boun...@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 ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] procd: remove /dev filter on uevents
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.--- Begin Message --- > -Original Message- > From: openwrt-devel [mailto:openwrt-devel-boun...@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 --- End Message --- ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
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 signature.asc Description: OpenPGP digital signature ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[OpenWrt-Devel] [PATCH] procd: remove /dev filter on uevents
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.--- Begin Message ---
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
---
plug/udevtrigger.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
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, [4], sizeof(devpath));
--
2.14.1
--- End Message ---
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
