Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages

2019-02-12 Thread Alexey Kardashevskiy



On 13/02/2019 04:18, Daniel Jordan wrote:
> On Tue, Feb 12, 2019 at 04:50:11PM +, Christopher Lameter wrote:
>> On Tue, 12 Feb 2019, Alexey Kardashevskiy wrote:
>>
>>> Now it is 3 independent accesses (actually 4 but the last one is
>>> diagnostic) with no locking around them. Why do not we need a lock
>>> anymore precisely? Thanks,
>>
>> Updating a regular counter is racy and requires a lock. It was converted
>> to be an atomic which can be incremented without a race.
> 
> Yes, though Alexey may have meant that the multiple reads of the atomic in
> decrement_pinned_vm are racy.

Yes, I meant this race, thanks for clarifying this.

>  It only matters when there's a bug that would
> make the counter go negative, but it's there.
> 
> And FWIW the debug print in try_increment_pinned_vm is also racy.
> 
> This fixes all that.  It doesn't try to correct the negative pinned_vm as the
> old code did because it's already a bug and adjusting the value by the 
> negative
> amount seems to do nothing but make debugging harder.
> 
> If it's ok, I'll respin the whole series this way (another point for common
> helper)

This looks good, thanks for fixing this.


> 
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
> b/drivers/vfio/vfio_iommu_spapr_tce.c
> index f47e020dc5e4..b79257304de6 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -53,25 +53,24 @@ static long try_increment_pinned_vm(struct mm_struct *mm, 
> long npages)
>   atomic64_sub(npages, >pinned_vm);
>   }
>  
> - pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
> - npages << PAGE_SHIFT,
> - atomic64_read(>pinned_vm) << PAGE_SHIFT,
> - rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
> + pr_debug("[%d] RLIMIT_MEMLOCK +%ld %lld/%lu%s\n", current->pid,
> + npages << PAGE_SHIFT, pinned << PAGE_SHIFT,
> + lock_limit, ret ? " - exceeded" : "");
>  
>   return ret;
>  }
>  
>  static void decrement_pinned_vm(struct mm_struct *mm, long npages)
>  {
> + s64 pinned;
> +
>   if (!mm || !npages)
>   return;
>  
> - if (WARN_ON_ONCE(npages > atomic64_read(>pinned_vm)))
> - npages = atomic64_read(>pinned_vm);
> - atomic64_sub(npages, >pinned_vm);
> - pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%lu\n", current->pid,
> - npages << PAGE_SHIFT,
> - atomic64_read(>pinned_vm) << PAGE_SHIFT,
> + pinned = atomic64_sub_return(npages, >pinned_vm);
> + WARN_ON_ONCE(pinned < 0);
> + pr_debug("[%d] RLIMIT_MEMLOCK -%ld %lld/%lu\n", current->pid,
> + npages << PAGE_SHIFT, pinned << PAGE_SHIFT,
>   rlimit(RLIMIT_MEMLOCK));
>  }
>  
> 

-- 
Alexey


Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages

2019-02-12 Thread Alexey Kardashevskiy



On 13/02/2019 05:56, Alex Williamson wrote:
> On Tue, 12 Feb 2019 17:56:18 +1100
> Alexey Kardashevskiy  wrote:
> 
>> On 12/02/2019 09:44, Daniel Jordan wrote:
>>> Beginning with bc3e53f682d9 ("mm: distinguish between mlocked and pinned
>>> pages"), locked and pinned pages are accounted separately.  The SPAPR
>>> TCE VFIO IOMMU driver accounts pinned pages to locked_vm; use pinned_vm
>>> instead.
>>>
>>> pinned_vm recently became atomic and so no longer relies on mmap_sem
>>> held as writer: delete.
>>>
>>> Signed-off-by: Daniel Jordan 
>>> ---
>>>  Documentation/vfio.txt  |  6 +--
>>>  drivers/vfio/vfio_iommu_spapr_tce.c | 64 ++---
>>>  2 files changed, 33 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
>>> index f1a4d3c3ba0b..fa37d65363f9 100644
>>> --- a/Documentation/vfio.txt
>>> +++ b/Documentation/vfio.txt
>>> @@ -308,7 +308,7 @@ This implementation has some specifics:
>>> currently there is no way to reduce the number of calls. In order to 
>>> make
>>> things faster, the map/unmap handling has been implemented in real mode
>>> which provides an excellent performance which has limitations such as
>>> -   inability to do locked pages accounting in real time.
>>> +   inability to do pinned pages accounting in real time.
>>>  
>>>  4) According to sPAPR specification, A Partitionable Endpoint (PE) is an 
>>> I/O
>>> subtree that can be treated as a unit for the purposes of partitioning 
>>> and
>>> @@ -324,7 +324,7 @@ This implementation has some specifics:
>>> returns the size and the start of the DMA window on the PCI bus.
>>>  
>>> VFIO_IOMMU_ENABLE
>>> -   enables the container. The locked pages accounting
>>> +   enables the container. The pinned pages accounting
>>> is done at this point. This lets user first to know what
>>> the DMA window is and adjust rlimit before doing any real job.
> 
> I don't know of a ulimit only covering pinned pages, so for
> documentation it seems more correct to continue referring to this as
> locked page accounting.
> 
>>> @@ -454,7 +454,7 @@ This implementation has some specifics:
>>>  
>>> PPC64 paravirtualized guests generate a lot of map/unmap requests,
>>> and the handling of those includes pinning/unpinning pages and updating
>>> -   mm::locked_vm counter to make sure we do not exceed the rlimit.
>>> +   mm::pinned_vm counter to make sure we do not exceed the rlimit.
>>> The v2 IOMMU splits accounting and pinning into separate operations:
>>>  
>>> - VFIO_IOMMU_SPAPR_REGISTER_MEMORY/VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY 
>>> ioctls
>>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
>>> b/drivers/vfio/vfio_iommu_spapr_tce.c
>>> index c424913324e3..f47e020dc5e4 100644
>>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
>>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>>> @@ -34,9 +34,11 @@
>>>  static void tce_iommu_detach_group(void *iommu_data,
>>> struct iommu_group *iommu_group);
>>>  
>>> -static long try_increment_locked_vm(struct mm_struct *mm, long npages)
>>> +static long try_increment_pinned_vm(struct mm_struct *mm, long npages)
>>>  {
>>> -   long ret = 0, locked, lock_limit;
>>> +   long ret = 0;
>>> +   s64 pinned;
>>> +   unsigned long lock_limit;
>>>  
>>> if (WARN_ON_ONCE(!mm))
>>> return -EPERM;
>>> @@ -44,39 +46,33 @@ static long try_increment_locked_vm(struct mm_struct 
>>> *mm, long npages)
>>> if (!npages)
>>> return 0;
>>>  
>>> -   down_write(>mmap_sem);
>>> -   locked = mm->locked_vm + npages;
>>> +   pinned = atomic64_add_return(npages, >pinned_vm);
>>> lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>>> -   if (locked > lock_limit && !capable(CAP_IPC_LOCK))
>>> +   if (pinned > lock_limit && !capable(CAP_IPC_LOCK)) {
>>> ret = -ENOMEM;
>>> -   else
>>> -   mm->locked_vm += npages;
>>> +   atomic64_sub(npages, >pinned_vm);
>>> +   }
>>>  
>>> -   pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%ld%s\n", current->pid,
>>> +   pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
>>> npages << PAGE_SHIFT,
>>> -   mm->locked_vm << PAGE_SHIFT,
>>> -   rlimit(RLIMIT_MEMLOCK),
>>> -   ret ? " - exceeded" : "");
>>> -
>>> -   up_write(>mmap_sem);
>>> +   atomic64_read(>pinned_vm) << PAGE_SHIFT,
>>> +   rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
>>>  
>>> return ret;
>>>  }
>>>  
>>> -static void decrement_locked_vm(struct mm_struct *mm, long npages)
>>> +static void decrement_pinned_vm(struct mm_struct *mm, long npages)
>>>  {
>>> if (!mm || !npages)
>>> return;
>>>  
>>> -   down_write(>mmap_sem);
>>> -   if (WARN_ON_ONCE(npages > mm->locked_vm))
>>> -   npages = mm->locked_vm;
>>> -   mm->locked_vm -= npages;
>>> -   pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%ld\n", 

Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages

2019-02-12 Thread Alex Williamson
On Tue, 12 Feb 2019 17:56:18 +1100
Alexey Kardashevskiy  wrote:

> On 12/02/2019 09:44, Daniel Jordan wrote:
> > Beginning with bc3e53f682d9 ("mm: distinguish between mlocked and pinned
> > pages"), locked and pinned pages are accounted separately.  The SPAPR
> > TCE VFIO IOMMU driver accounts pinned pages to locked_vm; use pinned_vm
> > instead.
> > 
> > pinned_vm recently became atomic and so no longer relies on mmap_sem
> > held as writer: delete.
> > 
> > Signed-off-by: Daniel Jordan 
> > ---
> >  Documentation/vfio.txt  |  6 +--
> >  drivers/vfio/vfio_iommu_spapr_tce.c | 64 ++---
> >  2 files changed, 33 insertions(+), 37 deletions(-)
> > 
> > diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> > index f1a4d3c3ba0b..fa37d65363f9 100644
> > --- a/Documentation/vfio.txt
> > +++ b/Documentation/vfio.txt
> > @@ -308,7 +308,7 @@ This implementation has some specifics:
> > currently there is no way to reduce the number of calls. In order to 
> > make
> > things faster, the map/unmap handling has been implemented in real mode
> > which provides an excellent performance which has limitations such as
> > -   inability to do locked pages accounting in real time.
> > +   inability to do pinned pages accounting in real time.
> >  
> >  4) According to sPAPR specification, A Partitionable Endpoint (PE) is an 
> > I/O
> > subtree that can be treated as a unit for the purposes of partitioning 
> > and
> > @@ -324,7 +324,7 @@ This implementation has some specifics:
> > returns the size and the start of the DMA window on the PCI bus.
> >  
> > VFIO_IOMMU_ENABLE
> > -   enables the container. The locked pages accounting
> > +   enables the container. The pinned pages accounting
> > is done at this point. This lets user first to know what
> > the DMA window is and adjust rlimit before doing any real job.

I don't know of a ulimit only covering pinned pages, so for
documentation it seems more correct to continue referring to this as
locked page accounting.

> > @@ -454,7 +454,7 @@ This implementation has some specifics:
> >  
> > PPC64 paravirtualized guests generate a lot of map/unmap requests,
> > and the handling of those includes pinning/unpinning pages and updating
> > -   mm::locked_vm counter to make sure we do not exceed the rlimit.
> > +   mm::pinned_vm counter to make sure we do not exceed the rlimit.
> > The v2 IOMMU splits accounting and pinning into separate operations:
> >  
> > - VFIO_IOMMU_SPAPR_REGISTER_MEMORY/VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY 
> > ioctls
> > diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
> > b/drivers/vfio/vfio_iommu_spapr_tce.c
> > index c424913324e3..f47e020dc5e4 100644
> > --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> > +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> > @@ -34,9 +34,11 @@
> >  static void tce_iommu_detach_group(void *iommu_data,
> > struct iommu_group *iommu_group);
> >  
> > -static long try_increment_locked_vm(struct mm_struct *mm, long npages)
> > +static long try_increment_pinned_vm(struct mm_struct *mm, long npages)
> >  {
> > -   long ret = 0, locked, lock_limit;
> > +   long ret = 0;
> > +   s64 pinned;
> > +   unsigned long lock_limit;
> >  
> > if (WARN_ON_ONCE(!mm))
> > return -EPERM;
> > @@ -44,39 +46,33 @@ static long try_increment_locked_vm(struct mm_struct 
> > *mm, long npages)
> > if (!npages)
> > return 0;
> >  
> > -   down_write(>mmap_sem);
> > -   locked = mm->locked_vm + npages;
> > +   pinned = atomic64_add_return(npages, >pinned_vm);
> > lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> > -   if (locked > lock_limit && !capable(CAP_IPC_LOCK))
> > +   if (pinned > lock_limit && !capable(CAP_IPC_LOCK)) {
> > ret = -ENOMEM;
> > -   else
> > -   mm->locked_vm += npages;
> > +   atomic64_sub(npages, >pinned_vm);
> > +   }
> >  
> > -   pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%ld%s\n", current->pid,
> > +   pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
> > npages << PAGE_SHIFT,
> > -   mm->locked_vm << PAGE_SHIFT,
> > -   rlimit(RLIMIT_MEMLOCK),
> > -   ret ? " - exceeded" : "");
> > -
> > -   up_write(>mmap_sem);
> > +   atomic64_read(>pinned_vm) << PAGE_SHIFT,
> > +   rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
> >  
> > return ret;
> >  }
> >  
> > -static void decrement_locked_vm(struct mm_struct *mm, long npages)
> > +static void decrement_pinned_vm(struct mm_struct *mm, long npages)
> >  {
> > if (!mm || !npages)
> > return;
> >  
> > -   down_write(>mmap_sem);
> > -   if (WARN_ON_ONCE(npages > mm->locked_vm))
> > -   npages = mm->locked_vm;
> > -   mm->locked_vm -= npages;
> > -   pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%ld\n", current->pid,
> > +   if (WARN_ON_ONCE(npages > 

Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages

2019-02-12 Thread Daniel Jordan
On Tue, Feb 12, 2019 at 04:50:11PM +, Christopher Lameter wrote:
> On Tue, 12 Feb 2019, Alexey Kardashevskiy wrote:
> 
> > Now it is 3 independent accesses (actually 4 but the last one is
> > diagnostic) with no locking around them. Why do not we need a lock
> > anymore precisely? Thanks,
> 
> Updating a regular counter is racy and requires a lock. It was converted
> to be an atomic which can be incremented without a race.

Yes, though Alexey may have meant that the multiple reads of the atomic in
decrement_pinned_vm are racy.  It only matters when there's a bug that would
make the counter go negative, but it's there.

And FWIW the debug print in try_increment_pinned_vm is also racy.

This fixes all that.  It doesn't try to correct the negative pinned_vm as the
old code did because it's already a bug and adjusting the value by the negative
amount seems to do nothing but make debugging harder.

If it's ok, I'll respin the whole series this way (another point for common
helper)

diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
b/drivers/vfio/vfio_iommu_spapr_tce.c
index f47e020dc5e4..b79257304de6 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -53,25 +53,24 @@ static long try_increment_pinned_vm(struct mm_struct *mm, 
long npages)
atomic64_sub(npages, >pinned_vm);
}
 
-   pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
-   npages << PAGE_SHIFT,
-   atomic64_read(>pinned_vm) << PAGE_SHIFT,
-   rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
+   pr_debug("[%d] RLIMIT_MEMLOCK +%ld %lld/%lu%s\n", current->pid,
+   npages << PAGE_SHIFT, pinned << PAGE_SHIFT,
+   lock_limit, ret ? " - exceeded" : "");
 
return ret;
 }
 
 static void decrement_pinned_vm(struct mm_struct *mm, long npages)
 {
+   s64 pinned;
+
if (!mm || !npages)
return;
 
-   if (WARN_ON_ONCE(npages > atomic64_read(>pinned_vm)))
-   npages = atomic64_read(>pinned_vm);
-   atomic64_sub(npages, >pinned_vm);
-   pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%lu\n", current->pid,
-   npages << PAGE_SHIFT,
-   atomic64_read(>pinned_vm) << PAGE_SHIFT,
+   pinned = atomic64_sub_return(npages, >pinned_vm);
+   WARN_ON_ONCE(pinned < 0);
+   pr_debug("[%d] RLIMIT_MEMLOCK -%ld %lld/%lu\n", current->pid,
+   npages << PAGE_SHIFT, pinned << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK));
 }
 



Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages

2019-02-12 Thread Christopher Lameter
On Tue, 12 Feb 2019, Alexey Kardashevskiy wrote:

> Now it is 3 independent accesses (actually 4 but the last one is
> diagnostic) with no locking around them. Why do not we need a lock
> anymore precisely? Thanks,

Updating a regular counter is racy and requires a lock. It was converted
to be an atomic which can be incremented without a race.


Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages

2019-02-11 Thread Alexey Kardashevskiy



On 12/02/2019 09:44, Daniel Jordan wrote:
> Beginning with bc3e53f682d9 ("mm: distinguish between mlocked and pinned
> pages"), locked and pinned pages are accounted separately.  The SPAPR
> TCE VFIO IOMMU driver accounts pinned pages to locked_vm; use pinned_vm
> instead.
> 
> pinned_vm recently became atomic and so no longer relies on mmap_sem
> held as writer: delete.
> 
> Signed-off-by: Daniel Jordan 
> ---
>  Documentation/vfio.txt  |  6 +--
>  drivers/vfio/vfio_iommu_spapr_tce.c | 64 ++---
>  2 files changed, 33 insertions(+), 37 deletions(-)
> 
> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> index f1a4d3c3ba0b..fa37d65363f9 100644
> --- a/Documentation/vfio.txt
> +++ b/Documentation/vfio.txt
> @@ -308,7 +308,7 @@ This implementation has some specifics:
> currently there is no way to reduce the number of calls. In order to make
> things faster, the map/unmap handling has been implemented in real mode
> which provides an excellent performance which has limitations such as
> -   inability to do locked pages accounting in real time.
> +   inability to do pinned pages accounting in real time.
>  
>  4) According to sPAPR specification, A Partitionable Endpoint (PE) is an I/O
> subtree that can be treated as a unit for the purposes of partitioning and
> @@ -324,7 +324,7 @@ This implementation has some specifics:
>   returns the size and the start of the DMA window on the PCI bus.
>  
>   VFIO_IOMMU_ENABLE
> - enables the container. The locked pages accounting
> + enables the container. The pinned pages accounting
>   is done at this point. This lets user first to know what
>   the DMA window is and adjust rlimit before doing any real job.
>  
> @@ -454,7 +454,7 @@ This implementation has some specifics:
>  
> PPC64 paravirtualized guests generate a lot of map/unmap requests,
> and the handling of those includes pinning/unpinning pages and updating
> -   mm::locked_vm counter to make sure we do not exceed the rlimit.
> +   mm::pinned_vm counter to make sure we do not exceed the rlimit.
> The v2 IOMMU splits accounting and pinning into separate operations:
>  
> - VFIO_IOMMU_SPAPR_REGISTER_MEMORY/VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY 
> ioctls
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
> b/drivers/vfio/vfio_iommu_spapr_tce.c
> index c424913324e3..f47e020dc5e4 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -34,9 +34,11 @@
>  static void tce_iommu_detach_group(void *iommu_data,
>   struct iommu_group *iommu_group);
>  
> -static long try_increment_locked_vm(struct mm_struct *mm, long npages)
> +static long try_increment_pinned_vm(struct mm_struct *mm, long npages)
>  {
> - long ret = 0, locked, lock_limit;
> + long ret = 0;
> + s64 pinned;
> + unsigned long lock_limit;
>  
>   if (WARN_ON_ONCE(!mm))
>   return -EPERM;
> @@ -44,39 +46,33 @@ static long try_increment_locked_vm(struct mm_struct *mm, 
> long npages)
>   if (!npages)
>   return 0;
>  
> - down_write(>mmap_sem);
> - locked = mm->locked_vm + npages;
> + pinned = atomic64_add_return(npages, >pinned_vm);
>   lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> - if (locked > lock_limit && !capable(CAP_IPC_LOCK))
> + if (pinned > lock_limit && !capable(CAP_IPC_LOCK)) {
>   ret = -ENOMEM;
> - else
> - mm->locked_vm += npages;
> + atomic64_sub(npages, >pinned_vm);
> + }
>  
> - pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%ld%s\n", current->pid,
> + pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
>   npages << PAGE_SHIFT,
> - mm->locked_vm << PAGE_SHIFT,
> - rlimit(RLIMIT_MEMLOCK),
> - ret ? " - exceeded" : "");
> -
> - up_write(>mmap_sem);
> + atomic64_read(>pinned_vm) << PAGE_SHIFT,
> + rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
>  
>   return ret;
>  }
>  
> -static void decrement_locked_vm(struct mm_struct *mm, long npages)
> +static void decrement_pinned_vm(struct mm_struct *mm, long npages)
>  {
>   if (!mm || !npages)
>   return;
>  
> - down_write(>mmap_sem);
> - if (WARN_ON_ONCE(npages > mm->locked_vm))
> - npages = mm->locked_vm;
> - mm->locked_vm -= npages;
> - pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%ld\n", current->pid,
> + if (WARN_ON_ONCE(npages > atomic64_read(>pinned_vm)))
> + npages = atomic64_read(>pinned_vm);
> + atomic64_sub(npages, >pinned_vm);
> + pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%lu\n", current->pid,
>   npages << PAGE_SHIFT,
> - mm->locked_vm << PAGE_SHIFT,
> + atomic64_read(>pinned_vm) << PAGE_SHIFT,
>  

[PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages

2019-02-11 Thread Daniel Jordan
Beginning with bc3e53f682d9 ("mm: distinguish between mlocked and pinned
pages"), locked and pinned pages are accounted separately.  The SPAPR
TCE VFIO IOMMU driver accounts pinned pages to locked_vm; use pinned_vm
instead.

pinned_vm recently became atomic and so no longer relies on mmap_sem
held as writer: delete.

Signed-off-by: Daniel Jordan 
---
 Documentation/vfio.txt  |  6 +--
 drivers/vfio/vfio_iommu_spapr_tce.c | 64 ++---
 2 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index f1a4d3c3ba0b..fa37d65363f9 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -308,7 +308,7 @@ This implementation has some specifics:
currently there is no way to reduce the number of calls. In order to make
things faster, the map/unmap handling has been implemented in real mode
which provides an excellent performance which has limitations such as
-   inability to do locked pages accounting in real time.
+   inability to do pinned pages accounting in real time.
 
 4) According to sPAPR specification, A Partitionable Endpoint (PE) is an I/O
subtree that can be treated as a unit for the purposes of partitioning and
@@ -324,7 +324,7 @@ This implementation has some specifics:
returns the size and the start of the DMA window on the PCI bus.
 
VFIO_IOMMU_ENABLE
-   enables the container. The locked pages accounting
+   enables the container. The pinned pages accounting
is done at this point. This lets user first to know what
the DMA window is and adjust rlimit before doing any real job.
 
@@ -454,7 +454,7 @@ This implementation has some specifics:
 
PPC64 paravirtualized guests generate a lot of map/unmap requests,
and the handling of those includes pinning/unpinning pages and updating
-   mm::locked_vm counter to make sure we do not exceed the rlimit.
+   mm::pinned_vm counter to make sure we do not exceed the rlimit.
The v2 IOMMU splits accounting and pinning into separate operations:
 
- VFIO_IOMMU_SPAPR_REGISTER_MEMORY/VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY ioctls
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
b/drivers/vfio/vfio_iommu_spapr_tce.c
index c424913324e3..f47e020dc5e4 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -34,9 +34,11 @@
 static void tce_iommu_detach_group(void *iommu_data,
struct iommu_group *iommu_group);
 
-static long try_increment_locked_vm(struct mm_struct *mm, long npages)
+static long try_increment_pinned_vm(struct mm_struct *mm, long npages)
 {
-   long ret = 0, locked, lock_limit;
+   long ret = 0;
+   s64 pinned;
+   unsigned long lock_limit;
 
if (WARN_ON_ONCE(!mm))
return -EPERM;
@@ -44,39 +46,33 @@ static long try_increment_locked_vm(struct mm_struct *mm, 
long npages)
if (!npages)
return 0;
 
-   down_write(>mmap_sem);
-   locked = mm->locked_vm + npages;
+   pinned = atomic64_add_return(npages, >pinned_vm);
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
-   if (locked > lock_limit && !capable(CAP_IPC_LOCK))
+   if (pinned > lock_limit && !capable(CAP_IPC_LOCK)) {
ret = -ENOMEM;
-   else
-   mm->locked_vm += npages;
+   atomic64_sub(npages, >pinned_vm);
+   }
 
-   pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%ld%s\n", current->pid,
+   pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
npages << PAGE_SHIFT,
-   mm->locked_vm << PAGE_SHIFT,
-   rlimit(RLIMIT_MEMLOCK),
-   ret ? " - exceeded" : "");
-
-   up_write(>mmap_sem);
+   atomic64_read(>pinned_vm) << PAGE_SHIFT,
+   rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
 
return ret;
 }
 
-static void decrement_locked_vm(struct mm_struct *mm, long npages)
+static void decrement_pinned_vm(struct mm_struct *mm, long npages)
 {
if (!mm || !npages)
return;
 
-   down_write(>mmap_sem);
-   if (WARN_ON_ONCE(npages > mm->locked_vm))
-   npages = mm->locked_vm;
-   mm->locked_vm -= npages;
-   pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%ld\n", current->pid,
+   if (WARN_ON_ONCE(npages > atomic64_read(>pinned_vm)))
+   npages = atomic64_read(>pinned_vm);
+   atomic64_sub(npages, >pinned_vm);
+   pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%lu\n", current->pid,
npages << PAGE_SHIFT,
-   mm->locked_vm << PAGE_SHIFT,
+   atomic64_read(>pinned_vm) << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK));
-   up_write(>mmap_sem);
 }
 
 /*
@@ -110,7 +106,7 @@ struct tce_container {
bool enabled;
bool v2;
bool