Re: [Xen-devel] [PATCH v5 7/7] VT-d: Fix vt-d Device-TLB flush timeout issue.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.02.16 at 09:47, <quan.xu@xxxxxxxxx> wrote:
>>  On February 18, 2016 4:36pm, <JBeulich@xxxxxxxx> wrote:
>> >>> On 18.02.16 at 08:36, <quan.xu@xxxxxxxxx> wrote:
>> >>  On February 17, 2016 10:41pm, <JBeulich@xxxxxxxx> wrote:
>> >> >>> On 05.02.16 at 11:18, <quan.xu@xxxxxxxxx> wrote:
>> >> > --- a/xen/drivers/passthrough/vtd/qinval.c
>> >> > +++ b/xen/drivers/passthrough/vtd/qinval.c
>> >> > +            if ( pci_hide_device(bus, devfn) )
>> >>
>> >> But now I'm _really_ puzzled: You acquire the exact lock that
>> >> pci_hide_device() acquires. Hence, unless I've overlooked an earlier
>> >> change,
>> > I
>> >> can't see this as other than an unconditional dead lock. Did you test
>> >> this
>> > code
>> >> path at all?
>> >
>> > Sorry, I didn't test this code path.
>> > I did test the follows:
>> >    1) Create domain with ATS device.
>> >    2) Attach / Detach ATS device.
>> >
>> > I think I could add a variation of pci_hide_device(), without
>> > "spin_lock(&pcidevs_lock) / spin_unlock(&pcidevs_lock)"
>> > Or "__init".
>> 
>> Which we already have - _pci_hide_device(). The function would just need to 
>> be
>> made non-static, but would otherwise fit your purposes since you also don't
>> need the alloc_pdev() the other function does.
> 
> But the _pci_hide_device() starts with '_', I think it is not a good idea to 
> make it non-static.
> I'd better introduce a new function to wrap the '_pci_hide_device()'. It is 
> difficult to name the new function.

Why add a wrapper - just rename the function (I indeed appreciate
you not wanting to globalize a function whose name starts with an
underscore), e.g. to pci_hide_device_locked() or
pci_hide_existing_device().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel