On 1/11/2019 1:54 PM, Luck, Tony wrote:
Jane,
It does look a bit fishy. But if there is an issue with “memdev”
changing once we release the locks,
then it will only help a little to move the “*flags = memdev->flags”
back before we release the locks.
Note that we access memdev one more time with the “return
memdev->physical_id”
Indeed, that line too.
But I’m not at all sure what the possible races are here. I hope
someone from the linx-nvdimm list
can comment. I think I just cut & pasted the bulk of those nested
loops from elsewhere. As far as I
know we don’t currently have any support for hotplug NFIT devices, so
I suspect there is currently
no race here.
I don't understand the bulk of the code, just trying to follow what you said
here. So if we *ever* decide to support hotplug NFIT device, what would remind
us to close the race here? Is there any harm to reference memdev while we
still have the locks?
Thanks!
-jane
-Tony
*From:*Jane Chu [mailto:[email protected]]
*Sent:* Friday, January 11, 2019 1:43 PM
*To:* Luck, Tony <[email protected]>
*Cc:* linux-nvdimm <[email protected]>
*Subject:* 23222f8f8dce6: acpi, nfit: Add function to look up nvdimm
device and provide SMBIOS handle
Hi, Tony,
I happen to be looking at the above patch, and noticed below change in
drivers/acpi/nfit/core.c
+ mutex_lock(&acpi_desc_lock);
+ list_for_each_entry(acpi_desc, &acpi_descs, list) {
+ mutex_lock(&acpi_desc->init_mutex);
+ list_for_each_entry(nfit_mem, &acpi_desc->dimms, list) {
+ memdev = __to_nfit_memdev(nfit_mem);
+ if (memdev->device_handle == device_handle) {
+ mutex_unlock(&acpi_desc->init_mutex);
+ mutex_unlock(&acpi_desc_lock);
+ *flags = memdev->flags; <-----
+ return memdev->physical_id;
+ }
+ }
+ mutex_unlock(&acpi_desc->init_mutex);
+ }
+ mutex_unlock(&acpi_desc_lock);
Is there a reason to retrieve the memdev->flags value after releasing
the locks?
Thanks!
-jane
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
