[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 07/10] xen: pci: add per-device locking
- To: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Tue, 28 Feb 2023 17:46:48 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=TzpvViBomllkuPX4Trt/Kd+pJisKBei7ccIpP0lPBsk=; b=AafQX8F6iJS25X9BENA2DAath+00PfG2rUcqtHyIRtwyYeGAIGo82SCdUNaMSHqFu1z2LLG4IZmGesk2i6P2QiEaHWpe+1/PdSlM8sCm7KdMl6OejhR3dJFF0Icsp2QgWNVI8cIiiaUMq1VtgzBIShXh7F1/xwVXPNtbqIjl9OjVllFtEhp2Gb0wRybMrsaPBzduVaNxaUeVH0Z2p5FvbOVumK/aeQL68isvoACArmeLlHL3ChgkjQbHRiEdXVVMJOsAmuG5g21xtk35m3f6JClQjQF3GKAokgD5YnGj3R0qMegmuI6cxVsNMb3RaFyhbxBCKBEjSpFCsw3MprvT2Q==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=l+z4NgV4rEY2ElrhhvtyQfajKANFOkYQ3/QQ1pU7+zsMEdVXk/oc8VkO1C79/uP8ps43/EHh8kxehb8Zzbd2DLU//7ELwH6gx8wSf2Pq0Yqc/2ZIQtO6+zZ4oikACFdpyQmFgDV0r5R+4b5aK0ONJbdWuNf0N6Zj5054aw7w5FeW3QnJg4WCjzilUOXrYw5U4xPFNnQigoPE+hrUHGVSu7bT73rwhaIy7v0ThJc3Mctlwzk3R0rkQDm+CjnAurJvnVfbOHFQfj+dGpmi0BzhYhbqjQtvfPrbBb+Y2Mjr3w9m0tD2rFvRHEo4jbzxjcKEtVIZ5manYn+AeOLipXpU6w==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
- Cc: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Tue, 28 Feb 2023 16:47:38 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 31.08.2022 16:11, Volodymyr Babchuk wrote:
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -203,10 +203,14 @@ static struct msi_desc *msixtbl_addr_to_desc(
>
> nr_entry = (addr - entry->gtable) / PCI_MSIX_ENTRY_SIZE;
>
> + pcidev_lock(entry->pdev);
> list_for_each_entry( desc, &entry->pdev->msi_list, list )
> if ( desc->msi_attrib.type == PCI_CAP_ID_MSIX &&
> - desc->msi_attrib.entry_nr == nr_entry )
> + desc->msi_attrib.entry_nr == nr_entry ) {
> + pcidev_unlock(entry->pdev);
> return desc;
This is a potentially problematic pattern: desc has a backlink to pdev,
just like "entry" has. _If_ locking is required here (and the
refcounting is insufficient), then it is questionable whether the lock
can actually be dropped before returning. The idea with refcounting was,
though, that entities holding a reference can be sure the pdev won't go
away.
But of course there's also the question what "access to device itself"
(as stated in the description) does (or does not) constitute. I think
it is pretty crucial that for every new lock it is spelled out clearly
what it protects.
Seeing the list iteration pattern here (and at least once below)
another question is whether a lock like the one here may want to be a
read/write one.
Jan
|
