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 15CDAC02198 for ; Thu, 13 Feb 2025 01:04:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9532E6B0082; Wed, 12 Feb 2025 20:04:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 903C96B0083; Wed, 12 Feb 2025 20:04:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7CA946B0085; Wed, 12 Feb 2025 20:04:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 5E7976B0082 for ; Wed, 12 Feb 2025 20:04:15 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 068CD814FE for ; Thu, 13 Feb 2025 01:04:15 +0000 (UTC) X-FDA: 83113125270.06.5FC9B3C Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) by imf12.hostedemail.com (Postfix) with ESMTP id 1B2D040005 for ; Thu, 13 Feb 2025 01:04:12 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=nZZv+k+o; spf=pass (imf12.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.182 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=1739408653; 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=YNBKlfsVmtlHTkH9IZhvvajaLh8zkK9XIaf8ryQGr5E=; b=zxFrz5bGQStf4dCQMLHwGwDgOJBK6KpdOXnOOBBDajY2t4Eckp+m8Iu/cCy7Ww/G1Sgzh9 wCRNulqKq135nOSYLJc9TxN8SEzaYS0OWY4+IAIg3/41yBXHA8phb2eC5R3dZUdysLk1XC MBfs/SW/0unVKONvPjJ5OazshthYr2U= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=nZZv+k+o; spf=pass (imf12.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.182 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=1739408653; a=rsa-sha256; cv=none; b=Q5BbSsUNktBN7ew7Bz1izecQhAh/wSslCBIz+CePDU+iPwW/YqtA04pYt89eNkGIBWqD+T cdtPfoSKKbOsTvyPRluhzkZplokFovMfvm6oLk53ZV7PnwPlDnPc/xV5H7vv8jHKkksDv8 QcuJm2xXvnotLZ0QLLQnbfoZUg6l868= Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-21f92258aa6so6580295ad.3 for ; Wed, 12 Feb 2025 17:04:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1739408652; x=1740013452; 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=YNBKlfsVmtlHTkH9IZhvvajaLh8zkK9XIaf8ryQGr5E=; b=nZZv+k+oYkUUSr85wLNwjL1/f3Tr6ckMMFhxSlDvYBYDtlRsG7969Q5YCixZvrTjoI a+7H2QQf+bH4DeS8nEiZQc9Lmf/QfxEWhBNr8ybUvV8ywmBHsMJc/KZgqSX3F6CgR9IN xw4njeBtuRuHftral8YY2EGHRhVkvfa2pFtsE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739408652; x=1740013452; 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=YNBKlfsVmtlHTkH9IZhvvajaLh8zkK9XIaf8ryQGr5E=; b=AhyQCgKo+8eM9uBrE/RE8VtnY4uwCjpKICenP1F0XpYcAWAV+9UxTe1r5Kfr2C8Zun elQ62ZuDp7CnALBiGaDVhmMXWn+xxiUlmy7bET7Nxg0PkJ5EOARuScKlEbKz49G2ooWf o4iNAYUQfzJibZHopY/VJ2BbAonuUE9kDjzQAxRz1wNmu0hpMJZBbvuUcjcILFR0gQyc r4jwpKZriRvLQ3e9L30xADlwnJPkZJIsokOY8VdnNvyiTa5113LvRwgpZW3ecf7WUhxn EjaQf9W0rD4IHQQFYwk+9vgh4bQjFKG/YOooxlkbliX5hd0gZHpLpdULcO5xnmM71ch0 oMoA== X-Forwarded-Encrypted: i=1; AJvYcCURe2nCSz4OKcwN7XEqSM5n0sA/AkC2AQ1iRdJ9MX6g4aKS56F8mE3j0TBMIRNMGJIHXwUj+fKbrw==@kvack.org X-Gm-Message-State: AOJu0YyOWtBAqe99udggt+7HeOnXQsMLywLNfvu/zjhje7kfHo7ZStsJ V87XuFV7PkF3N57rOQSpOGmyE6/rwuR19N07FlwfKJLlLYmbGbqTW5DS/KSNhQ== X-Gm-Gg: ASbGncvjWNkTfGIJoJ9TmmVeerBUDp+uNElu+m4Y6t1RCQtQVrA6YKYwK2zgwUGm0/+ uWf4aNfrs6AKM8lLZZjW8AZ39c8SACgAB634zs1T2eVBHPTjl482E24gHAojiGnjFHOZedbkK62 xO/cCynGSMZtpz26LER161u3gfMZQDNXI6zMedcOYITrRv1WfD6/kXxss5m/xVUmd+W1GXStUvt cjiCrR+tj6qHzzhmY1o83+2X0uTfJfA+jS0lj87DDYPt0qNSI+2Ge0WVfmWa+Y2jjSdxQCJvUDy yKKD7iL2LgmgKMqCFKo= X-Google-Smtp-Source: AGHT+IEsXd7pTwSdHVb6/w5n0BoUOPkwr/sRjXyRUZKPV0O4FUJWx/q+Ok9ezXMZIpRjdgj7hmkV+A== X-Received: by 2002:a05:6a20:914d:b0:1ee:6b16:983e with SMTP id adf61e73a8af0-1ee6b169967mr2949103637.11.1739408651929; Wed, 12 Feb 2025 17:04:11 -0800 (PST) Received: from google.com ([2401:fa00:8f:203:69f5:6852:451e:8142]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-adb5a5304a9sm61241a12.52.2025.02.12.17.04.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 12 Feb 2025 17:04:11 -0800 (PST) Date: Thu, 13 Feb 2025 10:04:05 +0900 From: Sergey Senozhatsky To: Yosry Ahmed Cc: Sergey Senozhatsky , Andrew Morton , Kairui Song , Minchan Kim , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 02/18] zram: permit preemption with active compression stream Message-ID: References: <20250212063153.179231-1-senozhatsky@chromium.org> <20250212063153.179231-3-senozhatsky@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 1B2D040005 X-Stat-Signature: 64tx11cugw6py55s6x8fp7m9k13kp34s X-HE-Tag: 1739408652-257990 X-HE-Meta: U2FsdGVkX1+xx7LAZsr5/cyEU+nIAHesx9NkY7ORu1qBQKW98XVNxHvAvwVdNAzzXo86mSENbO2NN+UCd+5vJhS/uGXmTVFVIFkKpkC7Yp588uOf+cZ2g1bkz5CGlO2+/SyEU/71Ly0bStLzQOMKwSsPsvFV4u8yi6yVZ3g8/s3mOOixQTNSuYmhAlX7E0hX1/GopfcSrJoqmvuVxyV+7aloJQyJ82DV0v6m9SG2KGGvFWMTUFJV43g8Gu0jjb56rkX+engri4CGgBK57dfTqeCsjQY9NRJ9du3giKJmeOCSrcluhvDoMRAZZHaszcKNpUSSYhyRe5qcQysb0i2nWLbiEY+p/6r6Kth2B142LRbttm6aWW/0VhQhWlYok3aWqFQd2QD9poM+3hlB51fpT51AOeO+r1ph1GY2/52h+XDHOWBJKgivGAS9qp3nO35vB3mNHY+ytiLmEXfEpZwMrGmri9tvpHz72/43CbfXpBjeE1tsP6psU79bfQslfsKd9SC0AAUQxxQexY+t2iUvsVeq0UF5x9L69M53vjwFcsgl6+MPC6k0Dt8srReGzvoTePQI3cGO2NTas+E3g0XV5sDvy7iwP6JPUjRFnuyNPJ5lBHNdWc0KQJS0e0FPMN8pnxvehDLQfSarNdpquWXK6fl5l951x/ThCcwlFfSY2Co/U8S4eOSA4oi519tGdV9bTcvRFOXOEeV1gLQDBtSOgiG86ee4e2raDcqljL9zzVG6DDo9RwI6DyeucEBLQROwVXhwGIjLK2m88mEFx6oWj9LOjHVkxCNvohFWHzWy0pBmd2SVPHSaj7SeLq68m/FUxsStjbF1yeLaYMXtTw2c+h6jZIWT+c9gOWES6w2ii3HR0uS6VhRlEP3pZCss5ESqXosGhdnAsPa2/xWDlvp1sHmQoovr9h1j/iiLcxU4X7fPC6rPbMUSn17jv3EGlVkduAICdcbM2G5AhfxW292 8wbKXvGB qlvdTGN8sr3V0Qo0TqmyAeZvuwJlJGjjKkKmOGhZ+7el6NTmPyhPr5r2d7CbEx0O7V4Ge4pyJED+CNizNtmA5AV013hDFEJBtJPH8rYJsqPGGhvlb0x/BUZPFbRUJGztPNYJgDGhn1J2BD5+M8gl/4Y3RkK4LZ7xE5aezT1JO33LhZj8RPLCcvjE+QyseN3TmLP9nBuH8j7RruBvbkF+a5RKMxtThgqDlha+1oj5PlNPKuTr0mcHJLvjxGO44oK6NYco6heRaJbLg2slI7Sf/+Zt1zvJQQ0LM+mV0c5P9mijfZcQs45+cefnGTF1XKHFz9P/Y8Fylb66WFxH+EjNOV4dgUDb+m5slHzCm2KW0hWTtZmnQFyBkqSRz00dW1FLY6Ek4 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/12 16:01), Yosry Ahmed wrote: > On Wed, Feb 12, 2025 at 03:27:00PM +0900, Sergey Senozhatsky wrote: > > Currently, per-CPU stream access is done from a non-preemptible > > (atomic) section, which imposes the same atomicity requirements on > > compression backends as entry spin-lock, and makes it impossible > > to use algorithms that can schedule/wait/sleep during compression > > and decompression. > > > > Switch to preemptible per-CPU model, similar to the one used > > in zswap. Instead of a per-CPU local lock, each stream carries > > a mutex which is locked throughout entire time zram uses it > > for compression or decompression, so that cpu-dead event waits > > for zram to stop using a particular per-CPU stream and release > > it. > > > > Suggested-by: Yosry Ahmed > > Signed-off-by: Sergey Senozhatsky > > --- > > drivers/block/zram/zcomp.c | 36 +++++++++++++++++++++++++---------- > > drivers/block/zram/zcomp.h | 6 +++--- > > drivers/block/zram/zram_drv.c | 20 +++++++++---------- > > 3 files changed, 39 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > > index bb514403e305..e83dd9a80a81 100644 > > --- a/drivers/block/zram/zcomp.c > > +++ b/drivers/block/zram/zcomp.c > > @@ -7,6 +7,7 @@ > > #include > > #include > > #include > > +#include > > What code changes prompt this? Just a missing header include. We use cpuhotplug. I actually think I wanted to replace cpu.h with it. > > #include > > #include > > > > @@ -54,6 +55,7 @@ static int zcomp_strm_init(struct zcomp *comp, struct zcomp_strm *zstrm) > > { > > int ret; > > > > + mutex_init(&zstrm->lock); > > I don't think we can initialize the mutex in the hotplug callback. I > think the following scenario is possible: > > CPU #1 CPU #2 > zcomp_stream_get() > zstrm = raw_cpu_ptr() > /* task migrated to CPU 2 */ > > CPU goes offline > zcomp_cpu_dead() > mutex_lock() > .. > mutex_unlock() > /* migrated task continues */ > zcomp_stream_get() > mutex_lock() > CPU goes online > mutex_init() > mutex_unlock() /* problem */ > > In this case we'll end up initializing the mutex on CPU #1 while CPU #2 > has it locked. When we unlocked it on CPU #2 we will corrupt it AFAICT. > > This is why I moved the mutex initialization out of the hotplug callback > in zswap. I suspect to do something similar for zram we'd need to do it > in zcomp_init()? Yeah, I think you are right. Let me take a look. > > ret = comp->ops->create_ctx(comp->params, &zstrm->ctx); > > if (ret) > > return ret; > > @@ -109,13 +111,29 @@ ssize_t zcomp_available_show(const char *comp, char *buf) > > > > struct zcomp_strm *zcomp_stream_get(struct zcomp *comp) > > { > > - local_lock(&comp->stream->lock); > > - return this_cpu_ptr(comp->stream); > > + 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 > > "we could have been migrated from** a CPU that has already destroyed its > stream"? Right? "from", "to"... what's the difference :)