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 44335CEB2FD for ; Tue, 1 Oct 2024 06:01:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BEAAE280051; Tue, 1 Oct 2024 02:01:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B9A09280036; Tue, 1 Oct 2024 02:01:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A6144280051; Tue, 1 Oct 2024 02:01:00 -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 81758280036 for ; Tue, 1 Oct 2024 02:01:00 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 06A961A0E7C for ; Tue, 1 Oct 2024 06:01:00 +0000 (UTC) X-FDA: 82623985080.10.CC61377 Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by imf10.hostedemail.com (Postfix) with ESMTP id 24C7AC000F for ; Tue, 1 Oct 2024 06:00:57 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=eEvvra5u; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727762355; a=rsa-sha256; cv=none; b=PXYLxcSTALDsp9HFkAALHlSoxvu/+x+EX0/1mvhWUs3O1sPkUHr1G9g61J2zdarbJkAFlD kcU3LDWLhGZtjWFv9DEcf4XQvkOmCfGhFuW1L3yzhBZeExtBpSKiMj6744sEcx6vdnurKH Ai5aAjGXjbQ/3XY2Mioo3E+B7mONhvI= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=eEvvra5u; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727762355; 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=dwlI9QyLO248HwJAU/UvsoCTF/S8s8/o8f7kwB2enIk=; b=F8c0n9Q82zD47t8HM9Dsx/gRkhIO/aMBFxkaKPf0d4GjLZa0nZ2wjVXw73I+u8zElwF60X rqdcFjzuYUNqpNseGa7t9KD0P3G03rwwZcqIcRaP9k7n6PDmAvw9gYELmxONId8gsriPuy dIl9r/lq1JB/i/RR3PLU+YhzHtDK3D0= Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-a90349aa7e5so755088766b.0 for ; Mon, 30 Sep 2024 23:00:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1727762456; x=1728367256; 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=dwlI9QyLO248HwJAU/UvsoCTF/S8s8/o8f7kwB2enIk=; b=eEvvra5uY4qy4NRNf4xhPI+1JmtT2piOBuYmCBYUdcb2FeWiRVWuv6QP0aLymSBrIz cu0uaX3uRPqpvRfbUzWaMkkkpORZ8aHht/wszBLBbpANp/kbVNgs3NcUfHUr8DPsA88b C945WQsGScgtfdKkh6CBRMsGltvnIyFLQ+NP9NwC394GwryHvASzk67VtDZSsg7b/itu maVNvLNxclKBWXjkdoGQKkoAqA3NNcx3q+PmOQQFx4T1zDT56opHgcFhXA5IYoy00swi aUb1yvzcIUP79w5bW667a32B2R9UnVoIXHzOWQHl8SG8F1fCmPhuLfcxTsPACE3lKWgD Liqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727762456; x=1728367256; 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=dwlI9QyLO248HwJAU/UvsoCTF/S8s8/o8f7kwB2enIk=; b=qMehLI919aAz0KJTRkqIIdO4ZsWwmVYxO9Wf1l+cEJqZXnDVqRbuL/AbHN20+B/cq/ QECpfOQkI0jWuvrbw3RN0QjKsHb7HemuWV19s+1bPp3qkpdPiV3YOXlbGLDw01Gu2Ewn vOes1pCrMzBBB5p2m3TeyKTHuPvhbvN3iXnS7i5VgwvJddrxFvrH0cJyam6UFQ4N/Zfv LcBXtVNMi6XDtSjSX1jJ9G7pVELxiFdiBVNWLIcX6sc+3bp3q9lFLvmavWM2hjF3U59+ +7bLAF+wPN4eXJfQHF50exJljnZ4GbWSnd6N9u43AJwJ6tjyx1X3YbZr707oFJj41YZo BqpA== X-Forwarded-Encrypted: i=1; AJvYcCWAw9EYwbwdv43nnx7Oyx3z32qyQo2RV1QvgNlX5Q8kXunxgEI9HGBbztius0jv+nGHCak7qyQYMw==@kvack.org X-Gm-Message-State: AOJu0YxANT+ORZKTTjzPRR6UPdxN7Ot5i5EVNwCrizATU+szCvb7PVbO sK8CrF93bewdakVOpWV/gZ4Se+j/JCxVOFHYZETHEqETptRdbI3goH7OSIO4Fgt7I+hN1upHJGJ f8S821GcD/4CpPe4LXuZqsgykXPlWK1kEXbr/ X-Google-Smtp-Source: AGHT+IFQilgykeE3aCG/7kVPkxptduUjRYi481CZtd6X2JJnlsxz7SKkhuTXukUNkuu24B+Hiu4QvRWFc6VF6+4Vt+U= X-Received: by 2002:a17:906:4787:b0:a8a:af0c:dba9 with SMTP id a640c23a62f3a-a93c49197e0mr1751746566b.16.1727762455925; Mon, 30 Sep 2024 23:00:55 -0700 (PDT) MIME-Version: 1.0 References: <20240930221221.6981-1-kanchana.p.sridhar@intel.com> <20240930221221.6981-7-kanchana.p.sridhar@intel.com> In-Reply-To: From: Yosry Ahmed Date: Mon, 30 Sep 2024 23:00:17 -0700 Message-ID: Subject: Re: [PATCH v9 6/7] mm: zswap: Support large folios in zswap_store(). To: "Sridhar, Kanchana P" Cc: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "hannes@cmpxchg.org" , "nphamcs@gmail.com" , "chengming.zhou@linux.dev" , "usamaarif642@gmail.com" , "shakeel.butt@linux.dev" , "ryan.roberts@arm.com" , "Huang, Ying" , "21cnbao@gmail.com" <21cnbao@gmail.com>, "akpm@linux-foundation.org" , "willy@infradead.org" , "Zou, Nanhai" , "Feghali, Wajdi K" , "Gopal, Vinodh" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 24C7AC000F X-Stat-Signature: oirtkfaawnpfmppsg3w4ta8k5ans67xp X-Rspam-User: X-HE-Tag: 1727762457-552932 X-HE-Meta: U2FsdGVkX1/2Ot9p2/1F+L4Bk1ql34U/cfE8VlSHTMwWzXc0OxkQE29zdbbGqRXLtnQDyJwV2M7c0wifqmj4vXp+A5B3RvB7s8r1V/LBp8t0+pNyD5iZeQJZbLnKr0Otl3hLNk9zB8JrZ9jHdDgEZtbXVy7RQmtxE7Dt5uR3ihXj2WKMDwfZHxBM5RDPaoQFNWkjl6+Rs/4xwltJ1jPeD0fU4sWPXmHVUZYFk6gX/7wyHljXLUgy3yrMrTv0AOoJiNK+b30CPdULpkQ0EP0/j+UYb1HTBhCVdGraZfL3LKDJytgGyzdjU7Aijn/yH+fOUH09T+i/wQO9CFsvcdu/J5BiyS5B3KMH8oko7PVX5jbJ88gFM2hTxg3hmR4FXzhtiMzO8QPNUobdmRhgszcqmEjTS20GAShOuAnHXKlZWhjfJKVgBky4Dcjnm29oXtu5Hs6PL4f6UKr9JrwqxLFgIB72FWFteervVExW+pX7GJcYmJcszySP4eEG9Db0B3xcwxhQFsi0Mk/nOdcIaDCxB+Yp2QZv2blAsbDTOJhvhridk9QHc1S0oav+KJwH29Akv+UaQqPjKSzDs2r6fqoC/ebMFHf2Wdw1Hfe00lhXOn/HxenKdrKRasPMgUkCGXDnydnWBR9ej11JiU6ZRIpOTskBGDhPVJKfRbGkGr1LGEN4MZ0SyPVUsd5JoUgVBqlpzrpfQItgzRv7u9l/S1iHuLGjQvzruRSyFHLbWO0PZpDj11AccAb30geiqNk9pZKlqbl/SKwbC9wiGHnK05tJxBh8ZjcUPwzdIQIcBAe4kl17aQftJLv6kSjZOmS1dlH1KpMr04UZO+femGxfCSxm3uWNE2kDLIWBeM7rlK6SI5gqiK1y7/yofh9siVdOC7H1ka6l3gCPc3sOJ4NvbQj7Ca2br6PrgT8TEyfkOeXovHsbCnjRA0CP6YgwGqyORzPiqDm1sgDQ3ktfHbOPqmB Nw1SbQwF D1gmapAn6XQa/30CQwyxqqhx8N3acV5BuycvI8KwMW5VcJ8W/fZfHnz0zZBD7tT8Z6VgF7m/2rXcfUFKqIOfEwc5n1DzIM9ldtoGy0LkZzzznQqQF3hyomMS/pgJCchK/6K4uLtMNViNpLKjg/qIJjd02aiNI/IzDL/GFSClM0p/8FFtVVPX8+O6JIBryotJp9GFIq71eSPs84QyOqqJw2xvK1kIcwhE9IOIPT+DEHF1HjTgR67FBWASEdk5d78dSLhjZbNnKYyawL4nME1L6COzfGA== 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: [..] > > > store_failed: > > > zpool_free(entry->pool->zpool, entry->handle); > > > -put_pool: > > > - zswap_pool_put(entry->pool); > > > -freepage: > > > +put_pool_objcg: > > > + zswap_pool_put(pool); > > > + obj_cgroup_put(objcg); > > > > I think if we reorder the function we can drop these calls, make the > > comments positioned a bit better, and centralize the entry > > initializations. I am also not a fan of passing a semi-initialized > > entry to zswap_compress() to get the pool pointer. > > > > Does the following diff improve things or did I miss something? > > We shouldn=E2=80=99t be adding the entry to the xarray before initializin= g its pool > and objcg, right? Please let me know if I am misunderstanding what you're > proposing in the diff. It should be safe. We already initialize entry->lru after we insert the entry in the tree. See the comment above the call to zswap_lru_add(). Basically we are protected against concurrent stores/loads through the folio lock, and are protected against writeback because the entry is not on the LRU yet. > > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index b74c8de996468..eac1f846886a6 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -881,7 +881,8 @@ static int zswap_cpu_comp_dead(unsigned int cpu, > > struct hlist_node *node) > > return 0; > > } > > > > -static bool zswap_compress(struct page *page, struct zswap_entry *entr= y) > > +static bool zswap_compress(struct page *page, struct zswap_entry *entr= y, > > + struct zswap_pool *pool) > > { > > struct crypto_acomp_ctx *acomp_ctx; > > struct scatterlist input, output; > > @@ -893,7 +894,7 @@ static bool zswap_compress(struct page *page, > > struct zswap_entry *entry) > > gfp_t gfp; > > u8 *dst; > > > > - acomp_ctx =3D raw_cpu_ptr(entry->pool->acomp_ctx); > > + acomp_ctx =3D raw_cpu_ptr(pool->acomp_ctx); > > > > mutex_lock(&acomp_ctx->mutex); > > > > @@ -926,7 +927,7 @@ static bool zswap_compress(struct page *page, > > struct zswap_entry *entry) > > if (comp_ret) > > goto unlock; > > > > - zpool =3D entry->pool->zpool; > > + zpool =3D pool->zpool; > > gfp =3D __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM; > > if (zpool_malloc_support_movable(zpool)) > > gfp |=3D __GFP_HIGHMEM | __GFP_MOVABLE; > > @@ -1435,23 +1436,11 @@ static bool zswap_store_page(struct page > > *page, > > entry =3D zswap_entry_cache_alloc(GFP_KERNEL, > > folio_nid(page_folio(page))); > > if (!entry) { > > zswap_reject_kmemcache_fail++; > > - goto reject; > > + return false; > > } > > > > - /* zswap_store() already holds a ref on 'objcg' and 'pool' */ > > - if (objcg) > > - obj_cgroup_get(objcg); > > - zswap_pool_get(pool); > > - > > - /* if entry is successfully added, it keeps the reference */ > > - entry->pool =3D pool; > > - > > - if (!zswap_compress(page, entry)) > > - goto put_pool_objcg; > > - > > - entry->swpentry =3D page_swap_entry(page); > > - entry->objcg =3D objcg; > > - entry->referenced =3D true; > > + if (!zswap_compress(page, entry, pool)) > > + goto compress_failed; > > > > old =3D xa_store(tree, swp_offset(entry->swpentry), entry, GFP_= KERNEL); > > if (xa_is_err(old)) { > > @@ -1470,6 +1459,16 @@ static bool zswap_store_page(struct page *page, > > if (old) > > zswap_entry_free(old); > > > > + /* > > + * The entry is successfully compressed and stored in the tree,= there is > > + * no further possibility of failure. Grab refs to the pool and= objcg. > > + * These refs will be dropped by zswap_entry_free() when the en= try is > > + * removed from the tree. > > + */ > > + zswap_pool_get(pool); > > + if (objcg) > > + obj_cgroup_get(objcg); > > + > > /* > > * We finish initializing the entry while it's already in xarra= y. > > * This is safe because: > > @@ -1480,26 +1479,22 @@ static bool zswap_store_page(struct page > > *page, > > * The publishing order matters to prevent writeback from se= eing > > * an incoherent entry. > > */ I am referring to the comment here ^ > > + entry->pool =3D pool; > > + entry->swpentry =3D page_swap_entry(page); > > + entry->objcg =3D objcg; > > + entry->referenced =3D true; > > if (entry->length) { > > *compressed_bytes +=3D entry->length; > > INIT_LIST_HEAD(&entry->lru); > > zswap_lru_add(&zswap_list_lru, entry); > > }