lkml.org 
[lkml]   [2016]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v9 04/12] vfio iommu: Add support for mediated devices
From
Date


On 10/26/2016 12:49 PM, Tian, Kevin wrote:
>> From: Alex Williamson [mailto:alex.williamson@redhat.com]
>> Sent: Monday, October 24, 2016 10:32 AM
>>
>>>>> -static long vfio_unpin_pages(unsigned long pfn, long npage,
>>>>> - int prot, bool do_accounting)
>>>>> +static long __vfio_unpin_pages_remote(struct vfio_iommu *iommu,
>>>>> + unsigned long pfn, long npage, int prot,
>>>>> + bool do_accounting)
>>>>
>>>> Have you noticed that it's kind of confusing that
>>>> __vfio_{un}pin_pages_remote() uses current, which does a
>>>> get_user_pages_fast() while "local" uses a provided task_struct and
>>>> uses get_user_pages_*remote*()? And also what was effectively local
>>>> (ie. we're pinning for our own use here) is now "remote" and pinning
>>>> for a remote, vendor driver consumer, is now "local". It's not very
>>>> intuitive.
>>>>
>
> I questioned this confusing naming in v8 too...
>

I do tried to address your concerns on v8.

>>>
>>> 'local' in local_domain was suggested to describe the domain for local
>>> page tracking. Earlier suggestions to have 'mdev' or 'noimmu' in this
>>> name were discarded. May be we should revisit what the name should be.
>>> Any suggestion?
>>>
>>> For local_domain, to pin pages, flow is:
>>>
>>> for local_domain
>>> |- vfio_pin_pages()
>>> |- vfio_iommu_type1_pin_pages()
>>> |- __vfio_pin_page_local()
>>> |- vaddr_get_pfn(task->mm)
>>> |- get_user_pages_remote()
>>>
>>> __vfio_pin_page_local() --> get_user_pages_remote()
>>
>>
>> In vfio.c we have the concept of an external user, perhaps that could
>> be continued here. An mdev driver would be an external, or remote
>> pinning.
>>
>
> I prefer to use remote here. It's aligned with underlying mm operations
>

Using 'remote' in this case is also confusing since it is already used
in this file. I liked Alex's suggestion to use external and I'll have
those changed in next version of patch set.

Kirti

\
 
 \ /
  Last update: 2016-10-26 17:06    [W:0.096 / U:0.596 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site