diff mbox series

[resend,RFC,7/9] s390/mm: no need for pte_alloc_map_lock() if we know the pmd is present

Message ID 20210909162248.14969-8-david@redhat.com (mailing list archive)
State New
Headers show
Series s390: fixes, cleanups and optimizations for page table walkers | expand

Commit Message

David Hildenbrand Sept. 9, 2021, 4:22 p.m. UTC 
pte_map_lock() is sufficient.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/s390/mm/pgtable.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

Comments

Claudio Imbrenda Sept. 14, 2021, 4:54 p.m. UTC | #1 
On Thu,  9 Sep 2021 18:22:46 +0200
David Hildenbrand <david@redhat.com> wrote:

> pte_map_lock() is sufficient.

Can you explain the difference and why it is enough?

> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/s390/mm/pgtable.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
> index 5fb409ff7842..4e77b8ebdcc5 100644
> --- a/arch/s390/mm/pgtable.c
> +++ b/arch/s390/mm/pgtable.c
> @@ -814,10 +814,7 @@ int set_guest_storage_key(struct mm_struct *mm, unsigned long addr,
>  	}
>  	spin_unlock(ptl);
>  
> -	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
> -	if (unlikely(!ptep))
> -		return -EFAULT;
> -
> +	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
>  	new = old = pgste_get_lock(ptep);
>  	pgste_val(new) &= ~(PGSTE_GR_BIT | PGSTE_GC_BIT |
>  			    PGSTE_ACC_BITS | PGSTE_FP_BIT);
> @@ -912,10 +909,7 @@ int reset_guest_reference_bit(struct mm_struct *mm, unsigned long addr)
>  	}
>  	spin_unlock(ptl);
>  
> -	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
> -	if (unlikely(!ptep))
> -		return -EFAULT;
> -
> +	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
>  	new = old = pgste_get_lock(ptep);
>  	/* Reset guest reference bit only */
>  	pgste_val(new) &= ~PGSTE_GR_BIT;
> @@ -977,10 +971,7 @@ int get_guest_storage_key(struct mm_struct *mm, unsigned long addr,
>  	}
>  	spin_unlock(ptl);
>  
> -	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
> -	if (unlikely(!ptep))
> -		return -EFAULT;
> -
> +	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
>  	pgste = pgste_get_lock(ptep);
>  	*key = (pgste_val(pgste) & (PGSTE_ACC_BITS | PGSTE_FP_BIT)) >> 56;
>  	paddr = pte_val(*ptep) & PAGE_MASK;
David Hildenbrand Sept. 14, 2021, 5:23 p.m. UTC | #2 
On 14.09.21 18:54, Claudio Imbrenda wrote:
> On Thu,  9 Sep 2021 18:22:46 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> pte_map_lock() is sufficient.
> 
> Can you explain the difference and why it is enough?

I didn't repeat the $subject:

"No need for pte_alloc_map_lock() if we know the pmd is present; 
pte_map_lock() is sufficient, because there isn't anything to allocate."

> 
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>   arch/s390/mm/pgtable.c | 15 +++------------
>>   1 file changed, 3 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
>> index 5fb409ff7842..4e77b8ebdcc5 100644
>> --- a/arch/s390/mm/pgtable.c
>> +++ b/arch/s390/mm/pgtable.c
>> @@ -814,10 +814,7 @@ int set_guest_storage_key(struct mm_struct *mm, unsigned long addr,
>>   	}
>>   	spin_unlock(ptl);
>>   
>> -	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
>> -	if (unlikely(!ptep))
>> -		return -EFAULT;
>> -
>> +	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
>>   	new = old = pgste_get_lock(ptep);
>>   	pgste_val(new) &= ~(PGSTE_GR_BIT | PGSTE_GC_BIT |
>>   			    PGSTE_ACC_BITS | PGSTE_FP_BIT);
>> @@ -912,10 +909,7 @@ int reset_guest_reference_bit(struct mm_struct *mm, unsigned long addr)
>>   	}
>>   	spin_unlock(ptl);
>>   
>> -	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
>> -	if (unlikely(!ptep))
>> -		return -EFAULT;
>> -
>> +	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
>>   	new = old = pgste_get_lock(ptep);
>>   	/* Reset guest reference bit only */
>>   	pgste_val(new) &= ~PGSTE_GR_BIT;
>> @@ -977,10 +971,7 @@ int get_guest_storage_key(struct mm_struct *mm, unsigned long addr,
>>   	}
>>   	spin_unlock(ptl);
>>   
>> -	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
>> -	if (unlikely(!ptep))
>> -		return -EFAULT;
>> -
>> +	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
>>   	pgste = pgste_get_lock(ptep);
>>   	*key = (pgste_val(pgste) & (PGSTE_ACC_BITS | PGSTE_FP_BIT)) >> 56;
>>   	paddr = pte_val(*ptep) & PAGE_MASK;
>
diff mbox series

Patch

diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
index 5fb409ff7842..4e77b8ebdcc5 100644
--- a/arch/s390/mm/pgtable.c
+++ b/arch/s390/mm/pgtable.c
@@ -814,10 +814,7 @@  int set_guest_storage_key(struct mm_struct *mm, unsigned long addr,
 	}
 	spin_unlock(ptl);
 
-	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
-	if (unlikely(!ptep))
-		return -EFAULT;
-
+	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
 	new = old = pgste_get_lock(ptep);
 	pgste_val(new) &= ~(PGSTE_GR_BIT | PGSTE_GC_BIT |
 			    PGSTE_ACC_BITS | PGSTE_FP_BIT);
@@ -912,10 +909,7 @@  int reset_guest_reference_bit(struct mm_struct *mm, unsigned long addr)
 	}
 	spin_unlock(ptl);
 
-	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
-	if (unlikely(!ptep))
-		return -EFAULT;
-
+	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
 	new = old = pgste_get_lock(ptep);
 	/* Reset guest reference bit only */
 	pgste_val(new) &= ~PGSTE_GR_BIT;
@@ -977,10 +971,7 @@  int get_guest_storage_key(struct mm_struct *mm, unsigned long addr,
 	}
 	spin_unlock(ptl);
 
-	ptep = pte_alloc_map_lock(mm, pmdp, addr, &ptl);
-	if (unlikely(!ptep))
-		return -EFAULT;
-
+	ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl);
 	pgste = pgste_get_lock(ptep);
 	*key = (pgste_val(pgste) & (PGSTE_ACC_BITS | PGSTE_FP_BIT)) >> 56;
 	paddr = pte_val(*ptep) & PAGE_MASK;