From mboxrd@z Thu Jan  1 00:00:00 1970
From: "=?UTF-8?Q?Jan_H._Sch=c3=b6nherr?=" <jschoenh@amazon.de>
Subject: Re: [PATCH] dax: Release PMD lock even when there is no PMD support
 in DAX
Date: Thu, 18 Jan 2018 15:27:12 +0100
Message-ID: <62c74e2b-1342-8a8c-f1a8-7daa285e6a0f@amazon.de>
References: <20180118133839.20587-1-jschoenh@amazon.de>
 <20180118140739.GA28322@bombadil.infradead.org>
 <20180118142207.GB28322@bombadil.infradead.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: Matthew Wilcox <mawilcox@microsoft.com>,
        Ross Zwisler <ross.zwisler@linux.intel.com>,
        linux-fsdevel@vger.kernel.org
To: Matthew Wilcox <willy@infradead.org>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from smtp-fw-4101.amazon.com ([72.21.198.25]:48383 "EHLO
        smtp-fw-4101.amazon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1755690AbeARO1S (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Thu, 18 Jan 2018 09:27:18 -0500
In-Reply-To: <20180118142207.GB28322@bombadil.infradead.org>
Content-Language: en-US
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On 01/18/2018 03:22 PM, Matthew Wilcox wrote:
> On Thu, Jan 18, 2018 at 06:07:39AM -0800, Matthew Wilcox wrote:
>> On Thu, Jan 18, 2018 at 02:38:39PM +0100, Jan H. Schönherr wrote:
>>> The function follow_pte_pmd() can theoretically return after having
>>> acquired a PMD lock, even when DAX was not compiled with
>>> CONFIG_FS_DAX_PMD.
>>
>> I don't think it can.  How would a PMD entry get into a DAX VMA if we
>> compiled the kernel without CONFIG_FS_DAX_PMD?
> 
> How about this patch instead?  Should shut up sparse nicely.

It would still skip the unlock, in case pmdp is !=NULL (and locked) after follow_pte_pmd().
So it wouldn't address, what I intended to address with the patch.

Regards
Jan

> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 78b72c48374e..fea1b64d111b 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -586,6 +586,12 @@ pgoff_address(pgoff_t pgoff, struct vm_area_struct *vma)
>  	return address;
>  }
>  
> +#ifdef CONFIG_FS_DAX_PMD
> +#define dax_pmd(pmdp)	pmdp
> +#else
> +#define dax_pmd(pmdp)	0
> +#endif
> +
>  /* Walk all mappings of a given index of a file and writeprotect them */
>  static void dax_mapping_entry_mkclean(struct address_space *mapping,
>  				      pgoff_t index, unsigned long pfn)
> @@ -621,8 +627,7 @@ static void dax_mapping_entry_mkclean(struct address_space *mapping,
>  		 *
>  		 * See Documentation/vm/mmu_notifier.txt
>  		 */
> -		if (pmdp) {
> -#ifdef CONFIG_FS_DAX_PMD
> +		if (dax_pmd(pmdp)) {
>  			pmd_t pmd;
>  
>  			if (pfn != pmd_pfn(*pmdp))
> @@ -638,7 +643,6 @@ static void dax_mapping_entry_mkclean(struct address_space *mapping,
>  			set_pmd_at(vma->vm_mm, address, pmdp, pmd);
>  unlock_pmd:
>  			spin_unlock(ptl);
> -#endif
>  		} else {
>  			if (pfn != pte_pfn(*ptep))
>  				goto unlock_pte;
>