Messages in this thread | | | Date | Thu, 17 Aug 2023 16:22:18 -0700 | Subject | Re: [PATCH v2 03/15] mshyperv: Introduce numa_node_to_proximity_domain_info | From | Dave Hansen <> |
| |
On 8/17/23 15:01, Nuno Das Neves wrote: > +static inline union hv_proximity_domain_info > +numa_node_to_proximity_domain_info(int node) > +{ > + union hv_proximity_domain_info proximity_domain_info; > + > + if (node != NUMA_NO_NODE) { > + proximity_domain_info.domain_id = node_to_pxm(node); > + proximity_domain_info.flags.reserved = 0; > + proximity_domain_info.flags.proximity_info_valid = 1; > + proximity_domain_info.flags.proximity_preferred = 1; > + } else { > + proximity_domain_info.as_uint64 = 0; > + } > + > + return proximity_domain_info; > +}
Pop quiz: What are the rules for the 30 bits of uninitialized data of proximity_domain_info.flags in the (node != NUMA_NO_NODE) case?
I actually don't know off the top of my head. I generally avoid bitfields, but if they were normal stack-allocated variable space, they'd be garbage.
I'd also *much* rather see the "as_uint64 = 0" coded up as a memset() or even explicitly zeroing all the same variables as the other half of the if(). As it stands, it's not 100% obvious that proximity_domain_info is 64 bits and that .as_uint64=0 zeroes the whole thing. It *WOULD* be totally obvious if it were a memset().
| |