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 23CA0C35FF1 for ; Wed, 19 Mar 2025 05:18:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 75B5D280002; Wed, 19 Mar 2025 01:18:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 707F0280001; Wed, 19 Mar 2025 01:18:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 58419280002; Wed, 19 Mar 2025 01:18:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 3A072280001 for ; Wed, 19 Mar 2025 01:18:40 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 1933CC1870 for ; Wed, 19 Mar 2025 05:18:40 +0000 (UTC) X-FDA: 83237145600.21.B03D57B Received: from mail-lj1-f178.google.com (mail-lj1-f178.google.com [209.85.208.178]) by imf20.hostedemail.com (Postfix) with ESMTP id 2BA631C0004 for ; Wed, 19 Mar 2025 05:18:37 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="ckhId/FD"; spf=pass (imf20.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.178 as permitted sender) smtp.mailfrom=ryncsn@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=1742361518; 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=dcdHh+ZqJmIjlzhWFYZhMTNXLUfn7HW8EBdWnfguqsg=; b=sGMloEHbswWP7l+HQFPC6BuIbOxdBaNTzRtLKvPSV0KqUwrk0OS/wdGEs0Zw66M0801I7n uGWPw5BLsFo8d8NwkjkBTJKGWh0wmeVJxcuAteON/QqXnfgCblEZNcB0ZYPT7pA4ZAJpDh fSBrp1d0PiInaOCZjpF3G5CO1E0RsEE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742361518; a=rsa-sha256; cv=none; b=D8h5d0T2EnTCPq/wKiCyqbPchQd0ShPfdVOBO0QdMB7FPI9+ew5BJ1mn3glxOcWGWBkoTq v4oLwXKlPXTyDAp1jgOzETLp2QRzYzHimd5Fl2ASgMQ1VzmvMfe/a733FpIhv+4dRAb4Nz Xf50FcNsWTfj3YUOKxBTnnG6sVZEQRs= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="ckhId/FD"; spf=pass (imf20.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.178 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-lj1-f178.google.com with SMTP id 38308e7fff4ca-307325f2436so61912941fa.0 for ; Tue, 18 Mar 2025 22:18:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1742361516; x=1742966316; 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=dcdHh+ZqJmIjlzhWFYZhMTNXLUfn7HW8EBdWnfguqsg=; b=ckhId/FDHD91MlebGf/vspNZkuuFD9A23LtC+PaeU5m7YMjwuWKxdG5AqwrFMLfNmC GDdp9y8hXmMVN8l9kz1/ehQkhEnRHHe4UH70u1RkLlIZRUaWM+ADUeimiPDl6drpZksj 9+D2MJNWtbp+npU7YIZFSJsesWwfb46E5I59yQ4f43dIsURBFEAZELOGARHHTD/9VZdr lz7/EA3ZirZ8EN9q06dzxOx8pidnxCNl/uIar+POUaT81yJP50jeUB/kuZAl+pTFtQUL pIq2+1iKW8KvIDfqBpmsaHJ+zdctsecFyopM/h3aiWu4Ng2P35NOvVuqbb00dN4eaN+S BLjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1742361516; x=1742966316; 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=dcdHh+ZqJmIjlzhWFYZhMTNXLUfn7HW8EBdWnfguqsg=; b=pv5eOHfjAXDnZ0+dY5jFtSMewkjN9EUZrayI87I4om0oDQ0nQha6rBtQS45sLwfrOZ iG3KYkQL43daomxCLnwpMRycXAbRePx3AjAYZB/T3Y9J5kiXI33al5uCBCmCOZJxZeQt EcZHnZM0cHVvVL8UEVhh7WzK83YrGonB7hw3PmFf4ps0XINbHigPOPjESnxNYtH0uLsu WFlHWHhlaBAv5R9s+wBseqDJs/EeQHpJUmwN5+wqqgGMhhVfeK5j7tfpbUIkej+TMzet vWwLJ+p25FUgRUBppPDUyB5k96LolamkltExk1Y4IrN6WK/hhNwz5xHHaInr5rnzfI7Q 0iag== X-Forwarded-Encrypted: i=1; AJvYcCVmCn9pj2jKNvoTfYCJXgbvQLgo794C9kWB4PUaj5mPyD0gsgQ/760+V3RmgCih+lM5+8NEFbbPCw==@kvack.org X-Gm-Message-State: AOJu0Yy5PCVI6UF58smSqxYgGm1Rjf5PdvDmw2K6JC1O7BJ8wanq/uE1 7JDxvbJhMzEwGWtDIYDvLVfCB/xExteovQouyEZaFJSXogzboQhMiyACsl7uA4ao4AyQcjzjAgz K/EJlSvmsgLHoZbc240cW/X0ZBTI= X-Gm-Gg: ASbGncuR2l1zUVd0DYUSUdeL/jNFhgxcnGGTdWi4nrWkdsOwlGfaTV/4/alFYI22UEK /sSexM4UPGKv6V8rM5jSb2kJ0mpsMVopCJlk6Kse6MvT/n5t3ShuqJJj+ZNeDBMYMtL5IUvWQ1W ET3G8fzeeMFF2kxnHj47PrqfpEQg== X-Google-Smtp-Source: AGHT+IGdIm89bRDmxx8rmw4XgWSt4s1PDbjkyc5EgX7IBl3BSoG0My4/EXOwZwnzzSY+8PHrDGc0nXfmWmNolvRHjj8= X-Received: by 2002:a05:651c:19a4:b0:30c:467f:cda2 with SMTP id 38308e7fff4ca-30d6a3c54b5mr5020861fa.10.1742361515916; Tue, 18 Mar 2025 22:18:35 -0700 (PDT) MIME-Version: 1.0 References: <20250318150614.6415-1-shikemeng@huaweicloud.com> <20250318150614.6415-3-shikemeng@huaweicloud.com> In-Reply-To: <20250318150614.6415-3-shikemeng@huaweicloud.com> From: Kairui Song Date: Wed, 19 Mar 2025 13:18:19 +0800 X-Gm-Features: AQ5f1JoR1KJI_JDaN2aLJ0kalXS1sxMROQERP5mVGN9MaAMurzFJ0nz6ilMyUV4 Message-ID: Subject: Re: [PATCH v2 2/8] mm: swap: remove unneeded VM_BUG_ON(*map != SWAP_HAS_CACHE) in swap_entry_range_free() To: Kemeng Shi Cc: akpm@linux-foundation.org, tim.c.chen@linux.intel.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 2BA631C0004 X-Stat-Signature: o95hnhxj16mq8jo1he8dsgdpdyh9e4ba X-HE-Tag: 1742361517-108626 X-HE-Meta: U2FsdGVkX1+vU0QjHxLgyF4KFY7dzn4VQTQt7pPIS/Beux8Aj0OZprsGjFLVx7w15PeezYIO+HqsnPCy1KwvueIK4BWEdwdDKoLZpkADQtFARPYQU3stp9wYlHIHliN7QSLjp175tT+R5Dq9GcG3wLQXG52zqAPFwX4X8iIKonneiiPap+wXXaP6Ixi5KxP90UxFGeEPZuJjSXPDgSuFC09NVAvlXqsaF5YcuASkur5twBt14u7CpwUDh39eLeep/U1CWSUlrLt+a/Bh+KiK9SaBFg8EUFGYsSnQFHH/25qKLcoUhyv9QWky8tpkqqVnhB+am3BQrVQKkMETJRoTfCKRTKUzkhwPB8cF9y3jWleN0LQ2rqOaplUC6tZn0/anz/JQiNltmzAAqTfmCcFlC+8XnkQ9hZNKFdfEnBJgtKBu+XgR1qkYcmsBpWjq/JrkFDzY82MpblICfy4C0xLEQoBfMPGm0f7zl7Up4Ps5i63OKZUgR2roBDtBpn2HxJ+jkd824oAgicbwqOFslVC/9B/VgK8ADdBdOYH1ytgTS5x27LCuWP2aJqPmPB8zbqAShB3YcS1Rw//pf0bXsASEGEyDhwoJQUzpLaVvnSkDgJ9Ij9byc9+l6/Y8e/d/fUtHentwgyDCbIQQWhj9xyhUqRZSHV0vjzR2GkqRSkuwB2dMudMvUmMlbW9H16mez8AuG1PRXjP6DJEJcbh1rRwoFIbv8c/Afhe5mkWUOOFFODeBzQrzaW7ZLhOyomAbYoTl8Jvy1bjL/kK3/Q4SrpOHKml9L1NFLe+O+V/mbViuiD+BXOwswb3pIwkvll4KNzzuQKzOy6qx86j/nefM0EI3DhFdvqA1dsaopZJZVUiWBqVIYlXiw+qkoslpqrDgyPGRJwTJvEZuaS1+o6XmGZqahgfufkrzmCmuuXj3mQtTZQhA+qJCp7UB0EdMfU/ssnmtqf9BflNpfNjZ9eZ6k+9 PAFYjVo9 MZKZ3+jdPqHTjTn02462x1ntQH4FrGRybvEQ3oIGHgUy98KAYh4sM1m5vTMP1qJoS/lKJwuHQYQbkYQLR0wgmIQpR2GkM2SdJC19luVPlDu6lMH0dcKujOtE3eFK/7T+S+aMs0VWa8Tkgz0qARvBuVfKNpboUYT2bVuqKkpQvS+dSklYnWMnoQ1xaoEOO9aeF6fWcosIZsai05/RZmLO4XQXHdCfPnMURnUU+dhJBjn7CGo4xSPKlSdXL8TVyJQQd6xqQE16nKiGfETcg6dKeYT05zYTZyibbNJxF2PKFkWl2bJuFXOpaAn0iHjp83m/ru5DkjtuI4d4hDgw3RlyYu7R1YJai/0ZPouo1UNJEuBeOoaUhoMU2IsJcANUBtbv5g2ykv2nP5jq8fZ3fV5QFnHmH5rucLDyxDyhqRNX0ZdNo54inxS+YIGr1VCEjyWRdOV/zJgsukIpGgPg= 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 Tue, Mar 18, 2025 at 2:10=E2=80=AFPM Kemeng Shi wrote: > > As all callers of swap_entry_range_free() have already ensured slots to > be freed are marked as SWAP_HAS_CACHE while holding the cluster lock, > the BUG_ON check can be safely removed. After this, the function > swap_entry_range_free() could drop any kind of last flag, rename it to > swap_entries_free() and update it's comment accordingly. > > This is a preparation to use swap_entries_free() to drop last ref count > and SWAP_MAP_SHMEM flag. > > Signed-off-by: Kemeng Shi > Reviewed-by: Tim Chen > --- > mm/swapfile.c | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 5a775456e26c..0aa7ce82c013 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -52,9 +52,9 @@ > static bool swap_count_continued(struct swap_info_struct *, pgoff_t, > unsigned char); > static void free_swap_count_continuations(struct swap_info_struct *); > -static void swap_entry_range_free(struct swap_info_struct *si, > - struct swap_cluster_info *ci, > - swp_entry_t entry, unsigned int nr_page= s); > +static void swap_entries_free(struct swap_info_struct *si, > + struct swap_cluster_info *ci, > + swp_entry_t entry, unsigned int nr_pages); > static void swap_range_alloc(struct swap_info_struct *si, > unsigned int nr_entries); > static bool folio_swapcache_freeable(struct folio *folio); > @@ -1463,7 +1463,7 @@ static unsigned char swap_entry_put(struct swap_inf= o_struct *si, > ci =3D lock_cluster(si, offset); > usage =3D swap_entry_put_locked(si, offset, 1); > if (!usage) > - swap_entry_range_free(si, ci, swp_entry(si->type, offset)= , 1); > + swap_entries_free(si, ci, swp_entry(si->type, offset), 1)= ; > unlock_cluster(ci); > > return usage; > @@ -1493,7 +1493,7 @@ static bool swap_entries_put_nr(struct swap_info_st= ruct *si, > for (i =3D 0; i < nr; i++) > WRITE_ONCE(si->swap_map[offset + i], SWAP_HAS_CACHE); > if (!has_cache) > - swap_entry_range_free(si, ci, entry, nr); > + swap_entries_free(si, ci, entry, nr); > unlock_cluster(ci); > > return has_cache; > @@ -1512,12 +1512,12 @@ static bool swap_entries_put_nr(struct swap_info_= struct *si, > } > > /* > - * Drop the last HAS_CACHE flag of swap entries, caller have to > - * ensure all entries belong to the same cgroup. > + * Drop the last flag(1, SWAP_HAS_CACHE or SWAP_MAP_SHMEM) of swap entri= es, > + * caller have to ensure all entries belong to the same cgroup and clust= er. > */ > -static void swap_entry_range_free(struct swap_info_struct *si, > - struct swap_cluster_info *ci, > - swp_entry_t entry, unsigned int nr_page= s) > +static void swap_entries_free(struct swap_info_struct *si, > + struct swap_cluster_info *ci, > + swp_entry_t entry, unsigned int nr_pages) > { > unsigned long offset =3D swp_offset(entry); > unsigned char *map =3D si->swap_map + offset; > @@ -1530,7 +1530,6 @@ static void swap_entry_range_free(struct swap_info_= struct *si, > > ci->count -=3D nr_pages; > do { > - VM_BUG_ON(*map !=3D SWAP_HAS_CACHE); Hi Kemeng Instead of just dropping this check, maybe it's better to change it to something like VM_BUG_ON(!*map); to catch potential SWAP double free? I've found this check very helpful for debugging, especially for catching concurrency problems. Or more strictly: VM_BUG_ON(*map !=3D SWAP_HAS_CACHE && *map !=3D 1 && *map !=3D SWAP_MAP_SHMEM);, you may introduce a helper to check if a entry is the "last map" like this and use it somewhere else too. > *map =3D 0; > } while (++map < map_end); > > @@ -1553,7 +1552,7 @@ static void cluster_swap_free_nr(struct swap_info_s= truct *si, > ci =3D lock_cluster(si, offset); > do { > if (!swap_entry_put_locked(si, offset, usage)) > - swap_entry_range_free(si, ci, swp_entry(si->type,= offset), 1); > + swap_entries_free(si, ci, swp_entry(si->type, off= set), 1); > } while (++offset < end); > unlock_cluster(ci); > } > @@ -1596,11 +1595,11 @@ void put_swap_folio(struct folio *folio, swp_entr= y_t entry) > > ci =3D lock_cluster(si, offset); > if (swap_only_has_cache(si, offset, size)) > - swap_entry_range_free(si, ci, entry, size); > + swap_entries_free(si, ci, entry, size); > else { > for (int i =3D 0; i < size; i++, entry.val++) { > if (!swap_entry_put_locked(si, offset + i, SWAP_H= AS_CACHE)) > - swap_entry_range_free(si, ci, entry, 1); > + swap_entries_free(si, ci, entry, 1); > } > } > unlock_cluster(ci); > -- > 2.30.0 >