Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jesper Dangaard Brouer <jbrouer@redhat.com>
CC:     Mel Gorman <mgorman@techsingularity.net>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Linux-MM <linux-mm@kvack.org>,
        "brouer@redhat.com" <brouer@redhat.com>
Subject: Re: [PATCH v3] mm/page_alloc: Further fix __alloc_pages_bulk() return
 value
Thread-Topic: [PATCH v3] mm/page_alloc: Further fix __alloc_pages_bulk()
 return value
Thread-Index: AQHXbO1toCeUhvRJMUGKkyLQssKzrasrJhsAgAAIrYA=
Date:   Tue, 29 Jun 2021 16:32:15 +0000
Message-ID: <9C7E1CE0-8AF8-4FCD-BABC-F09A2827CCBA@oracle.com>
References: <162497449506.16614.7781489905877008435.stgit@klimt.1015granger.net>
 <be9186d9-1e8e-4d99-ab0f-84c0518025c5@redhat.com>
In-Reply-To: <be9186d9-1e8e-4d99-ab0f-84c0518025c5@redhat.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Qed2CMFCjnomc6woHHn7euD9k+/YMtL1RGwoGU/NkxzfB+ZrAxZ1O0dx6u8j?=
 =?us-ascii?Q?P+kVPCIGI2VGB5a3LqVXBm9nxW1YOCbbcQNpSF00f/bvRwSfzt0NO3+DfuZS?=
 =?us-ascii?Q?BFQnk9Uw1lUaABSLX718dVEbmswU4+hqHGUD7Jz4VAOP7e/bSA/5AOqF2HeJ?=
 =?us-ascii?Q?pE19mn3GNc+TWUhUFUt4BEAqenVoRtZB2SkSlakMXUlwg38C9gxv0IS9bS3I?=
 =?us-ascii?Q?o9ARzESD2xUBM5Jkihctze45pQHYCNRWBNmkqLlv6XFWIHPX8axuS7VoQa9Y?=
 =?us-ascii?Q?35YPo+4wBmDCIqhDcb4aA+YqvN8kw21h9WQZ80/1h84WJoedGjUAtZ+zmcbF?=
 =?us-ascii?Q?JXdEImMX4/yVRPXOp32pvyzPSdIRrgr4mCP5PV3/Jw0whXmf/FFUN7zjkVLc?=
 =?us-ascii?Q?7k7viw76HxmWM+zGDQYsHUhW67fEy8N9y+IgTBtRWpyA3s74xdCT+D9TapDG?=
 =?us-ascii?Q?oXfY/yzAGL35jRQYXPcxLFUq2YN7dX/9Tv8hKgamR4ZIwizt4RT/nwZgnQYW?=
 =?us-ascii?Q?RvyjpwLS3IhikmObZp+Ao9BBSioJ3g7ik9Q1+aMe4WrP4NcsqYGforGsQyLq?=
 =?us-ascii?Q?vBh9kyb2x4zONRGG3+zSwMuIYEdasEC30cXcfkR2rdcn/q+Z6z/cBerzKfcA?=
 =?us-ascii?Q?6DfFUanm/VSsknv66dj5cysqz+EdKSa06EC26lWkT3QAWaA50kRXSj4qVwdB?=
 =?us-ascii?Q?rJKWYUy1Wdm0eZkZwhuWG9sPwhV3gBVqx1rWfPVn9Q4f5PDHUoBNofxPSphS?=
 =?us-ascii?Q?KckoPYnaQN2129kj7QLacXCnqucL+t5LC1DQWOrs2t82sHr9DemYmBD9hRoq?=
 =?us-ascii?Q?U0SmLLgL/p10GIFTHJpSi9Wc3ohgevFH7sxL10rbLUpnCTvHq8oicw/i8Ssn?=
 =?us-ascii?Q?yBTLnufCUqNBCvdYZIrRzPUBQuymNqP+PQzmAQxpZVfaNh+ajFfqElfLXADw?=
 =?us-ascii?Q?mSdO8pFnTR8N3MqubkV4xem5XcePQDsnS69nQ4aDfU8fBHgOiw+qqEklqM42?=
 =?us-ascii?Q?JprFlJ3D4pTiZlfL9PPTOYbaDcdlAEbUmepo5CaIyb1h6YL1ItwVinJ3VGh8?=
 =?us-ascii?Q?Ol97r25iRjRr9+nESqfaEUlNsrbB+WFfze8t2jnozn9Lz4/0VsGFGpNN2L1j?=
 =?us-ascii?Q?yxcTs0xcr4OONei2sjm0QChpZ3ul49AGEGzyQzm2OjmQ9ICOXds5n9PnqDIh?=
 =?us-ascii?Q?loCGkG+pButY6MXZGN3UJ1GklNZVZGXvdIWCovMAMZ4xxNDP8TJ68wLn+kTZ?=
 =?us-ascii?Q?8bUPeT9usCFEhJemksycabLLz1mjfyRuz+Kx1FotO6JaEo3mK7HHkExK3DtB?=
 =?us-ascii?Q?t46ZmEhz7evmxRPED9bBxGtH?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4B9C0C2185CA5443A3348C04064AA916@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4688.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6b2ccadb-95b6-4db9-872a-08d93b1b74aa
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Jun 2021 16:32:15.5989
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: LSQgZWdvcJHBFSd0Ht/UEKATSmc7QDm/RUTZbvGBNV9RFROEq0oiy0DmGYgrfY6oL65Br+hKqndz0sDyqrIoWw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3381
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10030 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 spamscore=0
 mlxlogscore=999 suspectscore=0 phishscore=0 bulkscore=0 adultscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104190000 definitions=main-2106290104
X-Proofpoint-GUID: coHdTLPLzWuS9YvvqvEU-UH03AAFrCyh
X-Proofpoint-ORIG-GUID: coHdTLPLzWuS9YvvqvEU-UH03AAFrCyh
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jun 29, 2021, at 12:01 PM, Jesper Dangaard Brouer <jbrouer@redhat.com>=
 wrote:
>=20
> On 29/06/2021 15.48, Chuck Lever wrote:
>=20
>> The call site in __page_pool_alloc_pages_slow() also seems to be
>> confused on this matter. It should be attended to by someone who
>> is familiar with that code.
>=20
> I don't think we need a fix for __page_pool_alloc_pages_slow(), as the ar=
ray is guaranteed to be empty.
>=20
> But a fix would look like this:
>=20
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index c137ce308c27..1b04538a3da3 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -245,22 +245,23 @@ static struct page *__page_pool_alloc_pages_slow(st=
ruct page_pool *pool,
>         if (unlikely(pp_order))
>                 return __page_pool_alloc_page_order(pool, gfp);
>=20
>         /* Unnecessary as alloc cache is empty, but guarantees zero count=
 */
> -       if (unlikely(pool->alloc.count > 0))
> +       if (unlikely(pool->alloc.count > 0))   // ^^^^^^^^^^^^^^^^^^^^^^
>                 return pool->alloc.cache[--pool->alloc.count];
>=20
>         /* Mark empty alloc.cache slots "empty" for alloc_pages_bulk_arra=
y */
>         memset(&pool->alloc.cache, 0, sizeof(void *) * bulk);
>=20
> +       /* bulk API ret value also count existing pages, but array is emp=
ty */
>         nr_pages =3D alloc_pages_bulk_array(gfp, bulk, pool->alloc.cache)=
;
>         if (unlikely(!nr_pages))
>                 return NULL;

Thanks, this check makes sense to me now.


>         /* Pages have been filled into alloc.cache array, but count is ze=
ro and
>          * page element have not been (possibly) DMA mapped.
>          */
> -       for (i =3D 0; i < nr_pages; i++) {
> +       for (i =3D pool->alloc.count; i < nr_pages; i++) {
>                 page =3D pool->alloc.cache[i];
>                 if ((pp_flags & PP_FLAG_DMA_MAP) &&
>                     unlikely(!page_pool_dma_map(pool, page))) {
>                         put_page(page);
>=20

--
Chuck Lever