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 EFF8FC4345F for ; Mon, 15 Apr 2024 07:04:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7055C6B0085; Mon, 15 Apr 2024 03:04:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6AF036B0087; Mon, 15 Apr 2024 03:04:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 528766B0088; Mon, 15 Apr 2024 03:04:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 3217A6B0085 for ; Mon, 15 Apr 2024 03:04:25 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id CD82140488 for ; Mon, 15 Apr 2024 07:04:24 +0000 (UTC) X-FDA: 82010877648.24.1F4CB6C Received: from mail-vs1-f46.google.com (mail-vs1-f46.google.com [209.85.217.46]) by imf08.hostedemail.com (Postfix) with ESMTP id 1158016001B for ; Mon, 15 Apr 2024 07:04:21 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=afkC9aev; spf=pass (imf08.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.46 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713164662; a=rsa-sha256; cv=none; b=v6d0Hh1c5DB2Q5B1/zIJGLH898jTUlHaG/f7a+teK8PmfX8KjuXuwHlnbFNE48eBq0nyjm 1OhFNpES8yUfHta9TphazpryQb+S+rRWGGTdXwBxFGm/uCuV/xU2v6WlAlHLzArQlix9Sn iZHsk+dYnm/vd1inH0rFZxixr0byqKU= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=afkC9aev; spf=pass (imf08.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.46 as permitted sender) smtp.mailfrom=21cnbao@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=1713164662; 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=zmCfKirzLXB4pHNEyVF8BmROY/YRx1yv0FznouV7Qmo=; b=V29+BxxdZP6Vyk8q5QknuqHnQTpkbMoVo55XijjkZYYPYLkL8JPw7CUWnH8dDLugZMTJG5 6KXnIyCrfOuiMNJJepsVtQE7QvGGEmgcsAmZ3AF9FiexcTOANzdUQlXF+CALjg6xyq2RR7 G1DiTpPhF18VImOXSPjazBOGT/gy/ag= Received: by mail-vs1-f46.google.com with SMTP id ada2fe7eead31-479c8f43ca5so921171137.0 for ; Mon, 15 Apr 2024 00:04:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713164661; x=1713769461; 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=zmCfKirzLXB4pHNEyVF8BmROY/YRx1yv0FznouV7Qmo=; b=afkC9aevpQ8q42ALZcRcSVmCy1H0VfvQGwZ1OF3CgF5lDwqi895/4IMSrBqb0iAwGS w1WH5nhS15XKYG9/PWy3qwYlnIUUh32m8UDE5EmQWo3KB0WWvTOAQRnfwzWdZ2NKgHpP ZTA+DZKUjoJy3IduuqQANUGrHZKjCywGKcrrA+y2g9hFS0wuOqnF+FukzW6OrvGkkgrY UVX4jSX0+3xqncke7/pQ7b0FeIU+GIijwAUx9AIGgQR6efBQ213Qw4gHVAeE/CRRfaWj 5N/V9fQJttZtMeSjXMrZsBGinVi8mQmgL9PcDLKUzwPPo3TEp7HU+xvsLGgkNY5xNVPv IFLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713164661; x=1713769461; 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=zmCfKirzLXB4pHNEyVF8BmROY/YRx1yv0FznouV7Qmo=; b=H+K+fo69m6R+GjIUsrfZFUSvKxikicdh25YhcI/yxfVYlbFEi2FxdnChM2NEm92wrL lTfS+VhZffk546QkziGYCs0EdXxlIekfNUetZaCdej52SAiYp9k3vxHMPjUf8Jr4ZW9G pzQPJwjeo4lf1FIOa+iZlDCJ4+FOiySWcp/k6sFZ3JKt3XhSQogN6RKnmpnPBFqVnEVI L/l9Hh/AAFvU1WPwwAMrtq37wTSIrzIkVSSEjnppZ/tAejbxHUOsulRSh7MgIQ1URWLg uD5SBMQrAuKwC8rNCQAYaLJehJxywQ6jh0FknOTssmofNQhgq5J3vCvmvZcWQKFz2tpI CTHA== X-Forwarded-Encrypted: i=1; AJvYcCWE1GgnqxNpW9lc/9eVIpNtQK/NwnuZnme7ao4+PyNCCWOfpbVZfvccQ7kzFKgwm2sD1glLG50qZhg/UVJCn/zm7r4= X-Gm-Message-State: AOJu0YynDzoPcIzZ+L+HeW9IOCZapkDn8dkjPOooDf8sGlRdDxE3IElP 4G4HUo0bgjzirci6nC7d0IpKwUEl3i9OFOUBN5jbOT7jMoRKtyVsRzdsG0z4SXc85skNLJkk8vv wHzann3YP9t4M5E5fBjsuv6tFpNA= X-Google-Smtp-Source: AGHT+IE2jObFNDSF11WjaIDSfio3KBBdInbtgqMFvQMLFBl5UDL3sbN4rd1KmnHIe1bBCzbbGich6hv/BVLDFJFluf8= X-Received: by 2002:a05:6102:dcc:b0:47a:4801:6738 with SMTP id e12-20020a0561020dcc00b0047a48016738mr8429860vst.18.1713164660339; Mon, 15 Apr 2024 00:04:20 -0700 (PDT) MIME-Version: 1.0 References: <20240409082631.187483-1-21cnbao@gmail.com> <20240409082631.187483-2-21cnbao@gmail.com> <87y19f2lq3.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <87y19f2lq3.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Barry Song <21cnbao@gmail.com> Date: Mon, 15 Apr 2024 19:04:08 +1200 Message-ID: Subject: Re: [PATCH v2 1/5] mm: swap: introduce swap_free_nr() for batched swap_free() To: "Huang, Ying" Cc: akpm@linux-foundation.org, linux-mm@kvack.org, baolin.wang@linux.alibaba.com, chrisl@kernel.org, david@redhat.com, hanchuanhua@oppo.com, hannes@cmpxchg.org, hughd@google.com, kasong@tencent.com, ryan.roberts@arm.com, surenb@google.com, v-songbaohua@oppo.com, willy@infradead.org, xiang@kernel.org, yosryahmed@google.com, yuzhao@google.com, ziy@nvidia.com, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 1158016001B X-Stat-Signature: n1en6stg4gi74pq61d3mbgafz9y5g6ak X-Rspam-User: X-HE-Tag: 1713164661-503639 X-HE-Meta: U2FsdGVkX193NosfGkJUcLQATDuq348f14xAcVVgAU8Cd77jC1eJDVdnemhb+DmB4CwTZhocK6n/g/lHH0AuNHEEsqFe5BNfY6zjQO/HnCfKgXH9UlVLNdfvVZtRgKjhElMNxMa3rq3XBOXpep4x2FUFI8IQ07xaSmz60550UNsrrdugVUu+N3m5rTJdclmZmg8OD2y8c8mf/Wq5Cg/T8qAvsNca1205eLUCTzpU1u1uzzrQU7Yio+e5P9aWVQOvZzHNMlwpR3vY5aAyPMQ96g7mZvSGDcUfxvDIfxggbmUWvWj6I+2tGhJhH0K1imsd0ftf/RMWKrTOHTUP+j/A3J+ED3GCC9CdO7AHn4kyXBpUu/JA1uup8byJdY5n26uyFXUILr8nxWIWMiJJhRNCfIxUkNYLNBM3zyu0jNpte0cUUQiNteAoA3KW1tDYG4quIRHkh3lfovJxSwWn3GVJS6wMdtF1z18GqUdd9LqW+RfFc+31UA3wwi7fSMhwftYciREWxD0maKXooXtFIEUDxlkV1L8xQtvrh8/qliRcURx1dkim9xTMOE9q+uO21pFNdua9KZ1ULHYoiQ/sOpvGxS0kcLO9hlZdpA83CS/6ipSEUzpRJ3jAXWddnWVYisuPY7l8ljcICTXpC2m0kQS1QfnXsiQ8YJl3lEGEFG3k44JVO83GUFSoAqeJHwDdPOsqqNxXUYJ9lkaO6gFIzyvq5trbD/ILHZk1n5XMfm3zi7HcxxF+VsPx008GIYukXrqZzAKveeunezjmnUB1ZogZlcAS89dKYYH0ECaFLVV9WoN8wUcUfBlb5sdkEgjaIixN+mnm0LDrFYhNtdDvr1E2Y3Dfn6ab3zDou6ogNgWcH/eZShl8eFly++hVGQYvIKIB7HmKWEyodRXTpWbVw54GIjJr+nI62Kb2Gef6jHCL+nyjv37C0JfDEDHGpgdqUqCNhL4sWsWIedTQGdi0oVT 8gAcXdAu qDFu8WeBh7i5bNibEZXcmnxZ9aRdhedl5FIALAgGSR/fXMKheAhd2RMOriqYa3E6XIGuAN4k8aOvwe18iy6vvfAke3G501uaFWIyECyfQn6qfucYbgJggwzkJU0uSrMbG3KBRRMZujBKlNo7VkNfJBAk9x8LvjAM48RIiLJZ2RsNBuQy/rMZdmYGyZ8XQZIPGJVPLt8AgX6Qf0eN/XnLQSiFCLBp2V+sCz+2yCytxvIy/GmZ1twPY90kakEj3UwuPmx1+g+YALrBXPZQsu4TmMgLfiw0BLae1YfpySDaOvWuICn4VmFyUVzTpvEKsaX6KuGQQJGN8ZDTU/Mi1G9JxtJJcgn1O77gIUfyKV7ec/czR3JQKcw34bFNxb5xa23uB15p+LxFZ53nesdzZHy6cwy1DQmuD2lxiFSyNc+wxKs4o2QY/blimibqGy8fZ1R8/8Hw3aTOXWmNhDju2jYCOCpaO72P4TrwDK1frT07iq0OaLSJ+VNBvdIxyycYu6et1W/dmUwBh0pnaS5NbMG9Ur9o7gg== 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 Mon, Apr 15, 2024 at 6:19=E2=80=AFPM Huang, Ying = wrote: > > Barry Song <21cnbao@gmail.com> writes: > > > From: Chuanhua Han > > > > While swapping in a large folio, we need to free swaps related to the w= hole > > folio. To avoid frequently acquiring and releasing swap locks, it is be= tter > > to introduce an API for batched free. > > > > Signed-off-by: Chuanhua Han > > Co-developed-by: Barry Song > > Signed-off-by: Barry Song > > --- > > include/linux/swap.h | 5 +++++ > > mm/swapfile.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 56 insertions(+) > > > > diff --git a/include/linux/swap.h b/include/linux/swap.h > > index 11c53692f65f..b7a107e983b8 100644 > > --- a/include/linux/swap.h > > +++ b/include/linux/swap.h > > @@ -483,6 +483,7 @@ extern void swap_shmem_alloc(swp_entry_t); > > extern int swap_duplicate(swp_entry_t); > > extern int swapcache_prepare(swp_entry_t); > > extern void swap_free(swp_entry_t); > > +extern void swap_free_nr(swp_entry_t entry, int nr_pages); > > extern void swapcache_free_entries(swp_entry_t *entries, int n); > > extern void free_swap_and_cache_nr(swp_entry_t entry, int nr); > > int swap_type_of(dev_t device, sector_t offset); > > @@ -564,6 +565,10 @@ static inline void swap_free(swp_entry_t swp) > > { > > } > > > > +void swap_free_nr(swp_entry_t entry, int nr_pages) > > +{ > > +} > > + > > static inline void put_swap_folio(struct folio *folio, swp_entry_t swp= ) > > { > > } > > diff --git a/mm/swapfile.c b/mm/swapfile.c > > index 28642c188c93..f4c65aeb088d 100644 > > --- a/mm/swapfile.c > > +++ b/mm/swapfile.c > > @@ -1356,6 +1356,57 @@ void swap_free(swp_entry_t entry) > > __swap_entry_free(p, entry); > > } > > > > +/* > > + * Free up the maximum number of swap entries at once to limit the > > + * maximum kernel stack usage. > > + */ > > +#define SWAP_BATCH_NR (SWAPFILE_CLUSTER > 512 ? 512 : SWAPFILE_CLUSTER= ) > > + > > +/* > > + * Called after swapping in a large folio, > > IMHO, it's not good to document the caller in the function definition. > Because this will discourage function reusing. ok. right now there is only one user that is why it is added. but i agree we can actually remove this. > > > batched free swap entries > > + * for this large folio, entry should be for the first subpage and > > + * its offset is aligned with nr_pages > > Why do we need this? This is a fundamental requirement for the existing kernel, folio's swap offset is naturally aligned from the first moment add_to_swap to add swapcache's xa. so this comment is describing the existing fact. In the future, if we want to support swap-out folio to discontiguous and not-aligned offsets, we can't pass entry as the parameter, we should instead pass ptep or another different data struct which can connect multiple discontiguous swap offsets. I feel like we only need "for this large folio, entry should be for the first subpage" and drop "and its offset is aligned with nr_pages", the latter is not important to this context at all. > > > + */ > > +void swap_free_nr(swp_entry_t entry, int nr_pages) > > +{ > > + int i, j; > > + struct swap_cluster_info *ci; > > + struct swap_info_struct *p; > > + unsigned int type =3D swp_type(entry); > > + unsigned long offset =3D swp_offset(entry); > > + int batch_nr, remain_nr; > > + DECLARE_BITMAP(usage, SWAP_BATCH_NR) =3D { 0 }; > > + > > + /* all swap entries are within a cluster for mTHP */ > > + VM_BUG_ON(offset % SWAPFILE_CLUSTER + nr_pages > SWAPFILE_CLUSTER= ); > > + > > + if (nr_pages =3D=3D 1) { > > + swap_free(entry); > > + return; > > + } > > Is it possible to unify swap_free() and swap_free_nr() into one function > with acceptable performance? IIUC, the general rule in mTHP effort is > to avoid duplicate functions between mTHP and normal small folio. > Right? I don't see why. but we have lots of places calling swap_free(), we may have to change them all to call swap_free_nr(entry, 1); the other possible way is making swap_free() a wrapper of swap_free_nr() always using 1 as the argument. In both cases, we are changing the semantics of swap_free_nr() to partially freeing large folio cases and have to drop "entry should be for the first subpage" then. Right now, the semantics is * swap_free_nr() for an entire large folio; * swap_free() for one entry of either a large folio or a small folio > > > + > > + remain_nr =3D nr_pages; > > + p =3D _swap_info_get(entry); > > + if (p) { > > + for (i =3D 0; i < nr_pages; i +=3D batch_nr) { > > + batch_nr =3D min_t(int, SWAP_BATCH_NR, remain_nr)= ; > > + > > + ci =3D lock_cluster_or_swap_info(p, offset); > > + for (j =3D 0; j < batch_nr; j++) { > > + if (__swap_entry_free_locked(p, offset + = i * SWAP_BATCH_NR + j, 1)) > > + __bitmap_set(usage, j, 1); > > + } > > + unlock_cluster_or_swap_info(p, ci); > > + > > + for_each_clear_bit(j, usage, batch_nr) > > + free_swap_slot(swp_entry(type, offset + i= * SWAP_BATCH_NR + j)); > > + > > + bitmap_clear(usage, 0, SWAP_BATCH_NR); > > + remain_nr -=3D batch_nr; > > + } > > + } > > +} > > + > > /* > > * Called after dropping swapcache to decrease refcnt to swap entries. > > */ > > put_swap_folio() implements batching in another method. Do you think > that it's good to use the batching method in that function here? It > avoids to use bitmap operations and stack space. Chuanhua has strictly limited the maximum stack usage to several unsigned long, so this should be safe. on the other hand, i believe this implementation is more efficient, as put_swap_folio() might lock/ unlock much more often whenever __swap_entry_free_locked returns 0. > > -- > Best Regards, > Huang, Ying Thanks Barry