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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4D1EEC433EF for ; Mon, 27 Sep 2021 20:37:19 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EB58561074 for ; Mon, 27 Sep 2021 20:37:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org EB58561074 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 890C96B0072; Mon, 27 Sep 2021 16:37:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 81A44900002; Mon, 27 Sep 2021 16:37:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 694ED6B0074; Mon, 27 Sep 2021 16:37:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0225.hostedemail.com [216.40.44.225]) by kanga.kvack.org (Postfix) with ESMTP id 56B366B0072 for ; Mon, 27 Sep 2021 16:37:18 -0400 (EDT) Received: from smtpin36.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 1244939475 for ; Mon, 27 Sep 2021 20:37:18 +0000 (UTC) X-FDA: 78634513356.36.5AE66C6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf03.hostedemail.com (Postfix) with ESMTP id B978730000AA for ; Mon, 27 Sep 2021 20:37:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632775037; 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=kVhbRAJmKSprlQgg0jXpRAyAty2Z7WA2uiN0tVCX8Ys=; b=EK4+E9wyQHfFtmBTeENRppnPqij8aO+2kGvrhi5KxOJu4QyI3CQxVCXs0QXBUNIKpLnkQh zbOWA9aiy4JySkV0klZ9v+vHNbg445J6RJ5MdILCk3W3AjCcB1OZqgnzGPknoaY/aOI8zj CE6YpzLPIh+hrl0qqdG08Pu9/9gJnY0= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-272-SPOu2vZYN9aHqPwLpEfUlw-1; Mon, 27 Sep 2021 16:37:15 -0400 X-MC-Unique: SPOu2vZYN9aHqPwLpEfUlw-1 Received: by mail-qk1-f198.google.com with SMTP id t2-20020a05620a450200b0045e34e4f9c7so11508117qkp.18 for ; Mon, 27 Sep 2021 13:37:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=kVhbRAJmKSprlQgg0jXpRAyAty2Z7WA2uiN0tVCX8Ys=; b=hBlliN4KNAxTKbPnC7TiDDlGtPhC+PO/F5wq9UGGeyF3Iaqt/7lOwIEH2VhjvQsC11 i8MG1yt52l8uyN/I0hutnI9PJTCng8lDPMMeUrMDwP2xPg5PlAbCQMEwFw70RUulviGj hQpw4YX1BdLVVxv+gMc42iniZs0Ctk4oRcQiCPXcfjqgtZAQ85KdFEWT6oNowZ2r7M1H P4+RI55ev8KIPo3nDSFO7A7jUV+mXGGBcOfETzO3jM1FW8QP0kyWkGyvoW92PAKvs3Dc KmaZ7Ge1nDOnn5B7MByNKlKwashJKY0T8xXVje0EzZBmviUVzBXVt5FGieGQx2fwYads hBqg== X-Gm-Message-State: AOAM5307RA4Ka8RHfX4rH8shdGk4tcIRXIeCSD8hUX0HK0CGxPpAI2mB OGyIhtt486gCM3JT4hAuOBLJHodJO6nNtlIxkcWI3Lcg2h2M2uhbqzHGN5gaxnQ8JWgGRa2EKkl 7CR9CMCR4zuQ= X-Received: by 2002:a37:9c12:: with SMTP id f18mr2000952qke.18.1632775035141; Mon, 27 Sep 2021 13:37:15 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwgG4CZqC/O53dsfz8veZQDVuVytxC0nWOpDZX1csnQsSimgxnlUx4eZ2tkw/L+ekLBM11Mgg== X-Received: by 2002:a37:9c12:: with SMTP id f18mr2000928qke.18.1632775034799; Mon, 27 Sep 2021 13:37:14 -0700 (PDT) Received: from t490s ([2607:fea8:56a2:9100::d3ec]) by smtp.gmail.com with ESMTPSA id j14sm11851289qtv.36.2021.09.27.13.37.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Sep 2021 13:37:14 -0700 (PDT) Date: Mon, 27 Sep 2021 16:37:12 -0400 From: Peter Xu To: Axel Rasmussen Cc: Linux MM , LKML , Andrew Morton , Andrea Arcangeli , Nadav Amit , Li Wang Subject: Re: [PATCH] mm/userfaultfd: selftests: Fix memory corruption with thp enabled Message-ID: References: <20210923232512.210092-1-peterx@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=EK4+E9wy; spf=none (imf03.hostedemail.com: domain of peterx@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: B978730000AA X-Stat-Signature: rj5ytbxa888zza88ekzcq7abmtyjpomn X-HE-Tag: 1632775037-529808 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Sep 27, 2021 at 10:49:39AM -0700, Axel Rasmussen wrote: > One small comment: > > I'd prefer to keep the "uffd_test_ops->release_pages(area_src);" > above, to ensure the src region is empty. It's not immediately obvious > to me that we overwrite *all* of the bytes in src when we initialize > it. (I'd have to go look at the definition of area_count and read the > loop carefully.) It may not be technically needed, but it makes the > guarantee that we're starting with a clean slate, free from any > changes from previous test cases, very clear + explicit. I think there're only two fields used, area_mutex and area_count. I'm not sure what's the initial idea from Andrea when the test case is merged, but IMHO it can be written as a struct too instead of using the long macros; struct could make it easier to undertand. And note again that we have your uffd_test_ctx_clear() called which contains munmap() of all the buffers before the release_pages() calls. It means at least for anon and shmem the pages won't be there 100% sure to me. hugetlbfs is the only one that may still keep the pages as the fs should hold another refcount on the inode, however as all the two fields got re-written anyway, so I think it'll be still very safe to drop the two release_pages(). > > Moving the release_pages(area_dst) down as you've done seems correct to me. > > Either way you can take: > > Reviewed-by: Axel Rasmussen > > > > > It's just that after the weekend when I look back I still don't see a 100% > > clean way to fix it yet. Mapping 4K PROT_NONE before/after each allocation is > > the most ideal but still looks tricky to me. > > > > Would you have time on looking for a better solution, so as to (see it a way > > to) complete what commit 8ba6e8640844 whats to do afterwards? > > Sure, it seems related to the other THP investigations we talked about > in the other thread, so I'm happy to look into it. > > Just to set expectations, progress may be slightly slow as I'm > balancing other work my employer wants done, and some upcoming time > off. But, I think with your patch the test is at least stable (not > flaky) enough that there is no *urgent* need for this, so it should be > fine. Thanks a lot on both reviewing the patch and willing to look into it. As long as we don't get any report for khugepaged (if it happens, I'll provide the PROT_NONE hack instead - that'll work 100% I believe but less clean; but for now IMHO we don't need to bother) then we don't need to rush on that. -- Peter Xu