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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 297BFC3DA64 for ; Thu, 1 Aug 2024 14:51:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8AC9F6B00AA; Thu, 1 Aug 2024 10:51:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 85D3D6B00AC; Thu, 1 Aug 2024 10:51:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7244D6B00AD; Thu, 1 Aug 2024 10:51:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 53FD76B00AA for ; Thu, 1 Aug 2024 10:51:08 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 006C640E90 for ; Thu, 1 Aug 2024 14:51:07 +0000 (UTC) X-FDA: 82403964216.07.EE364C7 Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) by imf06.hostedemail.com (Postfix) with ESMTP id 179F0180011 for ; Thu, 1 Aug 2024 14:51:05 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=I8fjwXeI; spf=pass (imf06.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.221.54 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722523810; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=rczSTFdLOpaX7aUhoEqBv+g5Ojw1S2oguQ1RPZSdPN8=; b=0fDJE9/XgNFpfCuPzP3Xu3pT55VZhYeQJq/4uYps+/8fvpBqXpMIunh09KBPw219BhYkbk 9EP1oGVdXj/KD3SqrWNY9IjRWb8STuw9ZQCSxcTBl2mR3l57FYHMeOv1tC7nAjW1/yQ+z1 MeIkQlVnKmzE8noHiUHZ2Sov0uNhu7A= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722523810; a=rsa-sha256; cv=none; b=D4IIzOt6sBCfbDaefW0kVnoWfWo1BxorE30qeNGDAp7jPasJ1umjxLQd+o99VJCHdseKee Vq5drer1qmcnCfTMwqIJX4+dewtd8HBUZA07SuOCfjhtHNZQn7EtzPO5JDFfNCnw529rWN UdaUxS0uIbF7RK6wlCHK834SQ5FbobQ= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=I8fjwXeI; spf=pass (imf06.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.221.54 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-3685b9c8998so3091599f8f.0 for ; Thu, 01 Aug 2024 07:51:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722523864; x=1723128664; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=rczSTFdLOpaX7aUhoEqBv+g5Ojw1S2oguQ1RPZSdPN8=; b=I8fjwXeITfqTT3cCZyScLD2Q6B+sVvsX0h2xM4byAO7kifNf1oNgi/2ZTEnQtdGe9E wBmR8pt0duP4t9sG3PPXy4BKHjGgde49Fc398XTVXxxbvHEoUpzjY1gpl+8X9oR+/kkT 5JF2jsQcYy9OS8/MBW55ImeUREuIHdzGiXJw+Qcd00tLevN5TgBu9TkyY+cNa2su/B7P nAs/bFrU5MERg41z5jEEZH1qpfoA/IBYySD2IECKBC5iYFR1OFJenhIN/KqH1axy4rYX SCI0YF04rUorQc95ImLLEJgmTi2XbpkXSKNnM8qoQh5SCOcJDWBGO7DfhlcasMTJGKh1 1zpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722523864; x=1723128664; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rczSTFdLOpaX7aUhoEqBv+g5Ojw1S2oguQ1RPZSdPN8=; b=p+/zedUNTMc4dY2NdS9ITp6wt8SSP+HuniJNBHByLR0iRWaKkAtl1nlvkFX/nmcfAk W+2yYnlwKq53MnzvAXQoW2OdvPduMUhJSQKrQq5R0HY4bHckylm9i+ElTNkFFQxBkFxB ++Mg+KMsOVgjNIfPoMPyAR1gsyCrTVg28SJGlXwJhEP9B+CralC1lp2LPc2gXYKqlaIZ X9x9NWcJtLLnpCQPLOCeIDmJ3+IhN9vSjOYOLQR5QboWeAP3SOu1R2RnI2IM4MTz9CBb 2FwmoblIPXpZpLFBPkIaGtROvp+OhADuhgPGCO4/6L5Cbcurh7n6JtNRKmMlIEUkatZ3 cuFw== X-Forwarded-Encrypted: i=1; AJvYcCXFK+bgGJfHNDVHHgt/BSr3u8Z/Ja4ZuFClmCCmZD+cb0R2O9Yo3AhnanPczC0umFL7YGHuNBFQTOdlxVicy052weU= X-Gm-Message-State: AOJu0YwGrBHSI+L7+iiq3VvZyEmwBesLq4CVODhD3z/NcVJ0MDSORF5l Zv6cRyfxQnFKP+GsloXgkKzoxT+jb1NS94Ii2XgkXdAutxHDjq8v+nxj+Kdhl8y4veMiOWcOQev bMtWlY2iJA2jYHF/l32PDuVPM0Uo= X-Google-Smtp-Source: AGHT+IFBcarHS8O1Pqb+2XFf22lRmwSFx41Gq01tcdJG7Q+ftLMblf/eBxn2kodE3JwR/SJEnUUR52D4GJl1AZJS99Y= X-Received: by 2002:adf:f952:0:b0:368:7583:54c7 with SMTP id ffacd0b85a97d-36bbc0e66c0mr56885f8f.8.1722523864202; Thu, 01 Aug 2024 07:51:04 -0700 (PDT) MIME-Version: 1.0 References: <20240731124505.2903877-1-linyunsheng@huawei.com> <20240731124505.2903877-2-linyunsheng@huawei.com> <03c555c5-a25d-434a-aed4-0f2f7aa65adf@huawei.com> In-Reply-To: <03c555c5-a25d-434a-aed4-0f2f7aa65adf@huawei.com> From: Alexander Duyck Date: Thu, 1 Aug 2024 07:50:27 -0700 Message-ID: Subject: Re: [PATCH net-next v12 01/14] mm: page_frag: add a test module for page_frag To: Yunsheng Lin Cc: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Andrew Morton , linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 179F0180011 X-Stat-Signature: 5gs6k9di57r6bnguwa6chjd7gmshjd8w X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1722523865-136328 X-HE-Meta: U2FsdGVkX1+VR6TIo634A5epy7qEr6lQx0MuBws/RbcIC7s2bTbEmZ0g3dl1FuG+ij9SToGMd3zgRvztk4fj3dE0nMGX4w99+XUgWW/JDSumkIQP0oPUdZpWbkk+9Qa4l2tZWoTU5FrVUovbj2eJJD7c1mwsc4W/aRDPGJbce9/7qAh4hjSFGXkSMzGLfx55/vySRtZB8SbQbpiRTQ7D90pchuq0UTgDlK3o0MlReaO0RipkFyv+XUjEF3EoHWsiT4IiEVTv6DnzTrTvYTi5cgiLbXL1fGeuzobEXDEK/tsmB6/G991Vg/aq6jsJF0hEDjJzTIn8DtyHUWocl8jpMFf+zv91l6ABUfBG/57Kfvx2mE/J+j1Kc8+BRugEpRw67s7z/4RM+QYZ7q07zf2m/BO/Ry1ZNTSSfMO6wQo73IbcpTsgASc1a4IAFu/QoU3cXaDuLuq5M6F4cBn1NRhtr0GUtFqX3GhBaCbddMITxuTQe/bchEgLtGVZ0yfZ9BH8mAp1u3g5qxRhWxQ0l+GDR4iLHCcT20xJQCbqsxxJS+xDuWUzKQfpKOo2UqBatZZcL18lsInGzwnEnpx9QqgUQiHNosl6tXzSN0NNYqyEC6Xe04+2/uXvh7jLdNJVD8/KuAKGnmtEQ7vSxKJRzYwrSgwqWPiXPBwDPTuQ6tm1LUkxy7KSt7Y/UH4ADvjuGymGrIhsNRpFKTFvNKUZmQ8EzzhG93k/N6Sf718va6R/bL/s3Q+/bcpD4TghTY0S5PvIpHDBa17qSB0fuJaJdw3BveCl8bG3/jZ0niBDrZnjnMODfU2DSd5O5xKJnx24DuM/G0Luz+g0lhCL0i7OzP7nvoNIdefRpTEUdcKMDLE/8pBn8LNp1utugbh8/kL0Qzpc8EjIjJxWAOf4JtsqUi9G60/i9oyQS+n8hxuHxKESizP+3OnLtwqyIS1Wp5Oj/7abgG2CLmWMEAd13IzTx/D Nk3fNfU9 TGnVhzrRcHNmP5110eCIf4xZUIXw1UsD+BaH5m4WdLayMo5OiL90wo5Uriny+ODo1e0bfMfmPwJ5b/HwkozYA+yEYPhKsKEmPkj0ohHIkqKD2aJKTIQ6oTlJvNoB72aJ2Xt9owKlfCkeEekjtdrFtj6XKczbopJKXnKlvWt3+nXMoExxN9s/6KWY/1q4gd2IlLLHLVxbJ7OxTKe4R76a9zZjw3EjzEkhLsE+UN1mz+4mNfM1taNk9NJjywdEkabt7Rlh6xDo7SMwCrOmsSo0gITJx9oBBqnt3TtXr48faIfCT5up1f1gJ0qZz3JD46uM7QWPAnQ+7E1BZBRCkgKxdF3XS/vqXKSfRFaEMtVWFobBTyGrBkvs1+84aX+wlIEottuhRKvhZD16S/NY= 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: List-Subscribe: List-Unsubscribe: On Thu, Aug 1, 2024 at 5:58=E2=80=AFAM Yunsheng Lin wrote: > > On 2024/8/1 2:29, Alexander Duyck wrote: > > On Wed, Jul 31, 2024 at 5:50=E2=80=AFAM Yunsheng Lin wrote: > >> > >> Basing on the lib/objpool.c, change it to something like a > >> ptrpool, so that we can utilize that to test the correctness > >> and performance of the page_frag. > >> > >> The testing is done by ensuring that the fragment allocated > >> from a frag_frag_cache instance is pushed into a ptrpool > >> instance in a kthread binded to a specified cpu, and a kthread > >> binded to a specified cpu will pop the fragment from the > >> ptrpool and free the fragment. > >> > >> We may refactor out the common part between objpool and ptrpool > >> if this ptrpool thing turns out to be helpful for other place. > > > > This isn't a patch where you should be introducing stuff you hope to > > refactor out and reuse later. Your objpoo/ptrpool stuff is just going > > to add bloat and overhead as you are going to have to do pointer > > changes to get them in and out of memory and you are having to scan > > per-cpu lists. You would be better served using a simple array as your > > threads should be stick to a consistent CPU anyway in terms of > > testing. > > > > I would suggest keeping this much more simple. Trying to pattern this > > after something like the dmapool_test code would be a better way to go > > for this. We don't need all this extra objpool overhead getting in the > > way of testing the code you should be focused on. Just allocate your > > array on one specific CPU and start placing and removing your pages > > from there instead of messing with the push/pop semantics. > > I am not sure if I understand what you meant here, do you meant something > like dmapool_test_alloc() does as something like below? > > static int page_frag_test_alloc(void **p, int blocks) > { > int i; > > for (i =3D 0; i < blocks; i++) { > p[i] =3D page_frag_alloc(&test_frag, test_alloc_len, GFP_= KERNEL); > > if (!p[i]) > goto pool_fail; > } > > for (i =3D 0; i < blocks; i++) > page_frag_free(p[i]); > > .... > } > > The above was my initial thinking too, I went to the ptrpool thing using > at least two CPUs as the below reason: > 1. Test the concurrent calling between allocing and freeing more throughl= y, > for example, page->_refcount concurrent handling, cache draining and > cache reusing code path will be tested more throughly. > 2. Test the performance impact of cache bouncing between different CPUs. > > I am not sure if there is a more lightweight implementation than ptrpool > to do the above testing more throughly. You can still do that with a single producer single consumer ring buffer/array and not have to introduce a ton of extra overhead for some push/pop approach. There are a number of different implementations for such things throughout the kernel. > > > > > Lastly something that is a module only tester that always fails to > > probe doesn't sound like it really makes sense as a standard kernel > > I had the same feeling as you, but when doing testing, it seems > convenient enough to do a 'insmod xxx.ko' for testing without a > 'rmmod xxx.ko' It means this isn't a viable module though. If it supports insmod to trigger your tests you should let it succeed, and then do a rmmod to remove it afterwards. Otherwise it is a test module and belongs in the selftest block. > > module. I still think it would make more sense to move it to the > > selftests tree and just have it build there as a module instead of > > I failed to find one example of test kernel module that is in the > selftests tree yet. If it does make sense, please provide an example > here, and I am willing to follow the pattern if there is one. You must not have been looking very hard. A quick grep for "module_init" in the selftest folder comes up with "tools/testing/selftests/bpf/bpf_testmod/" containing an example of a module built in the selftests folder. > > trying to force it into the mm tree. The example of dmapool_test makes > > sense as it could be run at early boot to run the test and then it > > I suppose you meant dmapool is built-in to the kernel and run at early > boot? I am not sure what is the point of built-in for dmapool, as it > only do one-time testing, and built-in for dmapool only waste some > memory when testing is done. There are cases where one might want to test on a system w/o console access such as an embedded system, or in the case of an environment where people run without an initrd at all. > > just goes quiet. This module won't load and will always just return > > -EAGAIN which doesn't sound like a valid kernel module to me. > > As above, it seems convenient enough to do a 'insmod xxx.ko' for testing > without a 'rmmod xxx.ko'. It is, but it isn't. The problem is it creates a bunch of ugliness in the build as you are a tristate that isn't a tristate as you are only building it if it is set to "m". There isn't anything like that currently in the mm tree.