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 3B830C27C6E for ; Thu, 13 Jun 2024 15:04:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B67386B0082; Thu, 13 Jun 2024 11:04:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AEF086B0098; Thu, 13 Jun 2024 11:04:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 942986B009E; Thu, 13 Jun 2024 11:04:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 6F9426B0082 for ; Thu, 13 Jun 2024 11:04:54 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 157E640167 for ; Thu, 13 Jun 2024 15:04:54 +0000 (UTC) X-FDA: 82226187708.03.D59B3C5 Received: from mail-qv1-f50.google.com (mail-qv1-f50.google.com [209.85.219.50]) by imf27.hostedemail.com (Postfix) with ESMTP id BD3F740039 for ; Thu, 13 Jun 2024 15:04:51 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="Ec/uY38F"; spf=pass (imf27.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.219.50 as permitted sender) smtp.mailfrom=nphamcs@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=1718291091; 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=APrVAMfdz5UzlM6te+OQj7RDwISoPjhwoAJLzTA+Yb4=; b=KqOqbbl9FcoKCH0q2Wkmko3+Mj0bGpXxm9WbEFIppCwtTyQ20tldRtG+jXF5X4nkS9d8O/ uOaihtcmxDZpMzuReRgEIlz4gvLl6jmy1rYMzAGJee48k8SNIXp4zYomDwRwoD+HksvaIH 3RFqDAmVjwC5DtlmPKf7MmFWW4sPIW8= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="Ec/uY38F"; spf=pass (imf27.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.219.50 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718291091; a=rsa-sha256; cv=none; b=yQ6yAo8n9UvW19bjS2O5+/cqrenOhZRn0m1lRt0rNXanAlQgLlzUPcvyDCW+CpcJlDZQmW 35IrKmOzoeoThFWN0WnW3fRy4mcL5PdbRdIarTRCRG8VLTrGfkmkqmPUQ+BLyaICvJja8R Bw0nj3GFWCHVYs1Vm5iElEJRpy9/9oA= Received: by mail-qv1-f50.google.com with SMTP id 6a1803df08f44-6adc63c2ee0so5915736d6.3 for ; Thu, 13 Jun 2024 08:04:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718291091; x=1718895891; 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=APrVAMfdz5UzlM6te+OQj7RDwISoPjhwoAJLzTA+Yb4=; b=Ec/uY38FNrW2LXHBQ5UhyPMB5nR1uG8M3e/z4djzxs3pjUR/1WdYhy+Ue26g3D4Rx1 SM1dECY3yCTQvbhPF4rkQXeMP3bLLFGZ2/tsCDNlEOJZ7oV7YmllbZHG9DyRNY36/0YQ SExallEPTLLDJGV+1TmHrl3FjEtWdvYzzWZT9Ir/XEli4qh1lZSoh9S98ZT1uCxFbCDA 3lPV7HWPktFWlTaFb1+WlnPgzupDIhWd3IQdVKQ8xxBta08O9Fb1C7SM8VUaI6fzNatI EL2IhXmPt7uwthlncT+Uva6o/4rBnlt9hJP3MpwcMaPFNXy5lmdwW7THY/qPVDJTxu1D 1S8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718291091; x=1718895891; 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=APrVAMfdz5UzlM6te+OQj7RDwISoPjhwoAJLzTA+Yb4=; b=mcgS/muBy+g3I11PN3vPX9+qIwhCZ7FW3AiMr+UVVnaVaiR8X7WoHksmRBK7S2jUub WEQUjhQ9km+/1PS68B4gQ8zxKRbi33ii8P+gGm7oXE1pC3JUnqUT0y9Vg6Ic5xbZeya6 QFj13g2NGUQlkzNzB4vpMe+JqD6Pha2dh4CXLjYu6b8ordCjT3anr3AA3FULKkJbXfqj 2x232DDQ6ywBaUF8xQBH05wsHaMCU7V4wyQggD7ubjQa/kQkoxxL/6Jy6Susz+c8vhFh 7y77FAIBiaeRy4o+XiXVXdv4GhFLFLgWNH2lVnY9wlTLZSoMBQHONvr0ZjJarzDKdtqC WoGg== X-Forwarded-Encrypted: i=1; AJvYcCWVnuhI3Ri9C2i/Q2doErghC/046N4mvJo7tFZi4vAWP1+/joeorWnOMaAj3vnF25dePOe3fvJ4tJJTMzbvKfmnyHs= X-Gm-Message-State: AOJu0YxH+n7jJPlAbs6CYPhtAUcaT//Qt3ctPu2YLzxt4fQXATa+/4oo 6vRev9jKBA1iIiqUsl38XemQqUCFVyDvEOWBqM1Sq52IAs4KZrl2kvgx1JhnY9sAxeRXQb0DfhK gVkwss97dDhUwYD0GO9dLMCq+YMs= X-Google-Smtp-Source: AGHT+IGU/QEpuTi/kwbsmlS2WdrVWU0zL2Qu2R/VLvOWuGqq4KLKtnVAdbBgWWeLmr1OLqdFCN9jK78F3SRfXTZlqgw= X-Received: by 2002:a05:6214:311d:b0:6b0:80f4:4505 with SMTP id 6a1803df08f44-6b1a64907d8mr51601676d6.33.1718291090593; Thu, 13 Jun 2024 08:04:50 -0700 (PDT) MIME-Version: 1.0 References: <20240608155316.451600-1-flintglass@gmail.com> <20240608155316.451600-2-flintglass@gmail.com> In-Reply-To: From: Nhat Pham Date: Thu, 13 Jun 2024 08:04:39 -0700 Message-ID: Subject: Re: [PATCH v1 1/3] mm: zswap: fix global shrinker memcg iteration To: Yosry Ahmed Cc: Takero Funaki , Johannes Weiner , Chengming Zhou , Jonathan Corbet , Andrew Morton , Domenico Cerasuolo , linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, Shakeel Butt Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: aromkk5dt865ebtihmoz3wag99n36r7n X-Rspam-User: X-Rspamd-Queue-Id: BD3F740039 X-Rspamd-Server: rspam02 X-HE-Tag: 1718291091-371144 X-HE-Meta: U2FsdGVkX19j3wy4uJ9jqtGvOv3i3q5Pa+L0KWiox3me6lYgcDxqIZHV3LBb5AnygBGYYZPxJvsZfZlCFp4mWLZPatbBDhmgAwCIyN+KcJWdRk7/BgXfZcR4m9GqXOPR06f6dvICF4IqJhq/+s6oELERi4MLjrEWg3mpVCAdayzWT7YKPjMJ0ycnYoTZQZ2HMGtrdYFBVQZGFEs0vAPz2+oV7k8zFSK1fCXv8QJ+fiR6PA2wBbNr1UkCH6Xrwr4FVImTD3TNrQvy7pQ8INgfeVW9wW4qrAb19zTaLBh6zOTH/a2aD/GNVmG6Y1BhKCCFbC2h5GoXiDzzOob5vm0MncHGagc1B/kxSDoEaXQ//GPSV5p5Oyr6ARTwJLUaEP9U9mTuqH21mVuG3vJyPVgF0QUXwetS41/f/MOuUWFO8iTJGNQsatyS3Kx2sxtTO+8sLCpzkHgNsEnd8imMUSY8kTRO3OPtWduNk5y0R0Mkpd60kEOXBFWYWVjaTESi7iMbdrdBnIhGJgOzNJe/Pf65BmnP9ZCTr01YaICdVGhN4eUv/sDkleK8DmtBypfwzF9a1mYMvF9SjPHn7sQqFUFSq0R1xWLtVj7Y4Yfr+VsSglDr3IH+inhNSr6B+Do8/WJ4FpxpmOaBzWCMxauJJTEHzjBr9JX/pOGiojgXRxKqlLHRSntDZWsNo8AnO+9OPiCxac0ihTjrOGux/HKf+Ak7YCtEz9aSEG+E3Ds/gl7Uv1EkkClrrNXzaKU3zp6pvo4rKN7DVGguJdSprnlxJc2oKbAioMDhT16x/TEULP+AOgr8hNhFLjoEGlDj5V4BHicEed186ftbXfFbnfje+EDWsV01MZCVPG5X9wrxr0V4icLqrEtKusynQdB7Q6nf+XboEWrmVJKu33GeRZQ2n/FHHdhpEPJO9zC/fg0MCALjL/xZtsA2omZbJzN6nERknakmXXERbwMFDNFNLhAQRoz 1BWzQCFv oX2vSqH+ueJsQV6Cg9y5Szs4qcbmzhowiLwGXIshhAPyUQT2rRmPn+y7DpaR28GZX7YXt5n5hiihzasOjXT8SPVVnERgvYYlP9zLFBP4T7b0J4JLx2wEWWLbvVdU1VOa/ktpKERRsAIM2JjgGULudjdsAFaiMWen0cgsK0S5ZE9Y5hF+zqtSIyHlq0YV00nm/0D7LiQoiC+Y8fOHMcK7f5s3OjJtswMA7+f36Apv8BFFjq3YUlNIMnS3qI4ZRdRQulryKxEkTvelKJGwoxe5cEJBKcNU/lY8yeM6F7g1pjaTXYmP4tCCOZnkNE6YBGWHZH+Wc1CZzPOB+iVtCkjN4LrpsuRNQ6vK3qJrBEgrb1McPuVkIReNSn7hmEelpXhNqcZNEUaF22NuudmtVt/qjqNOuKg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.004028, 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, Jun 12, 2024 at 7:58=E2=80=AFPM Yosry Ahmed = wrote: > > On Wed, Jun 12, 2024 at 7:36=E2=80=AFPM Takero Funaki wrote: > > > > 2024=E5=B9=B46=E6=9C=8813=E6=97=A5(=E6=9C=A8) 11:18 Yosry Ahmed : > > > > > > The corrected version of the cleaner should be: > > > > ```c > > > > void zswap_memcg_offline_cleanup(struct mem_cgroup *memcg) > > > > { > > > > /* lock out zswap shrinker walking memcg tree */ > > > > spin_lock(&zswap_shrink_lock); > > > > if (zswap_next_shrink =3D=3D memcg) { > > > > do { > > > > zswap_next_shrink =3D mem_cgroup_iter(NULL, > > > > zswap_next_shrink, NULL); > > > > spin_unlock(&zswap_shrink_lock); > > > > spin_lock(&zswap_shrink_lock); > > > > if (!zswap_next_shrink) > > > > break; > > > > } while (!mem_cgroup_online(zswap_next_shrink)); > > > > } > > > > spin_unlock(&zswap_shrink_lock); > > > > } > > > > ``` > > > > > > Is the idea here to avoid moving the iterator to another offline memc= g > > > that zswap_memcg_offline_cleanup() was already called for, to avoid > > > holding a ref on that memcg until the next run of zswap shrinking? > > > > > > If yes, I think it's probably worth doing. But why do we need to > > > release and reacquire the lock in the loop above? > > > > Yes, the existing cleaner might leave the offline, already-cleaned memc= g. > > > > The reacquiring lock is to not loop inside the critical section. > > In shrink_worker of v0 patch, the loop was restarted on offline memcg > > without releasing the lock. Nhat pointed out that we should drop the > > lock after every mem_cgroup_iter() call. v1 was changed to reacquire > > once per iteration like the cleaner code above. > > I am not sure how often we'll run into a situation where we'll be > holding the lock for too long tbh. It should be unlikely to keep > encountering offline memcgs for a long time. > > Nhat, do you think this could cause a problem in practice? I don't remember prescribing anything to be honest :) I think I was just asking why can't we just drop the lock, then "continue;". This is mostly for simplicity's sake. https://lore.kernel.org/linux-mm/CAKEwX=3DMwrRc43iM2050v5u-TPUK4Yn+a4G7+h6i= eKhpQ7WtQ=3DA@mail.gmail.com/ But I think as Takero pointed out, it would still skip over the memcg that was (concurrently) updated to zswap_next_shrink by the memcg offline callback.