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 8F322C02196 for ; Thu, 6 Feb 2025 07:47:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 298EC280002; Thu, 6 Feb 2025 02:47:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 248A7280001; Thu, 6 Feb 2025 02:47:13 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 11010280002; Thu, 6 Feb 2025 02:47:13 -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 E4946280001 for ; Thu, 6 Feb 2025 02:47:12 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 9293EC0EFD for ; Thu, 6 Feb 2025 07:47:12 +0000 (UTC) X-FDA: 83088739104.08.FCC3DA7 Received: from mail-pl1-f180.google.com (mail-pl1-f180.google.com [209.85.214.180]) by imf23.hostedemail.com (Postfix) with ESMTP id 7FADD140002 for ; Thu, 6 Feb 2025 07:47:10 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=LEkWnwSq; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf23.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.180 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738828030; a=rsa-sha256; cv=none; b=RD98isjtgq4dD39zustZK6dlXL6Iv0tOc5rNf1SitwzZUlXMRlKFL9BeDuHojST+AEgboP b8A/mgK2XaS28CidzgToZ6q5wsl7EdtSraVf7rap+HRTU4zMogS7qVXIJIeJh0suN7DgeP TVWlpwRKPrqsxtuC9K1kS/XFe2ngtpA= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=LEkWnwSq; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf23.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.180 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738828030; 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=biMRc3fGoaOBSDFj2CcD30wMdiQPWFVETBwCmRR/cHY=; b=QIwDDjv36SjCeHRTDKxbl6mZJ72iTpDE8/pnnSO4uwaQUtbhM+TR/fX8esW7p+NwFPFORT hloVtVBXmD4eq45uQiiE2vExrBJlCGQnbRmMMUK27pumJtVSYOJ5XkPm+nY5rMgPoUYwgj oO7HnVDRSKmCoZgUmbhDffD+zJZRiUY= Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-21c2f1b610dso15755115ad.0 for ; Wed, 05 Feb 2025 23:47:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1738828029; x=1739432829; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=biMRc3fGoaOBSDFj2CcD30wMdiQPWFVETBwCmRR/cHY=; b=LEkWnwSqajSF3EDIqdUxYa7hWKYw0SqrVRSBPZ3LuGnCoAhRJgwARolhxeoeY9lk4l +oFt2sNB9oZRE1McMRyYJ5bl7gLRSyzNX8jGFmh0Q9yQAcgc5Cw6mj8iLebDTwpjnykQ f+99ysDC/nAj4l0JAFtoTTnK5wkBImqGxYEn0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738828029; x=1739432829; h=in-reply-to:content-transfer-encoding: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=biMRc3fGoaOBSDFj2CcD30wMdiQPWFVETBwCmRR/cHY=; b=T9oEYmc7VoD6Yp2L2VT3iki61oDpiXUF9kqTKh1mF0CEDJWwcrW1SsekX2m9yigo12 fRxRUnibp+tkWZszsqJU4+5SyPxzdAli+LZZLWGeQcTkRcqJzHPgCh11jMS4AUEWPg5G QmFEfF/iHCp2ee6sgf2IfY+5eqnEYiEQ6Tv+q4H1y6HFPBmbYDzFK4tQouRDPD2TpvFZ V2K1Et6Mkvnk6KbFjKkdRk95d5jsgMvu2rvizS3irlo2FRA/dOIC8XcTyWIcz4RNtWrq 8bUh4skSp7yVwLbV9zn4AT3Mw6TB+yKFtXXG0zsc9flRojqBhO2EzJPVLkv74DYUPWz/ fvUw== X-Forwarded-Encrypted: i=1; AJvYcCV3+e8skHKNc1+26VmFpgKysHnDYfzEwoo0VGef9YA8HkYH0HbYPzY/CcxOHGrdUTQwwGhnuVTCOw==@kvack.org X-Gm-Message-State: AOJu0YzALBqVtC/hpkthX1bAW6l9dOf9fRAt3wS24niJt31iE4T0p7/y 5wB6WFiR95+TCOL3T0a1iQqJdi/mrVIHtVYoofx++r4B6VmVobiUqsuriCx7rQ== X-Gm-Gg: ASbGnctbi7vJSA48LQkLaSyp8Sn9YzeiZwwEwOf3q2cjjtTKdmpjGp5kRc+RlNlsd+u JzP35M90wgsBcF4f1YalFxSyKpRJkgJntSrpqI7oSaqdwGAaWvGkUwI7nLftiYeBCbQfN1wCgHf MOK3dLJamOsPP4ALytXTwokMfI7FzdqHWVK0I1CXNFzCkZjlKGvLGBgBsC/9gg51ZIgN+fEcrMW 7YCoOaGW1mI8htsbd1cahcoazwhtxPvHPBtXsuvbFtkjLRS8PwKf3X6sQK+dcMJz3Av0ZWUyAQD Bf8q0suOM7mkw7k4OdM= X-Google-Smtp-Source: AGHT+IFEIahDVpsRpMFTqaJvxTh7l1xv1hlgDIwPM1sGOynIktgBvR6HXJgMi6JHKvqQbqMMxnceSA== X-Received: by 2002:a62:f247:0:b0:730:399f:9593 with SMTP id d2e1a72fcca58-730399f9e50mr6203147b3a.2.1738828029329; Wed, 05 Feb 2025 23:47:09 -0800 (PST) Received: from google.com ([2401:fa00:8f:203:28ab:cea4:aa8a:127a]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-73048ae7558sm682026b3a.68.2025.02.05.23.47.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 23:47:08 -0800 (PST) Date: Thu, 6 Feb 2025 16:47:02 +0900 From: Sergey Senozhatsky To: Sebastian Andrzej Siewior Cc: Sergey Senozhatsky , Andrew Morton , Minchan Kim , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Hillf Danton , Mike Galbraith Subject: Re: [PATCHv4 01/17] zram: switch to non-atomic entry locking Message-ID: <46ttdyaiweca7qou2t3mx5jy7hefg3htrv4covt5htcex7zaq4@p6is3ukmtbhy> References: <20250131090658.3386285-1-senozhatsky@chromium.org> <20250131090658.3386285-2-senozhatsky@chromium.org> <20250131145536.d7a0483331b0ebfde80a5754@linux-foundation.org> <20250206073803.c2tiyIq6@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20250206073803.c2tiyIq6@linutronix.de> X-Rspam-User: X-Rspamd-Queue-Id: 7FADD140002 X-Rspamd-Server: rspam10 X-Stat-Signature: kw641izz6pqmzgomffqhaxyihyfj9ckj X-HE-Tag: 1738828030-971047 X-HE-Meta: U2FsdGVkX1+r9vrs6q8kGAgc3RbqSv4GjFetoZ4UPq1Rw9R9gtDnYrApC+UdCEoqSgt0XJ2P3nxPAEg0LxeWmfhSjn7gIoDzCbglItqVgT4SEJx6tSGK9NItUwyDDep5Vvq6muY/Tpg1BYoLMbcSpz6/QyEqgFo+3sNIoTjz4NgQNQeb1FqxHRVk44GeoUUVTtq67u2FkHGyc2d3vvsmKiRUAlEiCWBaMC9mnD7BvhHkMWQdvRuRAXsMbIXf98lc0FgPoS4t/4aMp1lX4GkMveMAWGmJYSRV2TfuEuww7rzSczN28bH/LAFfuM2INVLYt7vfD1K1wXHj7OUOQvbKzk4fA2IY7A+zNLfL9n8ez65iWtqslPqJWvbcLDA4f9vElnJkFpvzCgvMJLt5Ybsd/lUDYHBMRgLb9MFeSefIIl7sa23wg9InQln0dCEcJwllkPzBZSLA4SBJnMeAs1dnE6kF9X+urD3pP9PIE841PTbPZcg5uqMyjFpMA5iY0Xluy1TEdH+a2tVsZ1v810LlJJxr//crSN5M3JJCseE+4j1plEI1DQzzXp9Y+DLtkLWS4yjB7nVxzYUfev1KfGyE51mufY86woBRwcM7779ooOgVNk+dGAdgbARa4Jio67dYo/DhQwiiv1ME3Ikf/NUUWFl3QmRGXslv5ivQsk9anyiqrRhUi8Bs6b5/Tx2eRtQl1S1xxTq3XDGEN40goneRG0hKmAzEjOfeUT+JvlKrGlnlxUPJPZh51D27zJp8nkhrwHUkXh/eWSaoxZYxzy57yqdZBW191ryApiRHu748D5Pp4X4M5Hz4NHpiGKHoHmfUZdv3V0IKQ5euEcXfW0OmtKn0iHrDrJUHOfo7ds8bCEkT2Q1y8QxJeAzl0yuYTGE6Wvna4oUUG7IsKoAwWb0/z44G1Qn8bcjiY5DK7KPOGol+fE8Myd+VGolFTj/aXjA60K+kTj07nAtNSaDyuJQ vMHJYu90 Db2tOI/dZg9e+2PhB8gbzduFG2uV0EZyRVSWzbQvURDg2E8CqX5gHlnnJKHKHT8tg+y1DX/WEVXPobYUSGukohobxY+G9R+06TJTb7Og5285MPEFO/8awqzD0vqIzzorlxjZMcfN3BSQi3LXc8x/GAb8MF/78MCUf4EZGl7mel6g8d9c0HG5XGTOG6EbcvGlX7kKQI4BLlS+0D40zW6BBirUv+bQnPPgiUli6zWStgqKNeZaeNOEOWhPgljFax3iVYxAZZIF6W3VjGYyWfzS5dkvL67Fw9NNKM3Q18JRRFX9zr/cnxyyWc9iWXsja6tvi4dQKLphdwcTDo2VXEasZW40i+nGTRRaLcFjuUADDwk9wlt4DIJ92WiYE6QARivVRXfje 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 08:38), Sebastian Andrzej Siewior wrote: > On 2025-02-06 16:01:12 [+0900], Sergey Senozhatsky wrote: > > So I completely reworked this bit and we don't have that problem > > anymore, nor the problem of "inventing locking schemes in 2025". > > > > In short - we are returning back to bit-locks, what zram has been using > > until commit 9518e5bfaae19 ("zram: Replace bit spinlocks with a spinlock_t), > > not bit-spinlock these time around, that won't work with linux-rt, but > > wait_on_bit and friends. Entry lock is exclusive, just like before, > > but lock owner can sleep now, any task wishing to lock that same entry > > will wait to be woken up by the current lock owner once it unlocks the > > entry. For cases when lock is taken from atomic context (e.g. slot-free > > notification from softirq) we continue using TRY lock, which has been > > introduced by commit 3c9959e025472 ("zram: fix lockdep warning of free block > > handling"), so there's nothing new here. In addition I added some lockdep > > annotations, just to be safe. > > > > There shouldn't be too many tasks competing for the same entry. I can > > only think of cases when read/write (or slot-free notification if zram > > is used as a swap device) vs. writeback or recompression (we cannot have > > writeback and recompression simultaneously). > > So if I understand, you want to get back to bit spinlocks but sleeping > instead of polling. But why? Do you intend to have more locks per entry > so that you use the additional bits with the "lock"? zram is atomic right now, e.g. zram_read() lock entry by index # disables preemption map zsmalloc entry # possibly memcpy decompress unmap zsmalloc unlock entry # enables preemption That's a pretty long time to keep preemption disabled (e.g. using slow algorithm like zstd or deflate configured with high compression levels). Apart from that that, difficult to use async algorithms, which can e.g. wait for a H/W to become available, or algorithms that might want to allocate memory internally during compression/decompression, e.g. zstd). Entry lock is not the only lock in zram currently that makes it atomic, just one of. > > It currently looks like this: > > > … > > static __must_check bool zram_slot_try_lock(struct zram *zram, u32 index) > > { > > unsigned long *lock = &zram->table[index].flags; > > > > if (!test_and_set_bit_lock(ZRAM_ENTRY_LOCK, lock)) { > > #ifdef CONFIG_DEBUG_LOCK_ALLOC > > mutex_acquire(&zram->table[index].lockdep_map, 0, 0, _RET_IP_); > > #endif > > return true; > > } > > return false; > > } > > I hope the caller does not poll. Yeah, we don't that in the code. > > static void zram_slot_lock(struct zram *zram, u32 index) > > { > > unsigned long *lock = &zram->table[index].flags; > > > > WARN_ON_ONCE(!preemptible()); > > you want might_sleep() here instead. preemptible() works only on > preemptible kernels. And might_sleep() is already provided by > wait_on_bit_lock(). So this can go. wait_on_bit_lock() has might_sleep(). > > wait_on_bit_lock(lock, ZRAM_ENTRY_LOCK, TASK_UNINTERRUPTIBLE); > > #ifdef CONFIG_DEBUG_LOCK_ALLOC > > mutex_acquire(&zram->table[index].lockdep_map, 0, 0, _RET_IP_); > > #endif > > I would argue that you want this before the wait_on_bit_lock() simply > because you want to see a possible deadlock before it happens. Ack. > > static void zram_slot_unlock(struct zram *zram, u32 index) > > { > > unsigned long *lock = &zram->table[index].flags; > > > > clear_and_wake_up_bit(ZRAM_ENTRY_LOCK, lock); > > #ifdef CONFIG_DEBUG_LOCK_ALLOC > > mutex_release(&zram->table[index].lockdep_map, _RET_IP_); > > #endif > Also before. So it complains about release a not locked lock before it > happens. OK.