2018-10-15 01:36:59

by Frank Rowand

[permalink] [raw]
Subject: [PATCH] of: overlay: user space synchronization

From: Frank Rowand <[email protected]>

When an overlay is applied or removed, the live devicetree visible in
/proc/device-tree/, aka /sys/firmware/devicetree/base/, reflects the
changes. There is no method for user space to determine whether the
live devicetree was modified by overlay actions.

Provide a sysfs file, /sys/firmware/devicetree/tree_version, to allow
user space to determine if the live devicetree has remained unchanged
while a series of one or more accesses of /proc/device-tree/ occur.

The use of both dynamic devicetree modifications and overlay apply and
removal are not supported during the same boot cycle. Thus non-overlay
dynamic modifications are not reflected in the value of tree_version.

Example shell use:

done=1

while [ $done = 1 ] ; do

pre_version=$(cat /sys/firmware/devicetree/tree_version)
version=$pre_version
while [ $(( ${version} & 1 )) != 0 ] ; do
# echo is optional, sleep value can be tuned
echo "${version} sleeping"
sleep 2;
pre_version=$(cat /sys/firmware/devicetree/tree_version)
version=${pre_version}
done

# 'critical region'
# access /proc/device-tree/ one or more times

post_version=$(cat /sys/firmware/devicetree/tree_version)

if [ ${post_version} = ${pre_version} ] ; then
done=0
fi

done

Signed-off-by: Frank Rowand <[email protected]>
---
Documentation/ABI/testing/sysfs-firmware-ofw | 64 ++++++++++++++++++++++++---
drivers/of/base.c | 66 ++++++++++++++++++++++++++++
drivers/of/of_private.h | 2 +
drivers/of/overlay.c | 12 +++++
4 files changed, 137 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-firmware-ofw b/Documentation/ABI/testing/sysfs-firmware-ofw
index edcab3ccfcc0..4111797e8ed5 100644
--- a/Documentation/ABI/testing/sysfs-firmware-ofw
+++ b/Documentation/ABI/testing/sysfs-firmware-ofw
@@ -1,4 +1,10 @@
-What: /sys/firmware/devicetree/*
+What: /sys/firmware/devicetree/
+Date: November 2013
+Contact: Frank Rowand <[email protected]>, [email protected]
+Description:
+ Top level Open Firmware, aka devicetree, sysfs directory.
+
+What: /sys/firmware/devicetree/base
Date: November 2013
Contact: Grant Likely <[email protected]>, [email protected]
Description:
@@ -6,11 +12,6 @@ Description:
hardware, the device tree structure will be exposed in this
directory.

- It is possible for multiple device-tree directories to exist.
- Some device drivers use a separate detached device tree which
- have no attachment to the system tree and will appear in a
- different subdirectory under /sys/firmware/devicetree.
-
Userspace must not use the /sys/firmware/devicetree/base
path directly, but instead should follow /proc/device-tree
symlink. It is possible that the absolute path will change
@@ -20,13 +21,62 @@ Description:
filesystem support, and has largely the same semantics and
should be compatible with existing userspace.

- The contents of /sys/firmware/devicetree/ is a
+ The /sys/firmware/devicetree/base directory is the root
+ node of the devicetree.
+
+ The contents of /sys/firmware/devicetree/base is a
hierarchy of directories, one per device tree node. The
directory name is the resolved path component name (node
name plus address). Properties are represented as files
in the directory. The contents of each file is the exact
binary data from the device tree.

+What: /sys/firmware/devicetree/tree_version
+Date: October 2018
+KernelVersion: 4.20
+Contact: Frank Rowand <[email protected]>, [email protected]
+Description:
+ When an overlay is applied or removed, the live devicetree
+ visible in /proc/device-tree/, aka
+ /sys/firmware/devicetree/base/, reflects the changes.
+
+ tree_version provides a way for user space to determine if the
+ live devicetree has remained unchanged while a series of one
+ or more accesses of /proc/device-tree/ occur.
+
+ The use of both dynamic devicetree modifications and overlay
+ apply and removal are not supported during the same boot
+ cycle. Thus non-overlay dynamic modifications are not
+ reflected in the value of tree_version.
+
+ Example shell use of tree_version:
+
+ done=1
+
+ while [ $done = 1 ] ; do
+
+ pre_version=$(cat /sys/firmware/devicetree/tree_version)
+ version=$pre_version
+ while [ $(( ${version} & 1 )) != 0 ] ; do
+ # echo is optional, sleep value can be tuned
+ echo "${version} sleeping"
+ sleep 2;
+ pre_version=$(cat /sys/firmware/devicetree/tree_version)
+ version=${pre_version}
+ done
+
+ # 'critical region'
+ # access /proc/device-tree/ one or more times
+
+ post_version=$(cat /sys/firmware/devicetree/tree_version)
+
+ if [ ${post_version} = ${pre_version} ] ; then
+ done=0
+ fi
+
+ done
+
+
What: /sys/firmware/fdt
Date: February 2015
KernelVersion: 3.19
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 466e3c8582f0..ec0428e47577 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -151,9 +151,71 @@ int of_free_phandle_cache(void)
late_initcall_sync(of_free_phandle_cache);
#endif

+#define show_one(object) \
+ static ssize_t show_##object \
+ (struct kobject *kobj, struct attribute *attr, char *buf) \
+ { \
+ return sprintf(buf, "%u\n", object); \
+ }
+
+struct global_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct kobject *kobj,
+ struct attribute *attr, char *buf);
+ ssize_t (*store)(struct kobject *a, struct attribute *b,
+ const char *c, size_t count);
+};
+
+#define define_one_global_ro(_name) \
+static struct global_attr attr_##_name = \
+__ATTR(_name, 0444, show_##_name, NULL)
+
+/*
+ * tree_version must be unsigned so that at maximum value it wraps
+ * from an odd value (4294967295) to an even value (0).
+ */
+static u32 tree_version;
+
+show_one(tree_version);
+
+define_one_global_ro(tree_version);
+
+static struct attribute *of_attributes[] = {
+ &attr_tree_version.attr,
+ NULL
+};
+
+static const struct attribute_group of_attr_group = {
+ .attrs = of_attributes,
+};
+
+/*
+ * internal documentation
+ * tree_version_increment() - increment base version
+ *
+ * Before starting overlay apply or overlay remove, call this function.
+ * After completing overlay apply or overlay remove, call this function.
+ *
+ * caller must hold of_mutex
+ *
+ * Userspace can use the value of this variable to determine whether the
+ * devicetree has changed while accessing the devicetree. An odd value
+ * means a modification is underway. An even value means no modification
+ * is underway.
+ *
+ * The use of both (1) dynamic devicetree modifications and (2) overlay apply
+ * and removal are not supported during the same boot cycle. Thus non-overlay
+ * dynamic modifications are not reflected in the value of tree_version.
+ */
+void tree_version_increment(void)
+{
+ tree_version++;
+}
+
void __init of_core_init(void)
{
struct device_node *np;
+ int ret;

of_populate_phandle_cache();

@@ -172,6 +234,10 @@ void __init of_core_init(void)
/* Symlink in /proc as required by userspace ABI */
if (of_root)
proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
+
+ ret = sysfs_create_group(&of_kset->kobj, &of_attr_group);
+ if (ret)
+ pr_err("sysfs_create_group of_attr_group failed: %d\n", ret);
}

static struct property *__of_find_property(const struct device_node *np,
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 216175d11d3d..adf89f6bad81 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -31,6 +31,8 @@ struct alias_prop {
extern struct list_head aliases_lookup;
extern struct kset *of_kset;

+void tree_version_increment(void);
+
#if defined(CONFIG_OF_DYNAMIC)
extern int of_property_notify(int action, struct device_node *np,
struct property *prop, struct property *old_prop);
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index eda57ef12fd0..53b1810b1e03 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -770,6 +770,9 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
of_overlay_mutex_lock();
mutex_lock(&of_mutex);

+ /* live tree may change after this point, user space synchronization */
+ tree_version_increment();
+
ret = of_resolve_phandles(tree);
if (ret)
goto err_free_tree;
@@ -832,6 +835,9 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
free_overlay_changeset(ovcs);

out_unlock:
+ /* live tree change complete, user space synchronization */
+ tree_version_increment();
+
mutex_unlock(&of_mutex);
of_overlay_mutex_unlock();

@@ -1028,6 +1034,9 @@ int of_overlay_remove(int *ovcs_id)

mutex_lock(&of_mutex);

+ /* live tree may change after this point, user space synchronization */
+ tree_version_increment();
+
ovcs = idr_find(&ovcs_idr, *ovcs_id);
if (!ovcs) {
ret = -ENODEV;
@@ -1083,6 +1092,9 @@ int of_overlay_remove(int *ovcs_id)
free_overlay_changeset(ovcs);

out_unlock:
+ /* live tree change complete, user space synchronization */
+ tree_version_increment();
+
mutex_unlock(&of_mutex);

out:
--
Frank Rowand <[email protected]>



2018-10-15 08:26:23

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] of: overlay: user space synchronization

Hi Frank,

On Mon, Oct 15, 2018 at 3:36 AM <[email protected]> wrote:
> From: Frank Rowand <[email protected]>
>
> When an overlay is applied or removed, the live devicetree visible in
> /proc/device-tree/, aka /sys/firmware/devicetree/base/, reflects the
> changes. There is no method for user space to determine whether the
> live devicetree was modified by overlay actions.
>
> Provide a sysfs file, /sys/firmware/devicetree/tree_version, to allow
> user space to determine if the live devicetree has remained unchanged
> while a series of one or more accesses of /proc/device-tree/ occur.

Thanks for your patch!

> The use of both dynamic devicetree modifications and overlay apply and
> removal are not supported during the same boot cycle. Thus non-overlay
> dynamic modifications are not reflected in the value of tree_version.

What does this mean exactly, for users?
I am used to applying and removing overlays at run time (still
carrying Pantelis'
overlay configfs patches), but when would I use non-overlay dynamic
modifications?

> --- a/Documentation/ABI/testing/sysfs-firmware-ofw
> +++ b/Documentation/ABI/testing/sysfs-firmware-ofw
>
> +What: /sys/firmware/devicetree/tree_version
> +Date: October 2018
> +KernelVersion: 4.20
> +Contact: Frank Rowand <[email protected]>, [email protected]
> +Description:
> + When an overlay is applied or removed, the live devicetree
> + visible in /proc/device-tree/, aka
> + /sys/firmware/devicetree/base/, reflects the changes.
> +
> + tree_version provides a way for user space to determine if the
> + live devicetree has remained unchanged while a series of one
> + or more accesses of /proc/device-tree/ occur.
> +
> + The use of both dynamic devicetree modifications and overlay
> + apply and removal are not supported during the same boot
> + cycle. Thus non-overlay dynamic modifications are not
> + reflected in the value of tree_version.
> +
> + Example shell use of tree_version:
> +
> + done=1
> +
> + while [ $done = 1 ] ; do
> +
> + pre_version=$(cat /sys/firmware/devicetree/tree_version)
> + version=$pre_version
> + while [ $(( ${version} & 1 )) != 0 ] ; do
> + # echo is optional, sleep value can be tuned
> + echo "${version} sleeping"
> + sleep 2;
> + pre_version=$(cat /sys/firmware/devicetree/tree_version)
> + version=${pre_version}
> + done
> +
> + # 'critical region'
> + # access /proc/device-tree/ one or more times
> +
> + post_version=$(cat /sys/firmware/devicetree/tree_version)
> +
> + if [ ${post_version} = ${pre_version} ] ; then
> + done=0
> + fi
> +
> + done

Please say explicitly that tree_version contains a 32-bit unsigned
decimal number, which is incremented before and after every change.
I had to deduce that from the code.

IMHO that is more important than having the sample script here.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2018-10-15 18:10:40

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH] of: overlay: user space synchronization

On 10/15/18 01:24, Geert Uytterhoeven wrote:
> Hi Frank,
>
> On Mon, Oct 15, 2018 at 3:36 AM <[email protected]> wrote:
>> From: Frank Rowand <[email protected]>
>>
>> When an overlay is applied or removed, the live devicetree visible in
>> /proc/device-tree/, aka /sys/firmware/devicetree/base/, reflects the
>> changes. There is no method for user space to determine whether the
>> live devicetree was modified by overlay actions.
>>
>> Provide a sysfs file, /sys/firmware/devicetree/tree_version, to allow
>> user space to determine if the live devicetree has remained unchanged
>> while a series of one or more accesses of /proc/device-tree/ occur.
>
> Thanks for your patch!
>
>> The use of both dynamic devicetree modifications and overlay apply and
>> removal are not supported during the same boot cycle. Thus non-overlay
>> dynamic modifications are not reflected in the value of tree_version.
>
> What does this mean exactly, for users?
> I am used to applying and removing overlays at run time (still
> carrying Pantelis'
> overlay configfs patches), but when would I use non-overlay dynamic
> modifications?

To find some examples, 'git grep of_add_property'. (This is not a
comprehensive list, there are other similar functions.)

Some Powerpc systems use dynamic modifications of device trees, for
example adding and removing cpus.

Kexec uses dynamic modifications just before booting a new
kernel (so no interference with overlays). Devicetree
unittest uses it, with no interference with overlays.

There are also a few places platform code or a driver uses dynamic
modification. Possible conflicts with overlays are:
arch/arm/mach-mvebu/coherency.c
arch/arm/mach-omap2/timer.c

rcar_du_of.c is a known use that is grandfathered in. It currently
is not compatible with overlays.

drivers/macintosh/smu.c should not be an issue because I don't
expect any macintosh overlay.

Some of the reasons for not supporting both overlays and other
dynamic modifications on the same system might be possible to
resolve with additional code, but some might be difficult or
impossible to resolve. So that restriction might be loosened
or removed in the future. Some of the reasons are:

- dynamic modifications do not use the same locking mechanism
as overlay apply and removal, thus the devicetree could be
corrupted

- dynamic modifications of portions of the devicetree that
are the result of applying an overlay will (may?) cause
removal of the overlay to fail (or devicetree corruption?)

- (future concern: ) static validation of an overlay (or set
of overlays) against a specific base devicetree would not
be valid if the base devicetree is further modified by
dynamic modifications - this is theoretical since validation
is a currently under development feature and we do not know
what the final feature will look like

The plan for overlays has been to add specific use models (or
functionality or features) in a limited fashion initially, to
ensure that each feature is implemented to a sufficient degree
(in other words is not a hack that only works in limited
circumstances, such as the correct phase of the moon), works
robustly and is maintainable. Then as each feature or set
of features is found to be good enough, add more features.

I suspect that dynamic modification in general will remain not
compatible with overlays, with limited exceptions. Possible
exceptions would require stringent review and auditing, and
could incude devicetree unittest (even this one makes me
nervous) and some platform code (especially early boot code).


>> --- a/Documentation/ABI/testing/sysfs-firmware-ofw
>> +++ b/Documentation/ABI/testing/sysfs-firmware-ofw
>>
>> +What: /sys/firmware/devicetree/tree_version
>> +Date: October 2018
>> +KernelVersion: 4.20
>> +Contact: Frank Rowand <[email protected]>, [email protected]
>> +Description:
>> + When an overlay is applied or removed, the live devicetree
>> + visible in /proc/device-tree/, aka
>> + /sys/firmware/devicetree/base/, reflects the changes.
>> +
>> + tree_version provides a way for user space to determine if the
>> + live devicetree has remained unchanged while a series of one
>> + or more accesses of /proc/device-tree/ occur.
>> +
>> + The use of both dynamic devicetree modifications and overlay
>> + apply and removal are not supported during the same boot
>> + cycle. Thus non-overlay dynamic modifications are not
>> + reflected in the value of tree_version.
>> +
>> + Example shell use of tree_version:
>> +
>> + done=1
>> +
>> + while [ $done = 1 ] ; do
>> +
>> + pre_version=$(cat /sys/firmware/devicetree/tree_version)
>> + version=$pre_version
>> + while [ $(( ${version} & 1 )) != 0 ] ; do
>> + # echo is optional, sleep value can be tuned
>> + echo "${version} sleeping"
>> + sleep 2;
>> + pre_version=$(cat /sys/firmware/devicetree/tree_version)
>> + version=${pre_version}
>> + done
>> +
>> + # 'critical region'
>> + # access /proc/device-tree/ one or more times
>> +
>> + post_version=$(cat /sys/firmware/devicetree/tree_version)
>> +
>> + if [ ${post_version} = ${pre_version} ] ; then
>> + done=0
>> + fi
>> +
>> + done
>
> Please say explicitly that tree_version contains a 32-bit unsigned
> decimal number, which is incremented before and after every change.
> I had to deduce that from the code.

Good point. I'll add that.


>
> IMHO that is more important than having the sample script here.
>
> Gr{oetje,eeting}s,
>
> Geert
>


2018-10-15 20:40:13

by Alan Tull

[permalink] [raw]
Subject: Re: [PATCH] of: overlay: user space synchronization

On Mon, Oct 15, 2018 at 1:09 PM Frank Rowand <[email protected]> wrote:
>
> On 10/15/18 01:24, Geert Uytterhoeven wrote:
> >
> > Please say explicitly that tree_version contains a 32-bit unsigned
> > decimal number, which is incremented before and after every change.
> > I had to deduce that from the code.
>
> Good point. I'll add that.

Looking at the code, tree_version being odd or even means something.
tree_version is incremented every time the of_mutex is locked or
unlocked, such that:
* tree_version is odd == of_mutex is locked and the tree is
currently be in the process of being changed
* tree_version is even == of_version is unlocked again and the
changes are finished.

How about making this explicit in the interface by breaking it up into
two attributes:

/sys/firmware/devicetree/tree_lock == "locked" or "unlocked". If the
tree is locked, you know that the tree may still be changing and the
sysfs can't be trusted to be stable yet. Or maybe even more
specifically tree_overlay_lock?

/sys/firmware/devicetree/tree_version = a u32 that is incremented once
for every overlay added or removed.

Alan


>
>
> >
> > IMHO that is more important than having the sample script here.
> >
> > Gr{oetje,eeting}s,
> >
> > Geert
> >
>

2018-10-16 00:06:25

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH] of: overlay: user space synchronization

On 10/15/18 13:38, Alan Tull wrote:
> On Mon, Oct 15, 2018 at 1:09 PM Frank Rowand <[email protected]> wrote:
>>
>> On 10/15/18 01:24, Geert Uytterhoeven wrote:
>>>
>>> Please say explicitly that tree_version contains a 32-bit unsigned
>>> decimal number, which is incremented before and after every change.
>>> I had to deduce that from the code.
>>
>> Good point. I'll add that.
>
> Looking at the code, tree_version being odd or even means something.
> tree_version is incremented every time the of_mutex is locked or
> unlocked, such that:
> * tree_version is odd == of_mutex is locked and the tree is
> currently be in the process of being changed
> * tree_version is even == of_version is unlocked again and the
> changes are finished.
>
> How about making this explicit in the interface by breaking it up into
> two attributes:
>
> /sys/firmware/devicetree/tree_lock == "locked" or "unlocked". If the
> tree is locked, you know that the tree may still be changing and the
> sysfs can't be trusted to be stable yet. Or maybe even more
> specifically tree_overlay_lock?
>
> /sys/firmware/devicetree/tree_version = a u32 that is incremented once
> for every overlay added or removed.

I like the extra clarity of purpose that having two attributes provides,
but it makes the user space dance more difficult.

With a single attribute, the shell code is (updated from the patch
to remove the variable "version"):

done=1

while [ $done = 1 ] ; do

pre_version=$(cat /sys/firmware/devicetree/tree_version)
while [ $(( ${pre_version} & 1 )) != 0 ] ; do
# echo is optional, sleep value can be tuned
echo "${pre_version} tree locked, sleeping"
sleep 2;
pre_version=$(cat /sys/firmware/devicetree/tree_version)
done

# 'critical region'
# access /proc/device-tree/ one or more times

post_version=$(cat /sys/firmware/devicetree/tree_version)

if [ ${post_version} = ${pre_version} ] ; then
done=0
fi

done

With two attributes, the shell code is:


done=1

while [ $done = 1 ] ; do

# the order of the next three lines must not change
version=$(cat /sys/firmware/devicetree/tree_version)
pre_version=${version}
tree_lock=$(cat /sys/firmware/devicetree/tree_lock)
while [ tree_lock != "unlocked" ] ||
[ ${version} != ${pre_version} ] ; do
# echo is optional, sleep value can be tuned
echo "locked, sleeping"
sleep 2;
# the order of the next two lines must not change
pre_version=${version}
tree_lock=$(cat /sys/firmware/devicetree/tree_lock)
version=$(cat /sys/firmware/devicetree/tree_version)
done

# 'critical region'
# access /proc/device-tree/ one or more times

# the order of the next two lines must not change
post_version=$(cat /sys/firmware/devicetree/tree_version)
tree_lock=$(cat /sys/firmware/devicetree/tree_lock)

if [ ${tree_lock} = "unlocked" ] &&
[ ${post_version} = ${pre_version} ] ; then
done=0
fi

done


The two attribute example is untested, could have syntax errors, etc.
I'm also not sure that the logic is correct.

My opinion is that the extra shell complexity makes the two attribute
solution worse.

-Frank


>
> Alan
>
>
>>
>>
>>>
>>> IMHO that is more important than having the sample script here.
>>>
>>> Gr{oetje,eeting}s,
>>>
>>> Geert
>>>
>>
>

2018-10-16 19:41:18

by Alan Tull

[permalink] [raw]
Subject: Re: [PATCH] of: overlay: user space synchronization

On Mon, Oct 15, 2018 at 7:04 PM Frank Rowand <[email protected]> wrote:
>
> On 10/15/18 13:38, Alan Tull wrote:
> > On Mon, Oct 15, 2018 at 1:09 PM Frank Rowand <[email protected]> wrote:
> >>
> >> On 10/15/18 01:24, Geert Uytterhoeven wrote:
> >>>
> >>> Please say explicitly that tree_version contains a 32-bit unsigned
> >>> decimal number, which is incremented before and after every change.
> >>> I had to deduce that from the code.
> >>
> >> Good point. I'll add that.
> >
> > Looking at the code, tree_version being odd or even means something.
> > tree_version is incremented every time the of_mutex is locked or
> > unlocked, such that:
> > * tree_version is odd == of_mutex is locked and the tree is
> > currently be in the process of being changed
> > * tree_version is even == of_version is unlocked again and the
> > changes are finished.
> >
> > How about making this explicit in the interface by breaking it up into
> > two attributes:
> >
> > /sys/firmware/devicetree/tree_lock == "locked" or "unlocked". If the
> > tree is locked, you know that the tree may still be changing and the
> > sysfs can't be trusted to be stable yet. Or maybe even more
> > specifically tree_overlay_lock?
> >
> > /sys/firmware/devicetree/tree_version = a u32 that is incremented once
> > for every overlay added or removed.
>
> I like the extra clarity of purpose that having two attributes provides,
> but it makes the user space dance more difficult.
>
> With a single attribute, the shell code is (updated from the patch
> to remove the variable "version"):
>
> done=1
>
> while [ $done = 1 ] ; do
>
> pre_version=$(cat /sys/firmware/devicetree/tree_version)
> while [ $(( ${pre_version} & 1 )) != 0 ] ; do
> # echo is optional, sleep value can be tuned
> echo "${pre_version} tree locked, sleeping"
> sleep 2;
> pre_version=$(cat /sys/firmware/devicetree/tree_version)
> done
>
> # 'critical region'
> # access /proc/device-tree/ one or more times
>
> post_version=$(cat /sys/firmware/devicetree/tree_version)
>
> if [ ${post_version} = ${pre_version} ] ; then
> done=0
> fi
>
> done
>
> With two attributes, the shell code is:
>
>
> done=1
>
> while [ $done = 1 ] ; do
>
> # the order of the next three lines must not change
> version=$(cat /sys/firmware/devicetree/tree_version)
> pre_version=${version}
> tree_lock=$(cat /sys/firmware/devicetree/tree_lock)
> while [ tree_lock != "unlocked" ] ||
> [ ${version} != ${pre_version} ] ; do
> # echo is optional, sleep value can be tuned
> echo "locked, sleeping"
> sleep 2;
> # the order of the next two lines must not change
> pre_version=${version}
> tree_lock=$(cat /sys/firmware/devicetree/tree_lock)
> version=$(cat /sys/firmware/devicetree/tree_version)
> done
>
> # 'critical region'
> # access /proc/device-tree/ one or more times
>
> # the order of the next two lines must not change
> post_version=$(cat /sys/firmware/devicetree/tree_version)
> tree_lock=$(cat /sys/firmware/devicetree/tree_lock)
>
> if [ ${tree_lock} = "unlocked" ] &&
> [ ${post_version} = ${pre_version} ] ; then
> done=0
> fi
>
> done
>
>
> The two attribute example is untested, could have syntax errors, etc.
> I'm also not sure that the logic is correct.
>
> My opinion is that the extra shell complexity makes the two attribute
> solution worse.

Yes, I can see that now and agree with you here. Thanks for giving
the idea consideration. I'll review your v2 .

Alan

>
> -Frank
>
>
> >
> > Alan
> >
> >
> >>
> >>
> >>>
> >>> IMHO that is more important than having the sample script here.
> >>>
> >>> Gr{oetje,eeting}s,
> >>>
> >>> Geert
> >>>
> >>
> >