Re: replace in-memory rules array with match/action token list

Re: replace in-memory rules array with match/action token list

[Alan Jenkins]

> The in-memory rule array of a common desktop distro install took:
>   1151088 bytes
> with the token list:
>   109232 bytes tokens (6827 * 16 bytes), 71302 bytes buffer

Sounds great from a performance point of view.

It doesn't work for me though.  My simulation takes over 5 times as
long, and doesn't finish cleanly; i.e. there's still lots in
/dev/.udev/queue after the cpu usage has stopped.  I'll try to track it
down.

Re: replace in-memory rules array with match/action token list

[Alan Jenkins]

Alan Jenkins wrote:
>> The in-memory rule array of a common desktop distro install took:
>>   1151088 bytes
>> with the token list:
>>   109232 bytes tokens (6827 * 16 bytes), 71302 bytes buffer
>>     
>
> Sounds great from a performance point of view.
>
> It doesn't work for me though.  My simulation takes over 5 times as
> long, and doesn't finish cleanly; i.e. there's still lots in
> /dev/.udev/queue after the cpu usage has stopped.  I'll try to track it
> down.
>   
Sorry - the queue is cleaned up correctly.  I was just too impatient.

It's not a perfect test; I'm comparing

a391f49d7f5433e6204f35331b81391c2d110309 - good ..
b99028c96307e729303be8f6750418979a7488b9 - bad

which includes a few more commits, though the match/action list is
obviously the biggest one.

The bad version of udevd seems to be generating two extra uevents per
device, in addition to the ones generated by "udevadm trigger".  That
is, the output of "udevadm monitor --kernel" is three times as long!

It's not a change in udevmonitor or udevtrigger; I'm still using the
Ubuntu installed version of udevadm for testing.  I really don't
understand how this could happen.  Do you have any idea?

Thanks
Alan

Re: replace in-memory rules array with match/action token list

[Kay Sievers]

On Thu, Oct 23, 2008 at 13:34, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
> Alan Jenkins wrote:
>>> The in-memory rule array of a common desktop distro install took:
>>>   1151088 bytes
>>> with the token list:
>>>   109232 bytes tokens (6827 * 16 bytes), 71302 bytes buffer
>>>
>>
>> Sounds great from a performance point of view.
>>
>> It doesn't work for me though.  My simulation takes over 5 times as
>> long, and doesn't finish cleanly; i.e. there's still lots in
>> /dev/.udev/queue after the cpu usage has stopped.  I'll try to track it
>> down.
>>
> Sorry - the queue is cleaned up correctly.  I was just too impatient.
>
> It's not a perfect test; I'm comparing
>
> a391f49d7f5433e6204f35331b81391c2d110309 - good ..
> b99028c96307e729303be8f6750418979a7488b9 - bad
>
> which includes a few more commits, though the match/action list is
> obviously the biggest one.
>
> The bad version of udevd seems to be generating two extra uevents per
> device, in addition to the ones generated by "udevadm trigger".  That
> is, the output of "udevadm monitor --kernel" is three times as long!
>
> It's not a change in udevmonitor or udevtrigger; I'm still using the
> Ubuntu installed version of udevadm for testing.  I really don't
> understand how this could happen.  Do you have any idea?

Hmm, I don't see that here. Any rules you have, which may trigger
uevents, and which may behave differently now?
  grep uevent /etc/udev/rules.d/* /lib/udev/rules.d/* /dev/.udev/rules.d/*

Kay

Re: replace in-memory rules array with match/action token list

[Alan Jenkins]

Kay Sievers wrote:
> On Thu, Oct 23, 2008 at 13:34, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>   
>> Alan Jenkins wrote:
>>     
>>>> The in-memory rule array of a common desktop distro install took:
>>>>   1151088 bytes
>>>> with the token list:
>>>>   109232 bytes tokens (6827 * 16 bytes), 71302 bytes buffer
>>>>
>>>>         
>>> Sounds great from a performance point of view.
>>>
>>> It doesn't work for me though.  My simulation takes over 5 times as
>>> long, and doesn't finish cleanly; i.e. there's still lots in
>>> /dev/.udev/queue after the cpu usage has stopped.  I'll try to track it
>>> down.
>>>
>>>       
>> Sorry - the queue is cleaned up correctly.  I was just too impatient.
>>
>> It's not a perfect test; I'm comparing
>>
>> a391f49d7f5433e6204f35331b81391c2d110309 - good ..
>> b99028c96307e729303be8f6750418979a7488b9 - bad
>>
>> which includes a few more commits, though the match/action list is
>> obviously the biggest one.
>>
>> The bad version of udevd seems to be generating two extra uevents per
>> device, in addition to the ones generated by "udevadm trigger".  That
>> is, the output of "udevadm monitor --kernel" is three times as long!
>>
>> It's not a change in udevmonitor or udevtrigger; I'm still using the
>> Ubuntu installed version of udevadm for testing.  I really don't
>> understand how this could happen.  Do you have any idea?
>>     
>
> Hmm, I don't see that here. Any rules you have, which may trigger
> uevents, and which may behave differently now?
>   grep uevent /etc/udev/rules.d/* /lib/udev/rules.d/* /dev/.udev/rules.d/*
>
> Kay
>   

Nope.

Drat, I can't reproduce that any more.  It's still takes over five times
as long, but I can't see anything unexpected happening.

I tried using strace (piped through grep 'open.*uevent".*WR').  At that
point it seemed to stop happening, even when I rebooted and ran it
without strace.

All I can think is, I must have accidentally run "udevadm trigger" twice
before I started udevd.

Baaa

(sheepishly) Alan

Re: replace in-memory rules array with match/action token list

[Kay Sievers]

On Thu, Oct 23, 2008 at 16:16, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
> Kay Sievers wrote:
>> On Thu, Oct 23, 2008 at 13:34, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>>> Alan Jenkins wrote:
>>>
>>>>> The in-memory rule array of a common desktop distro install took:
>>>>>   1151088 bytes
>>>>> with the token list:
>>>>>   109232 bytes tokens (6827 * 16 bytes), 71302 bytes buffer
>>>>>
>>>> Sounds great from a performance point of view.

> It's still takes over five times
> as long, but I can't see anything unexpected happening.

Are you sure, you don't log/print debug messages somewhere?  We got a
few more dbg() messages in the code.

Kay

Re: replace in-memory rules array with match/action token list

[Alan Jenkins]

[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]

Kay Sievers wrote:
> On Thu, Oct 23, 2008 at 16:16, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>   
>> Kay Sievers wrote:
>>     
>>> On Thu, Oct 23, 2008 at 13:34, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>>>       
>>>> Alan Jenkins wrote:
>>>>
>>>>         
>>>>>> The in-memory rule array of a common desktop distro install took:
>>>>>>   1151088 bytes
>>>>>> with the token list:
>>>>>>   109232 bytes tokens (6827 * 16 bytes), 71302 bytes buffer
>>>>>>
>>>>>>             
>>>>> Sounds great from a performance point of view.
>>>>>           
>
>   
>> It's still takes over five times
>> as long, but I can't see anything unexpected happening.
>>     
>
> Are you sure, you don't log/print debug messages somewhere?  We got a
> few more dbg() messages in the code.
>
> Kay
>   

No.  I've done that before, and the messages ended up in
/var/log/daemon.log.  I checked it in case there were errors, and I
didn't see any debug messages there.  Oprofile said that most of the
time was in udevd, and it wasn't in a debug function.


Actually, the biggest increase was in "system" time.  read_hpet was the
top entry at 6%, hrtimers featured heavily in general, and I traced it
back to sys_nanosleep().  Which I thought might implicate
wait_for_file...  Hmm.  There's also a call usleep() in rename_netif(). 
Perhaps this error message is significant:

    "error changing netif name ath0 to ath1: Device or resource busy"

It only happens with the bad version.  The good version doesn't try to
rename the interface from "ath0".  I've attached my rules file.


But this error should cause rename_netif() to return without calling
usleep().  The only other high-res sleep in udevd is the usleep() in
wait_for_file().

And anticipating your next question:

    # grep WAIT /etc/udev/rules.d/*
    /etc/udev/rules.d/05-udev-early.rules:ACTION=="add",
KERNEL=="[0-9]*:[0-9]*", SUBSYSTEM=="scsi", WAIT_FOR_SYSFS="ioerr_cnt"


So I have two identifiables

 1)  The attached rules are being interpreted differently, changing the
name of the device.

 2)  wait_for_file is causing a lot more overhead for some reason.


Thanks
Alan

[-- Attachment #2: 70-persistent-net.rules --]
[-- Type: text/plain, Size: 972 bytes --]

# This file maintains persistent names for network interfaces.
# See udev(7) for syntax.
#
# Entries are automatically added by the 75-persistent-net-generator.rules
# file; however you are also free to add your own entries.

# PCI device 0x168c:0x001c (ath_pci)
SUBSYSTEM=="net", DRIVERS=="?*", ATTRS{address}=="00:15:af:87:43:9f", ATTRS{type}=="1", NAME="ath0"

# USB device 0x069a:0x0318 (cdc_ether)
SUBSYSTEM=="net", DRIVERS=="?*", ATTRS{address}=="00:1b:9e:19:45:8f", ATTR{type}=="1", NAME="eth0"

# PCI device 0x1969:0x2048 (atl2)
SUBSYSTEM=="net", DRIVERS=="?*", ATTRS{address}=="00:1f:c6:28:b4:f7", ATTR{type}=="1", NAME="eth1"

# USB device 0x:0x (cdc_ether)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="00:1b:9e:19:45:8f", ATTR{type}=="1", KERNEL=="eth*", NAME="eth2"

# PCI device 0x168c:0000:01:00.0 (ath5k_pci)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="00:15:af:87:43:9f", ATTR{type}=="1", KERNEL=="ath*", NAME="ath1"

Re: replace in-memory rules array with match/action token list

[Kay Sievers]

On Thu, Oct 23, 2008 at 18:10, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
> Kay Sievers wrote:
>> On Thu, Oct 23, 2008 at 16:16, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>>> Kay Sievers wrote:
>>>> On Thu, Oct 23, 2008 at 13:34, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>>>>> Alan Jenkins wrote:
>>>>>
>>>>>>> The in-memory rule array of a common desktop distro install took:
>>>>>>>   1151088 bytes
>>>>>>> with the token list:
>>>>>>>   109232 bytes tokens (6827 * 16 bytes), 71302 bytes buffer
>>>>>>>
>>>>>> Sounds great from a performance point of view.

> There's also a call usleep() in rename_netif().
> Perhaps this error message is significant:
>
>    "error changing netif name ath0 to ath1: Device or resource busy"
>
> It only happens with the bad version.  The good version doesn't try to
> rename the interface from "ath0".  I've attached my rules file.
>
> But this error should cause rename_netif() to return without calling
> usleep().

> And anticipating your next question:
>
>    # grep WAIT /etc/udev/rules.d/*
>    /etc/udev/rules.d/05-udev-early.rules:ACTION="add",
> KERNEL="[0-9]*:[0-9]*", SUBSYSTEM="scsi", WAIT_FOR_SYSFS="ioerr_cnt"

Yeah, this is no longer needed with recent kernels, and I don't have
it, so I didn't notice the wrong ordering.

>  1)  The attached rules are being interpreted differently, changing the
> name of the device.

> # PCI device 0x168c:0x001c (ath_pci)
> SUBSYSTEM="net", DRIVERS="?*", ATTRS{address}="00:15:af:87:43:9f", ATTRS{type}="1", NAME="ath0"

> # PCI device 0x168c:0000:01:00.0 (ath5k_pci)
> SUBSYSTEM="net", ACTION="add", DRIVERS="?*", ATTR{address}="00:15:af:87:43:9f", ATTR{type}="1", KERNEL="ath*", NAME="ath1"

For some reason you have duplicated MAC addresses here, which must not
happen. Might becaused by something like this bug:
  http://git.kernel.org/?p=linux/hotplug/udev.git;a=commitdiff;h›dca442ef3f7d54cea9827605ec635712009735

The new rule engine, does not special-case NAME=, so the last
assignment wins. Only NAME:= would prevent later changes as usual.

Thanks,
Kay