Received: by 2002:ac0:aed5:0:0:0:0:0 with SMTP id t21csp2011192imb;
        Sun, 3 Mar 2019 14:38:30 -0800 (PST)
X-Google-Smtp-Source: APXvYqy4SUG7vnh9U3MnLJplwV1hHUHUTCT1CrnnIHr7/RZzFAT43a1VWTNWMvJaobOEN+PvnuP4
X-Received: by 2002:a63:e03:: with SMTP id d3mr15751292pgl.245.1551652710445;
        Sun, 03 Mar 2019 14:38:30 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1551652710; cv=none;
        d=google.com; s=arc-20160816;
        b=IS449LF/IIeetn7XP64Rzf4cnivwkme8fcAmOvSg0Efd1vSoqqA3LvhxHQS1nAPZmJ
         0boWXJQ+ghQxbYtBoeY0qzOMdKF2XlcK5yf7PpQbI4O34ug0TVz3nDj0qXOcbujpdoVa
         XuUxQiO4WhIlNnUekVtyz3BtsRcaphSpUjif9mnXxFhbD3ezfq7NlLdxZfHX3fxu+AkN
         lgUfxydj5rDGDhlpSKQ6ZtfhgSK8elT/ge/kzDVgaUqUbSKaM0pqTvVUvjI784vE8tOU
         AZIIm5IGgP86ttt7gzHpiseg04+NeAucmYIJ4QQe0CvCv/S5p3n1+OwcsG+kDyTc7DrQ
         cI8w==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816;
        h=list-id:precedence:sender:dkim-signature:content-transfer-encoding
         :content-language:in-reply-to:mime-version:user-agent:date
         :message-id:from:references:cc:to:subject;
        bh=YtliewqCkw6AvjhQ1RBNtPdsYc4lcHNCvdnJ8EtyJKA=;
        b=cv3FclWXkCN0ox68lJMWn0A6GAfKwZcCLrsqkgxNmcsKUw85/8J7XrWo9RJ9VpVM65
         X0DCl3y7CKGk198owTPX8ZeiwzFor+t/RdcJqv/2Ayuags63DqGTqNGxnd2dN8kFbtOS
         2SHvN3+t4/2halkE4hWJH+Yn2GxuRdKSLrT6sEhxYJ8Nc3fNnO6FF3KSSD7URbMoZRv/
         CVe8PM3m3eYS0xIpbNxuFh6GTIGqyEqy/s0TEvIC5EQX0I/wD0Cz8U3t1xcB0I8rNzsq
         1y1uTVFKVwSt9E7AJKA0UH0eZc+ap832o0EdWEVOc5mS67qKeQEsPEEvFH4wGVzvAJLC
         urcg==
ARC-Authentication-Results: i=1; mx.google.com;
       dkim=pass header.i=@nvidia.com header.s=n1 header.b=Jjh466tN;
       spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
       dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67])
        by mx.google.com with ESMTP id x14si3754105pgh.98.2019.03.03.14.38.14;
        Sun, 03 Mar 2019 14:38:30 -0800 (PST)
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;
Authentication-Results: mx.google.com;
       dkim=pass header.i=@nvidia.com header.s=n1 header.b=Jjh466tN;
       spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
       dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1726001AbfCCWhx (ORCPT <rfc822;lans.zhang2008@gmail.com>
        + 99 others); Sun, 3 Mar 2019 17:37:53 -0500
Received: from hqemgate16.nvidia.com ([216.228.121.65]:4427 "EHLO
        hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1725923AbfCCWhx (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Sun, 3 Mar 2019 17:37:53 -0500
Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqemgate16.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA)
        id <B5c7c573f0000>; Sun, 03 Mar 2019 14:37:51 -0800
Received: from hqmail.nvidia.com ([172.20.161.6])
  by hqpgpgate101.nvidia.com (PGP Universal service);
  Sun, 03 Mar 2019 14:37:52 -0800
X-PGP-Universal: processed;
        by hqpgpgate101.nvidia.com on Sun, 03 Mar 2019 14:37:52 -0800
Received: from [10.2.174.18] (10.124.1.5) by HQMAIL101.nvidia.com
 (172.20.187.10) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Sun, 3 Mar
 2019 22:37:51 +0000
Subject: Re: [PATCH v2] RDMA/umem: minor bug fix and cleanup in error handling
 paths
To:     Artemy Kovalyov <artemyko@mellanox.com>,
        Ira Weiny <ira.weiny@intel.com>,
        "john.hubbard@gmail.com" <john.hubbard@gmail.com>
CC:     "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Jason Gunthorpe <jgg@ziepe.ca>,
        Doug Ledford <dledford@redhat.com>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
References: <20190302032726.11769-2-jhubbard@nvidia.com>
 <20190302202435.31889-1-jhubbard@nvidia.com>
 <20190302194402.GA24732@iweiny-DESK2.sc.intel.com>
 <2404c962-8f6d-1f6d-0055-eb82864ca7fc@mellanox.com>
X-Nvconfidentiality: public
From:   John Hubbard <jhubbard@nvidia.com>
Message-ID: <332021c5-ab72-d54f-85c8-b2b12b76daed@nvidia.com>
Date:   Sun, 3 Mar 2019 14:37:33 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <2404c962-8f6d-1f6d-0055-eb82864ca7fc@mellanox.com>
X-Originating-IP: [10.124.1.5]
X-ClientProxiedBy: HQMAIL108.nvidia.com (172.18.146.13) To
 HQMAIL101.nvidia.com (172.20.187.10)
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;
        t=1551652671; bh=YtliewqCkw6AvjhQ1RBNtPdsYc4lcHNCvdnJ8EtyJKA=;
        h=X-PGP-Universal:Subject:To:CC:References:X-Nvconfidentiality:From:
         Message-ID:Date:User-Agent:MIME-Version:In-Reply-To:
         X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language:
         Content-Transfer-Encoding;
        b=Jjh466tNs7neALILTrnqqdaBg5r2amMZAGGz5F57Zctna8B1vRhkkkQTlhtSKXWSH
         jUvt0AagFbvshV/+2Wc9KckWRtA1ZgwYm2izIIjnuDYBmJH3/U4jQuH7K533MOktV/
         Q1RlPM6jiwXv0UGWH5l57pPjR7DqQ73prR0D6xvfkiqt8N6VSYxehysA/sHYYOZ8R/
         fFTjXxTRPGke6l7FrUsDE5u8LKqBRM9RVVYqLJX9/Ro2liCZnTTOOiU998/8xlbZDA
         I9PpKpIccA1zCvsQbkO0wkGnzgIxv3f+W6C3D5jdn86wxddLCz/kt7gsTARrffRlbV
         8Px7sha/qYW4A==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/3/19 1:52 AM, Artemy Kovalyov wrote:
>=20
>=20
> On 02/03/2019 21:44, Ira Weiny wrote:
>>
>> On Sat, Mar 02, 2019 at 12:24:35PM -0800, john.hubbard@gmail.com wrote:
>>> From: John Hubbard <jhubbard@nvidia.com>
>>>
>>> ...
>>> 3. Dead code removal: the check for (user_virt & ~page_mask)
>>> is checking for a condition that can never happen,
>>> because earlier:
>>>
>>> =C2=A0=C2=A0=C2=A0=C2=A0 user_virt =3D user_virt & page_mask;
>>>
>>> ...so, remove that entire phrase.
>>>
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 bcnt -=3D min_t(=
size_t, npages << PAGE_SHIFT, bcnt);
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mutex_lock(&umem=
_odp->umem_mutex);
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 for (j =3D 0; j =
< npages; j++, user_virt +=3D PAGE_SIZE) {
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if =
(user_virt & ~page_mask) {
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 p +=3D PAGE_SIZE;
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 if (page_to_phys(local_page_list[j]) !=3D p) {
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D -EFAULT;
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 break;
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 }
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 put_page(local_page_list[j]);
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 continue;
>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>>> -
>>
>> I think this is trying to account for compound pages. (ie page_mask coul=
d
>> represent more than PAGE_SIZE which is what user_virt is being incriment=
ed by.)
>> But putting the page in that case seems to be the wrong thing to do?
>>
>> Yes this was added by Artemy[1] now cc'ed.
>=20
> Right, this is for huge pages, please keep it.
> put_page() needed to decrement refcount of the head page.
>=20

OK, thanks for explaining! Artemy, while you're here, any thoughts about th=
e
release_pages, and the change of the starting point, from the other part of=
 the=20
patch:

@@ -684,9 +677,11 @@ int ib_umem_odp_map_dma_pages(struct ib_umem_odp *umem=
_odp,=20
u64 user_virt,
	mutex_unlock(&umem_odp->umem_mutex);

  		if (ret < 0) {
-			/* Release left over pages when handling errors. */
-			for (++j; j < npages; ++j)
-				put_page(local_page_list[j]);
+			/*
+			 * Release pages, starting at the the first page
+			 * that experienced an error.
+			 */
+			release_pages(&local_page_list[j], npages - j);
  			break;
  		}
  	}

?

thanks,
--=20
John Hubbard
NVIDIA