From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 00041C43462 for ; Wed, 5 May 2021 13:24:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BD64061222 for ; Wed, 5 May 2021 13:24:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233528AbhEENZU (ORCPT ); Wed, 5 May 2021 09:25:20 -0400 Received: from mx2.suse.de ([195.135.220.15]:49522 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231774AbhEENZT (ORCPT ); Wed, 5 May 2021 09:25:19 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1620221062; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=aSIc/lDgtqLluSkN8lwArkLOvYYY16AGyEezAwvl+AY=; b=bvA4G+QBmgc3mkDjrCQOl7LWQXn8USiuFiw+pa8xnUItusUWADT+mFHhgCSn26vV+rBUU9 pOYdePL4OmedwlIo4eFr4C8mcM/1rW/BXH2AEwTL500kavySpYOB3rV/L0nthKhb61seS/ XCvo9Farq+XVMr9m+Zs/DULxlVV5QuQ= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id DF87CAE5E; Wed, 5 May 2021 13:24:21 +0000 (UTC) Date: Wed, 5 May 2021 15:24:19 +0200 From: Michal Hocko To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, Andrew Morton , "Michael S. Tsirkin" , Jason Wang , Alexey Dobriyan , Mike Rapoport , "Matthew Wilcox (Oracle)" , Oscar Salvador , Roman Gushchin , Alex Shi , Steven Price , Mike Kravetz , Aili Yao , Jiri Bohac , "K. Y. Srinivasan" , Haiyang Zhang , Stephen Hemminger , Wei Liu , Naoya Horiguchi , linux-hyperv@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v1 5/7] mm: introduce page_offline_(begin|end|freeze|unfreeze) to synchronize setting PageOffline() Message-ID: References: <20210429122519.15183-1-david@redhat.com> <20210429122519.15183-6-david@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210429122519.15183-6-david@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu 29-04-21 14:25:17, David Hildenbrand wrote: > A driver might set a page logically offline -- PageOffline() -- and > turn the page inaccessible in the hypervisor; after that, access to page > content can be fatal. One example is virtio-mem; while unplugged memory > -- marked as PageOffline() can currently be read in the hypervisor, this > will no longer be the case in the future; for example, when having > a virtio-mem device backed by huge pages in the hypervisor. > > Some special PFN walkers -- i.e., /proc/kcore -- read content of random > pages after checking PageOffline(); however, these PFN walkers can race > with drivers that set PageOffline(). > > Let's introduce page_offline_(begin|end|freeze|unfreeze) for > synchronizing. > > page_offline_freeze()/page_offline_unfreeze() allows for a subsystem to > synchronize with such drivers, achieving that a page cannot be set > PageOffline() while frozen. > > page_offline_begin()/page_offline_end() is used by drivers that care about > such races when setting a page PageOffline(). > > For simplicity, use a rwsem for now; neither drivers nor users are > performance sensitive. Please add a note to the PageOffline documentation as well. While are adding the api close enough an explicit note there wouldn't hurt. > Signed-off-by: David Hildenbrand As to the patch itself, I am slightly worried that other pfn walkers might be less tolerant to the locking than the proc ones. On the other hand most users shouldn't really care as they do not tend to touch the memory content and PageOffline check without any synchronization should be sufficient for those. Let's try this out and see where we get... Acked-by: Michal Hocko > --- > include/linux/page-flags.h | 5 +++++ > mm/util.c | 38 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 43 insertions(+) > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index b8c56672a588..e3d00c72f459 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h > @@ -767,6 +767,11 @@ PAGE_TYPE_OPS(Buddy, buddy) > */ > PAGE_TYPE_OPS(Offline, offline) > > +extern void page_offline_freeze(void); > +extern void page_offline_unfreeze(void); > +extern void page_offline_begin(void); > +extern void page_offline_end(void); > + > /* > * Marks pages in use as page tables. > */ > diff --git a/mm/util.c b/mm/util.c > index 54870226cea6..95395d4e4209 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -1013,3 +1013,41 @@ void mem_dump_obj(void *object) > } > pr_cont(" non-slab/vmalloc memory.\n"); > } > + > +/* > + * A driver might set a page logically offline -- PageOffline() -- and > + * turn the page inaccessible in the hypervisor; after that, access to page > + * content can be fatal. > + * > + * Some special PFN walkers -- i.e., /proc/kcore -- read content of random > + * pages after checking PageOffline(); however, these PFN walkers can race > + * with drivers that set PageOffline(). > + * > + * page_offline_freeze()/page_offline_unfreeze() allows for a subsystem to > + * synchronize with such drivers, achieving that a page cannot be set > + * PageOffline() while frozen. > + * > + * page_offline_begin()/page_offline_end() is used by drivers that care about > + * such races when setting a page PageOffline(). > + */ > +static DECLARE_RWSEM(page_offline_rwsem); > + > +void page_offline_freeze(void) > +{ > + down_read(&page_offline_rwsem); > +} > + > +void page_offline_unfreeze(void) > +{ > + up_read(&page_offline_rwsem); > +} > + > +void page_offline_begin(void) > +{ > + down_write(&page_offline_rwsem); > +} > + > +void page_offline_end(void) > +{ > + up_write(&page_offline_rwsem); > +} > -- > 2.30.2 > -- Michal Hocko SUSE Labs