Re: [PATCH] vpci: introduce per-domain lock to protect vpci structure

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.02.2022 14:36, Oleksandr Andrushchenko wrote:
> @@ -410,14 +428,37 @@ static void vpci_write_helper(const struct pci_dev 
> *pdev,
>               r->private);
>  }
>  
> +static bool vpci_header_write_lock(const struct pci_dev *pdev,
> +                                   unsigned int start, unsigned int size)
> +{
> +    /*
> +     * Writing the command register and ROM BAR register may trigger
> +     * modify_bars to run which in turn may access multiple pdevs while
> +     * checking for the existing BAR's overlap. The overlapping check, if 
> done
> +     * under the read lock, requires vpci->lock to be acquired on both 
> devices
> +     * being compared, which may produce a deadlock. It is not possible to
> +     * upgrade read lock to write lock in such a case. So, in order to 
> prevent
> +     * the deadlock, check which registers are going to be written and 
> acquire
> +     * the lock in the appropriate mode from the beginning.
> +     */
> +    if ( !vpci_offset_cmp(start, size, PCI_COMMAND, 2) )
> +        return true;
> +
> +    if ( !vpci_offset_cmp(start, size, pdev->vpci->header.rom_reg, 4) )
> +        return true;
> +
> +    return false;
> +}

A function of this name gives (especially at the call site(s)) the
impression of acquiring a lock. Considering that of the prefixes
neither "vpci" nor "header" are really relevant here, may I suggest
to use need_write_lock()?

May I further suggest that you either split the comment or combine
the two if()-s (perhaps even straight into single return statement)?
Personally I'd prefer the single return statement approach here ...

Jan