| Message ID | 20220817143458.335938-7-justin.he@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Modularize ghes_edac driver | expand |
From: Jia He > Sent: 17 August 2022 15:35 > > ghes_estatus_caches should be add rcu annotation to avoid sparse warnings. > drivers/acpi/apei/ghes.c:733:25: sparse: sparse: incompatible types in comparison expression > (different address spaces): > drivers/acpi/apei/ghes.c:733:25: sparse: struct ghes_estatus_cache [noderef] __rcu * > drivers/acpi/apei/ghes.c:733:25: sparse: struct ghes_estatus_cache * > drivers/acpi/apei/ghes.c:813:25: sparse: sparse: incompatible types in comparison expression > (different address spaces): > drivers/acpi/apei/ghes.c:813:25: sparse: struct ghes_estatus_cache [noderef] __rcu * > drivers/acpi/apei/ghes.c:813:25: sparse: struct ghes_estatus_cache * > > unrcu_pointer is to strip the __rcu in cmpxchg. > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Jia He <justin.he@arm.com> > --- > drivers/acpi/apei/ghes.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index 9272d963b57d..92ae58f4f7bb 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -144,7 +144,7 @@ struct ghes_vendor_record_entry { > static struct gen_pool *ghes_estatus_pool; > static unsigned long ghes_estatus_pool_size_request; > > -static struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE]; > +static struct ghes_estatus_cache __rcu *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE]; > static atomic_t ghes_estatus_cache_alloced; > > static int ghes_panic_timeout __read_mostly = 30; > @@ -834,8 +834,9 @@ static void ghes_estatus_cache_add( > } > /* new_cache must be put into array after its contents are written */ > smp_wmb(); > - if (slot != -1 && cmpxchg(ghes_estatus_caches + slot, > - slot_cache, new_cache) == slot_cache) { > + if (slot != -1 && unrcu_pointer(cmpxchg(ghes_estatus_caches + slot, > + RCU_INITIALIZER(slot_cache), > + RCU_INITIALIZER(new_cache)))) { Did you test this? There seems to be an == missing. David > if (slot_cache) > call_rcu(&slot_cache->rcu, ghes_estatus_cache_rcu_free); > } else > -- > 2.25.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
> -----Original Message----- > From: David Laight <David.Laight@ACULAB.COM> > Sent: Wednesday, August 17, 2022 11:39 PM > To: Justin He <Justin.He@arm.com>; Ard Biesheuvel <ardb@kernel.org>; Len > Brown <lenb@kernel.org>; James Morse <James.Morse@arm.com>; Tony > Luck <tony.luck@intel.com>; Borislav Petkov <bp@alien8.de>; Mauro > Carvalho Chehab <mchehab@kernel.org>; Robert Richter <rric@kernel.org>; > Robert Moore <robert.moore@intel.com>; Qiuxu Zhuo > <qiuxu.zhuo@intel.com>; Yazen Ghannam <yazen.ghannam@amd.com> > Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-edac@vger.kernel.org; devel@acpica.org; Rafael J . Wysocki > <rafael@kernel.org>; Shuai Xue <xueshuai@linux.alibaba.com>; Jarkko > Sakkinen <jarkko@kernel.org>; linux-efi@vger.kernel.org; > toshi.kani@hpe.com; nd <nd@arm.com>; kernel test robot <lkp@intel.com> > Subject: RE: [PATCH v2 6/7] apei/ghes: Use unrcu_pointer for cmpxchg > > From: Jia He > > Sent: 17 August 2022 15:35 > > > > ghes_estatus_caches should be add rcu annotation to avoid sparse > warnings. > > drivers/acpi/apei/ghes.c:733:25: sparse: sparse: incompatible types > > in comparison expression (different address spaces): > > drivers/acpi/apei/ghes.c:733:25: sparse: struct > ghes_estatus_cache [noderef] __rcu * > > drivers/acpi/apei/ghes.c:733:25: sparse: struct > ghes_estatus_cache * > > drivers/acpi/apei/ghes.c:813:25: sparse: sparse: incompatible types > > in comparison expression (different address spaces): > > drivers/acpi/apei/ghes.c:813:25: sparse: struct > ghes_estatus_cache [noderef] __rcu * > > drivers/acpi/apei/ghes.c:813:25: sparse: struct > ghes_estatus_cache * > > > > unrcu_pointer is to strip the __rcu in cmpxchg. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Jia He <justin.he@arm.com> > > --- > > drivers/acpi/apei/ghes.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index > > 9272d963b57d..92ae58f4f7bb 100644 > > --- a/drivers/acpi/apei/ghes.c > > +++ b/drivers/acpi/apei/ghes.c > > @@ -144,7 +144,7 @@ struct ghes_vendor_record_entry { static struct > > gen_pool *ghes_estatus_pool; static unsigned long > > ghes_estatus_pool_size_request; > > > > -static struct ghes_estatus_cache > > *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE]; > > +static struct ghes_estatus_cache __rcu > > +*ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE]; > > static atomic_t ghes_estatus_cache_alloced; > > > > static int ghes_panic_timeout __read_mostly = 30; @@ -834,8 +834,9 > @@ > > static void ghes_estatus_cache_add( > > } > > /* new_cache must be put into array after its contents are written */ > > smp_wmb(); > > - if (slot != -1 && cmpxchg(ghes_estatus_caches + slot, > > - slot_cache, new_cache) == slot_cache) { > > + if (slot != -1 && unrcu_pointer(cmpxchg(ghes_estatus_caches + slot, > > + RCU_INITIALIZER(slot_cache), > > + RCU_INITIALIZER(new_cache)))) { > > Did you test this? > There seems to be an == missing. Sorry about it, -- Cheers, Justin (Jia He)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 9272d963b57d..92ae58f4f7bb 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -144,7 +144,7 @@ struct ghes_vendor_record_entry { static struct gen_pool *ghes_estatus_pool; static unsigned long ghes_estatus_pool_size_request; -static struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE]; +static struct ghes_estatus_cache __rcu *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE]; static atomic_t ghes_estatus_cache_alloced; static int ghes_panic_timeout __read_mostly = 30; @@ -834,8 +834,9 @@ static void ghes_estatus_cache_add( } /* new_cache must be put into array after its contents are written */ smp_wmb(); - if (slot != -1 && cmpxchg(ghes_estatus_caches + slot, - slot_cache, new_cache) == slot_cache) { + if (slot != -1 && unrcu_pointer(cmpxchg(ghes_estatus_caches + slot, + RCU_INITIALIZER(slot_cache), + RCU_INITIALIZER(new_cache)))) { if (slot_cache) call_rcu(&slot_cache->rcu, ghes_estatus_cache_rcu_free); } else
ghes_estatus_caches should be add rcu annotation to avoid sparse warnings. drivers/acpi/apei/ghes.c:733:25: sparse: sparse: incompatible types in comparison expression (different address spaces): drivers/acpi/apei/ghes.c:733:25: sparse: struct ghes_estatus_cache [noderef] __rcu * drivers/acpi/apei/ghes.c:733:25: sparse: struct ghes_estatus_cache * drivers/acpi/apei/ghes.c:813:25: sparse: sparse: incompatible types in comparison expression (different address spaces): drivers/acpi/apei/ghes.c:813:25: sparse: struct ghes_estatus_cache [noderef] __rcu * drivers/acpi/apei/ghes.c:813:25: sparse: struct ghes_estatus_cache * unrcu_pointer is to strip the __rcu in cmpxchg. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Jia He <justin.he@arm.com> --- drivers/acpi/apei/ghes.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)