Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Zi Yan" <zi.yan@cs.rutgers.edu>
To:     "Huang, Ying" <ying.huang@intel.com>
Cc:     "Andrew Morton" <akpm@linux-foundation.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, "Al Viro" <viro@zeniv.linux.org.uk>,
        "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
        "Dan Williams" <dan.j.williams@intel.com>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH -mm] mm, gup: prevent pmd checking race in
 follow_pmd_mask()
Date:   Wed, 04 Apr 2018 11:02:26 -0400
Message-ID: <65E6BD75-FBA6-43AC-AC5A-B952DE409BC8@cs.rutgers.edu>
In-Reply-To: <20180404032257.11422-1-ying.huang@intel.com>
References: <20180404032257.11422-1-ying.huang@intel.com>
MIME-Version: 1.0
Content-Type: multipart/signed;
 boundary="=_MailMate_CB9590E2-828B-403D-9360-2645EC49EC8A_=";
 micalg=pgp-sha512; protocol="application/pgp-signature"
Received-SPF: None (protection.outlook.com: cs.rutgers.edu does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2018 15:02:32.4631
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 4c889636-a953-437e-b9ec-08d59a3d1896
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: b92d2b23-4d35-4470-93ff-69aca6632ffe
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR14MB1737
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

This is an OpenPGP/MIME signed message (RFC 3156 and 4880).

--=_MailMate_CB9590E2-828B-403D-9360-2645EC49EC8A_=
Content-Type: text/plain

On 3 Apr 2018, at 23:22, Huang, Ying wrote:

> From: Huang Ying <ying.huang@intel.com>
>
> mmap_sem will be read locked when calling follow_pmd_mask().  But this
> cannot prevent PMD from being changed for all cases when PTL is
> unlocked, for example, from pmd_trans_huge() to pmd_none() via
> MADV_DONTNEED.  So it is possible for the pmd_present() check in
> follow_pmd_mask() encounter a none PMD.  This may cause incorrect
> VM_BUG_ON() or infinite loop.  Fixed this via reading PMD entry again
> but only once and checking the local variable and pmd_none() in the
> retry loop.
>
> As Kirill pointed out, with PTL unlocked, the *pmd may be changed
> under us, so read it directly again and again may incur weird bugs.
> So although using *pmd directly other than pmd_present() checking may
> be safe, it is still better to replace them to read *pmd once and
> check the local variable for multiple times.

I see you point there. The patch wants to provide a consistent value
for all race checks. Specifically, this patch is trying to avoid the inconsistent
reads of *pmd for if-statements, which causes problem when both if-condition reads *pmd and
the statements inside "if" reads *pmd again and two reads can give different values.
Am I right about this?

If yes, the problem can be solved by something like:

if (!pmd_present(tmpval = *pmd)) {
    check tmpval instead of *pmd;
}

Right?

I just wonder if we need some general code for all race checks.

Thanks.

--
Best Regards
Yan Zi

--=_MailMate_CB9590E2-828B-403D-9360-2645EC49EC8A_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - https://gpgtools.org

iQEcBAEBCgAGBQJaxOkCAAoJEEGLLxGcTqbMZU8H/1RA0Gu02wy0myF4tXGBkeFp
6TPYuRa1qbFQcyl6+2u3jU2gmKEWNN2JSHErUk97CZrgzuZ44tjcyf7VCtWff26c
lGGI2hBnNiln8w9A2rsZcxV/rKcNbyZftiXUoosR/WVhqOzlqIa+6zbXz426FwJs
9fCpogLVZrAJio/cCRHaBsuZxw7HD94wUARB9ofHHLuDPbcaQH/a8H4jp/Funa28
hT6ngohFqquFCCTVLkOSiNush8juSXXrS0QSqf35FWZKaW9QyvqLvx82CsXgveJ7
y/oSN7+fL+NGiyrqGdImkFG/AQsjcp5zfKfTh+MV4rSuMSGhbHH0WtRTnQaaCvk=
=DDkr
-----END PGP SIGNATURE-----

--=_MailMate_CB9590E2-828B-403D-9360-2645EC49EC8A_=--