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

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

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.


> 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?

Thank you for you comments.
-grygorii


---
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 96001d5b7843..0448cc297445 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -81,8 +81,8 @@ struct dt_property {
 struct dt_device_node {
     const char *name;
     const char *type;
-    dt_phandle phandle;
     char *full_name;
+    dt_phandle phandle;
     domid_t used_by; /* By default it's used by dom0 */
 
     struct dt_property *properties;