[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [PATCH 07/37] xen/x86: use paddr_t for addresses in NUMA node structure
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@xxxxxxxx> > Sent: 2022年1月19日 15:55 > To: Wei Chen <Wei.Chen@xxxxxxx> > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; xen- > devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx > Subject: Re: [PATCH 07/37] xen/x86: use paddr_t for addresses in NUMA node > structure > > On 19.01.2022 07:33, Wei Chen wrote: > >> From: Jan Beulich <jbeulich@xxxxxxxx> > >> Sent: 2022年1月18日 23:23 > >> > >> On 23.09.2021 14:02, Wei Chen wrote: > >>> @@ -249,24 +250,26 @@ static int __init numa_emulation(u64 start_pfn, > >> u64 end_pfn) > >>> void __init numa_initmem_init(unsigned long start_pfn, unsigned long > >> end_pfn) > >>> { > >>> int i; > >>> + paddr_t start, end; > >>> > >>> #ifdef CONFIG_NUMA_EMU > >>> if ( numa_fake && !numa_emulation(start_pfn, end_pfn) ) > >>> return; > >>> #endif > >>> > >>> + start = pfn_to_paddr(start_pfn); > >>> + end = pfn_to_paddr(end_pfn); > >> > >> Nit: Would be slightly neater if these were the initializers of the > >> variables. > > > > But if this function returns in numa_fake && !numa_emulation, > > will the two pfn_to_paddr operations be waste? > > And what harm would that do? Ok, two or three instructions waste in init time will not take too much harm. I will move them above with initializers. > > >>> @@ -441,7 +441,7 @@ void __init srat_parse_regions(u64 addr) > >>> acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat)) > >>> return; > >>> > >>> - srat_region_mask = pdx_init_mask(addr); > >>> + srat_region_mask = pdx_init_mask((u64)addr); > >> > >> I don't see the need for a cast here. > >> > > > > current addr type has been changed to paddr_t, but pdx_init_mask > > accept parameter type is u64. I know paddr_t is a typedef of > > u64 on Arm64/32, or unsinged long on x86. In current stage, > > their machine byte-lengths are the same. But in case of future > > changes I think an explicit case here maybe better? > > It may only ever be an up-cast, yet the compiler would do a widening > conversion (according to the usual type conversion rules) for us > anyway no matter whether there's a cast. Down-casts (in the general > compiler case, i.e. considering a wider set than just gcc and clang) > sometimes need making explicit to silence compiler warnings about > truncation, but I've not observed any compiler warning when widening > values. Ok, I will drop that cast. > > Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |