Subject: Re: [patch v2 1/2] mm, mmu_notifier: annotate mmu notifiers with
 blockable invalidate callbacks
To: Andrew Morton <akpm@linux-foundation.org>,
        David Rientjes <rientjes@google.com>
Cc: Michal Hocko <mhocko@suse.com>, Andrea Arcangeli <aarcange@redhat.com>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>, Oded Gabbay <oded.gabbay@gmail.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        David Airlie <airlied@linux.ie>, Joerg Roedel <joro@8bytes.org>,
        Doug Ledford <dledford@redhat.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Mike Marciniszyn <mike.marciniszyn@intel.com>,
        Sean Hefty <sean.hefty@intel.com>, Dimitri Sivanich <sivanich@sgi.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= <jglisse@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org
References: <alpine.DEB.2.10.1712111409090.196232@chino.kir.corp.google.com>
 <alpine.DEB.2.10.1712141329500.74052@chino.kir.corp.google.com>
 <20171215150429.f68862867392337f35a49848@linux-foundation.org>
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Message-ID: <cafa6cdb-886b-b010-753f-600ae86f5e71@I-love.SAKURA.ne.jp>
Date: Sat, 16 Dec 2017 16:14:07 +0900
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: <20171215150429.f68862867392337f35a49848@linux-foundation.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1652
Lines: 40

On 2017/12/16 8:04, Andrew Morton wrote:
>> The implementation is steered toward an expensive slowpath, such as after
>> the oom reaper has grabbed mm->mmap_sem of a still alive oom victim.
> 
> some tweakage, please review.
> 
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: mm-mmu_notifier-annotate-mmu-notifiers-with-blockable-invalidate-callbacks-fix
> 
> make mm_has_blockable_invalidate_notifiers() return bool, use rwsem_is_locked()
> 

> @@ -240,13 +240,13 @@ EXPORT_SYMBOL_GPL(__mmu_notifier_invalid
>   * Must be called while holding mm->mmap_sem for either read or write.
>   * The result is guaranteed to be valid until mm->mmap_sem is dropped.
>   */
> -int mm_has_blockable_invalidate_notifiers(struct mm_struct *mm)
> +bool mm_has_blockable_invalidate_notifiers(struct mm_struct *mm)
>  {
>  	struct mmu_notifier *mn;
>  	int id;
> -	int ret = 0;
> +	bool ret = false;
>  
> -	WARN_ON_ONCE(down_write_trylock(&mm->mmap_sem));
> +	WARN_ON_ONCE(!rwsem_is_locked(&mm->mmap_sem));
>  
>  	if (!mm_has_notifiers(mm))
>  		return ret;

rwsem_is_locked() test isn't equivalent with __mutex_owner() == current test, is it?
If rwsem_is_locked() returns true because somebody else has locked it, there is
no guarantee that current thread has locked it before calling this function.

down_write_trylock() test isn't equivalent with __mutex_owner() == current test, is it?
What if somebody else held it for read or write (the worst case is registration path),
down_write_trylock() will return false even if current thread has not locked it for
read or write.

I think this WARN_ON_ONCE() can not detect incorrect call to this function.