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 23069C02194 for ; Fri, 7 Feb 2025 02:56:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 789E16B007B; Thu, 6 Feb 2025 21:56:11 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 713A46B0082; Thu, 6 Feb 2025 21:56:11 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 58D3E6B0083; Thu, 6 Feb 2025 21:56:11 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 325536B007B for ; Thu, 6 Feb 2025 21:56:11 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 97DA314108C for ; Fri, 7 Feb 2025 02:56:10 +0000 (UTC) X-FDA: 83091634500.21.DA67D94 Received: from mail-pl1-f180.google.com (mail-pl1-f180.google.com [209.85.214.180]) by imf26.hostedemail.com (Postfix) with ESMTP id A65EF140008 for ; Fri, 7 Feb 2025 02:56:08 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=KYaTpb+K; spf=pass (imf26.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.180 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738896968; a=rsa-sha256; cv=none; b=ww2tru9sQinoXtCmq13g3TBU9ImX6FXgRggBBT1u03dgY1EXhGctUCL/FrzbYIvSFf0d71 E5m0pWJkl+6oZRTkDVj4Q237K1PoHStqHTrjlyedWPH/12IeCg05bCdX1vPGHltPAa+haf dPCPd3gXRMy6h2OFbI3iUj9B33OtL40= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=KYaTpb+K; spf=pass (imf26.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.180 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738896968; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=MOd4R4QD4yTRTYqUIyIfC9qaMS5rYTZOemKwXt2pRVo=; b=F4ehd3Sq3LcyUGldXBVJBDa4djHCeCzaNymjngPmdynZ/hnd5T1MAnDQNBBA9uyIqFH9Ag Q7rD6JVEC+McE36nRTOvLmGh+OaZTqWrQ/ZdQSr0gJgMfZYtpV9FDq7DbdUk2FrHoHiP0D bBg7M7uDWS4YYLX0+J/VZdFkYfzBMrE= Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-21f464b9a27so18773105ad.1 for ; Thu, 06 Feb 2025 18:56:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1738896967; x=1739501767; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=MOd4R4QD4yTRTYqUIyIfC9qaMS5rYTZOemKwXt2pRVo=; b=KYaTpb+KNNtBfJWJou6fHyvyss2MYLlBgfo/DaFHQiVATPVWUrdUgOB51fWbieQSUs oKgVX/JDbQyxvqrtjV427fy6nHqSrwCO3MP/re2ioq9Cm1NMKuZSbreN1um9SjAa44Wj /mAb0eCg+qi1fxssjwwE5OnPsDr/CCq23xM8Y= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738896967; x=1739501767; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=MOd4R4QD4yTRTYqUIyIfC9qaMS5rYTZOemKwXt2pRVo=; b=nmbPsMN6PeV4BqOOJPj4rMBgof1OXNlvB6g7SVXNnP7dWAmLq//mVzKEi7bZs714XE zk1iVZlIy2LXB5WKGpPHrxKnc85esNC4NliNqmX8Ebc4cLQ9+IO3ccQxSm/JZIFiR/wd j65DsYZoz80WDbytVV74OqyoBBWHV8wT3z2vCs880YP81H401fDd5UAnzftaE6srYjg9 Plw9pnpOqGyq2wW6r2TVN6SMG0gOWLBi9JL3bGLcv7yMdoy/njB2FQyYxxWOW22tdkXk bykAWiLXabTb9Vi8pnx3hZKiFOWrwOP9hx2wwk+nUZSR5NvKq72cM2kuSTdw8sURf7XF KOcQ== X-Forwarded-Encrypted: i=1; AJvYcCVhJpt9woNJs7jcbXIuVvgOUoFWpNN5fQy53owCGMeNCpUpnYMkusMlbAPUPfgJ+ltK4HI/EQrjgw==@kvack.org X-Gm-Message-State: AOJu0YwaIIHIdK+l0s6BX7qMyzXSkeEp34vKTx8ISVgaL6VW4pHEF4px GOdqUvvjKJS7/NL2zsPZsUKXHHnIe/5pivWxRPWjO6boNRSEokhpZ39AQLclpw== X-Gm-Gg: ASbGncso/oxL3nKJSw33woR1D6Wd/u4Y3BxKZGTLfMZy6YF0tIidMzp8GSbOceUA8mt Kg2KSDhhsrNJga0vW1F76Tr5rBrI0uUnOs3ZuC3Z9a4M2DQQGGd0nX2xcrPi4dsMwBHkSFio4jt ++wWZEt8uTZJAu4MgydcSB0NTgCqMeHYBKq352u/PwmEHw6IYXykPjoDc3wknoI8+uDTdq4NAGn 2nPDrtfPzktB6Ht+ylCii1nCZFkpZ6v2tXwW6MLvexwJgZAd40TVmP0UT1mRrHQ4z74wPfOTm4n GhMBvhhXXjR+L38kRA0= X-Google-Smtp-Source: AGHT+IGoEzmid0eg+e77/XKk9RTP/A15aAjm+SUgP6OzJG2D8+UvoM/vG7vUjR51J9sTmXBHfa2nEA== X-Received: by 2002:a17:903:234e:b0:215:bb50:6a05 with SMTP id d9443c01a7336-21f4e1cb7b0mr22889305ad.9.1738896967436; Thu, 06 Feb 2025 18:56:07 -0800 (PST) Received: from google.com ([2401:fa00:8f:203:28ab:cea4:aa8a:127a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21f36881e14sm20056445ad.207.2025.02.06.18.56.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2025 18:56:06 -0800 (PST) Date: Fri, 7 Feb 2025 11:56:01 +0900 From: Sergey Senozhatsky To: Yosry Ahmed Cc: Sergey Senozhatsky , Kairui Song , Andrew Morton , Minchan Kim , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv4 02/17] zram: do not use per-CPU compression streams Message-ID: References: <20250131090658.3386285-1-senozhatsky@chromium.org> <20250131090658.3386285-3-senozhatsky@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: A65EF140008 X-Stat-Signature: if1jm159kthedzrxp6zkzt8dmsfzbd3y X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1738896968-589219 X-HE-Meta: U2FsdGVkX19DldwRm8dr/J4OMoT6dpURb/6mqGKMGyb1QyrbL0eTaoGQz3rdjnzciAL1gepiuf6Qr0hsojFgO4H7Ll66rlxgFxKNz9k7V8NABXfHyl4DEqlZyj3SHfjc443ks6qbW8SJ17oZn2feoYnnP7r2RnH0CbwyZnSeAOrKoWSccC921AaF6GTXUO+malH8ZiNlp/RIjpNZHaN5GIxbtKSuJGQ7zjAau3U3K6ko91YgoZv/Wc31Wa/CfJ0ClKf5YjvxPUY40RMM9HeEVh4Wn4IKW6L9GYpzrki4mMEKezz9pjzI+hiPY7iAncqLVKDr2GZo7/l3CUZ9oUMzUAVJ8IJgWX5+kL0Zfvjn3QwRZokKbwn2dveCpo/3DFwJv6o/GalLhQKT//U9Q+VW54wQrvH7eKtumZWvKzwq48clsGdE/jWEYoE5v5u0vAhEzgGGwhC96KnvaX1wJOByRfiwU8OecVKX9U5JDHhYSBMRgp4JmW+zhGPkBaVEQD0lQl4umA6qoRoI8XbTfxSDOCRwPf7oXQXXLOlrxljNKoGx7dVvdmV2xMJf9lqRzsrTsNYKV6FlWe+4Q4Pat88b8tNuCeEnrB/ZH3bTgimyybQVnS5VSVDvZ64P4cIP7CrwRXdJujc+nSFKniTISFvQccimXu/FzXYmY0ey2n6MqQiG5kXFGrp90vwA5wv/tJMR/2KtOI4VkMSF7rfQZrVXajH491w+i+GjAqNvhrTDDraE+PiU2+DJolkEp5NCLGeLVeyBb3acVy2sesdFkWqzncO2IE2hkreFdSUEf2OFnlNJWY45raCdWlTyG8Jvx1uWTkygglmizTfF/Fqodx2IQykA4E9qnvqepLAdLKuGzM8PfBWlKQr5oOKxoGzPVeqbas6n6a7fefc90q2jCIoS7nwByOQJKN38gtbInFjow0//5Zvu9Koyod994gnhoqgaVXAAiCm9nfYhcGLx7rh I1BQir8M TqXgHGO/seHAnSgaSVGNwmrkZmBGbepAGwW3zzR8JUfzSmzAbjYyfhg1iLkZb7qReYRrICnGy82ECR0d8SW128Pb1+p6EWsgPxu51p/iAiTDnPhtG3BJuH4BJ5Uz5n4u//pJKMP2s4lvB6hdz/UIPta+bJ2V/BlR0cyu1rkNVx+1nOgE3lpTeFuMn4eXP2w/hKBiaeq5q71QG99cZyxKkjj67D2WzOCNaaMwnXo/JD3Euf0GY7nfp4Zpcmqzf4XzHNRkpBdPEyi9IivMc+Wot7cQJ11bDLUCxKAiTmdbjRkILRIrdex8n7AcLHPlZc3CwAZ/CThecGRLZt1VkYfGlqU/eNYBx3ie7MHsjnPBT23//f9E= 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 (25/02/06 16:16), Yosry Ahmed wrote: > > So for spin-lock contention - yes, but that lock really should not > > be so visible. Other than that we limit the number of compression > > streams to the number of the CPUs and permit preemption, so it should > > be the same as the "preemptible per-CPU" streams, roughly. > > I think one other problem is that with a pool of streams guarded by a > single lock all CPUs have to be serialized on that lock, even if there's > enough streams for all CPUs in theory. Yes, at the same time it guards list-first-entry, which is not exceptionally expensive. Yet, somehow, it still showed up on Kairui's radar. I think there was also a problem with how on-demand streams were constructed - GFP_KERNEL allocations from a reclaim path, which is a tiny bit problematic and deadlock-ish. > > The difference, perhaps, is that we don't pre-allocate streams, but > > allocate only as needed. This has two sides: one side is that later > > allocations can fail, but the other side is that we don't allocate > > streams that we don't use. Especially secondary streams (priority 1 > > and 2, which are used for recompression). I didn't know it was possible > > to use per-CPU data and still have preemption enabled at the same time. > > So I'm not opposed to the idea of still having per-CPU streams and do > > what zswap folks did. > > Note that it's not a free lunch. If preemption is allowed there is > nothing holding keeping the CPU that you're using its data, and it can > be offlined. I see that zcomp_cpu_dead() would free the compression > stream from under its user in this case. Yes, I took same approach as you did in zswap - we are holding the mutex that cpu-dead is blocked on as long as the stream is being used. struct zcomp_strm *zcomp_stream_get(struct zcomp *comp) { for (;;) { struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream); /* * Inspired by zswap * * stream is returned with ->mutex locked which prevents * cpu_dead() from releasing this stream under us, however * there is still a race window between raw_cpu_ptr() and * mutex_lock(), during which we could have been migrated * to a CPU that has already destroyed its stream. If so * then unlock and re-try on the current CPU. */ mutex_lock(&zstrm->lock); if (likely(zstrm->buffer)) return zstrm; mutex_unlock(&zstrm->lock); } } void zcomp_stream_put(struct zcomp_strm *zstrm) { mutex_unlock(&zstrm->lock); } int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node) { struct zcomp *comp = hlist_entry(node, struct zcomp, node); struct zcomp_strm *zstrm = per_cpu_ptr(comp->stream, cpu); mutex_lock(&zstrm->lock); zcomp_strm_free(comp, zstrm); mutex_unlock(&zstrm->lock); return 0; } > We had a similar problem recently in zswap and it took me a couple of > iterations to properly fix it. In short, you need to synchronize the CPU > hotplug callbacks with the users of the compression stream to make sure > the stream is not freed under the user. Agreed.