Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] hugetlbfs: Take read_lock on i_mmap for PMD sharing
To:     Matthew Wilcox <willy@infradead.org>,
        Waiman Long <longman@redhat.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Davidlohr Bueso <dave@stgolabs.net>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Will Deacon <will.deacon@arm.com>
References: <20191107190628.22667-1-longman@redhat.com>
 <20191107195441.GF11823@bombadil.infradead.org>
From:   Mike Kravetz <mike.kravetz@oracle.com>
Message-ID: <ed46ef09-7766-eb80-a4ad-4c72d8dba188@oracle.com>
Date:   Thu, 7 Nov 2019 13:49:55 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.2.0
MIME-Version: 1.0
In-Reply-To: <20191107195441.GF11823@bombadil.infradead.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 11/7/19 11:54 AM, Matthew Wilcox wrote:
> Are there other current users of the write lock that could use a read lock?
> At first blush, it would seem that unmap_ref_private() also only needs
> a read lock on the i_mmap tree.  I don't think hugetlb_change_protection()
> needs the write lock either.  Nor retract_page_tables().

I believe that the semaphore still needs to be held in write mode while
calling huge_pmd_unshare (as is done in the call sites above).  Why?
There is this check for sharing in huge_pmd_unshare,

	if (page_count(virt_to_page(ptep)) == 1)
		return 0;	// implies no sharing

Note that huge_pmd_share now increments the page count with the semaphore
held just in read mode.  It is OK to do increments in parallel without
synchronization.  However, we don't want anyone else changing the count
while that check in huge_pmd_unshare is happening.  Hence, the need for
taking the semaphore in write mode.
-- 
Mike Kravetz