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 75ABEC3DA7F for ; Thu, 1 Aug 2024 02:07:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EAB7E6B00A7; Wed, 31 Jul 2024 22:07:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E5BB86B00A8; Wed, 31 Jul 2024 22:07:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CFC286B00AB; Wed, 31 Jul 2024 22:07:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id AC5816B00A7 for ; Wed, 31 Jul 2024 22:07:22 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 3198DA082D for ; Thu, 1 Aug 2024 02:07:22 +0000 (UTC) X-FDA: 82402039524.01.5A94CAB Received: from mail-ua1-f44.google.com (mail-ua1-f44.google.com [209.85.222.44]) by imf05.hostedemail.com (Postfix) with ESMTP id 6067E100003 for ; Thu, 1 Aug 2024 02:07:19 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=fR3mnKtQ; spf=pass (imf05.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.44 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=1722477983; 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=yPgCfrE6s1SqmvRg/f6N9INj2p53v7nDyKO2JNF8QsM=; b=HD34X7B1Slj9ybb4XlnhbIWl4TGB4O3x3Hp3F4yAdtCLy7w0fUkn1x0jk2YDeG1cNOv4ep grAttqeRY5pHToc0lysgYBR2QhsaLLj5TPt8J1e/4oRD6fFXzwS4bLzbs6tyWLJ2YEZ2Ui YdXqawrWHkrg4ro5JgOtWI76JeS1mA8= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722477983; a=rsa-sha256; cv=none; b=XovMlNwV4pvaoEpCYJXV5B3uajTdeeT0G+Db8kI1TYYreyGz/sT1ovCFEIB9C2hJsvvcgo ffj7fDxzwdCOGqCirPRaiMKYp7TmSCIXmgeZPY6c7kdiGZjL3Z5VQahY8x3LN9Q++VRKu7 Uo9PPDIz5K+fsQ4KXzzKKdSuklYRI0k= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=fR3mnKtQ; spf=pass (imf05.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.44 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-ua1-f44.google.com with SMTP id a1e0cc1a2514c-822eb80f6e4so1740440241.2 for ; Wed, 31 Jul 2024 19:07:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722478038; x=1723082838; 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=yPgCfrE6s1SqmvRg/f6N9INj2p53v7nDyKO2JNF8QsM=; b=fR3mnKtQ1ls+XWoZMMuB8L4DwaahUT0Io4UZhXb+a5lJoew+4qpvZjTHlqyNCJ+bvX mFGqJ4IS2eoTL6GGMkOw/tGalNM0/cyCzeZChAjN3aBQkAcLXrXiaSzDv8VHs14xE2Ws KLESOdTMOPC445em2XS2FEBTR72GNXUvd0LlAKxp9bee8z8wiHSlPKv5NyOCKBIOYhmS pAbLdlezAYD/GWJ7Qud/YH8gtCAilshQZAFyoY98DNbWCydNo1Z7LMTGX9GOMReDEd/Q u5TAqbZGd1Dr++/tbWThiGwba3nDCxDsVxsm79MLvmHroLpdfWpgi0736Y3ldwu6hxzA slTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722478038; x=1723082838; 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=yPgCfrE6s1SqmvRg/f6N9INj2p53v7nDyKO2JNF8QsM=; b=BWi0I6f3AoM4w3HtO0A7tXYL7tLE9kG6UxMQfSMP5dGC/CiwiHkc/Is0dmqOgkZGSy zH5L6ZykrG89qPdq3Y5D5nJSvgNLR7rDgLK7pFCrUzsNctbN0cmSnfXzbK/ZcH2/Znw8 M4BnwCFNU/iWSQBKbJTYIdugn2VKP53vtyioJKrqr34z3IhdV7TzQWnkI3t3NRHzro/f R1YTrmNkPZp8PdkGjHEqyv6VHiKsSPoNvijQW9dILgc8XQ363JVNMajTIrqbcLfsRDxM fvwWaDROKVqOd3rWvmy7xBfY/u2i97Dq0E6kUWQZ4umBwJoyn4ovhvVd1VbKVf+BJcFl i6dA== X-Forwarded-Encrypted: i=1; AJvYcCVj/qcgdVhdbq/AvvBBLzTNC8LgsKDBZE+9OAe1WGyu5I0EHaVNIx5q12GUMCG+wZORmDMG3p8DCMRUK/2hfOzwm2g= X-Gm-Message-State: AOJu0Yy/YW9FlSehhVRoMTUUBUPBbrbALEutRwkyUSKA/73bJePIeBA9 looHRlX9gihgoRkWo2WZ/dxe+yauP0lQzwpBjW13R9vZO+w1MFUq2beAGAztltCTwRgFgNrlw0J xNlVjoeYfWubiwdBTL92tPRfQ8kg= X-Google-Smtp-Source: AGHT+IE15fWelSv58xepqg5vJWMkjAUBZiws655TLHnU0T2zfZn+o3havsm1pNxPRCjVgSzJVtKiU/Rn3UseSXu3czY= X-Received: by 2002:a05:6102:38d4:b0:493:b0c2:ad3c with SMTP id ada2fe7eead31-4945099bb12mr1786504137.22.1722478038194; Wed, 31 Jul 2024 19:07:18 -0700 (PDT) MIME-Version: 1.0 References: <20240730071339.107447-1-21cnbao@gmail.com> <20240730071339.107447-2-21cnbao@gmail.com> <874j86oubf.fsf@yhuang6-desk2.ccr.corp.intel.com> <87zfpynf2r.fsf@yhuang6-desk2.ccr.corp.intel.com> <871q39kq0e.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <871q39kq0e.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Barry Song <21cnbao@gmail.com> Date: Thu, 1 Aug 2024 10:07:05 +0800 Message-ID: Subject: Re: [PATCH 1/1] mm: swap: add nr argument in swapcache_prepare and swapcache_clear to support large folios To: "Huang, Ying" Cc: akpm@linux-foundation.org, linux-mm@kvack.org, baolin.wang@linux.alibaba.com, chrisl@kernel.org, david@redhat.com, hannes@cmpxchg.org, hughd@google.com, kaleshsingh@google.com, kasong@tencent.com, linux-kernel@vger.kernel.org, mhocko@suse.com, minchan@kernel.org, nphamcs@gmail.com, ryan.roberts@arm.com, senozhatsky@chromium.org, shakeel.butt@linux.dev, shy828301@gmail.com, surenb@google.com, v-songbaohua@oppo.com, willy@infradead.org, xiang@kernel.org, yosryahmed@google.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 6067E100003 X-Stat-Signature: rx9tk4di6acwqsxkbzoayhg3tw1dzkor X-HE-Tag: 1722478039-310035 X-HE-Meta: U2FsdGVkX1+sTXAgDg3MjcFcmxA7mJsMaPLA5mQwX8vVShLiWMcw5SiyKZqI65wIaF4CHfU6oFUbr9PhPWBQUUcA5TOtG4/zw0F0CBpgQ6oCtVMmgacx7nhN8S8+oeQ5dn6v+rKTn87v68J7Lkw6w2K0wcB53rQGnB6XAflzHFM6lZQXAP1UBQ0sk8lf0oeYf+xwAUBx6J6P0SiNRPuciKQLqJoAp7/tlf/5i/JpHdjmqJqKjQfBQWryvoiKZe6UVXV/uLpOmVfn/yOv25eG5JCNscTxMvrTbU7X5cukCZpfRvdX7AG+W9R/bv4iY0+eFBqCxosjxq1ePLyC/AszRz/xo9fR8TCIxHSCtlWZUitmGK1XYrAJSxoShQDfTmYzjL8OfqSDL5TiVimp7YwrCXVwhQxAgh/O7QQYMRQ+ZIuQKKDXW+aT1//6jIqe4YEiT56+Cgpq6YRkTDVDL+GwcVJuCvrohUEa+s2SfjbyDhFsMkZkkLjRkl/0shYlX8hzODOqUhQMRwmMKrHw71YjP0QR2gwB/eTmHyK75Ea0zOdgBMOdKITy0vKIpF2raqIbqNaHkDDw8r26fW132VmEr05qLtt+PFnRCqPhQ2ko2JFTwjY41eXNYaoKpqaLQCdz6J7XKHbyvb8n7b39xU++PuVacBFSLOC8t+s75hBwY9k/iVaTk4vghb4s5t9wADxHBEwpdg+Sl8ceEMiEfGh9TH4e0RNnRQ0mJJlxuwimMQiouG3UYryObKpjT2Ux96vIvstDX7gmlrR1gWlPxuvrpqGPtepvcdKNImEPvK2syo2+bgu5L52t5dE5i+wE+DLVW8WpSFhMWCYeUcjbpHm89X9NK0/vsAdD816rgjVq73poIhqH56o1+DS3VSZy2YkvTdd7p1Xio7jHw9bKCjPvCBOZgFvClG9ZK5BLZpxxgHGfuEsUTHsAdDJcz6PlpVouBYJr2uqhlnuDlbwPLvb qvIlqEgq EhMeCj+rX3j2aPLaOZoS3nQtXUes5GiuR6EftqKG+WbT0lGIIxSNYl/u1iuBSk3aRXk7sHLiVDaBmCNePVNSLUDScVGYO8vrO9E22zDLvrrnpLx2rMNZiSLbajUpuvXZrlpwrH+3nZ4cJDjVD+mIaMtVsgoek1aWYmhl5NgZRgLt/JVpArAlpBts93kCI96T1yPR9N0c4GqJxMtH8AMubX9XTSGgdXriUh0FQdrGd0dgV9qNVjNkIBpyMsv+gLC2Mj1aXpuAwkdSUtDfYXafEglhzbDWF7RCpU+yY2o6hQhAtmastxDK+aq8JwGPm6bTMb/dmsMuFMYbRywLu3axF9u+DH27f6jzdjvs/DuR4wZlylZ0+dPjj0J581tf3BN85sNEnoPevCHnb8cfML60kVKY1cGSnTk8j2qDtKDcbp92F/bxBNIjvKVh18/Ffc1tgvc9XGC3HYyl5dEqwxwEG5lKDvkjaUYooEVcDGZYq8HAR0187onW2SiQtBA== 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 9:13=E2=80=AFAM Huang, Ying w= rote: > > Barry Song <21cnbao@gmail.com> writes: > > > On Wed, Jul 31, 2024 at 4:28=E2=80=AFPM Huang, Ying wrote: > >> > >> Barry Song <21cnbao@gmail.com> writes: > >> > >> > On Wed, Jul 31, 2024 at 4:14=E2=80=AFPM Huang, Ying wrote: > >> >> > >> >> Hi, Barry, > >> >> > >> >> Barry Song <21cnbao@gmail.com> writes: > >> >> > >> >> > From: Barry Song > >> >> > > >> >> > Right now, swapcache_prepare() and swapcache_clear() supports one= entry > >> >> > only, to support large folios, we need to handle multiple swap en= tries. > >> >> > > >> >> > To optimize stack usage, we iterate twice in __swap_duplicate(): = the > >> >> > first time to verify that all entries are valid, and the second t= ime > >> >> > to apply the modifications to the entries. > >> >> > > >> >> > Currently, we're using nr=3D1 for the existing users. > >> >> > > >> >> > Reviewed-by: Baolin Wang > >> >> > Signed-off-by: Barry Song > >> >> > --- > >> >> > include/linux/swap.h | 4 +- > >> >> > mm/memory.c | 6 +-- > >> >> > mm/swap.h | 5 ++- > >> >> > mm/swap_state.c | 2 +- > >> >> > mm/swapfile.c | 101 +++++++++++++++++++++++++------------= ------ > >> >> > 5 files changed, 68 insertions(+), 50 deletions(-) > >> >> > > >> >> > diff --git a/include/linux/swap.h b/include/linux/swap.h > >> >> > index ba7ea95d1c57..5b920fa2315b 100644 > >> >> > --- a/include/linux/swap.h > >> >> > +++ b/include/linux/swap.h > >> >> > @@ -480,7 +480,7 @@ extern int get_swap_pages(int n, swp_entry_t = swp_entries[], int order); > >> >> > extern int add_swap_count_continuation(swp_entry_t, gfp_t); > >> >> > extern void swap_shmem_alloc(swp_entry_t); > >> >> > extern int swap_duplicate(swp_entry_t); > >> >> > -extern int swapcache_prepare(swp_entry_t); > >> >> > +extern int swapcache_prepare(swp_entry_t entry, int nr); > >> >> > 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); > >> >> > @@ -554,7 +554,7 @@ static inline int swap_duplicate(swp_entry_t = swp) > >> >> > return 0; > >> >> > } > >> >> > > >> >> > -static inline int swapcache_prepare(swp_entry_t swp) > >> >> > +static inline int swapcache_prepare(swp_entry_t swp, int nr) > >> >> > { > >> >> > return 0; > >> >> > } > >> >> > diff --git a/mm/memory.c b/mm/memory.c > >> >> > index 833d2cad6eb2..b8675617a5e3 100644 > >> >> > --- a/mm/memory.c > >> >> > +++ b/mm/memory.c > >> >> > @@ -4081,7 +4081,7 @@ vm_fault_t do_swap_page(struct vm_fault *vm= f) > >> >> > * reusing the same entry. It's undetectabl= e as > >> >> > * pte_same() returns true due to entry reu= se. > >> >> > */ > >> >> > - if (swapcache_prepare(entry)) { > >> >> > + if (swapcache_prepare(entry, 1)) { > >> >> > /* Relax a bit to prevent rapid rep= eated page faults */ > >> >> > schedule_timeout_uninterruptible(1)= ; > >> >> > goto out; > >> >> > @@ -4387,7 +4387,7 @@ vm_fault_t do_swap_page(struct vm_fault *vm= f) > >> >> > out: > >> >> > /* Clear the swap cache pin for direct swapin after PTL unl= ock */ > >> >> > if (need_clear_cache) > >> >> > - swapcache_clear(si, entry); > >> >> > + swapcache_clear(si, entry, 1); > >> >> > if (si) > >> >> > put_swap_device(si); > >> >> > return ret; > >> >> > @@ -4403,7 +4403,7 @@ vm_fault_t do_swap_page(struct vm_fault *vm= f) > >> >> > folio_put(swapcache); > >> >> > } > >> >> > if (need_clear_cache) > >> >> > - swapcache_clear(si, entry); > >> >> > + swapcache_clear(si, entry, 1); > >> >> > if (si) > >> >> > put_swap_device(si); > >> >> > return ret; > >> >> > diff --git a/mm/swap.h b/mm/swap.h > >> >> > index baa1fa946b34..7c6330561d84 100644 > >> >> > --- a/mm/swap.h > >> >> > +++ b/mm/swap.h > >> >> > @@ -59,7 +59,7 @@ void __delete_from_swap_cache(struct folio *fol= io, > >> >> > void delete_from_swap_cache(struct folio *folio); > >> >> > void clear_shadow_from_swap_cache(int type, unsigned long begin, > >> >> > unsigned long end); > >> >> > -void swapcache_clear(struct swap_info_struct *si, swp_entry_t en= try); > >> >> > +void swapcache_clear(struct swap_info_struct *si, swp_entry_t en= try, int nr); > >> >> > struct folio *swap_cache_get_folio(swp_entry_t entry, > >> >> > struct vm_area_struct *vma, unsigned long addr); > >> >> > struct folio *filemap_get_incore_folio(struct address_space *map= ping, > >> >> > @@ -120,7 +120,7 @@ static inline int swap_writepage(struct page = *p, struct writeback_control *wbc) > >> >> > return 0; > >> >> > } > >> >> > > >> >> > -static inline void swapcache_clear(struct swap_info_struct *si, = swp_entry_t entry) > >> >> > +static inline void swapcache_clear(struct swap_info_struct *si, = swp_entry_t entry, int nr) > >> >> > { > >> >> > } > >> >> > > >> >> > @@ -172,4 +172,5 @@ static inline unsigned int folio_swap_flags(s= truct folio *folio) > >> >> > return 0; > >> >> > } > >> >> > #endif /* CONFIG_SWAP */ > >> >> > + > >> >> > >> >> NITPICK: Is it necessary to add a blank line here? But I don't thi= nk a > >> >> new version is necessary if this is the only change needed. > >> > > >> > No need to add a blank line; it was probably a mistake I made in Vim= . > >> > > >> >> > >> >> > #endif /* _MM_SWAP_H */ > >> >> > diff --git a/mm/swap_state.c b/mm/swap_state.c > >> >> > index a1726e49a5eb..b06f2a054f5a 100644 > >> >> > --- a/mm/swap_state.c > >> >> > +++ b/mm/swap_state.c > >> >> > @@ -477,7 +477,7 @@ struct folio *__read_swap_cache_async(swp_ent= ry_t entry, gfp_t gfp_mask, > >> >> > /* > >> >> > * Swap entry may have been freed since our caller = observed it. > >> >> > */ > >> >> > - err =3D swapcache_prepare(entry); > >> >> > + err =3D swapcache_prepare(entry, 1); > >> >> > if (!err) > >> >> > break; > >> >> > > >> >> > diff --git a/mm/swapfile.c b/mm/swapfile.c > >> >> > index 5f73a8553371..757d38a86f56 100644 > >> >> > --- a/mm/swapfile.c > >> >> > +++ b/mm/swapfile.c > >> >> > @@ -3363,7 +3363,7 @@ void si_swapinfo(struct sysinfo *val) > >> >> > } > >> >> > > >> >> > /* > >> >> > - * Verify that a swap entry is valid and increment its swap map = count. > >> >> > + * Verify that nr swap entries are valid and increment their swa= p map counts. > >> >> > * > >> >> > * Returns error code in following case. > >> >> > * - success -> 0 > >> >> > @@ -3373,60 +3373,77 @@ void si_swapinfo(struct sysinfo *val) > >> >> > * - swap-cache reference is requested but the entry is not used= . -> ENOENT > >> >> > * - swap-mapped reference requested but needs continued swap co= unt. -> ENOMEM > >> >> > */ > >> >> > -static int __swap_duplicate(swp_entry_t entry, unsigned char usa= ge) > >> >> > +static int __swap_duplicate(swp_entry_t entry, unsigned char usa= ge, int nr) > >> >> > { > >> >> > struct swap_info_struct *p; > >> >> > struct swap_cluster_info *ci; > >> >> > unsigned long offset; > >> >> > unsigned char count; > >> >> > unsigned char has_cache; > >> >> > - int err; > >> >> > + int err, i; > >> >> > > >> >> > p =3D swp_swap_info(entry); > >> >> > > >> >> > offset =3D swp_offset(entry); > >> >> > + VM_WARN_ON(nr > SWAPFILE_CLUSTER - offset % SWAPFILE_CLUSTE= R); > >> >> > ci =3D lock_cluster_or_swap_info(p, offset); > >> >> > > >> >> > - count =3D p->swap_map[offset]; > >> >> > + err =3D 0; > >> >> > + for (i =3D 0; i < nr; i++) { > >> >> > + count =3D p->swap_map[offset + i]; > >> >> > > >> >> > - /* > >> >> > - * swapin_readahead() doesn't check if a swap entry is vali= d, so the > >> >> > - * swap entry could be SWAP_MAP_BAD. Check here with lock h= eld. > >> >> > - */ > >> >> > - if (unlikely(swap_count(count) =3D=3D SWAP_MAP_BAD)) { > >> >> > - err =3D -ENOENT; > >> >> > - goto unlock_out; > >> >> > - } > >> >> > + /* > >> >> > + * swapin_readahead() doesn't check if a swap entry= is valid, so the > >> >> > + * swap entry could be SWAP_MAP_BAD. Check here wit= h lock held. > >> >> > + */ > >> >> > + if (unlikely(swap_count(count) =3D=3D SWAP_MAP_BAD)= ) { > >> >> > + err =3D -ENOENT; > >> >> > + goto unlock_out; > >> >> > + } > >> >> > > >> >> > - has_cache =3D count & SWAP_HAS_CACHE; > >> >> > - count &=3D ~SWAP_HAS_CACHE; > >> >> > - err =3D 0; > >> >> > + has_cache =3D count & SWAP_HAS_CACHE; > >> >> > + count &=3D ~SWAP_HAS_CACHE; > >> >> > > >> >> > - if (usage =3D=3D SWAP_HAS_CACHE) { > >> >> > + if (usage =3D=3D SWAP_HAS_CACHE) { > >> >> > + /* set SWAP_HAS_CACHE if there is no cache = and entry is used */ > >> >> > + if (!has_cache && count) > >> >> > + continue; > >> >> > + else if (has_cache) /* someone = else added cache */ > >> >> > + err =3D -EEXIST; > >> >> > + else /* no users= remaining */ > >> >> > + err =3D -ENOENT; > >> >> > > >> >> > - /* set SWAP_HAS_CACHE if there is no cache and entr= y is used */ > >> >> > - if (!has_cache && count) > >> >> > - has_cache =3D SWAP_HAS_CACHE; > >> >> > - else if (has_cache) /* someone else add= ed cache */ > >> >> > - err =3D -EEXIST; > >> >> > - else /* no users remaini= ng */ > >> >> > - err =3D -ENOENT; > >> >> > + } else if (count || has_cache) { > >> >> > > >> >> > - } else if (count || has_cache) { > >> >> > + if ((count & ~COUNT_CONTINUED) < SWAP_MAP_M= AX) > >> >> > + continue; > >> >> > + else if ((count & ~COUNT_CONTINUED) > SWAP_= MAP_MAX) > >> >> > + err =3D -EINVAL; > >> >> > + else if (swap_count_continued(p, offset + i= , count)) > >> >> > + continue; > >> >> > >> >> IIUC, this will make the change to swap map directly instead of > >> >> verification. If the verification failed for some entry later, the > >> >> count will be wrong? Or I missed something? > >> > > >> > To avoid using a bitmap or a larger stack, we actually verify during > >> > the first iteration. > >> > This ensures that by the second iteration, we can safely commit the > >> > modification. > >> > > >> > I actually put some words in the changelog :-) > >> > > >> > To optimize stack usage, we iterate twice in __swap_duplicate(): the > >> > first time to verify that all entries are valid, and the second time > >> > to apply the modifications to the entries. > >> > >> Yes, I have seen it and I think that it is a good strategy. > >> > >> But, IIUC, swap_count_continued() will change the higher bits of the > >> swap_map instead of verifying. Or, my understanding is wrong? > >> > > > > Ying, your understanding is 100% correct. but the code also has nothing > > broken. we didn't extend swap_duplicate() to have argument nr, > > so all users which can set usage=3D1 will definitely have nr=3D1. > > > > int swap_duplicate(swp_entry_t entry) > > { > > int err =3D 0; > > > > while (!err && __swap_duplicate(entry, 1, 1) =3D=3D -ENOMEM) > > err =3D add_swap_count_continuation(entry, GFP_ATOMIC); > > return err; > > } > > I understand that we don't have requirements to support "usage =3D=3D 1 &= & > nr > 1" case for __swap_duplicate() at least for now. > > > Maybe I can add a VM_WARN_ON to warn those people who might > > want to extend swap_duplicate()? in that case, things could be quite > > tricky. > > > > --- a/mm/swapfile.c > > +++ b/mm/swapfile.c > > @@ -3386,6 +3386,7 @@ static int __swap_duplicate(swp_entry_t entry, > > unsigned char usage, int nr) > > > > offset =3D swp_offset(entry); > > VM_WARN_ON(nr > SWAPFILE_CLUSTER - offset % SWAPFILE_CLUSTER); > > + VM_WARN_ON(usage =3D=3D 1 && nr > 1); > > ci =3D lock_cluster_or_swap_info(p, offset); > > > > err =3D 0; > > Please add this. And, I think that we need to make it explicit in patch > description and comments to avoid potential confusing. cool. make sense to me. I will post something for Andrew to squash into. > > And, because it's hard to implement the verify and change strategy if > "usage =3D=3D 1". Can we only use that strategy for "usage =3D=3D > SWAP_HAS_CACHE"? I believe Baolin also needs the case for shmem. I don't feel a strong need to split two logics(1 and non-1) as the code will be quite ugly :-) > > -- > Best Regards, > Huang, Ying > > >> >> > >> >> > + else > >> >> > + err =3D -ENOMEM; > >> >> > + } else > >> >> > + err =3D -ENOENT; /* unused= swap entry */ > >> >> > > >> >> > - if ((count & ~COUNT_CONTINUED) < SWAP_MAP_MAX) > >> >> > + if (err) > >> >> > + goto unlock_out; > >> >> > + } > >> >> > + > >> >> > + for (i =3D 0; i < nr; i++) { > >> >> > + count =3D p->swap_map[offset + i]; > >> >> > + has_cache =3D count & SWAP_HAS_CACHE; > >> >> > + count &=3D ~SWAP_HAS_CACHE; > >> >> > + > >> >> > + if (usage =3D=3D SWAP_HAS_CACHE) > >> >> > + has_cache =3D SWAP_HAS_CACHE; > >> >> > + else if ((count & ~COUNT_CONTINUED) < SWAP_MAP_MAX) > >> >> > count +=3D usage; > >> >> > - else if ((count & ~COUNT_CONTINUED) > SWAP_MAP_MAX) > >> >> > - err =3D -EINVAL; > >> >> > - else if (swap_count_continued(p, offset, count)) > >> >> > - count =3D COUNT_CONTINUED; > >> >> > else > >> >> > - err =3D -ENOMEM; > >> >> > - } else > >> >> > - err =3D -ENOENT; /* unused swap en= try */ > >> >> > + count =3D COUNT_CONTINUED; > >> >> > > >> >> > - if (!err) > >> >> > - WRITE_ONCE(p->swap_map[offset], count | has_cache); > >> >> > + WRITE_ONCE(p->swap_map[offset + i], count | has_cac= he); > >> >> > + } > >> >> > > >> >> > unlock_out: > >> >> > unlock_cluster_or_swap_info(p, ci); > >> >> > @@ -3439,7 +3456,7 @@ static int __swap_duplicate(swp_entry_t ent= ry, unsigned char usage) > >> >> > */ > >> >> > void swap_shmem_alloc(swp_entry_t entry) > >> >> > { > >> >> > - __swap_duplicate(entry, SWAP_MAP_SHMEM); > >> >> > + __swap_duplicate(entry, SWAP_MAP_SHMEM, 1); > >> >> > } > >> >> > > >> >> > /* > >> >> > @@ -3453,29 +3470,29 @@ int swap_duplicate(swp_entry_t entry) > >> >> > { > >> >> > int err =3D 0; > >> >> > > >> >> > - while (!err && __swap_duplicate(entry, 1) =3D=3D -ENOMEM) > >> >> > + while (!err && __swap_duplicate(entry, 1, 1) =3D=3D -ENOMEM= ) > >> >> > err =3D add_swap_count_continuation(entry, GFP_ATOM= IC); > >> >> > return err; > >> >> > } > >> >> > > >> >> > /* > >> >> > - * @entry: swap entry for which we allocate swap cache. > >> >> > + * @entry: first swap entry from which we allocate nr swap cache= . > >> >> > * > >> >> > - * Called when allocating swap cache for existing swap entry, > >> >> > + * Called when allocating swap cache for existing swap entries, > >> >> > * This can return error codes. Returns 0 at success. > >> >> > * -EEXIST means there is a swap cache. > >> >> > * Note: return code is different from swap_duplicate(). > >> >> > */ > >> >> > -int swapcache_prepare(swp_entry_t entry) > >> >> > +int swapcache_prepare(swp_entry_t entry, int nr) > >> >> > { > >> >> > - return __swap_duplicate(entry, SWAP_HAS_CACHE); > >> >> > + return __swap_duplicate(entry, SWAP_HAS_CACHE, nr); > >> >> > } > >> >> > > >> >> > -void swapcache_clear(struct swap_info_struct *si, swp_entry_t en= try) > >> >> > +void swapcache_clear(struct swap_info_struct *si, swp_entry_t en= try, int nr) > >> >> > { > >> >> > unsigned long offset =3D swp_offset(entry); > >> >> > > >> >> > - cluster_swap_free_nr(si, offset, 1, SWAP_HAS_CACHE); > >> >> > + cluster_swap_free_nr(si, offset, nr, SWAP_HAS_CACHE); > >> >> > } > >> >> > > >> >> > struct swap_info_struct *swp_swap_info(swp_entry_t entry) > >> >> > >> >> -- > >> >> Best Regards, > >> >> Huang, Ying > >> > Thanks Barry