From: Joe Jin <joe.jin@oracle.com> To: Juergen Gross <jgross@suse.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: xen-devel <xen-devel@lists.xenproject.org> Subject: Re: Question regarding swiotlb-xen in Linux kernel Date: Fri, 19 Apr 2019 07:40:52 -0700 [thread overview] Message-ID: <d1ca67c3-23cd-12ef-2913-4e1afb62e061@oracle.com> (raw) In-Reply-To: <a0ec7385-bc7d-92c4-538b-cad8001fa5ef@suse.com> On 4/18/19 10:47 PM, Juergen Gross wrote: > On 19/04/2019 00:31, Joe Jin wrote: >> On 4/18/19 2:09 PM, Boris Ostrovsky wrote: >>> On 4/18/19 3:36 AM, Juergen Gross wrote: >>>> I'm currently investigating a problem related to swiotlb-xen. With a >>>> specific driver a customer is capable to trigger a situation where a >>>> MFN is mapped to multiple dom0 PFNs at the same time. There is no >>>> guest involved, so this is not related to grants. >>>> >>>> Wit a debug kernel I have managed to track the inconsistency to a >>>> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >>>> where the region was obviously not contiguous. >>>> >>>> xen_swiotlb_free_coherent() contains: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) || >>>> range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>>> >>>> Shouldn't it be either: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) && >>>> !range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>> >>> +Joe >>> >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html >>> >>> (The discussion happened in >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) >>> >>> And looks like we dropped it. Or was there a reason we haven't picked it up? >> >> I remembered the concern was whether memory not from Xen. > > The current coding is wrong. I agree with you, your patch same with I sent before, I'm good to have it. > > I believe we should have at least a WARN_ONCE() in case > xen_swiotlb_free_coherent() is being called with non-contiguous memory. > And it should not call xen_destroy_contiguous_region() in that case. It's potential issue, if alloc/free is not in pairs, it will be a problem. > > Joe, did you observe such cases? I'm asking because the backtraces I > have give me no clue _why_ the memory was non-contiguous. The calling > function allocated the memory via xen_swiotlb_alloc_coherent() and when > freeing it again it was not contiguous. Not sure if you have commit 7250f422 "xen-swiotlb: use actually allocated size on check physical continuous"? without this commit, it's possible, after apply the commit, we did not hit such kind cases. Thanks, Joe > > Another topic is the question whether we should really call > xen_destroy_contiguous_region() when freeing the memory if there was no > need to use xen_create_contiguous_region() when allocating it. > > > Juergen > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
WARNING: multiple messages have this Message-ID (diff)
From: Joe Jin <joe.jin@oracle.com> To: Juergen Gross <jgross@suse.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: xen-devel <xen-devel@lists.xenproject.org> Subject: Re: [Xen-devel] Question regarding swiotlb-xen in Linux kernel Date: Fri, 19 Apr 2019 07:40:52 -0700 [thread overview] Message-ID: <d1ca67c3-23cd-12ef-2913-4e1afb62e061@oracle.com> (raw) Message-ID: <20190419144052.yw6BQJC6OMde0zJ8EUQ3RrRJqVA7ETkRZ17Ivqr7IWM@z> (raw) In-Reply-To: <a0ec7385-bc7d-92c4-538b-cad8001fa5ef@suse.com> On 4/18/19 10:47 PM, Juergen Gross wrote: > On 19/04/2019 00:31, Joe Jin wrote: >> On 4/18/19 2:09 PM, Boris Ostrovsky wrote: >>> On 4/18/19 3:36 AM, Juergen Gross wrote: >>>> I'm currently investigating a problem related to swiotlb-xen. With a >>>> specific driver a customer is capable to trigger a situation where a >>>> MFN is mapped to multiple dom0 PFNs at the same time. There is no >>>> guest involved, so this is not related to grants. >>>> >>>> Wit a debug kernel I have managed to track the inconsistency to a >>>> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >>>> where the region was obviously not contiguous. >>>> >>>> xen_swiotlb_free_coherent() contains: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) || >>>> range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>>> >>>> Shouldn't it be either: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) && >>>> !range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>> >>> +Joe >>> >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html >>> >>> (The discussion happened in >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) >>> >>> And looks like we dropped it. Or was there a reason we haven't picked it up? >> >> I remembered the concern was whether memory not from Xen. > > The current coding is wrong. I agree with you, your patch same with I sent before, I'm good to have it. > > I believe we should have at least a WARN_ONCE() in case > xen_swiotlb_free_coherent() is being called with non-contiguous memory. > And it should not call xen_destroy_contiguous_region() in that case. It's potential issue, if alloc/free is not in pairs, it will be a problem. > > Joe, did you observe such cases? I'm asking because the backtraces I > have give me no clue _why_ the memory was non-contiguous. The calling > function allocated the memory via xen_swiotlb_alloc_coherent() and when > freeing it again it was not contiguous. Not sure if you have commit 7250f422 "xen-swiotlb: use actually allocated size on check physical continuous"? without this commit, it's possible, after apply the commit, we did not hit such kind cases. Thanks, Joe > > Another topic is the question whether we should really call > xen_destroy_contiguous_region() when freeing the memory if there was no > need to use xen_create_contiguous_region() when allocating it. > > > Juergen > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-04-19 14:40 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-18 7:36 Question regarding swiotlb-xen in Linux kernel Juergen Gross 2019-04-18 7:36 ` [Xen-devel] " Juergen Gross 2019-04-18 21:09 ` Boris Ostrovsky 2019-04-18 21:09 ` [Xen-devel] " Boris Ostrovsky 2019-04-18 22:31 ` Joe Jin 2019-04-18 22:31 ` [Xen-devel] " Joe Jin 2019-04-19 5:47 ` Juergen Gross 2019-04-19 5:47 ` [Xen-devel] " Juergen Gross 2019-04-19 14:40 ` Joe Jin [this message] 2019-04-19 14:40 ` Joe Jin 2019-04-19 15:28 ` Dongli Zhang 2019-04-19 15:28 ` [Xen-devel] " Dongli Zhang
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=d1ca67c3-23cd-12ef-2913-4e1afb62e061@oracle.com \ --to=joe.jin@oracle.com \ --cc=boris.ostrovsky@oracle.com \ --cc=jgross@suse.com \ --cc=konrad.wilk@oracle.com \ --cc=xen-devel@lists.xenproject.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.