Re: [PATCH] device-tree: optimize dt_device_for_passthrough()

[Julien Grall <julien@xxxxxxx>]

Hi,

On 11/02/2025 15:14, Grygorii Strashko wrote:
> On 11.02.25 14:32, Julien Grall wrote:
> > On 11/02/2025 11:57, Orzel, Michal wrote:
> > > On 11/02/2025 12:18, Grygorii Strashko wrote:
> > > > The dt_device_for_passthrough() is called many times during Xen
> > > > initialization and Dom0 creation. On every call it traverses struct
> > > > dt_device_node properties list and compares compares properties name 
> > > > with
> > > double "compares"
> 10x
>
> > > > "xen,passthrough" which is runtime overhead. This can be optimized by
> > > Are you sure? Looking at the calls, it's almost only used at boot 
> > > except for dt
> > > overlay.
> > >
> > > > marking dt_device_node as passthrough while unflattening DT.
> > > >
> > > > This patch introduced new struct dt_device_node property 
> > > > "is_passthrough"
> > > > which is filled if "xen,passthrough" property is present while 
> > > > unflattening
> > > > DT and dt_device_for_passthrough() just return it's value.
> > > In the past we were skeptical about adding new fields to the 
> > > dt_device_node
> > > structure for use cases like this one. I would say this optimization 
> > > is not
> > > worth it. Also, why would you optimize dt_device_for_passthrough but not
> > > e.g. dt_device_is_available.
> > So we are trading speed with memory usage. It looks like we may be 
> > using a padding, although I didn't check whether the existing 
> > structure could be packed...
> Actually I see it consumes nothing due to alignments:
> - before sizeof(dt_device_node)=144
> - after sizeof(dt_device_node)=144
>
> But it could be made bool is_passthrough:1; together with other bools, 
> and probably moved at the end of struct dt_device_node.
> By the way, below diff can save 8 bytes on arm64 per struct dt_device_node.
>
> > > You can check with other Arm maintainers.
> > Before forging an opinion, I'd like to see some numbers showing the 
> > performance improvement. Also, is this impacting only boot?
> Sry, indeed only boot, need to be more specific.
>
> My DT: ~700 nodes
> Number of calls till the end of create_dom0():
> (XEN) =============== dt_device_for_passthrough=2684 
> dt_device_is_available=1429.
> I've enabled console_timestamps=boot and did 5 boots and calculated 
> average - it gives ~20 microseconds improvement.
This doesn't seem to be worth it. But I also don't know what's the 
normal boot time on your system... I guess we are still in seconds?
> > Also, I agree with Michal, if this is a concern for 
> > dt_device_device_for_passthrough(). Then it would be a concern for a 
> > few others calls using dt_find_property(). Are you going to modify all 
> > of them?
> I follow the rule one patch one functional change. Regarding further 
> optimization - I think only generic properties can be optimized and 
> personally I see now only "xen,passthrough" and may be "status".
> Ok. 20 microseconds. it's probably more like a measurement error margin.
> Please advice if i should continue or drop?
See above. Regardless that, would you be able to provide a bit more 
information of your end goal? Are you intending to be able to boot 
Xen/dom0/dom0less guest in less than N milliseconds? How far are you 
from that target? Are those numbers done on the latest Xen?
Asking because there are probably bigger place where optimization can be 
done first.
Cheers,

--
Julien Grall