Re: [PATCH v3] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()

From: Frank Rowand
Date: Fri Feb 16 2018 - 17:20:34 EST


On 02/16/18 01:04, Chintan Pandya wrote:
>
>
> On 2/15/2018 6:22 AM, frowand.list@xxxxxxxxx wrote:
>> From: Frank Rowand <frank.rowand@xxxxxxxx>
>>
>> Create a cache of the nodes that contain a phandle property. Use this
>> cache to find the node for a given phandle value instead of scanning
>> the devicetree to find the node. If the phandle value is not found
>> in the cache, of_find_node_by_phandle() will fall back to the tree
>> scan algorithm.
>>

< snip >

>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index ad28de96e13f..ab545dfa9173 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -91,10 +91,69 @@ int __weak of_node_to_nid(struct device_node *np)
>> Â }
>> Â #endif
>> Â +static struct device_node **phandle_cache;
>> +static u32 phandle_cache_mask;
>> +
>> +/*
>> + * Assumptions behind phandle_cache implementation:
>> + *ÂÂ - phandle property values are in a contiguous range of 1..n
>> + *
>> + * If the assumptions do not hold, then
>> + *ÂÂ - the phandle lookup overhead reduction provided by the cache
>> + *ÂÂÂÂ will likely be less
>> + */
>> +static void of_populate_phandle_cache(void)
>> +{
>> +ÂÂÂ unsigned long flags;
>> +ÂÂÂ u32 cache_entries;
>> +ÂÂÂ struct device_node *np;
>> +ÂÂÂ u32 phandles = 0;
>> +
>> +ÂÂÂ raw_spin_lock_irqsave(&devtree_lock, flags);
>> +
>> +ÂÂÂ kfree(phandle_cache);
>
> I couldn't understood this. Everything else looks good to me.

I will be adding a call to of_populate_phandle_cache() from the
devicetree overlay code. I put the kfree here so that the previous
cache memory is freed when a new cache is created.

Adding the call from the overlay code is not done in this
series because I have a patch series modifying overlays and
I do not want to create a conflict or ordering between that
series and that patch. The lack of the call from overlay
code means that overlay code will gain some of the overhead
reduction from this patch, but possibly not the entire reduction.


>
>> +ÂÂÂ phandle_cache = NULL;
>> +
>> +ÂÂÂ for_each_of_allnodes(np)

< snip >