Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.117.160 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.117.160; helo=mail.nvidia.com; pr=C
Message-ID: <cb706d3f-1fa8-2047-e65c-e1dc1fa6821f@nvidia.com>
Date:   Tue, 23 May 2023 19:20:31 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH 1/2] mmu_notifiers: Restore documentation for
 .invalidate_range()
Content-Language: en-US
To:     Alistair Popple <apopple@nvidia.com>,
        Andrew Morton <akpm@linux-foundation.org>, <will@kernel.org>,
        <catalin.marinas@arm.com>, David Rientjes <rientjes@google.com>
CC:     <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
        <robin.murphy@arm.com>, <nicolinc@nvidia.com>,
        <linux-arm-kernel@lists.infradead.org>, <kvm@vger.kernel.org>,
        <jgg@nvidia.com>, <zhi.wang.linux@gmail.com>,
        Sean Christopherson <seanjc@google.com>
References: <3cece716fc09724793aa832e755abfc9d70a8bb3.1684892404.git-series.apopple@nvidia.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <3cece716fc09724793aa832e755abfc9d70a8bb3.1684892404.git-series.apopple@nvidia.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 May 2023 02:20:48.9621
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: ae10a83b-453c-4668-657a-08db5bfd7d5d
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.160];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT059.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB8607
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 5/23/23 18:47, Alistair Popple wrote:
> The .invalidate_range() callback is called by
> mmu_notifier_invalidate_range() which is often called while holding
> the ptl spin-lock. Therefore any implementations of this callback must
> not sleep. This was originally documented when the call back was added
> in commit 0f0a327fa12c ("mmu_notifier: add the callback for
> mmu_notifier_invalidate_range()") but appears to have been

Thanks for digging into this. I expect that you're on the right
track, I'm just wondering about something still:

> inadvertently removed by commit 5ff7091f5a2c ("mm, mmu_notifier:
> annotate mmu notifiers with blockable invalidate callbacks").

Was it really inadvertent, though? The initial patch proposed said this:

"Also remove a bogus comment about invalidate_range() always being called
under the ptl spinlock." [1]

I've added David Rientjes to CC.

I almost think we should rename the callback to something with
"non blocking" or similar in the name. It not great to have to
do this much research to figure out the intent. And it still feels
backwards.


[1] https://lore.kernel.org/all/alpine.DEB.2.10.1801091339570.240101@chino.kir.corp.google.com/T/#u


> 
> Restore the comment to make it clear that .invalidate_range()
> callbacks may not sleep.
> 
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> ---
>   include/linux/mmu_notifier.h | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> index 64a3e05..447d757 100644
> --- a/include/linux/mmu_notifier.h
> +++ b/include/linux/mmu_notifier.h
> @@ -200,6 +200,9 @@ struct mmu_notifier_ops {
>   	 * external TLB range needs to be flushed. For more in depth
>   	 * discussion on this see Documentation/mm/mmu_notifier.rst
>   	 *
> +	 * The invalidate_range() function is called under the ptl
> +	 * spin-lock and not allowed to sleep.
> +	 *
>   	 * Note that this function might be called with just a sub-range
>   	 * of what was passed to invalidate_range_start()/end(), if
>   	 * called between those functions.
> 
> base-commit: 44c026a73be8038f03dbdeef028b642880cf1511

Hooray for --base! :)


thanks,
-- 
John Hubbard
NVIDIA