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 E45A6E77188 for ; Wed, 8 Jan 2025 05:34:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 10B376B0083; Wed, 8 Jan 2025 00:34:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0BC8A6B0089; Wed, 8 Jan 2025 00:34:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E9E316B008A; Wed, 8 Jan 2025 00:34:54 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id CC5776B0083 for ; Wed, 8 Jan 2025 00:34:54 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 688051C810B for ; Wed, 8 Jan 2025 05:34:54 +0000 (UTC) X-FDA: 82983170508.27.B6797CF Received: from mail-qv1-f46.google.com (mail-qv1-f46.google.com [209.85.219.46]) by imf09.hostedemail.com (Postfix) with ESMTP id 93F4D140008 for ; Wed, 8 Jan 2025 05:34:52 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cXYyXI0O; spf=pass (imf09.hostedemail.com: domain of yosryahmed@google.com designates 209.85.219.46 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=1736314492; 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=GGGTpbai52pQZ2nEPdY6iTgX2oG72nzLWb46afIeUik=; b=Y0upgar0Y7UHJVTwQwtiNwnCdvUQfGL1qLN6KtdhYnSZyxOVrDC6cdIVEna+afgingBAZY ORO6FWRmpHQsyJZGCRQJou+0CB3R41I81vxXHIaggy3hB+XyXj57Sx0cdghgQVQ+KybXie 8co8WKTg1n0y6EuhfhzQm7lnCTCO2+o= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cXYyXI0O; spf=pass (imf09.hostedemail.com: domain of yosryahmed@google.com designates 209.85.219.46 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736314492; a=rsa-sha256; cv=none; b=KqRw90QxYz5AUjHJCkMlR8Bv0FAtDzGpJRtl4H88tZUjnI3iHD/OdGsYYDsli0Hn5/Ukqg eVGpQpEYggIl4xa+uCxvsisVZ6WeJ37IYzC+hrr53IXPiP1MwvuL6bXpU/c2O2QAY/gIRK gRG4oDpwJlJYnqyXgXf9CedR75FtNbg= Received: by mail-qv1-f46.google.com with SMTP id 6a1803df08f44-6d8f65ef5abso138044426d6.3 for ; Tue, 07 Jan 2025 21:34:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1736314492; x=1736919292; 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=GGGTpbai52pQZ2nEPdY6iTgX2oG72nzLWb46afIeUik=; b=cXYyXI0OyXsK+S5oEUOo3evEK4UOgc8WyYqZNmpfjG0WbhpwTXy6I63pdAOGqho1QL tUGSgKSRvgvl9WksZK7XDhMPsLTdRvnTi6SlBxQtvjLJqseKBO9yhAT/bVULS/nYXVSU A0sEoa1mtuLZ89Z5MUyW3vTSSRmNtigI8wtRPpYDcvVZV0QTfKydaHY8a3N6ygk7aiDo DO36gKJPkRw5nEWYUlia+XGZb5Tj48JGrctVcNp7CnL47yQpLn3LyHbDfBmPwDhuHflo qx2yj9nEXtPj+ILh70xNbo90jSiLOt0kjW7LQzZLCuM1tmPE4v7+IRg0zxM8HcwNPYLV V9mw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736314492; x=1736919292; 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=GGGTpbai52pQZ2nEPdY6iTgX2oG72nzLWb46afIeUik=; b=dg+PdgZo4o0C5jUHVvKuix1bo1FLs6h8y3CrG0xExWr+E651RjuhwfAfw+CdylXFKx XEApf0rsQ5i7XVzj+yHNpMgea0i6sUa0Emxsz+8nZKMAaPwepfYnyMkqE7LaAsBxf9Zd pCtwx+4iZGDdpfA5j//FKHKOjhibua0y1oLtixYM4ftQNWKvOXOziwgftGJOryTGPAfi UzaHa0gOo7m+Ghi2f7Ks7ge0Nyz6G3og1JxTtYPCv4hlltxzv9/oWhXoei2QbeWJz/rL uXZQ3XxK4bV7Y4gL19ZxqJisefGrUZsAtOWNspVqZD2ko1fGWS8SEW9ApCifnUSiYpDg TbTg== X-Forwarded-Encrypted: i=1; AJvYcCXwnE/1qjw1Hib18w8rw1mNU+BSXS91cF+UPd5j4cJ62RLfcnwFQl5iF0zjf7/sRMpoQk5UloLzDw==@kvack.org X-Gm-Message-State: AOJu0YzYpK2/wzUAGx1+of6Trpf+0x3EwSxOIzR6LVZcZi0Cmja1GiQG l5L62m9ucKwRPUTZFNx5wci6KjAyzeJBha6JyNlaSAIN7YKLULOhoZ0aty6B5QZgJ10I4jBmBWJ MB/Dsn8lmAwTATfiBO6tjl3+oHuNdMcWpt4vx X-Gm-Gg: ASbGncsG47NcSixBAAtkhIUKMEa+RbTSOsJRTlqUplb+lmgfuO8ODpJnZY9Nk42r08l rYdwui7rI8UPlj/ASXn4mTdsnxKN4FfOzDRA= X-Google-Smtp-Source: AGHT+IEpGa22i1VWzWBqEUP7CYJT6s+0vIGD0eCOK6ljUBlWUp59HTcAocd/QJCSOpwC9GUWBBt7NGz3fuW/ofcDVAY= X-Received: by 2002:ad4:5bcc:0:b0:6d8:a9dc:5a65 with SMTP id 6a1803df08f44-6df9b32b347mr23989286d6.45.1736314491553; Tue, 07 Jan 2025 21:34:51 -0800 (PST) MIME-Version: 1.0 References: <20250107222236.2715883-1-yosryahmed@google.com> <20250107222236.2715883-2-yosryahmed@google.com> <857acdc4-c4b7-44ea-a67d-2df83ca245ed@linux.dev> In-Reply-To: <857acdc4-c4b7-44ea-a67d-2df83ca245ed@linux.dev> From: Yosry Ahmed Date: Tue, 7 Jan 2025 21:34:15 -0800 X-Gm-Features: AbW1kvbuH4zFO1DrM-8N0ej8AsO7sS4UchNazjpirfR1-uGUBsBqYVkPxlt_Tt0 Message-ID: Subject: Re: [PATCH v2 2/2] mm: zswap: disable migration while using per-CPU acomp_ctx To: Chengming Zhou , Nhat Pham Cc: Johannes Weiner , Andrew Morton , Vitaly Wool , Barry Song , Sam Sun , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" , "Sridhar, Kanchana P" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 93F4D140008 X-Rspamd-Server: rspam12 X-Stat-Signature: no5x3nfty1wdth6xftyy3wz7h8b4w9zo X-Rspam-User: X-HE-Tag: 1736314492-302149 X-HE-Meta: U2FsdGVkX1/ntHxPsIEZqPdyDDTiy6sncE8D7eMUrp5+ydq3fkvucyjJws+iO0qR3nOFWLTt8GWbzBG2s0OFzlFcCo0Xh2N6duYctWX8cXqWNcl4t6QmiPhOFl28QekbB+BTIdI5ITAyg3WesswZbbeYcrvTiLy7xAVnxqM2nITvMpesVU43Y1DrSfe9eoqQ0PdAB8dqmIvneiKIKd8bPaUXB1kYSICCiZKbn37NRxO+Wsvhp/QHgL7/nJDYv/XlVtZj74R5yvdUw1EVT+VEJcvcRhAwuJHPgd+362vpeq0xOZdZi+fzjg9igEX0iW7lw2h1ApaY6+Gy/hKGyZjZI4fAooA+aQ8SJJouGUouiUFpEEcdDPRjIKX5WnUnkSCapum8A8o/vEe1TBbLfWdo2KEoUD56RddAwZjS3qpUxdD+qnPHrUr2qj4/P+MwpynP0TAejMroBv00+HBCNkiXI4o1ymCeVOK99NXKyNKR8Em0WWKEYmhP9UJ7CzZfzMCi263JDvqGXWLQqDsHd6pM+MbgxTxwkkQi+3l05BWJc4+oMDHLvIQaoXCn0IuRWBKrV/ymYbwT3Pvp/Os4eLoVpNrJ3o6hy+f50d8i/aXoabTmALxlvm+BIx1l52+3yEAbXhi3tXA7Yh4QBiV6itYZNTfkIhk4loFLjDpXx828LUP4WGhQG7cB5Jpw1QPbIP1o0M1uCrzW7Z9BAr1P51NNgiyOUv25RursGibwBED4c9dkns2Ev8VZo+fZpBHOUhVQMywgQOtCpXd7njzlK+0F7wuIcEYC4B8LPvUdVA49A+2rjh/5zlf1oaWY7R+UKuHm++J7WN4SBq5ZGUUK2FYdSiUMQa8NQvlo5+hUmXg760s4u+5l2YV/NWXVMFGD5ipf7o3ZXRbwkqp1fzPWik5v+OOLhEWZXGwEN29/ZTLUuaPPLC4/WJbQhX1vyXsEFlZqhv2wpAbQ7n36A+q2ISb F17mXJHM cgNfRa11LtaQfLWLdI7bpOPb2/waDoxZz7DfEMIcddPqlEqlIAIr2fHSo/sQVzQMtz0Yhbp2kaGUz8grcKOBeAiQBr1gxk5/CsicDUI55jM8yYN8/LcM3u+Gpbqtm1JOxF55ybRKXsjR0PreeE+BbKeyO3zUTslTEwH5xCNeg7Q/axBebXf4n2m3KtXf7cvBac+qxiDYg3knAEpVuChoubQeqEL36infcVDpI/vFoeIDGZnQQz8A/kfmmSWQMTWJ1c+pr5HtB9yBCzwr8Ga5r5soOfk3CrnqPLL8z+pqifKKD91FsogVt3ZIXaA== 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, Jan 7, 2025 at 9:00=E2=80=AFPM Chengming Zhou wrote: > > On 2025/1/8 12:46, Nhat Pham wrote: > > On Wed, Jan 8, 2025 at 9:34=E2=80=AFAM Yosry Ahmed wrote: > >> > >> > >> Actually, using the mutex to protect against CPU hotunplug is not too > >> complicated. The following diff is one way to do it (lightly tested). > >> Johannes, Nhat, any preferences between this patch (disabling > >> migration) and the following diff? > > > > I mean if this works, this over migration diasbling any day? :) > > > >> > >> diff --git a/mm/zswap.c b/mm/zswap.c > >> index f6316b66fb236..4d6817c679a54 100644 > >> --- a/mm/zswap.c > >> +++ b/mm/zswap.c > >> @@ -869,17 +869,40 @@ static int zswap_cpu_comp_dead(unsigned int cpu, > >> struct hlist_node *node) > >> struct zswap_pool *pool =3D hlist_entry(node, struct zswap_po= ol, node); > >> struct crypto_acomp_ctx *acomp_ctx =3D per_cpu_ptr(pool->acom= p_ctx, cpu); > >> > >> + mutex_lock(&acomp_ctx->mutex); > >> if (!IS_ERR_OR_NULL(acomp_ctx)) { > >> if (!IS_ERR_OR_NULL(acomp_ctx->req)) > >> acomp_request_free(acomp_ctx->req); > >> + acomp_ctx->req =3D NULL; > >> if (!IS_ERR_OR_NULL(acomp_ctx->acomp)) > >> crypto_free_acomp(acomp_ctx->acomp); > >> kfree(acomp_ctx->buffer); > >> } > >> + mutex_unlock(&acomp_ctx->mutex); > >> > >> return 0; > >> } > >> > >> +static struct crypto_acomp_ctx *acomp_ctx_get_cpu_locked( > >> + struct crypto_acomp_ctx __percpu *acomp_ctx) > >> +{ > >> + struct crypto_acomp_ctx *ctx; > >> + > >> + for (;;) { > >> + ctx =3D raw_cpu_ptr(acomp_ctx); > >> + mutex_lock(&ctx->mutex); > > > > I'm a bit confused. IIUC, ctx is per-cpu right? What's protecting this > > cpu-local data (including the mutex) from being invalidated under us > > while we're sleeping and waiting for the mutex? Please correct me if I am wrong, but my understanding is that memory allocated with alloc_percpu() is allocated for each *possible* CPU, and does not go away when CPUs are offlined. We allocate the per-CPU crypto_acomp_ctx structs with alloc_percpu() (including the mutex), so they should not go away with CPU offlining. OTOH, we allocate the crypto_acomp_ctx.acompx, crypto_acomp_ctx.req, and crypto_acomp_ctx.buffer only for online CPUs through the CPU hotplug notifiers (i.e. zswap_cpu_comp_prepare() and zswap_cpu_comp_dead()). These are the resources that can go away with CPU offlining, and what we need to protect about. The approach I am taking here is to hold the per-CPU mutex in the CPU offlining code while we free these resources, and set crypto_acomp_ctx.req to NULL. In acomp_ctx_get_cpu_locked(), we hold the mutex of the current CPU, and check if crypto_acomp_ctx.req is NULL. If it is NULL, then the CPU is offlined between raw_cpu_ptr() and acquiring the mutex, and we retry on the new CPU that we end up on. If it is not NULL, then we are guaranteed that the resources will not be freed by CPU offlining until acomp_ctx_put_unlock() is called and the mutex is unlocked. > > Yeah, it's not safe, we can only use this_cpu_ptr(), which will disable > preempt (so cpu offline can't kick in), and get refcount of ctx. Since > we can't mutex_lock in the preempt disabled section. My understanding is that the purpose of this_cpu_ptr() disabling preemption is to prevent multiple CPUs accessing per-CPU data of a single CPU concurrently. In the zswap case, we don't really need that because we use the mutex to protect against it (and we cannot disable preemption anyway).