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 8229FC3DA7F for ; Mon, 5 Aug 2024 02:06:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EEC856B007B; Sun, 4 Aug 2024 22:06:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E9D0C6B0082; Sun, 4 Aug 2024 22:06:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D64006B0085; Sun, 4 Aug 2024 22:06:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id B75BD6B007B for ; Sun, 4 Aug 2024 22:06:47 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 69BD7C1C48 for ; Mon, 5 Aug 2024 02:06:47 +0000 (UTC) X-FDA: 82416553254.26.A7FAF3F Received: from out-184.mta1.migadu.com (out-184.mta1.migadu.com [95.215.58.184]) by imf05.hostedemail.com (Postfix) with ESMTP id 4FE53100017 for ; Mon, 5 Aug 2024 02:06:45 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=fbNEId2C; spf=pass (imf05.hostedemail.com: domain of chengming.zhou@linux.dev designates 95.215.58.184 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722823575; a=rsa-sha256; cv=none; b=l8i9zHojYgI1D/w68/VcJHBN+/EaQXKFVz1W+4NSZPCF/8E8oOkrJlcO9gXzIrtDwzzGXc icyTWVexJ47eCbhab/EMZ7vJiQIy8Cduvchcby/OtaVLq4O2Jbm19uwCerePGLMDVtfxAa COixLLzoyvdgfNmRCeqGzoOdUfAgq1I= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=fbNEId2C; spf=pass (imf05.hostedemail.com: domain of chengming.zhou@linux.dev designates 95.215.58.184 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722823575; 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=l44H4QWdGe86dD8zQeD5JdGzKItvgmz16BSy3TEXbJU=; b=Kl+6Us0+qvkMMKE0Z8d+XVJOhfK5v9a7OQgiUZrBZPhlgUgQUiH6FLteIXEhhs1hfIJnzM UevoueJ5dsHV/sSA1wCS8hDyxTqL3QwYjPyyp5FsSinrXuIgX5g6KNDsLP/kYX7h0NifWX XREmRlUU0sJMDLjjfjMws1PGLf0gnWs= Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1722823602; h=from:from: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; bh=l44H4QWdGe86dD8zQeD5JdGzKItvgmz16BSy3TEXbJU=; b=fbNEId2C4gLy6nbRlUcLD4xGv/QOhM21IgsaQuP1c1FeJkXLdg59LRcTK9BZ2J03zzIzq/ G6Uvg/3/z0CEjO1BoqkIDHErxZ4aOEWfZgBnH5k0tpdCkVokkHF2qkz6OuqSW385xAwTZO TAoApuhi/7Pq8yklJ4WetZi+X8O/AdA= Date: Mon, 5 Aug 2024 10:06:33 +0800 MIME-Version: 1.0 Subject: Re: [PATCH] mm: zswap: make the lock critical section obvious in shrink_worker() To: Yosry Ahmed , Andrew Morton Cc: Johannes Weiner , Nhat Pham , Takero Funaki , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240803053306.2685541-1-yosryahmed@google.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: <20240803053306.2685541-1-yosryahmed@google.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT X-Stat-Signature: 7bar4tokoyd3edi3dfgajopiyjy5n4n5 X-Rspamd-Queue-Id: 4FE53100017 X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1722823605-932754 X-HE-Meta: U2FsdGVkX1+0Eo7YltPC9L/fsAdpNUendQfoNA+6q6Iga5r/HZJkPfCWQeqvLvW9VBvnxH1az/C0Uq8+GRN3+omhqRzPpWHYzUrpJlphp7i7kx1CW6L0vuEPTSeqIeTux0NobXvMtCeM1bo40LUkVetRXryEeFPSOJP6omp8iIppNR5zoU1kCuGOWfaM4xDqFsWU3fPuxNenFsOQA6qlE3OzFIAVSS8vGQroMCKgUafOnbXg0ZqqVRVgvys3dIitkdS0Q2wMEEoyKbBfd28OL75NV9hcDkW9umknPmZ5mzZ47YSljW/FCgLXw+Bk2tP5rhDXWKhM7kxzgPi3dpC2T4ZTlmevdMBIdIF7u2m+aIVNxAduuw10+uq62TbLsjfb2sB8UgGCVezZgGM67AJKMK/NMPzsfY3pMlVsJ/4iu80SV/u7mVnol2UUYEhqCZufSm6cKE6T7ySirG62a73dv/EfAEwA73KNzYM1iO2pbQ4ZJm6ZrPlC+a93BNECNsXE9kViEiw0qmZfDdjhlaORq8SXbyp2kR0Oxzy8DwA7DQNEa5UAtYYF6c3U95bXm0mDnekWwZaJVD8Rwh2Pz9ag8Cu0Gm3VmNSCp+UEObr0jjesjQU0CxpIDkqZcQjZ5FJNR8XEL+ETMzyuozDGZEodIixLftpbefQw6PcF55hBV5nAaCKAavKlsMQj8dTErYwXjG1inSW3ZZIZ5K87x9AkjA5IinJWoSPNa5I01R+IPefee4rxEA800pdPirqSS7pa0u1BNDCgWYQxXOehYsMcAlfsn0HA1M+WPa/33YnAMTMNjwwFWBJQnMQBLkP7bi7RqEPWGhC8a2gJssQistqmiwE0YYdKAxxfc987f5JvR7vkSfojV59tiSKm1188FHd2BHU4Jaad2N40x/cDbSny2Df3m3uS6eVpw9MiW4dJJRq8Ix7FYiNOoZ34p1HBy6YTKGq8P/mfU7/+3ktAd/e jw/QezUe re4eP+6+zViL8vrdYpItB2alpUWo/FmWzlhn5974SisfqWkdWr2Y1nl5HWgNRfCaG2R9c1nM3dFsi5Df/8COgfFZ9ago/F9BM/+9eUg4OcQdFLmZgLuAAlEpc/H2VxyihfW4tUXjY3rdXV0Zl6hBkWTnlPL1ntUBjBNJF9/dxuG51IYCUsMLK/cCOb59Vy4Avbs35 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 2024/8/3 13:33, Yosry Ahmed wrote: > Move the comments and spin_{lock/unlock}() calls around in > shrink_worker() to make it obvious the lock is protecting the loop > updating zswap_next_shrink. > > Signed-off-by: Yosry Ahmed Yeah, it's clearer. Reviewed-by: Chengming Zhou Thanks. > --- > > This is intended to be squashed into "mm: zswap: fix global shrinker > memcg iteration". > > --- > mm/zswap.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index babf0abbcc765..df620eacd1d11 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1364,24 +1364,22 @@ static void shrink_worker(struct work_struct *w) > * until the next run of shrink_worker(). > */ > do { > - spin_lock(&zswap_shrink_lock); > - > /* > * Start shrinking from the next memcg after zswap_next_shrink. > * When the offline cleaner has already advanced the cursor, > * advancing the cursor here overlooks one memcg, but this > * should be negligibly rare. > + * > + * If we get an online memcg, keep the extra reference in case > + * the original one obtained by mem_cgroup_iter() is dropped by > + * zswap_memcg_offline_cleanup() while we are shrinking the > + * memcg. > */ > + spin_lock(&zswap_shrink_lock); > do { > memcg = mem_cgroup_iter(NULL, zswap_next_shrink, NULL); > zswap_next_shrink = memcg; > } while (memcg && !mem_cgroup_tryget_online(memcg)); > - /* > - * Note that if we got an online memcg, we will keep the extra > - * reference in case the original reference obtained by mem_cgroup_iter > - * is dropped by the zswap memcg offlining callback, ensuring that the > - * memcg is not killed when we are reclaiming. > - */ > spin_unlock(&zswap_shrink_lock); > > if (!memcg) {