Subject: Re: [patch 1/2] mm, mmu_notifier: annotate mmu notifiers with
 blockable invalidate callbacks
To: =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        David Rientjes <rientjes@google.com>,
        Dimitri Sivanich <sivanich@hpe.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
        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>,
        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>,
        LKML <linux-kernel@vger.kernel.org>, linux-mm <linux-mm@kvack.org>
References: <alpine.DEB.2.10.1712111409090.196232@chino.kir.corp.google.com>
 <20171212200542.GJ5848@hpe.com>
 <alpine.DEB.2.10.1712121326280.134224@chino.kir.corp.google.com>
 <d6487124-b613-6614-f355-14b7388a8ae3@amd.com>
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Message-ID: <d435cf68-1073-7bdb-c5e7-c28f3e15bcb0@I-love.SAKURA.ne.jp>
Date: Wed, 13 Dec 2017 19:26:31 +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: <d6487124-b613-6614-f355-14b7388a8ae3@amd.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1834
Lines: 38

On 2017/12/13 18:34, Christian König wrote:
> Am 12.12.2017 um 22:28 schrieb David Rientjes:
>> On Tue, 12 Dec 2017, Dimitri Sivanich wrote:
>>
>>>> --- a/drivers/misc/sgi-gru/grutlbpurge.c
>>>> +++ b/drivers/misc/sgi-gru/grutlbpurge.c
>>>> @@ -298,6 +298,7 @@ struct gru_mm_struct *gru_register_mmu_notifier(void)
>>>>               return ERR_PTR(-ENOMEM);
>>>>           STAT(gms_alloc);
>>>>           spin_lock_init(&gms->ms_asid_lock);
>>>> +        gms->ms_notifier.flags = 0;
>>>>           gms->ms_notifier.ops = &gru_mmuops;
>>>>           atomic_set(&gms->ms_refcnt, 1);
>>>>           init_waitqueue_head(&gms->ms_wait_queue);
>>>> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
>>> There is a kzalloc() just above this:
>>>     gms = kzalloc(sizeof(*gms), GFP_KERNEL);
>>>
>>> Is that not sufficient to clear the 'flags' field?
>>>
>> Absolutely, but whether it is better to explicitly document that the mmu
>> notifier has cleared flags, i.e. there are no blockable callbacks, is
>> another story.  I can change it if preferred.
> 
> Actually I would invert the new flag, in other words specify that an MMU notifier will never sleep.
> 
> The first reason is that we have 8 blocking notifiers and 5 not blocking if I counted right. So it is actually more common to sleep than not to.
> 
> The second reason is to be conservative and assume the worst, e.g. that the flag is forgotten when a new notifier is added.

I agree. Some out of tree module might forget to set the flags.

Although you don't need to fix out of tree modules, as a troubleshooting
staff at a support center, I want to be able to identify the careless module.

I guess specifying the flags at register function would be the best, for
an attempt to call register function without knowing this change will
simply results in a build failure.