[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v9 02/16] vpci: use per-domain PCI lock to protect vpci structure


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 19 Sep 2023 17:55:42 +0200
  • 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=p0ZPEyw7aeGQ7+pn2Et2gUiel0EI+zK3LZNm12STbD0=; b=RYOLRwlvHSltC3kd2i80UHIVL1SnodBhJNMXOKajFxNtemaliMrNFarIHsAamNHyMqjmO6df+2w2ePtJX7unPQp5uZP575GvRFQJeClMoqfKPhDYpGSyMRrKcSxx+Aiqcm1ErRQnN+cN6vyuA26ZLb8N8MQ+Jb+Slo4pR2KVtV2DRLHnMKMTsoinB1e/LGSWbWbZDZY9ceITu1DwXnMsjxQnTNbtj2JGctGUh5AE82+cAdbME1E3q1w887+F+9oVTigT1v4IGplg70Y9J5cCPUC1q+bsP/NcoR0FDmrVxKyJ2VQ1Z6Cp/c0NWVreyp1ptsnVfLEPPKzvYIwtHiIk+Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jHrzlItGfPiY0lZWzuRDhuDJOBQeguUyuQUNVoDClN20/8x1lhOESH6G9SrZIerCSRQzOD9HSaBbJALI8oNHCpKOC6Wv76TljZIFju+4rCuep7mJKATFypay4IDXWnwsn8jmAVE670fDYdHfVpWyx86AO6K7TFRZo71m/Djkfqcv4fDC0zb683PRilZLiHn8zZcM6pETfyFoOA7n7vbd4Bv2sWUmiSV9U4RTKcGMbdW+eBDFBDw7OOy7s0fCyIOAX8wVm5Bkp6wF81wppbky8Jh+o31BPIoDhbTW+YBkO6E3bpG2wWIhpWGy/9L0mNLfV5GWDoeXlEIPY9adF5TytQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stewart Hildebrand <stewart.hildebrand@xxxxxxx>, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Tue, 19 Sep 2023 15:56:25 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 19.09.2023 17:39, Roger Pau Monné wrote:
> On Tue, Aug 29, 2023 at 11:19:42PM +0000, Volodymyr Babchuk wrote:
>> @@ -2908,7 +2908,13 @@ int allocate_and_map_msi_pirq(struct domain *d, int 
>> index, int *pirq_p,
>>  
>>      msi->irq = irq;
>>  
>> -    pcidevs_lock();
>> +    /*
>> +     * If we are called via vPCI->vMSI path, we already are holding
>> +     * d->pci_lock so there is no need to take pcidevs_lock, as it
>> +     * will cause lock inversion.
>> +     */
>> +    if ( !rw_is_locked(&d->pci_lock) )
>> +        pcidevs_lock();
> 
> This is not a safe expression to use, rw_is_locked() just returns
> whether the lock is taken, but not if it's taken by the current CPU.
> This is fine to use in assertions and debug code, but not in order to
> take lock ordering decisions I'm afraid.
> 
> You will likely need to move the locking to the callers of the
> function.

Along the lines of a later comment, I think it would by rw_is_write_locked()
here anyway. Noting that xen/rwlock.h already has an internal
_is_write_locked_by_me(), it would in principle be possible to construct
something along the lines of what the comment says. But it would certainly
be better if that could be avoided.

As to the comment: A lock inversion cannot be used to justify not
acquiring a necessary lock. The wording therefore wants adjusting (if the
logic was to stay).

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.