Messages in this thread | | | Date | Thu, 25 Apr 2019 03:01:40 -0600 | From | "Jan Beulich" <> | Subject | Re: [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region() |
| |
>>> On 23.04.19 at 20:36, <jgross@suse.com> wrote: > On 23/04/2019 19:05, Stefano Stabellini wrote: >> On Tue, 23 Apr 2019, Juergen Gross wrote: >>> Instead of always calling xen_destroy_contiguous_region() in case the >>> memory is DMA-able for the used device, do so only in case it has been >>> made DMA-able via xen_create_contiguous_region() before. >>> >>> This will avoid a lot of xen_destroy_contiguous_region() calls for >>> 64-bit capable devices. >>> >>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1 >>> flag of the first allocated page can be used for remembering. >> >> Although the patch looks OK, this sentence puzzles me. Why do you say >> that the memory in question is owned by swiotlb-xen? Because it was >> returned by xen_alloc_coherent_pages? Both the x86 and the Arm >> implementation return fresh new memory, hence, it should be safe to set >> the PageOwnerPriv1 flag? >> >> My concern with this approach is with the semantics of PG_owner_priv_1. >> Is a page marked with PG_owner_priv_1 only supposed to be used by the >> owner? > > The owner of the page is free to use the flag. > > Like Grant pages are marked by the grant driver using this flag. And > Xen page tables are using it in PV-guests for indicating a "Pinned" > page table.
Considering the background of the series, isn't such multi-purpose use of the flag a possible problem? You're already suspecting a wrong call into here. The function finding the flag set (but for another reason) might add to the confusion. But I realize there are only so many page flags available.
Perhaps the freeing function should, first thing, check the handed space actually matches the criteria (within dma_mask and contiguous)?
Jan
| |