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 C70A1CA0ED3 for ; Wed, 4 Sep 2024 07:17:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 443916B00A2; Wed, 4 Sep 2024 03:17:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F3318D0228; Wed, 4 Sep 2024 03:17:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2970A6B00A5; Wed, 4 Sep 2024 03:17:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 088238D0228 for ; Wed, 4 Sep 2024 03:17:32 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id A562880E24 for ; Wed, 4 Sep 2024 07:17:31 +0000 (UTC) X-FDA: 82526200302.05.D46581D Received: from mail-ua1-f51.google.com (mail-ua1-f51.google.com [209.85.222.51]) by imf26.hostedemail.com (Postfix) with ESMTP id E0F5C140013 for ; Wed, 4 Sep 2024 07:17:29 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=UJ7tZE1F; spf=pass (imf26.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.51 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=1725434154; 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=m53gn9oU2Gmb4XGafm+y8O/WGo92nnWgTKjnY2RkI3c=; b=d4uZ7DJCvtUwmK9sqbnUjZ75ScKRRHxCImPNfU758yuQLOtPgEvNQRCjzyX3hKXLqeMCJl UcS5v0Vy+neTYBGulbgXRtEDxKb65Qtj7LfxHVrJ+kOsSkwMjyd3g/FwhOAmuKva6gDEWe 8zmoa5njOUUsM/W13X7wCwXBaBPBtOU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725434154; a=rsa-sha256; cv=none; b=ogbOXplvCOoBa4yXORo472twhkPbCkLVCtgnGvRbXIr6NjuuJrLeXWw+jFJhM7XdjVlv8k nzm+8XGbg4DliyBGtzfCLFJXGKb/+6VqF/DECliYboc9e1kTjr1rIww9oJl9NtukY06AQU OW3UgNi6qvNcvThK+ObgvjPs7pCuxII= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=UJ7tZE1F; spf=pass (imf26.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.51 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-ua1-f51.google.com with SMTP id a1e0cc1a2514c-846bcb525f7so1334060241.3 for ; Wed, 04 Sep 2024 00:17:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725434249; x=1726039049; 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=m53gn9oU2Gmb4XGafm+y8O/WGo92nnWgTKjnY2RkI3c=; b=UJ7tZE1FqI1wcSnRiUOzXudx8yO1200wYhoUhGKgryzhnkgmzyPCMGfZR0BrVrz+HS JwsKjyPwmACW2WOqL6R7mY4bfT5JMb8keJyn4bogg4zfucrocJ74l7ouO5o86p+tMUft zy1Q8QX20txhO8ZbRm99Rq+K1XbTNZunoYQP0M3UpFEHJVuhkm7IE+BEaCR9KPuilbPn nmqIbA9QNUT1zMspxVgW2y48LUDEl8KcPBq2tRRkbRhy0j+X+6sVbY33BzBDHMO3Luor Z5HIWYU+IDKy/R1KR7CtkazNjedf79bwxBwI1oe4MrZ6j5kQGuNN20ELin/6LiHqrFEU LkiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725434249; x=1726039049; 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=m53gn9oU2Gmb4XGafm+y8O/WGo92nnWgTKjnY2RkI3c=; b=LqwhMTxOw43z6cSAcs8H1T0O+JXZet7+MI46+mnySPCKlZWvc8FjLRJFPACIvdNj62 fPNSOkr+PbL7SO//gC+QfKpUzHdaEiWapcfl25ud+swmGZsBTc2jZ42e/uWTBZPGDK21 qA0bx7LUQjOgpEw0PoNSDqvT/tVdCIaBgPaXsP3DUjarj5lbCCR5Ip6LfYy1p+zErtCr 94AlnjXfEmryl5obw0Zx9wqRdz1OWhxdkheFbdnr5cQKQXvdeh+THF1Cn8ZkTqAKC4ju P6Sk+XxpwJBraQgt6i5VndPKAyuVf/GoeiUGHD8bTajPI1Y1f+Ei43natQPurlvXIpi+ kK0g== X-Forwarded-Encrypted: i=1; AJvYcCWVJc5FdEq9ihz2JHZqe4y8fXkL05bsC+f2U6AiLJFmI27jMd/utAV9cJNpxdTRvbnvlthgy6dGoA==@kvack.org X-Gm-Message-State: AOJu0YzzsSKzFG92C8xspMk1y2wjWJ6dvgyn78a4bgjj727cePAehjlk U0uVbxzU4Qmh/AbLYjZFEB5SMCj3kvU1WW1QjosuO6R2t7Kf5gpvNA8XxAxUX2d9MCK7M1VsCqt tpriHaAN2gkTqPCVNUvDkqQd2EO0= X-Google-Smtp-Source: AGHT+IGT/af0TrsVSNkBeCAbwaAQwZphy+O2CC3+7hz7TlUrzzYtI0q9KoADO0Sdp+wRToUZlG4ZVG0NgrwEUcRexC4= X-Received: by 2002:a05:6122:3695:b0:4f5:2a41:d10e with SMTP id 71dfb90a1353d-5009dc39752mr14108187e0c.10.1725434248701; Wed, 04 Sep 2024 00:17:28 -0700 (PDT) MIME-Version: 1.0 References: <20240612124750.2220726-2-usamaarif642@gmail.com> <20240904055522.2376-1-21cnbao@gmail.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Wed, 4 Sep 2024 19:17:17 +1200 Message-ID: Subject: Re: [PATCH v4 1/2] mm: store zero pages to be swapped out in a bitmap To: Yosry Ahmed Cc: usamaarif642@gmail.com, akpm@linux-foundation.org, chengming.zhou@linux.dev, david@redhat.com, hannes@cmpxchg.org, hughd@google.com, kernel-team@meta.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, nphamcs@gmail.com, shakeel.butt@linux.dev, willy@infradead.org, ying.huang@intel.com, hanchuanhua@oppo.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: E0F5C140013 X-Stat-Signature: i4tz8xmbmdze9j7a5dipammq4hkbasd8 X-HE-Tag: 1725434249-922986 X-HE-Meta: U2FsdGVkX18YlQ4n2Ej4UywEj2+It4jdYsT6D5l+J6FZyOntNOh8xV34K8CzBWjM10VOv89IcIlEfFk0nL/ruiRdYqfJb5Hy70jINo/QDdevqcGBnvBZ9qSwr3+mhsxN0oW2JK9/jM5HpOIefu0zrW0sXF67+jmQ+6IREZCSC0YAX/7cAWYnn85vSXaqk2X4WM52aJC4E0Qcrd/LdUPDmL50g70ul9mjc+jUjqsurhRmMI0Quwgy8tAl3FaWq70BfuULwmMv4c610jwCmGE2qL5XigsprrXqbdt4bYAit0QppcqJl46/LIEwAY9Qv7Ll9f0+b1y1mgM2oThlPgWxLuLPQxccGen3lf7xakw2mYV3TIAADcTP8Yiy4c/Nd3Hp+51jycisud8zFTO9M9EviR/pTZd5sUDI376CoUvIpqvNvnNa8kcRE6IAi3Jn2kLtroxA1nu0H5iADuwB7qIKO5WhWhASo6DoK4RvxwfXWeWuyFlQyp97txV5XBO8tUscpH0GS2/FUsLgUAfROjdc4YwoyjzW7OG5Ht86XqgMDLaB1e88bkEz3ui4QQNbBy6mmMZBM8t/V4an+ODcVJkVtVdN20aG9+GagOuRLAlUH+Qe2xdaJZzlD1T8A0Q5ofpgO3vz/eSKLhBYwVBVIwyPIXz40BVcbAjgZXpGLcmiRC7NTF1M2S1YEYVOC5OxieV+MZhcoT8o3JloBV4ndJzhdlt7VLDOisXAoYgyX6bOGjjvvsb9Fmk0pIvd/99BvAgxdJuqARbNNwMTcY7Sw/hC25fVHKeX+nMx/N3Jsbs7afe3RvOH7tAG1vhKg8GOEwfn3OfH/l2SEfx0nqdM/nMgUoGTxfKtjMry1ehiQNC62SHTgawv0SHnd4hWIt7c+7PdikHFpn8uh2zBJfUmGPnny3Dvm/p2HEAfSQ5hB9zR9tzWVBYJf6F5HDmAc8WiJj+OMP1AgrRL2DLkIQXaf2r Cv2+DAwe +AYNeoGkZxdpU7j9LBCijZmtN6oTVaHrWaI9QlEB64e3XnqIFZFxdZ01RNaixO4LBbyuxPd0DGVHnVN3hylmuEUb+QiTz6KsoAMmehUq7YkZ1AsUp8kEVY5t8htwm4U/P0CPjgbvLmXVODnzJf0D4Xvz9On09Z6Jn+gk+DYzXA3Lut8VyY3v6DssDGjiB2V0AkZLR2yeSptnWl0EU1B5WaYg4A+B6IyqTGqRqdL/Z00W4CRFIA+a4poSOZDjxwtS7Q2j6kYWffnHZpl55nFg3xSt8nyM3/SJPK8akAFTa905JJ8f0VQFrDBL3en1P3J1LwYq+j66JrJ/yjnKZbMhcYwj/EdVjhF4sfHlTgtARZJDLaZDy54pLRWO7OuHbrONZCIyW 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 Wed, Sep 4, 2024 at 7:12=E2=80=AFPM Yosry Ahmed = wrote: > > [..] > > > @@ -426,6 +515,26 @@ static void sio_read_complete(struct kiocb *iocb= , long ret) > > > mempool_free(sio, sio_pool); > > > } > > > > > > +static bool swap_read_folio_zeromap(struct folio *folio) > > > +{ > > > + unsigned int idx =3D swap_zeromap_folio_test(folio); > > > + > > > + if (idx =3D=3D 0) > > > + return false; > > > + > > > + /* > > > + * Swapping in a large folio that is partially in the zeromap= is not > > > + * currently handled. Return true without marking the folio u= ptodate so > > > + * that an IO error is emitted (e.g. do_swap_page() will sigb= us). > > > + */ > > > + if (WARN_ON_ONCE(idx < folio_nr_pages(folio))) > > > + return true; > > > > Hi Usama, Yosry, > > > > I feel the warning is wrong as we could have the case where idx=3D=3D0 > > is not zeromap but idx=3D1 is zeromap. idx =3D=3D 0 doesn't necessarily > > mean we should return false. > > Good catch. Yeah if idx =3D=3D 0 is not in the zeromap but other indices > are we will mistakenly read the entire folio from swap. > > > > > What about the below change which both fixes the warning and unblocks > > large folios swap-in? > > But I don't see how that unblocks the large folios swap-in work? We > still need to actually handle the case where a large folio being > swapped in is partially in the zeromap. Right now we warn and unlock > the folio without calling folio_mark_uptodate(), which emits an IO > error. I placed this in mm/swap.h so that during swap-in, it can filter out any la= rge folios where swap_zeromap_entries_count() is greater than 0 and less than nr. I believe this case would be quite rare, as it can only occur when some sma= ll folios that are swapped out happen to have contiguous and aligned swap slots. > > > > > diff --git a/mm/page_io.c b/mm/page_io.c > > index 4bc77d1c6bfa..7d7ff7064e2b 100644 > > --- a/mm/page_io.c > > +++ b/mm/page_io.c > > @@ -226,26 +226,6 @@ static void swap_zeromap_folio_clear(struct folio = *folio) > > } > > } > > > > -/* > > - * Return the index of the first subpage which is not zero-filled > > - * according to swap_info_struct->zeromap. > > - * If all pages are zero-filled according to zeromap, it will return > > - * folio_nr_pages(folio). > > - */ > > -static unsigned int swap_zeromap_folio_test(struct folio *folio) > > -{ > > - struct swap_info_struct *sis =3D swp_swap_info(folio->swap); > > - swp_entry_t entry; > > - unsigned int i; > > - > > - for (i =3D 0; i < folio_nr_pages(folio); i++) { > > - entry =3D page_swap_entry(folio_page(folio, i)); > > - if (!test_bit(swp_offset(entry), sis->zeromap)) > > - return i; > > - } > > - return i; > > -} > > - > > /* > > * We may have stale swap cache pages in memory: notice > > * them here and get rid of the unnecessary final write. > > @@ -524,9 +504,10 @@ static void sio_read_complete(struct kiocb *iocb, = long ret) > > > > static bool swap_read_folio_zeromap(struct folio *folio) > > { > > - unsigned int idx =3D swap_zeromap_folio_test(folio); > > + unsigned int nr_pages =3D folio_nr_pages(folio); > > + unsigned int nr =3D swap_zeromap_entries_count(folio->swap, nr_= pages); > > > > - if (idx =3D=3D 0) > > + if (nr =3D=3D 0) > > return false; > > > > /* > > @@ -534,7 +515,7 @@ static bool swap_read_folio_zeromap(struct folio *f= olio) > > * currently handled. Return true without marking the folio upt= odate so > > * that an IO error is emitted (e.g. do_swap_page() will sigbus= ). > > */ > > - if (WARN_ON_ONCE(idx < folio_nr_pages(folio))) > > + if (WARN_ON_ONCE(nr < nr_pages)) > > return true; > > > > folio_zero_range(folio, 0, folio_size(folio)); > > diff --git a/mm/swap.h b/mm/swap.h > > index f8711ff82f84..2d59e9d89e95 100644 > > --- a/mm/swap.h > > +++ b/mm/swap.h > > @@ -80,6 +80,32 @@ static inline unsigned int folio_swap_flags(struct f= olio *folio) > > { > > return swp_swap_info(folio->swap)->flags; > > } > > + > > +/* > > + * Return the number of entries which are zero-filled according to > > + * swap_info_struct->zeromap. It isn't precise if the return value > > + * is larger than 0 and smaller than nr to avoid extra iterations, > > + * In this case, it means entries haven't consistent zeromap. > > + */ > > +static inline unsigned int swap_zeromap_entries_count(swp_entry_t entr= y, int nr) > > +{ > > + struct swap_info_struct *sis =3D swp_swap_info(entry); > > + unsigned long offset =3D swp_offset(entry); > > + unsigned int type =3D swp_type(entry); > > + unsigned int n =3D 0; > > + > > + for (int i =3D 0; i < nr; i++) { > > + entry =3D swp_entry(type, offset + i); > > + if (test_bit(offset + i, sis->zeromap)) { > > + if (i !=3D n) > > + return i; > > + n++; > > + } > > + } > > + > > + return n; > > +} > > + > > #else /* CONFIG_SWAP */ > > struct swap_iocb; > > static inline void swap_read_folio(struct folio *folio, struct swap_io= cb **plug) > > @@ -171,6 +197,11 @@ static inline unsigned int folio_swap_flags(struct= folio *folio) > > { > > return 0; > > } > > + > > +static inline unsigned int swap_zeromap_entries_count(swp_entry_t entr= y, int nr) > > +{ > > + return 0; > > +} > > #endif /* CONFIG_SWAP */ > > > > #endif /* _MM_SWAP_H */ > > > > > + > > > + folio_zero_fill(folio); > > > + folio_mark_uptodate(folio); > > > + return true; > > > +} > > > + > > > static void swap_read_folio_fs(struct folio *folio, struct swap_iocb= **plug) > > > { > > > struct swap_info_struct *sis =3D swp_swap_info(folio->swap); > > > @@ -515,8 +624,9 @@ void swap_read_folio(struct folio *folio, bool sy= nchronous, > > > psi_memstall_enter(&pflags); > > > } > > > delayacct_swapin_start(); > > > - > > > - if (zswap_load(folio)) { > > > + if (swap_read_folio_zeromap(folio)) { > > > + folio_unlock(folio); > > > + } else if (zswap_load(folio)) { > > > folio_mark_uptodate(folio); > > > folio_unlock(folio); > > > } else if (data_race(sis->flags & SWP_FS_OPS)) { > > > diff --git a/mm/swapfile.c b/mm/swapfile.c > > > index f1e559e216bd..48d8dca0b94b 100644 > > > --- a/mm/swapfile.c > > > +++ b/mm/swapfile.c > > > @@ -453,6 +453,8 @@ static unsigned int cluster_list_del_first(struct= swap_cluster_list *list, > > > static void swap_cluster_schedule_discard(struct swap_info_struct *s= i, > > > unsigned int idx) > > > { > > > + unsigned int i; > > > + > > > /* > > > * If scan_swap_map_slots() can't find a free cluster, it wil= l check > > > * si->swap_map directly. To make sure the discarding cluster= isn't > > > @@ -461,6 +463,13 @@ static void swap_cluster_schedule_discard(struct= swap_info_struct *si, > > > */ > > > memset(si->swap_map + idx * SWAPFILE_CLUSTER, > > > SWAP_MAP_BAD, SWAPFILE_CLUSTER); > > > + /* > > > + * zeromap can see updates from concurrent swap_writepage() a= nd swap_read_folio() > > > + * call on other slots, hence use atomic clear_bit for zeroma= p instead of the > > > + * non-atomic bitmap_clear. > > > + */ > > > + for (i =3D 0; i < SWAPFILE_CLUSTER; i++) > > > + clear_bit(idx * SWAPFILE_CLUSTER + i, si->zeromap); > > > > > > cluster_list_add_tail(&si->discard_clusters, si->cluster_info= , idx); > > > > > > @@ -482,7 +491,7 @@ static void __free_cluster(struct swap_info_struc= t *si, unsigned long idx) > > > static void swap_do_scheduled_discard(struct swap_info_struct *si) > > > { > > > struct swap_cluster_info *info, *ci; > > > - unsigned int idx; > > > + unsigned int idx, i; > > > > > > info =3D si->cluster_info; > > > > > > @@ -498,6 +507,8 @@ static void swap_do_scheduled_discard(struct swap= _info_struct *si) > > > __free_cluster(si, idx); > > > memset(si->swap_map + idx * SWAPFILE_CLUSTER, > > > 0, SWAPFILE_CLUSTER); > > > + for (i =3D 0; i < SWAPFILE_CLUSTER; i++) > > > + clear_bit(idx * SWAPFILE_CLUSTER + i, si->zer= omap); > > > unlock_cluster(ci); > > > } > > > } > > > @@ -1059,9 +1070,12 @@ static void swap_free_cluster(struct swap_info= _struct *si, unsigned long idx) > > > { > > > unsigned long offset =3D idx * SWAPFILE_CLUSTER; > > > struct swap_cluster_info *ci; > > > + unsigned int i; > > > > > > ci =3D lock_cluster(si, offset); > > > memset(si->swap_map + offset, 0, SWAPFILE_CLUSTER); > > > + for (i =3D 0; i < SWAPFILE_CLUSTER; i++) > > > + clear_bit(offset + i, si->zeromap); > > > cluster_set_count_flag(ci, 0, 0); > > > free_cluster(si, idx); > > > unlock_cluster(ci); > > > @@ -1336,6 +1350,7 @@ static void swap_entry_free(struct swap_info_st= ruct *p, swp_entry_t entry) > > > count =3D p->swap_map[offset]; > > > VM_BUG_ON(count !=3D SWAP_HAS_CACHE); > > > p->swap_map[offset] =3D 0; > > > + clear_bit(offset, p->zeromap); > > > dec_cluster_info_page(p, p->cluster_info, offset); > > > unlock_cluster(ci); > > > > > > @@ -2597,6 +2612,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, s= pecialfile) > > > free_percpu(p->cluster_next_cpu); > > > p->cluster_next_cpu =3D NULL; > > > vfree(swap_map); > > > + bitmap_free(p->zeromap); > > > kvfree(cluster_info); > > > /* Destroy swap account information */ > > > swap_cgroup_swapoff(p->type); > > > @@ -3123,6 +3139,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, s= pecialfile, int, swap_flags) > > > goto bad_swap_unlock_inode; > > > } > > > > > > + p->zeromap =3D bitmap_zalloc(maxpages, GFP_KERNEL); > > > + if (!p->zeromap) { > > > + error =3D -ENOMEM; > > > + goto bad_swap_unlock_inode; > > > + } > > > + > > > if (p->bdev && bdev_stable_writes(p->bdev)) > > > p->flags |=3D SWP_STABLE_WRITES; > > > > > > -- > > > 2.43.0 > > > > > > Thanks barry