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 224C7C021BE for ; Thu, 27 Feb 2025 12:05:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3E6F66B007B; Thu, 27 Feb 2025 07:05:40 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 397506B0082; Thu, 27 Feb 2025 07:05:40 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 25EA36B008A; Thu, 27 Feb 2025 07:05:40 -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 074FB6B007B for ; Thu, 27 Feb 2025 07:05:39 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 213CE1411B9 for ; Thu, 27 Feb 2025 12:05:39 +0000 (UTC) X-FDA: 83165595198.12.FA1AA4E Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf04.hostedemail.com (Postfix) with ESMTP id BFCE640006 for ; Thu, 27 Feb 2025 12:05:36 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=pChr+Dlg; dkim=pass header.d=linutronix.de header.s=2020e header.b=5yfdr6jI; spf=pass (imf04.hostedemail.com: domain of bigeasy@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=bigeasy@linutronix.de; dmarc=pass (policy=none) header.from=linutronix.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740657937; 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=VgpeASaw3foJsIkn7/JOj3RC9lzZoTbMn9rZNSif0zU=; b=I7UXqUIiFD6/vzh2/RW9wNoasoQ/0L6m2LQMdfppkbQ2Bd8hi8FoJxrmD5/5MsADMO3MUV CWgxBfUhZDrcwbfFOXNd1g/El5Ma1XXzTGuaNAJDZAtFcXKQpKb9Uqjl6xRDD+QKdOX7ES H8rkCn3Aqwb1W0qZb1cwwBPk/0NWa00= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=pChr+Dlg; dkim=pass header.d=linutronix.de header.s=2020e header.b=5yfdr6jI; spf=pass (imf04.hostedemail.com: domain of bigeasy@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=bigeasy@linutronix.de; dmarc=pass (policy=none) header.from=linutronix.de ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740657937; a=rsa-sha256; cv=none; b=1+6PXQZoM3CWLbvEKk5ivDmfUzkqBvDyAwGXRSJL3TgAhO0uPFGYOvTLFEODyjwCQmtxWT kIBQP77q2HqWiCEuOex7DebYxlM7otf3fKU3OUo6L4UVF1Y3hGUQFT1YLSRmWcPc7CUmzb bD3A60I9Trf0JdM93ZSrzKZeRAJkKJo= Date: Thu, 27 Feb 2025 13:05:32 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1740657934; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VgpeASaw3foJsIkn7/JOj3RC9lzZoTbMn9rZNSif0zU=; b=pChr+DlgAb76BJa81EjYlLMMdyZCRXaBTw7hOWGGc8O4fJgwndNT6jKrTI5A3abMRvaspU xN293YQHK5bpERucSTKTuV16tUh+Ajn3roDZTmtdNrPl0YsuICGQnw3DEvjb8hDK43g3zD BrUi0HobaXmlqX1/978VRJ8xpNQYqkeUUMMT6PonsgbcEhI943kjT7cL1qz+tycSqTazw3 DmmsQmaqYszjXBXoZHzMtDDyR65/Sc+MG6YaMc5Zr9ZlAOKCEuazel/0WLs1C7Hpw4Vrsx eDQ660is/b58laNJL6siozoaIFsT3szkZXlMp/oyibtl4IFVvcdLePkHhanH2A== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1740657934; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VgpeASaw3foJsIkn7/JOj3RC9lzZoTbMn9rZNSif0zU=; b=5yfdr6jI/Pd+03S//vazmRT6JL/Odq5LRfHjxJSy2DpUkWInboEXPPd5IRYc7OVVGHGQ+F 0b7vuFFx3tKQ7sDQ== From: Sebastian Andrzej Siewior To: Sergey Senozhatsky Cc: 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: <20250227120532.OsZr4v2A@linutronix.de> References: <20250221222958.2225035-1-senozhatsky@chromium.org> <20250221222958.2225035-2-senozhatsky@chromium.org> <20250224081956.knanS8L_@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: BFCE640006 X-Stat-Signature: 8ondn3cksaapthwzmdmtpn7pdgd9etz5 X-HE-Tag: 1740657936-508254 X-HE-Meta: U2FsdGVkX18QAbVTfdif6NqH68Jlis2wQJf7ywkz7cVX5UFKW6oyHii73KE7nIYiPpJpR0poLhbYj99aQWrkrCcS0npbXscgznFCXD8jtNqzrpvGyt0mQDyabIl1lUh3BkHJ8rFqfigRk9aC8cOtUnEawsJRrnTajDCwTl2flFui4tnNrhjC9jZ+Oic9ZREITPRrkRWjMia4c5MZzF7GfZ3Rt5/dBHUKhrdhep613Gcz1FKRm23KzmDgBhiwH7Fj7Un4NHRLWibpKG+HYTI/mew9OE07N5L1QstJ6faARiKTj6m4HmxJE+I0cRHtbP2QFXcOovn9LpSDMW+J3sV5/ZN0d8BCPjP7peZ6aSmIjCtbBw5czb1Iyfdr6un2lKTiHDP5aEctjCDAsBtsOO6iFEcy0eirzV0g4VpR2fgGK4gbUz9ctRvv+0gdeyeNRV3v0yWMKcV8uUExlUJ2dOuM6ctoIFTqfa1lFsDz7g7knoH39Ntz8du7zh7dEOdnjlGIelasuzFEwDJHNf7PbX4+GN+J1YLV6riYiFQjf7UWCHUa8F92y3O4zdZdDbhjMzb/lzr3zySb3dkj2ltdw0FnUjWKwZmHWR6YTrMH7D+AL3Ar7wTPXjQoVxwevYLD4ziUnZAdxw/6djgZ3OdlD3ASm8VjuoJIh8N1YTIOtuljmgzma8WkkgTAqwqOJtQCTa7adEHFNvSkkXBX+ODDjahDIZX9INnG72eFY+kPfVtWcDCbaMka1w3EcXx2yHCIko/bqg3vFKBEiUaRFylNPY/SgQXY7PxNVgPuNVpQYO6VWlweMTQse1NV8p6n4aNFIlpdKp1Oez+HdqMtw8rNdjpAs4e97JGHClVWG0UuSwIESnOX7EjYNCWR3GZPht0sm3Qtu382fUwzRytbVWNdk9WLzNU/uKY5CttDTZ/um7SuYeuaI+DPzI4uzd/8+CggJ2MccE2qKuU4L0BDw+zqeC1 6sqjG4AC Z9QoADq+kr3iDMVRGFkOOaJxuvcHgRi5ndnawdM8SxzO6IzgM+VRHmH6KqxLaoroUgfKOzOSPk/xff0WOdZfcAKf142ObBjOXL3uu5o8yHYbCDvxHMWHwdQVg0e/3EjB3iQg3EKcQvpil5fK9AACX98CaxbLQu5v3nX/9W3CMJNFzpNE0NvBvcme89jXTKl6Y8ybf 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 2025-02-25 13:51:31 [+0900], Sergey Senozhatsky 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. > > > 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 ;) Sebastian