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

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 9/19/23 11:39, Roger Pau Monné wrote:
> On Tue, Aug 29, 2023 at 11:19:42PM +0000, Volodymyr Babchuk wrote:
>> diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
>> index 8f2b59e61a..a0733bb2cb 100644
>> --- a/xen/drivers/vpci/msi.c
>> +++ b/xen/drivers/vpci/msi.c
>> @@ -318,15 +321,28 @@ void vpci_dump_msi(void)
>>                       * holding the lock.
>>                       */
>>                      printk("unable to print all MSI-X entries: %d\n", rc);
>> -                    process_pending_softirqs();
>> -                    continue;
>> +                    goto pdev_done;
>>                  }
>>              }
>>
>>              spin_unlock(&pdev->vpci->lock);
>> + pdev_done:
>> +            /*
>> +             * Unlock lock to process pending softirqs. This is
>> +             * potentially unsafe, as d->pdev_list can be changed in
>> +             * meantime.
>> +             */
>> +            read_unlock(&d->pci_lock);
>>              process_pending_softirqs();
>> +            if ( !read_trylock(&d->pci_lock) )
>> +            {
>> +                printk("unable to access other devices for the domain\n");
>> +                goto domain_done;
> 
> Shouldn't the domain_done label be after the read_unlock(), so that we
> can proceed to try to dump the devices for the next domain?  With the
> proposed code a failure to acquire one of the domains pci_lock
> terminates the dump.
> 
>> +            }
>>          }
>> +        read_unlock(&d->pci_lock);
>>      }
>> + domain_done:
>>      rcu_read_unlock(&domlist_read_lock);
>>  }
>>

With the label moved, a no-op expression after the label is needed to make the 
compiler happy:

            }
        }
        read_unlock(&d->pci_lock);
 domain_done:
        (void)0;
    }
    rcu_read_unlock(&domlist_read_lock);
}


If the no-op is omitted, the compiler may complain (gcc 9.4.0):

drivers/vpci/msi.c: In function ‘vpci_dump_msi’:
drivers/vpci/msi.c:351:2: error: label at end of compound statement
  351 |  domain_done:
      |  ^~~~~~~~~~~