xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Quan Xu <quan.xu@intel.com>
Cc: "dario.faggioli@citrix.com" <dario.faggioli@citrix.com>,
	Feng Wu <feng.wu@intel.com>, Kevin Tian <kevin.tian@intel.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [Patch v11 3/3] vt-d: fix vt-d Device-TLB flush timeout issue
Date: Fri, 17 Jun 2016 02:40:34 -0600	[thread overview]
Message-ID: <5763D3A202000078000F5F46@prv-mh.provo.novell.com> (raw)
In-Reply-To: <945CA011AD5F084CBEA3E851C0AB28894B8E6E7D@SHSMSX103.ccr.corp.intel.com>

>>> On 17.06.16 at 10:15, <quan.xu@intel.com> wrote:
> On June 17, 2016 3:01 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> >>> On 17.06.16 at 08:08, <quan.xu@intel.com> wrote:
>> 
>> > On June 16, 2016 5:05 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> >> >>> On 16.06.16 at 10:42, <quan.xu@intel.com> wrote:
>> >> > On June 02, 2016 7:07 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> >> >> >>> On 01.06.16 at 11:05, <quan.xu@intel.com> wrote:
>> >> >> > +static void dev_invalidate_iotlb_timeout(struct iommu *iommu, u16
>> did,
>> >> >> > +                                         struct pci_ats_dev
>> >> >> > +*ats_dev) {
>> >> >> > +    struct domain *d = NULL;
>> >> >> > +    struct pci_dev *pdev;
>> >> >> > +
>> >> >> > +    if ( test_bit(did, iommu->domid_bitmap) )
>> >> >> > +        d = rcu_lock_domain_by_id(iommu->domid_map[did]);
>> >> >> > +
>> >> >> > +    /*
>> >> >> > +     * In case the domain has been freed or the IOMMU domid bitmap
>> is
>> >> >> > +     * not valid, the device no longer belongs to this domain.
>> >> >> > +     */
>> >> >> > +    if ( d == NULL )
>> >> >> > +        return;
>> >> >> > +
>> >> >> > +    pcidevs_lock();
>> >> >> > +
>> >> >> > +    for_each_pdev(d, pdev)
>> >> >> > +    {
>> >> >> > +        if ( (pdev->seg == ats_dev->seg) &&
>> >> >> > +             (pdev->bus == ats_dev->bus) &&
>> >> >> > +             (pdev->devfn == ats_dev->devfn) )
>> >> >> > +        {
>> >> >> > +            ASSERT(pdev->domain);
>> >> >> > +            list_del(&pdev->domain_list);
>> >> >> > +            pdev->domain = NULL;
>> >> >> > +            pci_hide_existing_device(pdev);
>> >> >> > +            break;
>> >> >> > +        }
>> >> >> > +    }
>> >> >> > +
>> >> >> > +    pcidevs_unlock();
>> >> >>
>> >> >> ... this loop (and locking). (Of course such a change may better
>> >> >> be done in another preparatory patch.)
>> >> >>
>> >> >
>> >> > To eliminate the locking?  I am afraid the locking is still a must
>> >> > here even without the loop, also referring  to device_assigned()..
>> >>
>> >> If the entire loop gets eliminated, what would be left is
>> >>
>> >>     pcidevs_lock();
>> >>     pcidevs_unlock();
>> >>
>> >> which I don't think does any good.
>> >
>> > Why? I can't follow it..
>> 
>> I don't understand your question. Can you explain what use above code
>> sequence is, in your opinion? Or else - what does the "why"
>> refer to?
>> 
> 
> Ah, there may be a gap between us. without this loop,  these pdev operation 
> should be still there, such as,
> 
> 
> +    ASSERT(pdev->domain);
> +    list_del(&pdev->domain_list);
> +    pdev->domain = NULL;
> +    pci_hide_existing_device(pdev);
> 
> So, the left is not only:
>    pcidevs_lock();
>    pcidevs_unlock();

Oh, indeed. My bad.

Jan


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

  reply	other threads:[~2016-06-17  8:40 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-01  9:05 [Patch v11 0/3] VT-d Device-TLB flush issue Xu, Quan
2016-06-01  9:05 ` [Patch v11 1/3] IOMMU: add a timeout parameter for device IOTLB invalidation Xu, Quan
2016-06-02 10:24   ` Jan Beulich
2016-06-15  2:55     ` Xu, Quan
2016-06-01  9:05 ` [Patch v11 2/3] vt-d: synchronize for Device-TLB flush one by one Xu, Quan
2016-06-02 10:49   ` Jan Beulich
2016-06-01  9:05 ` [Patch v11 3/3] vt-d: fix vt-d Device-TLB flush timeout issue Xu, Quan
2016-06-02 11:07   ` Jan Beulich
2016-06-16  8:42     ` Xu, Quan
2016-06-16  9:04       ` Jan Beulich
2016-06-17  6:08         ` Xu, Quan
2016-06-17  7:00           ` Jan Beulich
2016-06-17  8:15             ` Xu, Quan
2016-06-17  8:40               ` Jan Beulich [this message]
2016-06-22 15:54             ` Xu, Quan
2016-06-22 16:18               ` Jan Beulich
2016-06-23  2:08                 ` Xu, Quan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5763D3A202000078000F5F46@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=dario.faggioli@citrix.com \
    --cc=feng.wu@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=quan.xu@intel.com \
    --cc=xen-devel@lists.xen.org \
    --subject='Re: [Patch v11 3/3] vt-d: fix vt-d Device-TLB flush timeout issue' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).