[RFC] Invalidating udev cache...

[RFC] Invalidating udev cache...

[Denys Dmytriyenko]

On Tue, Mar 03, 2009 at 08:16:48AM -0700, Tom Rini wrote:
> On Tue, Mar 03, 2009 at 08:24:44AM +0100, Koen Kooi wrote:
> > On 03-03-09 06:11, Denys Dmytriyenko wrote:
> >> On Thu, Dec 04, 2008 at 06:41:50PM +0100, Koen Kooi wrote:
> >>> On 04-12-08 18:09, Mike (mwester) wrote:
> >>>> Mike (mwester) wrote:
> >>>>
> >>>>> A slight improvement would be to make the dev.tar file dependent upon
> >>>>> the bootargs; i.e. invalidate /etc/dev.tar file if the boot command line
> >>>>> doesn't match the current command line.  This could be a very fast
> >>>>> operation, just "cmp /proc/cmdline /etc/dev_cmdline" or similar.
> >>>> As I consider this further, we could actually just save and compare
> >>>> /proc/atags if that's present on the device in question (falling back to
> >>>> /proc/cmdline if not present).  That would catch *any* changes passed in
> >>>> to the kernel via the bootloader.
> >>>>
> >>>> Flashing a new kernel would seem to be another logical place to
> >>>> invalidate the cache, so adding a comparison of "uname -rv" would be a
> >>>> reasonable way to catch that.
> >>> That should indeed take care of bootargs and kernel version changes. I'm
> >>> still tempted to add option 2) to all that :)
> >>
> >> Sorry for bringing up this old discussion. Are there any plans to implement
> >> the above checks in udev?
> >
> > Plans yes, time no :(
> 
> Can we make sure they're optional?

I'd like to revive this old topic and propose a patch, which implements most 
of the above points. I'm not sure about making it optional and /proc/atags. 
So, anyway, the patch will follow, please provide comments.

-- 
Denys

[PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Denys Dmytriyenko]

Signed-off-by: Denys Dmytriyenko <denis@denix.org>
---
 recipes/udev/udev-124/init |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/recipes/udev/udev-124/init b/recipes/udev/udev-124/init
index 9e11f08..840cddd 100644
--- a/recipes/udev/udev-124/init
+++ b/recipes/udev/udev-124/init
@@ -34,7 +34,10 @@ LANG=C awk "\$2 == \"/dev\" && \$4 == \"tmpfs\" { exit 1 }" /proc/mounts && {
         mkdir -m 0755 /dev/shm
 }
 
-if [ -e /etc/dev.tar ]; then
+# Invalidate udev cache if the kernel or its bootargs/cmdline have changed
+[ -x /bin/uname ] && /bin/uname -mrspv > /tmp/uname || touch /tmp/uname
+[ -r /proc/cmdline ] && cat /proc/cmdline > /tmp/cmdline || touch /tmp/cmdline
+if [ -e /etc/dev.tar ] && cmp -s /tmp/uname /etc/uname && cmp -s /tmp/cmdline /etc/cmdline; then
 	(cd /; tar xf /etc/dev.tar)
 	not_first_boot=1
 fi
@@ -50,10 +53,15 @@ kill_udevd > "/dev/null" 2>&1
 		if [ "$not_first_boot" != "" ];then
 			/sbin/udevadm trigger --subsystem-nomatch=tty --subsystem-nomatch=mem --subsystem-nomatch=vc --subsystem-nomatch=vtconsole --subsystem-nomatch=misc --subsystem-nomatch=dcon --subsystem-nomatch=pci_bus  --subsystem-nomatch=graphics  --subsystem-nomatch=backlight --subsystem-nomatch=video4linux  --subsystem-nomatch=platform
 			(/sbin/udevadm settle --timeout=3; /sbin/udevadm control env STARTUP=)&
+			rm -f /tmp/uname
+			rm -f /tmp/cmdline
 		else
 			/sbin/udevadm trigger
 			/sbin/udevadm settle
+			echo -n " and populating dev cache"
 			(cd /; tar cf /etc/dev.tar dev)
+			mv /tmp/uname /etc/uname
+			mv /tmp/cmdline /etc/cmdline
 		fi
 
 echo
-- 
1.6.0.6

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Koen Kooi]

On 15-04-09 01:49, Denys Dmytriyenko wrote:

> +			mv /tmp/uname /etc/uname
> +			mv /tmp/cmdline /etc/cmdline

I think we should put those in /etc/udev or /var/lib/udev if udev will 
be the only consumer.

regards,

Koen

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Holger Schurig]

> On 15-04-09 01:49, Denys Dmytriyenko wrote:
> > +			mv /tmp/uname /etc/uname
> > +			mv /tmp/cmdline /etc/cmdline
>
> I think we should put those in /etc/udev or /var/lib/udev if
> udev will be the only consumer.

+1, e.g.

/etc/udev/saved.uname
/etc/udev/saved.cmdline

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Denys Dmytriyenko]

On Wed, Apr 15, 2009 at 08:54:04AM +0200, Holger Schurig wrote:
> > On 15-04-09 01:49, Denys Dmytriyenko wrote:
> > > +			mv /tmp/uname /etc/uname
> > > +			mv /tmp/cmdline /etc/cmdline
> >
> > I think we should put those in /etc/udev or /var/lib/udev if
> > udev will be the only consumer.
> 
> +1, e.g.
> 
> /etc/udev/saved.uname
> /etc/udev/saved.cmdline

Good point, I'll be sending the update. Are there any other 
comments/suggestions?

-- 
Denys

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Otavio Salvador]

On Wed, Apr 15, 2009 at 9:06 PM, Denys Dmytriyenko <denis@denix.org> wrote:
> On Wed, Apr 15, 2009 at 08:54:04AM +0200, Holger Schurig wrote:
>> > On 15-04-09 01:49, Denys Dmytriyenko wrote:
>> > > +                 mv /tmp/uname /etc/uname
>> > > +                 mv /tmp/cmdline /etc/cmdline
>> >
>> > I think we should put those in /etc/udev or /var/lib/udev if
>> > udev will be the only consumer.
>>
>> +1, e.g.
>>
>> /etc/udev/saved.uname
>> /etc/udev/saved.cmdline
>
> Good point, I'll be sending the update. Are there any other
> comments/suggestions?

It would be nice to, while on that, evaluate the possibility
of updating udev; 0.141 fixes a serious security whole
and even if most embedded systems usually runs as root,
there're many that doesn't.

-- 
Otavio Salvador                  O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854         http://projetos.ossystems.com.br

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Koen Kooi]

On 16-04-09 14:34, Otavio Salvador wrote:
> On Wed, Apr 15, 2009 at 9:06 PM, Denys Dmytriyenko<denis@denix.org>  wrote:
>> On Wed, Apr 15, 2009 at 08:54:04AM +0200, Holger Schurig wrote:
>>>> On 15-04-09 01:49, Denys Dmytriyenko wrote:
>>>>> +                 mv /tmp/uname /etc/uname
>>>>> +                 mv /tmp/cmdline /etc/cmdline
>>>>
>>>> I think we should put those in /etc/udev or /var/lib/udev if
>>>> udev will be the only consumer.
>>>
>>> +1, e.g.
>>>
>>> /etc/udev/saved.uname
>>> /etc/udev/saved.cmdline
>>
>> Good point, I'll be sending the update. Are there any other
>> comments/suggestions?
>
> It would be nice to, while on that, evaluate the possibility
> of updating udev; 0.141 fixes a serious security whole
> and even if most embedded systems usually runs as root,
> there're many that doesn't.

Also 141 will allow us to reduce the recipe to something like 'inherit 
autotools_stage' :)

regards,

Koen

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Tom Rini]

On Tue, Apr 14, 2009 at 07:49:58PM -0400, Denys Dmytriyenko wrote:

> Signed-off-by: Denys Dmytriyenko <denis@denix.org>
> ---
>  recipes/udev/udev-124/init |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)

So, for making it optional, how about adding something to /etc/default
to enable / disable dev caching all together?  And, I believe
/proc/atags would just be cp /proc/atags /tmp/atags || touch /tmp/atags.

-- 
Tom Rini

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Koen Kooi]

On 16-04-09 18:33, Tom Rini wrote:
> On Tue, Apr 14, 2009 at 07:49:58PM -0400, Denys Dmytriyenko wrote:
>
>> Signed-off-by: Denys Dmytriyenko<denis@denix.org>
>> ---
>>   recipes/udev/udev-124/init |   10 +++++++++-
>>   1 files changed, 9 insertions(+), 1 deletions(-)
>
> So, for making it optional, how about adding something to /etc/default
> to enable / disable dev caching all together?  And, I believe
> /proc/atags would just be cp /proc/atags /tmp/atags || touch /tmp/atags.

Do we really want atags in there? As the name says, they are ARM TAGs, 
and only when kexec is enabled. Does it buy us anything over /proc/cmdline?

regards,

Koen

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Tom Rini]

On Sat, Apr 18, 2009 at 11:31:38AM +0200, Koen Kooi wrote:
> On 16-04-09 18:33, Tom Rini wrote:
>> On Tue, Apr 14, 2009 at 07:49:58PM -0400, Denys Dmytriyenko wrote:
>>
>>> Signed-off-by: Denys Dmytriyenko<denis@denix.org>
>>> ---
>>>   recipes/udev/udev-124/init |   10 +++++++++-
>>>   1 files changed, 9 insertions(+), 1 deletions(-)
>>
>> So, for making it optional, how about adding something to /etc/default
>> to enable / disable dev caching all together?  And, I believe
>> /proc/atags would just be cp /proc/atags /tmp/atags || touch /tmp/atags.
>
> Do we really want atags in there? As the name says, they are ARM TAGs,  
> and only when kexec is enabled. Does it buy us anything over 
> /proc/cmdline?

I'll leave that up to mwester as it was his suggestion originally.

-- 
Tom Rini

Re: [PATCH] udev 124: add cache invalidation logic on kernel change or its bootargs/cmdline

[Mike (mwester)]

Tom Rini wrote:
> On Sat, Apr 18, 2009 at 11:31:38AM +0200, Koen Kooi wrote:
>> On 16-04-09 18:33, Tom Rini wrote:
>>> On Tue, Apr 14, 2009 at 07:49:58PM -0400, Denys Dmytriyenko wrote:
>>>
>>>> Signed-off-by: Denys Dmytriyenko<denis@denix.org>
>>>> ---
>>>>   recipes/udev/udev-124/init |   10 +++++++++-
>>>>   1 files changed, 9 insertions(+), 1 deletions(-)
>>> So, for making it optional, how about adding something to /etc/default
>>> to enable / disable dev caching all together?  And, I believe
>>> /proc/atags would just be cp /proc/atags /tmp/atags || touch /tmp/atags.
>> Do we really want atags in there? As the name says, they are ARM TAGs,  
>> and only when kexec is enabled. Does it buy us anything over 
>> /proc/cmdline?
> 
> I'll leave that up to mwester as it was his suggestion originally.

Executive Summary: I don't really care one way or another.


Detail for why this was mentioned in the first place:

The discussion that led up the ARM ATAG suggestion involved an
exploration of all the possible things that could happen to a device
that might cause trouble because of a stale cache.  Using /proc/cmdline
is good, but the suggestion to use the ATAG list was offered because it
captures not just the command line itself, but might also capture other
hardware changes.

As a specific example, consider the use case where one takes a bootable
SD-card image from one om-gta02 device, and plugs it into a different
one with a different hardware version (significant on the om-gta0x
devices).  The use of /proc/cmdline will result in a stale cache; the
use of /proc/atags will catch that the revision number in the atag list
has changed.

Is that sufficient reason to use /proc/atags?  I think that's a pretty
far-fetched and unusual use-case, and if someone is doing that, they
should expect things to break.

So I have no real-world concern if just /proc/cmdline is used.


Regards,
Mike (mwester)