Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752567AbdLMKhl (ORCPT ); Wed, 13 Dec 2017 05:37:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55842 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752102AbdLMKhe (ORCPT ); Wed, 13 Dec 2017 05:37:34 -0500 Subject: Re: [patch 1/2] mm, mmu_notifier: annotate mmu notifiers with blockable invalidate callbacks To: Tetsuo Handa , =?UTF-8?Q?Christian_K=c3=b6nig?= , David Rientjes , Dimitri Sivanich Cc: Andrew Morton , Michal Hocko , Andrea Arcangeli , Benjamin Herrenschmidt , Paul Mackerras , Oded Gabbay , Alex Deucher , David Airlie , Joerg Roedel , Doug Ledford , Jani Nikula , Mike Marciniszyn , Sean Hefty , Dimitri Sivanich , Boris Ostrovsky , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , LKML , linux-mm References: <20171212200542.GJ5848@hpe.com> From: Paolo Bonzini Message-ID: Date: Wed, 13 Dec 2017 11:37:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 13 Dec 2017 10:37:34 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2073 Lines: 44 On 13/12/2017 11:26, Tetsuo Handa wrote: > 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. Specifying them in the ops would have the same effect and it would be even better, as you don't have to split the information across two places. Paolo