Patchwork [08/20] udev-cache: Create cache asynchronously

login
register
mail settings
Submitter Richard Tollerton
Date Aug. 4, 2014, 9:18 p.m.
Message ID <87zjfkvtrj.fsf@weregild.amer.corp.natinst.com>
Download mbox | patch
Permalink /patch/77261/
State New
Headers show

Comments

Richard Tollerton - Aug. 4, 2014, 9:18 p.m.
Otavio Salvador <otavio@ossystems.com.br> writes:

> I am not sure about this one. I see the value you are adding here but
> I worry how often something can be connected during this process and
> change the contents along the way. Did you see something as that
> during your tests?

No, but point taken. I didn't really test "physical" hotplug during
bootup -- and in particular, for this to be a persisting issue, the
added/removed device must be removed/added during this boot -- but I
suppose that is totally possible.

This reflects an underlying race condition, starting at the computation
of NEWDATA/OLDDATA in the udev initscript, and ending in the tarball
creation in udev-cache. I suppose that we can mitigate this by spinning
in udev-cache until the devcache stops changing...? Cheesy, untested,
dirty RFC follows:



> -- 
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://code.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
> --
Otavio Salvador - Aug. 4, 2014, 9:22 p.m.
On Mon, Aug 4, 2014 at 6:18 PM, Richard Tollerton <rich.tollerton@ni.com> wrote:
> Otavio Salvador <otavio@ossystems.com.br> writes:
>
>> I am not sure about this one. I see the value you are adding here but
>> I worry how often something can be connected during this process and
>> change the contents along the way. Did you see something as that
>> during your tests?
>
> No, but point taken. I didn't really test "physical" hotplug during
> bootup -- and in particular, for this to be a persisting issue, the
> added/removed device must be removed/added during this boot -- but I
> suppose that is totally possible.
>
> This reflects an underlying race condition, starting at the computation
> of NEWDATA/OLDDATA in the udev initscript, and ending in the tarball
> creation in udev-cache. I suppose that we can mitigate this by spinning
> in udev-cache until the devcache stops changing...? Cheesy, untested,
> dirty RFC follows:

You need to check the list of files taken by 'tar' and compare. This
mitigates it but does not really solves the issue.

Maybe we could "copy" the nodes to a tmp area and use this as a
snapshot? The copy should be fast and makes it mostly "atomic".
Richard Tollerton - Aug. 4, 2014, 9:37 p.m.
Otavio Salvador <otavio@ossystems.com.br> writes:

> On Mon, Aug 4, 2014 at 6:18 PM, Richard Tollerton <rich.tollerton@ni.com> wrote:
>> Otavio Salvador <otavio@ossystems.com.br> writes:
>>
>>> I am not sure about this one. I see the value you are adding here but
>>> I worry how often something can be connected during this process and
>>> change the contents along the way. Did you see something as that
>>> during your tests?
>>
>> No, but point taken. I didn't really test "physical" hotplug during
>> bootup -- and in particular, for this to be a persisting issue, the
>> added/removed device must be removed/added during this boot -- but I
>> suppose that is totally possible.
>>
>> This reflects an underlying race condition, starting at the computation
>> of NEWDATA/OLDDATA in the udev initscript, and ending in the tarball
>> creation in udev-cache. I suppose that we can mitigate this by spinning
>> in udev-cache until the devcache stops changing...? Cheesy, untested,
>> dirty RFC follows:
>
> You need to check the list of files taken by 'tar' and compare. This
> mitigates it but does not really solves the issue.

Hmm, I'm confused. If we relied entirely on /dev contents, how would we
detect (and correct for) if a device was removed between the execution
of udev and of udev-cache?

> Maybe we could "copy" the nodes to a tmp area and use this as a
> snapshot? The copy should be fast and makes it mostly "atomic".

I agree that the copy of the contents of /dev needs to be as fast as
possible -- your about using tmpfs as a staging area is well taken. But
beyond that, it seems to me that the root issue you articulated is about
ensuring adequate synchronization of udev.cache and the tarball;
rereading CMP_FILE_LIST in udev-cache thus seems unavoidable.

> -- 
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://code.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
Otavio Salvador - Aug. 4, 2014, 9:41 p.m.
On Mon, Aug 4, 2014 at 6:37 PM, Richard Tollerton <rich.tollerton@ni.com> wrote:
> Otavio Salvador <otavio@ossystems.com.br> writes:
>
>> On Mon, Aug 4, 2014 at 6:18 PM, Richard Tollerton <rich.tollerton@ni.com> wrote:
>>> Otavio Salvador <otavio@ossystems.com.br> writes:
>>>
>>>> I am not sure about this one. I see the value you are adding here but
>>>> I worry how often something can be connected during this process and
>>>> change the contents along the way. Did you see something as that
>>>> during your tests?
>>>
>>> No, but point taken. I didn't really test "physical" hotplug during
>>> bootup -- and in particular, for this to be a persisting issue, the
>>> added/removed device must be removed/added during this boot -- but I
>>> suppose that is totally possible.
>>>
>>> This reflects an underlying race condition, starting at the computation
>>> of NEWDATA/OLDDATA in the udev initscript, and ending in the tarball
>>> creation in udev-cache. I suppose that we can mitigate this by spinning
>>> in udev-cache until the devcache stops changing...? Cheesy, untested,
>>> dirty RFC follows:
>>
>> You need to check the list of files taken by 'tar' and compare. This
>> mitigates it but does not really solves the issue.
>
> Hmm, I'm confused. If we relied entirely on /dev contents, how would we
> detect (and correct for) if a device was removed between the execution
> of udev and of udev-cache?
>
>> Maybe we could "copy" the nodes to a tmp area and use this as a
>> snapshot? The copy should be fast and makes it mostly "atomic".
>
> I agree that the copy of the contents of /dev needs to be as fast as
> possible -- your about using tmpfs as a staging area is well taken. But
> beyond that, it seems to me that the root issue you articulated is about
> ensuring adequate synchronization of udev.cache and the tarball;
> rereading CMP_FILE_LIST in udev-cache thus seems unavoidable.

Yes, that's why I think we need:

 - copy contents to a staging area
 - generate the tarball

This way we avoid the following steps:

 - generate the list of contents
 - generate the tarball
 - generate tarball's contents
 - compare, loop if does not match.

Makes sense?
Richard Tollerton - Aug. 4, 2014, 10:07 p.m.
Otavio Salvador <otavio@ossystems.com.br> writes:

> On Mon, Aug 4, 2014 at 6:37 PM, Richard Tollerton <rich.tollerton@ni.com> wrote:
>> Otavio Salvador <otavio@ossystems.com.br> writes:
>>
>>> On Mon, Aug 4, 2014 at 6:18 PM, Richard Tollerton <rich.tollerton@ni.com> wrote:
>>>> Otavio Salvador <otavio@ossystems.com.br> writes:
>>>>
>>>>> I am not sure about this one. I see the value you are adding here but
>>>>> I worry how often something can be connected during this process and
>>>>> change the contents along the way. Did you see something as that
>>>>> during your tests?
>>>>
>>>> No, but point taken. I didn't really test "physical" hotplug during
>>>> bootup -- and in particular, for this to be a persisting issue, the
>>>> added/removed device must be removed/added during this boot -- but I
>>>> suppose that is totally possible.
>>>>
>>>> This reflects an underlying race condition, starting at the computation
>>>> of NEWDATA/OLDDATA in the udev initscript, and ending in the tarball
>>>> creation in udev-cache. I suppose that we can mitigate this by spinning
>>>> in udev-cache until the devcache stops changing...? Cheesy, untested,
>>>> dirty RFC follows:
>>>
>>> You need to check the list of files taken by 'tar' and compare. This
>>> mitigates it but does not really solves the issue.
>>
>> Hmm, I'm confused. If we relied entirely on /dev contents, how would we
>> detect (and correct for) if a device was removed between the execution
>> of udev and of udev-cache?
>>
>>> Maybe we could "copy" the nodes to a tmp area and use this as a
>>> snapshot? The copy should be fast and makes it mostly "atomic".
>>
>> I agree that the copy of the contents of /dev needs to be as fast as
>> possible -- your about using tmpfs as a staging area is well taken. But
>> beyond that, it seems to me that the root issue you articulated is about
>> ensuring adequate synchronization of udev.cache and the tarball;
>> rereading CMP_FILE_LIST in udev-cache thus seems unavoidable.
>
> Yes, that's why I think we need:
>
>  - copy contents to a staging area
>  - generate the tarball
>
> This way we avoid the following steps:
>
>  - generate the list of contents
>  - generate the tarball
>  - generate tarball's contents
>  - compare, loop if does not match.
>
> Makes sense?

Yes, with one friendly amendment: "copy contents" means copying
$CMP_FILE_LIST in addition to /dev. Then we rebuild cache.data from
that.

> -- 
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://code.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Patch

diff --git a/meta/recipes-core/udev/udev/udev-cache b/meta/recipes-core/udev/udev/udev-cache
index 9ad3b4d..3b744c5 100644
--- a/meta/recipes-core/udev/udev/udev-cache
+++ b/meta/recipes-core/udev/udev/udev-cache
@@ -28,6 +28,13 @@  fi
 [ -e "$DEVCACHE_CURRENT_SYSCONF" ] || exit 0
 [ "${VERBOSE}" == "no" ] || echo "found; building cache."
 
+# assume readfiles and CMP_FILE_LIST are copied from udev initscript
+readfiles "$DEVCACHE_SYSCONF"
+DATA2="$READDATA"
+
+while true; do
+	DATA1="$DATA2"
+
 	if [ ! `readlink -f /bin/tar` = "/bin/tar.tar" ]; then
 		(
 			find /dev \( -type b -o -type c -o -type l \) -xdev | cut -c 2- | xargs tar czf "${DEVCACHE}.tmp" -C / $BUSYBOX_DEVCACHE_CREATE_OPTS && \
@@ -42,4 +49,13 @@  else
 		) &
 	fi
 
+	readfiles $CMP_FILE_LIST
+	DATA2="$READDATA$RULELIST"
+	if [ "$DATA1" = "$DATA2" ]; then
+		break
+	else
+		echo "$DATA2" > "$DEVCACHE_CURRENT_SYSCONF"
+	fi
+done
+
 exit 0