| Message ID | 20230602004824.20731-14-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dynamic node programming using overlay dtbo | expand |
Hi, On 02/06/2023 01:48, Vikram Garhwal wrote: > Dynamic programming ops will modify the dt_host and there might be other > function which are browsing the dt_host at the same time. To avoid the race > conditions, adding rwlock for browsing the dt_host. Reading this sentence, it sounds like you are adding the rwlock in this patch. However, this is not the case. Also, the rwlock is not only for browsing but also add new node. So how about ", we will need to add a rwlock to protect access to the dt_host". > But adding rwlock in > device_tree.h causes following circular dependency: > device_tree.h->rwlock.h->smp.h->asm/smp.h->device_tree.h > > To fix this, removed the "#include <xen/device_tree.h> and forward declared > "struct dt_device_node". > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> > Reviewed-by: Henry Wang <Henry.Wang@arm.com> > Reviewed-by: Michal Orzel <michal.orzel@amd.com> > --- > xen/arch/arm/include/asm/smp.h | 3 ++- > xen/arch/arm/smpboot.c | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/include/asm/smp.h b/xen/arch/arm/include/asm/smp.h > index a37ca55bff..b12949ba8a 100644 > --- a/xen/arch/arm/include/asm/smp.h > +++ b/xen/arch/arm/include/asm/smp.h > @@ -3,13 +3,14 @@ > > #ifndef __ASSEMBLY__ > #include <xen/cpumask.h> > -#include <xen/device_tree.h> > #include <asm/current.h> > #endif > > DECLARE_PER_CPU(cpumask_var_t, cpu_sibling_mask); > DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask); > > +struct dt_device_node; Can you add the declaration just above arch_cpu_init()? This will make clearer why the forward declaration is necessary. > + > #define cpu_is_offline(cpu) unlikely(!cpu_online(cpu)) > > #define smp_processor_id() get_processor_id() > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index e107b86b7b..eeb76cd551 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -10,6 +10,7 @@ > #include <xen/cpu.h> > #include <xen/cpumask.h> > #include <xen/delay.h> > +#include <xen/device_tree.h> > #include <xen/domain_page.h> > #include <xen/errno.h> > #include <xen/init.h> Cheers,
diff --git a/xen/arch/arm/include/asm/smp.h b/xen/arch/arm/include/asm/smp.h index a37ca55bff..b12949ba8a 100644 --- a/xen/arch/arm/include/asm/smp.h +++ b/xen/arch/arm/include/asm/smp.h @@ -3,13 +3,14 @@ #ifndef __ASSEMBLY__ #include <xen/cpumask.h> -#include <xen/device_tree.h> #include <asm/current.h> #endif DECLARE_PER_CPU(cpumask_var_t, cpu_sibling_mask); DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask); +struct dt_device_node; + #define cpu_is_offline(cpu) unlikely(!cpu_online(cpu)) #define smp_processor_id() get_processor_id() diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index e107b86b7b..eeb76cd551 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -10,6 +10,7 @@ #include <xen/cpu.h> #include <xen/cpumask.h> #include <xen/delay.h> +#include <xen/device_tree.h> #include <xen/domain_page.h> #include <xen/errno.h> #include <xen/init.h>
Dynamic programming ops will modify the dt_host and there might be other function which are browsing the dt_host at the same time. To avoid the race conditions, adding rwlock for browsing the dt_host. But adding rwlock in device_tree.h causes following circular dependency: device_tree.h->rwlock.h->smp.h->asm/smp.h->device_tree.h To fix this, removed the "#include <xen/device_tree.h> and forward declared "struct dt_device_node". Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> Reviewed-by: Henry Wang <Henry.Wang@arm.com> Reviewed-by: Michal Orzel <michal.orzel@amd.com> --- xen/arch/arm/include/asm/smp.h | 3 ++- xen/arch/arm/smpboot.c | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)