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 E7CE2C2BB3F for ; Wed, 15 Nov 2023 20:13:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5F46B80027; Wed, 15 Nov 2023 15:13:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5A44680022; Wed, 15 Nov 2023 15:13:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 46CF580027; Wed, 15 Nov 2023 15:13:19 -0500 (EST) 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 3194E80022 for ; Wed, 15 Nov 2023 15:13:19 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 09B97C0668 for ; Wed, 15 Nov 2023 20:13:19 +0000 (UTC) X-FDA: 81461288118.30.1077789 Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by imf28.hostedemail.com (Postfix) with ESMTP id 3F6CAC0011 for ; Wed, 15 Nov 2023 20:13:17 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="BLKKq7i/"; spf=pass (imf28.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.45 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1700079197; 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=l44iogGkycFObP7ldrLRFPDR8OL/5GpOZwnm3btFsOA=; b=wTOz5z/7Q6GH7fXKd3CABD8r48HW+oz+008mfHzWzrQ5uPo4GCQXe3J3I3SldP/5/P3yl+ HhuHtkhQhZVRbFD9eWRs5PTMUwLOh06HJTrPnIhBE6k6Egq7+AecinS8h1jw4QYy6bxfEa yXh8ydM2rHSa42MBj5eLon+8kloitnw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1700079197; a=rsa-sha256; cv=none; b=8LocDQjFqZLHHK+AOP06xhDb0pwVJZ0+E6pTWaOejuv6w1Q8dIUYjM7VJBEfjCBsu6AuP/ 62K3x25U3WkBcUt/O4Qxryu6NrYOtbSfxOewgfiLXTE778oWPXpYnwacIHiEqFF0hss329 KinMoRJ/avqi+Ghrt99EVdH8xClW9WE= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="BLKKq7i/"; spf=pass (imf28.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.45 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-9becde9ea7bso256422566b.0 for ; Wed, 15 Nov 2023 12:13:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1700079196; x=1700683996; 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=l44iogGkycFObP7ldrLRFPDR8OL/5GpOZwnm3btFsOA=; b=BLKKq7i/WmvY5/AiPUqKWqSwIaGBQfRpyM19BC4cM2bLSWN22bKyItob4hM5eNPElb GqDPzy/ukcPc+nfWQCug1DPvy4MELja0z+aLWPeFEvzGls7RhMmll0pMfS/HVwGpF4z2 YM0BC/R4LdDLh2ozDbf/y+pCbQb7EdL0Se9z5GtFbPqqnSYw5lQn7kKEjUgP+bxAZYr1 UPwCLLR0NIiJb3u/lqcQwh+gCDCVPw8Gsi+KNiMVbB3PNbBPEEfquHpyuANHc0d+jIKc 54g1s/jlmjQ8vM2p8QUaVyQ/GeHSPtKiNM6nGB0Sb4u66UxGUTO7Y5e03sXFqh6OuIFW v2/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700079196; x=1700683996; 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=l44iogGkycFObP7ldrLRFPDR8OL/5GpOZwnm3btFsOA=; b=SJoL94pZxsNdmU1razRStx7eU4vfJQglz7uqKqpkO7+K8zSrz9KKog56KuWducbz8x L4ABRfMeZaYLGMUFchtY75pnPzNun5tCk06RcdCtNypq6aLVWvJpgOtYG+Z+BS+aQ1an K4qCTV//yZUP6GHH/iYoWE44IY0Qyso5cjLmaatxOZAv4jdj45oh2x0mTF4Nwfc1Yp9R GIFnKG3velaZNwNYbNblBYO/byR0aZd+hqF7lgdT7LctgbEZ3NWztByXQoS+ImaH2dG4 4EgkrZdxRN7k5heUyiFWuDWOg8Qa02TuQ/WupYzMHyQeFZq1460NjfAaZPDz3evUzQaa /NBg== X-Gm-Message-State: AOJu0YyL4JIB9STUWP8dYoipZ/5hMcv8a5fGKfiCUltMwcWIkfC9PHHB VPAJnXlC9FdGzF3Q35mmJFVPXZQTKkS08LQY60c12Q== X-Google-Smtp-Source: AGHT+IFiuslHc5GoYPVTg55zJ6Hp9XqFP3VtNmMulCWUAvsjgsNcTOeJSsiXri+rcjv2D49T/xj/VqlOLdtl0G8jS3o= X-Received: by 2002:a17:906:3c04:b0:9ae:614e:4560 with SMTP id h4-20020a1709063c0400b009ae614e4560mr6301269ejg.29.1700079195532; Wed, 15 Nov 2023 12:13:15 -0800 (PST) MIME-Version: 1.0 References: <20231113130601.3350915-1-hezhongkun.hzk@bytedance.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 15 Nov 2023 12:12:36 -0800 Message-ID: Subject: Re: [External] Re: [PATCH] mm:zswap: fix zswap entry reclamation failure in two scenarios To: =?UTF-8?B?6LS65Lit5Z2k?= Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, nphamcs@gmail.com, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Ying Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 3F6CAC0011 X-Rspam-User: X-Stat-Signature: wx56akogj6rcqic514o9xx78a5cx4aek X-Rspamd-Server: rspam03 X-HE-Tag: 1700079196-848384 X-HE-Meta: U2FsdGVkX19zP7/FKaSef4a9RCV1KsDIHNZzrXTaFurrs/+3ASLMwQxcvebdiSl2SPc0vw1Qzm3tR6KmlvgIuRkaPo3yUR+EmDv9pAFX5W8M1t33IDD5EkgW+jwrY5IY9iP/36COmlQ7mm1IeBh2xwK9Ak+Kq64esMpNK/yYQqLdqeXj6PyreAEL8I2S9vVC4ZwRzWWHcS54+8gjnci95L2LWPYTENWPuHCm48+dqPyVZmZuw6Xn6Nmm5doF47r05dK0z3GOBY5M1sX+9va+IBbINCfBKn62o8t05yBx9LgNXpaOqqzAnYO9HlN2Y3E8nrR2EK5AuiVexzegkzOMIZPvg17LHP9xMJt/Ckbs2bx5w+5CIEVoX6mtxkePecXFrc55XdFT130Wt702SiO1HwLURGpr1FQ9iCot9sGFOXvXLMq46FlNRAKfRX+PA5uBzEHZsX++jbQZMqTvDAlmzXyOBv/W6nV7KGtl8QKxTXzuVkI4/gVV+s9Fj5txNze2O4PxGQMEn7/z6c0GW+lDlIiAKW4S/b0giETolZOxY1+2KwcIYmrWwoNiVS+So1mfwv5ioylu2OYwPPZ4uRiWYtPRD9LyAFN5vxTUoQfZD9md6+dB5JvyjsR0IBTIvUizC58vw8pIs7T4iHiRilWdeMVcihk6/DeYNdlqV+8LBBxdzYjhYT435JC+ixpNt/hdubItfcACstjUWIaXaguHNUT7Eal45MI5PH1XUw9n3aBJL+A77QQKwhpAtxm0DYtfNZH2cryY+UwW6YrEvYN7bLG8yGDXisLldocZ2dQHQkWutjKSbulJ+zDPzf340aUj15YAywCqbpYfYIlpqdFNQ+kHOQjOwVMC7D4QTfHtGO/hCQRyS4q21WkJcYR2mEJpmMl+Rw2fVQU9FQN8Y58EQZkV3dMpDDu7zrx2kG104lu9ip6sPOjx1oJzjgzVKPcWLMcqrdAniapF4rNCvmI 93cweuJI GW0+2QzTLP3CfqdJx8DHjwxlkPL3FJCFcK4Qx/niiL8x5aW9URboCkq+7Y14HLODEbGQYEwYhjlrOJ7dFnJbWVsCaqp1xKXn5PDReMx5Lz8NmZxjs2UT5WnZKJoQ+dvAUUTdZV6TE2YInpcCxqOlNgPnwD4S6RDnjV9kFQGG86N4TYPqUnUY29pKf+CbD0SFDqPfDB9HbEu7BK3DNEJOPvx9H625PT0Xa/BuEUerSUOQwLuTmMfwTzhc78ILjCNeveY4x/PRjsIUU7tOp+xxkLz7AbgfquqatG4h6 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, Nov 15, 2023 at 4:53=E2=80=AFAM =E8=B4=BA=E4=B8=AD=E5=9D=A4 wrote: > > > For case (1), I think a cleaner solution would be to move the > > zswap_invalidate() call from swap_range_free() (which is called after > > the cached slots are freed) to __swap_entry_free_locked() if the usage > > goes to 0. I actually think conceptually this makes not just for > > zswap_invalidate(), but also for the arch call, memcg uncharging, etc. > > Slots caching is a swapfile optimization that should be internal to > > swapfile code. Once a swap entry is freed (i.e. swap count is 0 AND > > not in the swap cache), all the hooks should be called (memcg, zswap, > > arch, ..) as the swap entry is effectively freed. The fact that > > swapfile code internally batches and caches slots should be > > transparent to other parts of MM. I am not sure if the calls can just > > be moved or if there are underlying assumptions in the implementation > > that would be broken, but it feels like the right thing to do. > > Good idea, This is indeed a clear solution. I'll try it in another > patch later. > > > > > For case (2), I don't think zswap can just decide to free the entry. > > > > In that case, the page is in the swap cache pointing to a swp_entry > > which has a corresponding zswap entry, and the page is clean because > > it is already in swap/zswap, so we don't need to write it out again > > unless it is redirtied. If zswap just drops the entry, and reclaim > > tries to reclaim the page in the swap cache, it will drop the page > > assuming that there is a copy in swap/zswap (because it is clean). Now > > we lost all copies of the page. > > > > Am I missing something? > > > > Ah, my bad. Missed the step of marking the page as dirty. > Please have a look, just like zswap_exclusive_loads_enabled, > set page dity so that it can be pageout again. > if (!page_was_allocated) { > if (!count) { > set_page_dirty(page); > ret =3D 0; > } else > ret =3D -EEXIST; > put_page(page); > } I think we may need to try to lock the folio. Otherwise we may race with reclaim reading the dirty bit before we set it. Taking a step back, this seems like we are going behind exclusive loads. We "should" keep the page in zswap as exclusive loads are disabled and the page is not yet invalidated from zswap (the swap entry is still in use). What you are trying to do here is sneakily drop the page from zswap as if we wrote it back, but we didn't. We just know that it was already loaded from zswap. We are essentially making the previous load exclusive retroactively. Is there a reason why exclusive loads cannot be enabled to achieve the same result in the (arguably) correct way? > Thanks for your feedback, Yosry.