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 C50C0C19F2E for ; Thu, 27 Feb 2025 12:42:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5D8FD280004; Thu, 27 Feb 2025 07:42:36 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 58914280003; Thu, 27 Feb 2025 07:42:36 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 42960280004; Thu, 27 Feb 2025 07:42:36 -0500 (EST) 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 25943280003 for ; Thu, 27 Feb 2025 07:42:36 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id CA04880D8C for ; Thu, 27 Feb 2025 12:42:35 +0000 (UTC) X-FDA: 83165688270.28.B5C33B0 Received: from mail-pl1-f181.google.com (mail-pl1-f181.google.com [209.85.214.181]) by imf18.hostedemail.com (Postfix) with ESMTP id CEC921C0046 for ; Thu, 27 Feb 2025 12:42:33 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=eJWCgCGK; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf18.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.181 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740660154; a=rsa-sha256; cv=none; b=UtoSXWDtj5T6WG7D/ushel1hOvXEV1ISXcuSkPwfaj8ZYFdb0H7qEfAi3miSF/3RowaT5I 61AC5wcLUhCz8CEcTqjXKv+/BWsUtIwp/iVAQWaAa64qozNgWOvdC//PAvkToVF28Cguko PvlhGQSr4Trr2DENzzA/xj7wsDdpl1k= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740660153; 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=JO39NonkIXySEEdjexhrhful6hOdxXOoHMCUy3lutDs=; b=f3RYlH/1Qer4Un0ltxw1oSG7SFlCV6nfKB4jHXnBA3o3CC7fzPiHFs3JHqrDxbE1b0jXYI ZVBCI8MgJIIrTqDF2wd+xcbSqzqNRPNV80svH+0F/eh16mp4qq//JcnR9yYVrEfs1Cxi5g CiIJXdpJyjP0kOoMms9UfaitKFBZM2o= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=eJWCgCGK; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf18.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.181 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org Received: by mail-pl1-f181.google.com with SMTP id d9443c01a7336-2234e4b079cso13188725ad.1 for ; Thu, 27 Feb 2025 04:42:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1740660153; x=1741264953; 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=JO39NonkIXySEEdjexhrhful6hOdxXOoHMCUy3lutDs=; b=eJWCgCGK94rMchMQOpYOE6yIPePZ02K1IMaeTu2KipGxo/q16ZAHkNgknQswu/vtTZ umC26oR20+igQd6b8LEUdQgPT1T2eKWjBaB91vpPdCf3wcpVQ71VBkutnZbHmnof1/+7 NgeqtW0SNCGKEK+pVlaRWkDuR9zVEjscmNyrA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740660153; x=1741264953; 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=JO39NonkIXySEEdjexhrhful6hOdxXOoHMCUy3lutDs=; b=LlYI/aumM49DH8hC3KK6NWte5yrbuMGddLfDP6ZoQFCoC1jMvGraNQjXraZ2rxPq1n eYrXhGjLOBFbY2bwUuwMMfH008Pkkwp3ov9aYvgaPG5gM6DWb2HNkJm8x/8eFvciI1Kb t31Jxg/CQYtxdyAzqJBF9zx8Fri3j+7DmidV5JW39zEU8e1p0jhuEAa9uFKYXrJGEIiT /cy9vSZWFMo+fmKASzSKYhk4bqzSGLdsB3Gd9vxnZUyQhRmlmumhTqRccy0yYgKr228b 4NpaTsXD5/ytHq3C/GtzJloysZL0lHQf0/SCMxik3CCwJC7kNcLoUSfNLzEW5Bm2ls2d JW/g== X-Forwarded-Encrypted: i=1; AJvYcCWCFUoCk0RD0RJPrJHAHw4E5bZSQY1aN52MvCtWYV2OcsqFtA2LwpHRDrLR7JyB1jDaVbnRXdJ3Ww==@kvack.org X-Gm-Message-State: AOJu0Yz3CxAwMi7cDkHsI+6/8+6LodMkIG7hWAx6Bn43gv7ly1+PgBdE 9oV+6Bac4vDeCf6CauNFhQXqPdnLC6IFicEXg4HwglGR5N5vWLIhOjLnYqh0aA== X-Gm-Gg: ASbGncvVt4e1DfnfgrgI9umZhRmtfUl1fdEtPDUMV63fcfYe/PB63HWXaP3wyIb6ATW AzcoViXf/IcqdDHGGBVjgMW8RYU42SnPrbunf7bSgKRRFo748Le2a11WblgqCg5AXGZBKUCDWfo jhdHX1qFW+uNcwoBjsHD4c+ksYeuMRUHpl4iGLkpWwwv+nAL32qmWhoSFveIVD0g1DanjA2h/Hh aMGvMboLM5X0SK4P6YnegkdOndpgRODD/8QMnWwOjrZh/Ln9iFafRqWxrmcfuM4SwFB+hohOLPt Pn1FPEtGeDBxESE2m6PeYC3m/xq7Mw== X-Google-Smtp-Source: AGHT+IHQDfnD4YCR20HmdNquT6tjT8Vc6oH3OvR16gag9lE7O1kspFHtXL7T10uSf6M1PlhbDM0jsQ== X-Received: by 2002:a17:903:41d1:b0:21f:6ce6:7243 with SMTP id d9443c01a7336-22307e79375mr163730555ad.51.1740660152660; Thu, 27 Feb 2025 04:42:32 -0800 (PST) Received: from google.com ([2401:fa00:8f:203:a9c0:1bc1:74e3:3e31]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-223504c5c67sm13289625ad.135.2025.02.27.04.42.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Feb 2025 04:42:31 -0800 (PST) Date: Thu, 27 Feb 2025 21:42:25 +0900 From: Sergey Senozhatsky To: Sebastian Andrzej Siewior Cc: Sergey Senozhatsky , Andrew Morton , Yosry Ahmed , Hillf Danton , Kairui Song , Minchan Kim , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v8 01/17] zram: sleepable entry locking Message-ID: References: <20250221222958.2225035-1-senozhatsky@chromium.org> <20250221222958.2225035-2-senozhatsky@chromium.org> <20250224081956.knanS8L_@linutronix.de> <20250227120532.OsZr4v2A@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250227120532.OsZr4v2A@linutronix.de> X-Rspamd-Queue-Id: CEC921C0046 X-Rspamd-Server: rspam08 X-Rspam-User: X-Stat-Signature: ce4rm5j36z479dijdpfg75npmt3fnyos X-HE-Tag: 1740660153-850055 X-HE-Meta: U2FsdGVkX1+qGNAGWpZ5cVuSwgFI+HNFYkM4CmbZmWjuW3m4wOn4JVT6Ciw7NmOy+PyR1c+JdlTj4wFX3lv/oV4xr7v8k8Dp+MydtQwtDv9iaXjCfKs/5eo64SQOtgu9JdPkgd3KeMw45YhuQLiTveZl2NG0QjLnqvrAi0jPjCWTg337HMLVN7lImSYpSw8jONiSjF4OawjNrHULDPqoGyw7Z9w9U1PDfSjkbiLGc9dEIlhSgUjZkQqrETlJhsbu9SDb7opq5ZC93z9Jwju75BN9aM6aPiTTQ3aub3DDDqKja8KyH7IZmUO54+j795TRDhPjYgLV4b7fCZsKngysqgVPS+J+FflScR2pd2h+AJpXSrOjae6TNDi8tCkgTWr7w81v+x9E+NYLumuwdYVmReux1rzlTzyGNzA/08fIzN8e26PD0SPhxOw7KTGuo+JU0xBKwq0xDvZ9c8xQibhXOEuVbFWIwVYepizb7hnSA46cRPEp6zS5WxqC0H1uKjkX34zse2i4a9mOn0ZzHqaP3sw+p7CQq71xRXs4P9AiNDjfbEJ9+WIbnVfDHqvhRiz5xom9uK+FtwtMmizCOXsRtfnc/Ke43gRvg81HcCibAuHhKDivPTLnvz90PIIS+M/Uur5nrwGSmgHiL+bd7qAAPqF2EzTwISHDaVVhQ7ikrmGFB+7U+j9PlIMkTOE83hykQw2s35o/WxP48UY4TRifIxKroo6rcukX2MN8rMSV9G8pCtixxlTDbdkh6QyYiP6EwwcgEdH+1a0bxV+UC7FmzHwE+YJotpYSXWjBRJTWZHpM6myOuMLQk18yCM/vJC9/0+t/sGby6kVu9T7uN9GAbR6k7nY0Uwd11lRgtZbdIaRxABfICCJYRExJhWMjtneohexE6aRu90PGxnwdu4Y2nK5uJ2UomSJege+fkEaiG8tIjJ3+zk3eBBiYznAtUTljZp++btsmUXwaBq/CyWF BY2cypOe u/n+FNbMc+f5Z6+nQOqTh/qrS410NkDTsh6D4fAnqhv1KVyI/iDVNm1GKE7l9HxDAtVnLhEkVNGrCrLMiQ29FHrLqDpe5PjiiLG/WJzwTAca027p1NPTw/+VJ7LPZi8v9P9FQ2hqu7BvqwBPW/S/3cC2eg6rWf9kmN0lehdLw6o5hMW/TE5GYsoVzQrPXiKh5rnArA7uQNU/6B1Ykr/hVeuGPt81slMAj36sW3XQlqjhw/vM/pIbKbgE3tKSbDw8ilRQC74n13zU3V4iO9z11i/al+OnEvwet3gTmXz5tUi63ebBIaNsXSi/+olsoz539rVZv6UN4vNs0OR8q7UNYvKH9U3afsxptc+7lpNL7F+Njjto= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000003, 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/27 13:05), Sebastian Andrzej Siewior wrote: > > > > +static void zram_slot_lock_init(struct zram *zram, u32 index) > > > > { > > > > - return spin_trylock(&zram->table[index].lock); > > > > + lockdep_init_map(slot_dep_map(zram, index), > > > > + "zram->table[index].lock", > > > > + zram_lock_class(zram), 0); > > > > +} > > > Why do need zram_lock_class and slot_dep_map? As far as I can tell, you > > > init both in the same place and you acquire both in the same place. > > > Therefore it looks like you tell lockdep that you acquire two locks > > > while it would be enough to do it with one. > > > > Sorry, I'm not that familiar with lockdep, can you elaborate? > > I don't think we can pass NULL as lock-class to lockdep_init_map(), > > this should trigger `if (DEBUG_LOCKS_WARN_ON(!key))` as far as I > > can tell. I guess it's something else that you are suggesting? > > ach. Got it. What about > > | static void zram_slot_lock_init(struct zram *zram, u32 index) > | { > | static struct lock_class_key __key; > | > | lockdep_init_map(slot_dep_map(zram, index), > | "zram->table[index].lock", > | &__key, 0); > | } > > So every lock coming from zram belongs to the same class. Otherwise each > lock coming from zram_slot_lock_init() would belong to a different class > and for lockdep it would look like they are different locks. But they > are used always in the same way. I see. I thought that they key was "shared" between zram meta table entries because the key is per-zram device, which sort of made sense (we can have different zram devices in a system - one swap, a bunch mounted with various file-systems on them). I can do a 'static key', one for all zram devices. > > > > static void zram_slot_lock(struct zram *zram, u32 index) > > > > { > > > > - spin_lock(&zram->table[index].lock); > > > > + unsigned long *lock = &zram->table[index].flags; > > > > + > > > > + mutex_acquire(slot_dep_map(zram, index), 0, 0, _RET_IP_); > > > > + wait_on_bit_lock(lock, ZRAM_ENTRY_LOCK, TASK_UNINTERRUPTIBLE); > > > > + lock_acquired(slot_dep_map(zram, index), _RET_IP_); > > > > > > This looks odd. The first mutex_acquire() can be invoked twice by two > > > threads, right? The first thread gets both (mutex_acquire() and > > > lock_acquired()) while, the second gets mutex_acquire() and blocks on > > > wait_on_bit_lock()). > > > > Hmm why is this a problem? ... and I'm pretty sure it was you who > > suggested to put mutex_acquire() before wait_on_bit_lock() [1] ;) > > Sure. I was confused that you issue it twice. I didn't noticed the d in > lock_acquired(). So you have one for lockdep and one for lockstat. That > is okay ;) Cool!