of: overlay: user space synchronization
diff mbox series

Message ID 1539567282-1326-1-git-send-email-frowand.list@gmail.com
State New, archived
Headers show
Series
  • of: overlay: user space synchronization
Related show

Commit Message

Frank Rowand Oct. 15, 2018, 1:34 a.m. UTC
From: Frank Rowand <frank.rowand@sony.com>

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 <frank.rowand@sony.com>
---
 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(-)

Comments

Geert Uytterhoeven Oct. 15, 2018, 8:24 a.m. UTC | #1
Hi Frank,

On Mon, Oct 15, 2018 at 3:36 AM <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@sony.com>
>
> 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 <frowand.list@gmail.com>, devicetree@vger.kernel.org
> +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
Frank Rowand Oct. 15, 2018, 6:09 p.m. UTC | #2
On 10/15/18 01:24, Geert Uytterhoeven wrote:
> Hi Frank,
> 
> On Mon, Oct 15, 2018 at 3:36 AM <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> 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 <frowand.list@gmail.com>, devicetree@vger.kernel.org
>> +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
>
Alan Tull Oct. 15, 2018, 8:38 p.m. UTC | #3
On Mon, Oct 15, 2018 at 1:09 PM Frank Rowand <frowand.list@gmail.com> 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
> >
>
Frank Rowand Oct. 16, 2018, 12:04 a.m. UTC | #4
On 10/15/18 13:38, Alan Tull wrote:
> On Mon, Oct 15, 2018 at 1:09 PM Frank Rowand <frowand.list@gmail.com> 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
>>>
>>
>
Alan Tull Oct. 16, 2018, 7:39 p.m. UTC | #5
On Mon, Oct 15, 2018 at 7:04 PM Frank Rowand <frowand.list@gmail.com> wrote:
>
> On 10/15/18 13:38, Alan Tull wrote:
> > On Mon, Oct 15, 2018 at 1:09 PM Frank Rowand <frowand.list@gmail.com> 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
> >>>
> >>
> >

Patch
diff mbox series

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 <frowand.list@gmail.com>, devicetree@vger.kernel.org
+Description:
+		Top level Open Firmware, aka devicetree, sysfs directory.
+
+What:		/sys/firmware/devicetree/base
 Date:		November 2013
 Contact:	Grant Likely <grant.likely@arm.com>, devicetree@vger.kernel.org
 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 <frowand.list@gmail.com>, devicetree@vger.kernel.org
+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: