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=-7.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 EBD45C433DB for ; Thu, 7 Jan 2021 22:33:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B6EAD235FF for ; Thu, 7 Jan 2021 22:33:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728926AbhAGWdN (ORCPT ); Thu, 7 Jan 2021 17:33:13 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:59635 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727107AbhAGWdK (ORCPT ); Thu, 7 Jan 2021 17:33:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1610058703; h=from:from:reply-to:subject:subject: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=sH1Oz4zaoHH/onOXGglouMWNOSaa/V+B/VSZ6QQ9VbE=; b=etDT8riLzV4nmOOtYIF9HWMFiJAosee8GI0/rrv90dxBvR22tpdNQjRcAO3BpNOBIMz7UI aEHvAIAoUT6TndnF78t+APnOus7P5/WScFVQFYB6Z7UCLOEic1AR0RTOMkbinemW7de0Jp 6gwyzHW0HerdVk2UJXBYunikZul15vY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-144-28Qz9KUBPwau2GZF_C58Ag-1; Thu, 07 Jan 2021 17:31:39 -0500 X-MC-Unique: 28Qz9KUBPwau2GZF_C58Ag-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E41A7800D62; Thu, 7 Jan 2021 22:31:36 +0000 (UTC) Received: from mail (ovpn-112-222.rdu2.redhat.com [10.10.112.222]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0CA0C6268F; Thu, 7 Jan 2021 22:31:30 +0000 (UTC) Date: Thu, 7 Jan 2021 17:31:29 -0500 From: Andrea Arcangeli To: Linus Torvalds Cc: Linux-MM , Linux Kernel Mailing List , Yu Zhao , Andy Lutomirski , Peter Xu , Pavel Emelyanov , Mike Kravetz , Mike Rapoport , Minchan Kim , Will Deacon , Peter Zijlstra , Hugh Dickins , "Kirill A. Shutemov" , Matthew Wilcox , Oleg Nesterov , Jann Horn , Kees Cook , John Hubbard , Leon Romanovsky , Jason Gunthorpe , Jan Kara , Kirill Tkhai Subject: Re: [PATCH 2/2] mm: soft_dirty: userfaultfd: introduce wrprotect_tlb_flush_pending Message-ID: References: <20210107200402.31095-1-aarcange@redhat.com> <20210107200402.31095-3-aarcange@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.0.4 (2020-12-30) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 07, 2021 at 01:29:43PM -0800, Linus Torvalds wrote: > On Thu, Jan 7, 2021 at 12:59 PM Andrea Arcangeli wrote: > > > > The problem is it's not even possible to detect reliably if there's > > really a long term GUP pin because of speculative pagecache lookups. > > So none of the normal code _needs_ that any more these days, which is > what I think is so nice. Any pinning will do the COW, and then we have > the logic to make sure it stays writable, and that keeps everything > nicely coherent and is all fairly simple. > > And yes, it does mean that if somebody then explicitly write-protects > a page, it may end up being COW'ed after all, but if you first pinned > it, and then started playing with the protections of that page, why > should you be surprised? > > So to me, this sounds like a "don't do that then" situation. > > Anybody who does page pinning and wants coherency should NOT TOUCH THE > MAPPING IT PINNED. > > (And if you do touch it, it's your own fault, and you get to keep both > of the broken pieces) > > Now, I do agree that from a QoI standpoint, it would be really lovely > if we actually enforced it. I'm not entirely sure we can, but maybe it > would be reasonable to use that > > mm->has_pinned && page_maybe_dma_pinned(page) > > at least as the beginning of a heuristic. > > In fact, I do think that "page_maybe_dma_pinned()" could possibly be > made stronger than it is. Because at *THAT* point, we might say "we > know a pinned page always must have a page_mapcount() of 1" - since as > part of pinning it and doing the GUP_PIN, we forced the COW, and then > subsequent fork() operations enforce it too. > > So I do think that it might be possible to make that clear_refs code > notice "this page is pinned, I can't mark it WP without the pinning > coherency breaking". > > It might not even be hard. But admittedly I'm somewhat handwaving > here, and I might not have thought of some situation. I suppose the objective would be to detect when it's a transient pin (as an O_DIRECT write) and fail clear_refs with an error for all other cases of real long term pins that need to keep reading at full PCI bandwidth, without extra GUP invocations after the wp_copy_page run. Because of the speculative lookups are making the count unstable, it's not even enough to use mmu notifier and never use FOLL_GET in GUP to make it safe again (unlike what I mentioned in a previous email). Random memory corruption will still silently materialize as result of the speculative lookups in the above scenario. My whole point here in starting this new thread to suggest page_count in do_wp_page is an untenable solution is that such commit silently broke every single long term PIN user that may be used in combination of clear_refs since 2013. Silent memory corruption undetected or a detectable error out of clear_refs, are both different side effects the same technical ABI break that rendered clear_refs fundamentally incompatible with clear_refs, so detecting it or not still an ABI break that is. I felt obliged to report there's something much deeper and fundamentally incompatible between the page_count in do_wp_page any wrprotection of exclusive memory in place, as if used in combination with any RDMA for example. The TLB flushing and the mmap_read/write_lock were just the tip of the iceberg and they're not the main concern anymore. In addition, the inefficiency caused by the fact the page_count effect is multiplied by 512 times or 262144 while the mapcount remains 4k granular, makes me think the page_count is unsuitable to be used there and this specific cure with page_count in do_wp_page, looks worse than the initial zygote disease. Thanks, Andrea