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;
MIME-Version: 1.0
References: <20220901072119.37588-1-david@redhat.com> <YxDdycTur733hMgt@xz-m1.local>
 <fa0bb4b1-3edd-eb5a-7ad6-dff785d88d8f@redhat.com> <YxDghv54uHYMGCfG@xz-m1.local>
 <c9dc3f22-4a72-9b9d-7a74-ad77fe4f3b6e@redhat.com> <CAHbLzkpdUMq2fMxqx-NgSZ2VLBU5RGqSpJRWH6eRrYymErAbaw@mail.gmail.com>
 <YxD00K1lv151X/eq@xz-m1.local>
In-Reply-To: <YxD00K1lv151X/eq@xz-m1.local>
From:   Yang Shi <shy828301@gmail.com>
Date:   Thu, 1 Sep 2022 11:35:40 -0700
Message-ID: <CAHbLzkrA-jKzTBq_Mn3NZYc91exovv1gH2LNzetCNiVu8+W6Kg@mail.gmail.com>
Subject: Re: [PATCH v1] mm/gup: adjust stale comment for RCU GUP-fast
To:     Peter Xu <peterx@redhat.com>
Cc:     David Hildenbrand <david@redhat.com>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.vnet.ibm.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Jerome Marchand <jmarchan@redhat.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Hugh Dickins <hughd@google.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        John Hubbard <jhubbard@nvidia.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Sep 1, 2022 at 11:07 AM Peter Xu <peterx@redhat.com> wrote:
>
> On Thu, Sep 01, 2022 at 10:50:48AM -0700, Yang Shi wrote:
> > Yeah, because THP collapse does copy the data before clearing pte. If
> > we want to remove pmdp_collapse_flush() by just clearing pmd, we
> > should clear *AND* flush pte before copying the data IIRC.
>
> Yes tlb flush is still needed.  IIUC the generic pmdp_collapse_flush() will
> still be working (with the pte level flushing there) but it should just
> start to work for all archs, so potentially we could drop the arch-specific
> pmdp_collapse_flush()s, mostly the ppc impl.

I'm don't know why powperpc needs to have its specific
pmdp_collapse_flush() in the first place, not only the mandatory IPI
broadcast, but also the specific implementation of pmd tlb flush. But
anyway the IPI broadcast could be removed at least IMO.

>
> This also reminded me that the s390 version of pmdp_collapse_flush() is a
> bit weird, since it doesn't even have the tlb flush there.  I feel like
> it's broken but I can't really tell whether something I've overlooked.
> Worth an eye on.

I don't know why. But if s390 doesn't flush tlb in
pmdp_collapse_flush(), then there may be data integrity problem since
the page is still writable when copying the data because pte is
cleared after data copying. Or s390 hardware does flush tlb
automatically?

>
> --
> Peter Xu
>