On Tue, 22 Apr 2014 08:16:09 -0700, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > On Tue, Apr 22, 2014 at 09:10:35AM -0500, Rob Herring wrote: > > On Tue, Apr 22, 2014 at 9:06 AM, Grant Likely <grant.likely@xxxxxxxxxx> wrote: > > > On Thu, 17 Apr 2014 09:32:16 -0500, Rob Herring <robherring2@xxxxxxxxx> wrote: > > >> On Thu, Apr 17, 2014 at 2:48 AM, Xiubo Li <Li.Xiubo@xxxxxxxxxxxxx> wrote: > > >> > The of_update_property() is intented to update a property in a node > > >> > and if the property does not exist, will add it. > > >> > > > >> > The second search of the property is possibly won't be found, that > > >> > maybe removed by other thread just before the second search begain. > > >> > > > >> > Using the __of_find_property() and __of_add_property() instead and > > >> > move them into lock operations. > > >> > > >> I have a fix in my tree from Guenter which handles the race of the > > >> property being removed. However, your change will be needed when we > > >> have overlays and the node itself can be removed. So this is 3.16 > > >> material and will need to be rebased on Guenter's fix. > > > > > > I didn't think Guenter's patch actually fixed a race. It only fixed a > > > double checking of the oldprop pointer. Do I have it wrong? > > > > Yes, the commit message should have been clearer. The problem was that > > found was checked after updating the sysfs files. > > > Fixing the race was more of a side effect. It wasn't actually clear to me > at the time that there was a race; I realized it only after I read above > exchange. > > To clarify: My understanding is that the property could have been removed > after the call to of_find_property() found it. Thus, the actual search > for it can fail, and the check if it was found is necessary to prevent > action on the (now removed) old property. > > Did I get this right ? Not quite. Your change was correct for the existing code. The second oldprop check was not needed since there was no way to get there with it being NULL. However, your understanding of the race condition is correct. Xiubo fixed it by moving the of_find_property() call under the spinlock so there is no possibility for the property to disappear between find and update. g. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
