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 D959AE77188 for ; Wed, 8 Jan 2025 15:36:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6A4DB6B007B; Wed, 8 Jan 2025 10:36:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 655226B0082; Wed, 8 Jan 2025 10:36:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 51D686B0083; Wed, 8 Jan 2025 10:36:58 -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 34B376B007B for ; Wed, 8 Jan 2025 10:36:58 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id A741AA1275 for ; Wed, 8 Jan 2025 15:36:57 +0000 (UTC) X-FDA: 82984687674.02.0454793 Received: from mail-qv1-f49.google.com (mail-qv1-f49.google.com [209.85.219.49]) by imf04.hostedemail.com (Postfix) with ESMTP id AB74F40003 for ; Wed, 8 Jan 2025 15:36:55 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=iZKPnT3n; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.219.49 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=1736350615; a=rsa-sha256; cv=none; b=rXKn2ylNGyDDwxUVjBtsrYVmNSyjJT0YRbc/t/cgKhGWGcwaE5zcZDW1o4uoe+nD4y7thJ L8DSX9DoCwMQSeifsqGgpUHwKqtaL+46baXgHQ0t+ZHYM6WWejikFE2qdntJ/3MDO1Q0iL yycSS9hshqLDprClV3vRA3uGUGVCs14= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=iZKPnT3n; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.219.49 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=1736350615; 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=axQHKpC6dD7D4GwLmKC298J+SeI1ebHuNyZt1l3JX/0=; b=7WzArh/lN2R4BEIM/YLkiZ9fWV9INnmc3IuToKrihUd12cZsF92wqSVOXNEIHGRxvGP66W Ku8IbRN6nG2CVLdRgqb9QyL8f/VANN4lxN1sFkJvthv71tHj7UKxPW2vkOcs4iA47YflhZ a3VUkiUiB1KJXqf1wJ6u0/mM5GdRNBU= Received: by mail-qv1-f49.google.com with SMTP id 6a1803df08f44-6d8e8cb8605so91872986d6.0 for ; Wed, 08 Jan 2025 07:36:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1736350615; x=1736955415; 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=axQHKpC6dD7D4GwLmKC298J+SeI1ebHuNyZt1l3JX/0=; b=iZKPnT3nHW/D/dQpuDZvSWoMnBzLVFBSGpG2V/NMpu6xZfJvOOx//38tRjzQSUUa9U EuWsEOlSQH5yuIWRct0JS+QaMAX4zi5UB4h4wypbEETwsnY7sXidiNHEE717qtFpog2+ uRqwQ33Wm23Owi1SaOByGn2wZB6pJnhIgWZKMAIKxDvF9J8J5sHovd9aMt7BseQjr2XX +P56u22xn8oAt+yKHKeBgJRaguCoyi3He1VL8g8+QpqqOMP/LEVVx6DSnQW9q1zSoXcg JvZ7Z8PkuuA0+nyk4uL+5jhGmDvgLkqeDRcMQ33JfjxjbgNHBGAdN1lhuTOff9VIajig rQhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736350615; x=1736955415; 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=axQHKpC6dD7D4GwLmKC298J+SeI1ebHuNyZt1l3JX/0=; b=C8w3Hlf2uutbJxe8ok7g4uVw9J/uM/VrY+vGCOHTBRBlSDapzPAqUDjRAP2qXpe0+Q yxiD/8eQNzvoPe1CRAV6D224Xe4TzsNQpib+mZuTmxICc/GVWQa78BuFs62jTaCXbx5N J5X5Mf2sOETEGPEDtgXWrcuzxjoh0YOlhzZPhbRNrvuKz3ybIo/AEv6fGaWAIg+OSw5o WaHwgikpdaK3w2xnCy6Nx1hUUt0XqtUqZt4qi6In0/LHK++rkH6NY7lSdr65uRKyI2JL R4bS3vGAE7R9yq6+khbbNhweshi4vLyihSd+ucfasVDlOsGpO+LWEEudaVagOxPZeDte unLw== X-Forwarded-Encrypted: i=1; AJvYcCWZ0N9Z8+4gbtW/nb0YQmT3VYrq0SQByXY9d3MR9SsQaT3XEOfSoIKFae1PpmUt29bM51s96XprWw==@kvack.org X-Gm-Message-State: AOJu0YwybfqQNvZKYgsaJlL+ClxcO0wNNK9LVnEsVjWXHM+525Nwr3Lo KpdTKQkhIuLAljOJumV2PweM+V2jZkSqoERN8WxnkB0b32wJZlaMPoheI6HgzzhfwZYODVLE5uZ W9A63wrZP/Jg+ZY+i3KbG7FHMhWe3GxcKN/hK X-Gm-Gg: ASbGncstzWKbz7fRdQpzdHqf3RZqtFsvZsh3OwLU+Yz5hJqeeuBGczw6Tdq6kuVKEt+ Yn58BSOxk2UAojYFNWh+BdkcbPUL8+CdDzLY= X-Google-Smtp-Source: AGHT+IHE30RdvHFwHrl8DYNVDwiHIEwA6KNtLh6e615xOvu6roP1M8aszxEP2FT6kKhU7XY6R/omU63DFXee33mPD48= X-Received: by 2002:a05:6214:1313:b0:6d8:a7f7:c3a1 with SMTP id 6a1803df08f44-6df9b2e71afmr50749576d6.40.1736350614534; Wed, 08 Jan 2025 07:36:54 -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: From: Yosry Ahmed Date: Wed, 8 Jan 2025 07:36:17 -0800 X-Gm-Features: AbW1kvaG4--WHdAxY2l6bGR_njCr75NcnNoIpCKSzpY1SUWHqWRh9wna1Yl5_WY Message-ID: Subject: Re: [PATCH v2 2/2] mm: zswap: disable migration while using per-CPU acomp_ctx To: Barry Song <21cnbao@gmail.com> Cc: Chengming Zhou , Nhat Pham , Johannes Weiner , Andrew Morton , Vitaly Wool , 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: AB74F40003 X-Stat-Signature: bmmoyc45kodfuhbgrj91bhpyhnzg7hr5 X-Rspam-User: X-Rspamd-Server: rspam09 X-HE-Tag: 1736350615-903214 X-HE-Meta: U2FsdGVkX1/cJfGGvfVAePm2uWfUQJD5egcIgj0Jb6mZ0r8II51mtAwSv6PwkO0d4gsGMWNgI43oVDA0MLFNn3EvCC64aevRFSrx44Fy5NXuS4nD7NK/9pkq8Wi3ddHlmKxwWZM32M6MN60YBKNU5ko+efDR4NL+cpmrh6rLNbN9qElDHofTdrpwiderbcXYhmIFThgQqshJwUyYAL+3ncT9N2dE8/Fb/0baNTrflHysySBiMXicYAylFtxzMzoh07KinQoWyq4576ucI/Rf3E5f2JaezMZJYB1nJQ4KcJARCQBNasyfobp8o1XfuroPjwBdYCHpCEgpUaQsT5IIYPDNTs/fNeHHJLmKzo4x+rU/RwTbpr69Co/51zxo+pQrUR3uHORd8eV+DhvR0j+/zL8xiVnXQBz6VcbB0cG0LdKkhF10uP5pCzxcyy4qbNOMwHnRsKl1HVfu7l3KZ2RhKQU8ceiHbcx9hl4N5hMJbc0dAH/fPa0E4vuvpvOAHrWqdkQi2OxHUSNU8LBDCNrzeZzU6o9v5O2dpe1SaxFDRqx8PZZBwPKb1t7rkqBus4yxV4uP77x6MlXG6tdUzgdkTBKmMovGMPRj4ThOz2kkQiAzVPhyQdj1dOw76+d1O4XKBi3IIE14c3sGPWuTkQD4WFexZBqNxjwmU+XlzHKDtBFJ/Pezj+O9ifm049ovcLXJE7BxvwMwJmQn+Mbhk4T4pqlXzXsIj0HnM3s2C0a8YfRlc7+wCF6ixQwE4wDcxlYjam72hrPZzUUxl7IbKC0DJuCu4dlnVdmwCGY5HBC6zCj3YPOYi3BNV3aiuR76Hw78vx1YMaj/0HvWnIABBxoxwzuk136ALrfR8+/lZqq9gI0wlLCAowbZCOC+UEBf26/xBYnAvDKGcyS5GHDGGlFkCZ+jnMsM4jn3MBboNhEX8g068DU/1p/JPBSAK4lqrCuxNOhWe+sGW8eVhCleGF0 paDSNJyU FhrAEPJmBbSBrCWWh/3OSJ61T6yD+efHsHx1ZUjBcE/Pw4mcDhwwYy3SWqhP31rdNQB4buxoKfSpyQuMs2jnVuvuLZPIorC8i7DX+Sh8GxqZHO/g6qw1K9oxJ9tp03jnu842r/h6vyYTjDf+lCgTLBDPX7LVi0SxFV/vQDEo7vpjeePOFp3CPFvjLT9wZukujzmyMLEjwlbDDQ3853ROz4gcFNOsxs7or6YWvKjQqdA7x5KYBgTKQXWg5VpUFNU1fCzl8CPzb7+E61g9Ej63H9AJvtHedYhKYYs+mions5ZE2BKG7C+W+9jVnbDYC3y8XpE2CsILLBGPhEb4= 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 11:57=E2=80=AFPM Barry Song <21cnbao@gmail.com> wrot= e: > > On Wed, Jan 8, 2025 at 6:56=E2=80=AFPM Yosry Ahmed wrote: > > > > On Tue, Jan 7, 2025 at 9:34=E2=80=AFPM Yosry Ahmed wrote: > > > > > > 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 no= t too > > > > >> complicated. The following diff is one way to do it (lightly tes= ted). > > > > >> 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 in= t cpu, > > > > >> struct hlist_node *node) > > > > >> struct zswap_pool *pool =3D hlist_entry(node, struct zs= wap_pool, node); > > > > >> struct crypto_acomp_ctx *acomp_ctx =3D per_cpu_ptr(pool= ->acomp_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), s= o > > > 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. > > > > ..and now that I explain all of this I am wondering if the complexity > > is warranted in the first place. It goes back all the way to the first > > zswap commit, so I can't tell the justification for it. > > Personally, I would vote for the added complexity, as it avoids the > potential negative side effects of reverting the scheduler's optimization > for selecting a suitable CPU for a woken-up task and I have been looking > for an approach to resolve it by cpuhotplug lock (obviously quite hacky > and more complex than using this mutex) Oh, I was not talking about my proposed diff, but the existing logic that allocates the requests and buffers in the hotplug callbacks instead of just using alloc_percpu() to allocate them once for each possible CPU. I was wondering if there are actual setups where this matters and a significant amount of memory is being saved. Otherwise we should simplify things and just rip out the hotplug callbacks. Anyway, for now I will cleanup and send the mutex diff as a new patch. > > for (;;) in acomp_ctx_get_cpu_locked() is a bit tricky but correct and > really interesting, maybe it needs some comments. > > > > > I am not sure if they are setups that have significantly different > > numbers of online and possible CPUs. Maybe we should just bite the > > bullet and just allocate everything with alloc_percpu() and rip out > > the hotplug callbacks completely. > > Thanks > Barry