Re: [patch 10/12] [PATCH] kvm-s390: storage key interface

[Martin Schwidefsky <schwidefsky@de.ibm.com>]

On Sat, 10 Dec 2011 13:35:39 +0100
Carsten Otte <cotte@de.ibm.com> wrote:

> --- a/arch/s390/mm/pgtable.c
> +++ b/arch/s390/mm/pgtable.c
> @@ -393,6 +393,33 @@ out_unmap:
>  }
>  EXPORT_SYMBOL_GPL(gmap_map_segment);
> 
> +static pmd_t *__pmdp_for_addr(struct mm_struct *mm, unsigned long addr)
> +{
> +	struct vm_area_struct *vma;
> +	pgd_t *pgd;
> +	pud_t *pud;
> +	pmd_t *pmd;
> +
> +	vma = find_vma(mm, addr);
> +	if (!vma || (vma->vm_start > addr))
> +		return ERR_PTR(-EFAULT);
> +
> +	pgd = pgd_offset(mm, addr);
> +	pud = pud_alloc(mm, pgd, addr);
> +	if (!pud)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pmd = pmd_alloc(mm, pud, addr);
> +	if (!pmd)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (!pmd_present(*pmd) &&
> +	    __pte_alloc(mm, vma, pmd, addr))
> +		return ERR_PTR(-ENOMEM);
> +
> +	return pmd;
> +}
> +
>  /*
>   * this function is assumed to be called with mmap_sem held
>   */

The __pmdp_for_addr function is fine for the usage in __gmap_fault.

> @@ -806,6 +820,26 @@ int s390_enable_sie(void)
>  }
>  EXPORT_SYMBOL_GPL(s390_enable_sie);
> 
> +pte_t *ptep_for_addr(unsigned long addr)
> +{
> +	pmd_t *pmd;
> +	pte_t *pte;
> +
> +	down_read(&current->mm->mmap_sem);
> +
> +	pmd = __pmdp_for_addr(current->mm, addr);
> +	if (IS_ERR(pmd)) {
> +		pte = (pte_t *)pmd;
> +		goto up_out;
> +	}
> +
> +	pte = pte_offset(pmd, addr);
> +up_out:
> +	up_read(&current->mm->mmap_sem);
> +	return pte;
> +}
> +EXPORT_SYMBOL_GPL(ptep_for_addr);
> +
>  #if defined(CONFIG_DEBUG_PAGEALLOC) && defined(CONFIG_HIBERNATION)
>  bool kernel_page_present(struct page *page)
>  {

There is a fundamental locking sanfu. The pointer the the page table
entry is only valid until the mmap_sem is released. The down_read/up_read
has to be done in the caller.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.