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 8453BC02192 for ; Mon, 3 Feb 2025 21:11:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 00D936B0085; Mon, 3 Feb 2025 16:11:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id EFE766B0088; Mon, 3 Feb 2025 16:11:28 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DED906B0089; Mon, 3 Feb 2025 16:11:28 -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 BC2FF6B0085 for ; Mon, 3 Feb 2025 16:11:28 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 7354DC0490 for ; Mon, 3 Feb 2025 21:11:28 +0000 (UTC) X-FDA: 83079879456.17.401ACD6 Received: from out-188.mta1.migadu.com (out-188.mta1.migadu.com [95.215.58.188]) by imf02.hostedemail.com (Postfix) with ESMTP id AE8228000F for ; Mon, 3 Feb 2025 21:11:26 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=u+vLNvVj; spf=pass (imf02.hostedemail.com: domain of yosry.ahmed@linux.dev designates 95.215.58.188 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738617086; 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=jnXiGEsRLvwqZF8PATt6EDp1c9/m1BOHmpU4ny8u7CA=; b=IaEQbDArooj9qBhyzEKmYoZBqQ9PATPRbGGK4CChnqCxsKFm3ux37m+8mwcKFwZxtS06SO iYMCw0PGnOKP5dC5abzL1La18y30F7fGdLHbFr2nePx+hjEDnObwRxeaEHyb+Ylkba9tbx R5EI8YKCNHbXf/nzK7wEnzZwC+b1vJ4= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=u+vLNvVj; spf=pass (imf02.hostedemail.com: domain of yosry.ahmed@linux.dev designates 95.215.58.188 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738617086; a=rsa-sha256; cv=none; b=vzJBL274pjJMfsnNjTq4VZ0nzQsI/kQni9NEIlqlckqVSqAOplfQF11GkR5eCvKjxYb51/ kXtcbGdLwE2FadPWrD2kUiPNDKdw5WEa2V7J5kOnA0Fo9HShmgAfj8lefUCKx8sLib05Hb 2di/Voqh7JJtxZEpyQWmtVnv99fkmWE= Date: Mon, 3 Feb 2025 21:11:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1738617084; 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=jnXiGEsRLvwqZF8PATt6EDp1c9/m1BOHmpU4ny8u7CA=; b=u+vLNvVjQka6QnN381FMvUXEjlpKoy0O5E7ZWr8J1jmLeeoBpBS06gL+H/JZ17Tt+VImT9 rIMMYrvZyyty2S/x4fkTwqpsOrBuzl67xWS4v/w5fgmvwEyowo61R2zLR0LsSSaLMF70vG 0XzRgNjz5QdRE/k+I4HWJRjuzIJLl+8= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yosry Ahmed To: Sergey Senozhatsky Cc: Andrew Morton , Minchan Kim , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv4 14/17] zsmalloc: make zspage lock preemptible Message-ID: References: <20250131090658.3386285-1-senozhatsky@chromium.org> <20250131090658.3386285-15-senozhatsky@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: AE8228000F X-Stat-Signature: o14ib8zutk7hoaryt1yw5qyykhzxybto X-Rspamd-Server: rspam08 X-Rspam-User: X-HE-Tag: 1738617086-981911 X-HE-Meta: U2FsdGVkX1/eXVi96zAh0HLdwUvAwVhnXcsx+2ov9LRLcbOCefEAj8oCsFJykt0P55r5VUfZfPR88SpQXtbExHeYst7MytV37v2Ke1C07Y5Twyy4WthO6sBqWnei+LYvM7yfcyhlO7OLb7hS/9SZgYrRqKsxXEbLI3802e8YURlSUaf/E2IIxsz38gojvfxkPY1TFhARr0OS5w72WbgzK0nEBW5t4AFQ7XaW+DDKmEoPrMwWKEHJkTThihbKe8N8cD6fyZYgH56y+l2467qYVO5imP+W1li1RHrP2FQbkia1hu+//fBa7yq28HeQr/QfkKFIOp/Axk7F7YNbXa3CwWcM5NtpYyxb+KBCa7iVkqfGeREQDpWiWZHLY89VxS0n9G4DK+F9b+ZDCWXhKwQPtQMEQFcrinFzchWDiVWuJfE/EdWdNAP6GAhKLz3y1oEiw1GF7awkOW/fmq6uHJoU2/ChNjDiX/X4Wrkov4DrZ7uDmuC5j+Xre6o4cxtZMfNOVMRMBAw/sha4Xt58Ju2Nd41DEPORgbu0G90yy1D2fyiu0YJNYQD5AXCdyQ+jw15UMFesZPJPhovt8WyNTh1wBJSGzEaSAAoS2Kn8WqTqaVhHRcPDwy4f0WJjCn95ZPC9kdrm0eGK7qS0huCnmHOr3LShHhTarqvN8mRuw8fseyAiCRZ7inVrP/d9yRO/UAwoCKhHcIh3xblcWNql5EvkxR94l8FLdXuRspedVc1ZQUKhMkjmYV5ke6uT4dsFskkKs6Z3qnemTWk1RueDlrv3n6b1LHqbY6tzKZ8WzyvjUt836S1NP7AZo8RTVS/cMjxoBOhrrKUI+sv6npo50Ls+RkBLhnHePYMLLCLcnlZ+nUO3Ay4CQbgIjGDJ0ypXx/O+rpu8n3+m6jhCN7wmxNchGMlzAaKXdQWpeSxC137QCTOai7v8MPS6ZYp5xE9PIuLUR14rFEDiGhAUlmETEv9 4EODiHSu vMEsxLRINpJPkcxWVVnlgYqze18JRP3kPCDaHcUQy6fZrsUMg/XEGhhiT0fRajIR7nZgxLLfdSR8ZHX6RSxEb4Or0sTDbkQCvrysjrB87+exr6nxjX9j4cShe0xZbJERCziYXP4zKtNe5R+jeF0GThzn8filRuTlYG0JeijGf6LCA0x37fOgHewAr8mfGA3G1yMkqlfTWonyQwe/LyYC/YaaU/F+BMC0hhCVQp9GNAVgfBysnQzYoq3+iZ+Q/K3nHJop6WAboV1KOJtNnXKmHirDaPeC5lrbOQdioFS4nMpdxPe7YS95ByxiFuKEykHSdrw7tv7ud5UCjCBAvQcTCiooEQmyW7gMHbgj81uOx/he2SEfql6E0hpnzp60xGpPQ5SwL1wvC9Vs5sk5IBIFkf7211BIQZ1VhTx+8BQk+m53SY2w= 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 Mon, Feb 03, 2025 at 12:13:49PM +0900, Sergey Senozhatsky wrote: > On (25/01/31 15:51), Yosry Ahmed wrote: > > > +static void zspage_read_lock(struct zspage *zspage) > > > +{ > > > + atomic_t *lock = &zspage->lock; > > > + int old = atomic_read(lock); > > > + > > > + do { > > > + if (old == ZS_PAGE_WRLOCKED) { > > > + cpu_relax(); > > > + old = atomic_read(lock); > > > + continue; > > > + } > > > + } while (!atomic_try_cmpxchg(lock, &old, old + 1)); > > > +} > > > + > > > +static void zspage_read_unlock(struct zspage *zspage) > > > +{ > > > + atomic_dec(&zspage->lock); > > > +} > > > + > > > +static bool zspage_try_write_lock(struct zspage *zspage) > > > +{ > > > + atomic_t *lock = &zspage->lock; > > > + int old = ZS_PAGE_UNLOCKED; > > > + > > > + preempt_disable(); > > > + if (atomic_try_cmpxchg(lock, &old, ZS_PAGE_WRLOCKED)) > > > > FWIW, I am usually afraid to manually implement locking like this. For > > example, queued_spin_trylock() uses atomic_try_cmpxchg_acquire() not > > atomic_try_cmpxchg(), and I am not quite sure what could happen without > > ACQUIRE semantics here on some architectures. > > I looked into it a bit, wasn't sure either. Perhaps we can switch > to acquire/release semantics, I'm not an expert on this, would highly > appreciate help. > > > We also lose some debugging capabilities as Hilf pointed out in another > > patch. > > So that zspage lock should have not been a lock, I think, it's a ref-counter > and it's being used as one > > map() > { > page->users++; > } > > unmap() > { > page->users--; > } > > migrate() > { > if (!page->users) > migrate_page(); > } Hmm, but in this case we want migration to block new map/unmap operations. So a vanilla refcount won't work. > > > Just my 2c. > > Perhaps we can sprinkle some lockdep on it. For instance: Honestly this looks like more reason to use existing lock primitives to me. What are the candidates? I assume rw_semaphore, anything else? I guess the main reason you didn't use a rw_semaphore is the extra memory usage. Seems like it uses ~32 bytes more than rwlock_t on x86_64. That's per zspage. Depending on how many compressed pages we have per-zspage this may not be too bad. For example, if a zspage has a chain length of 4, and the average compression ratio of 1/3, that's 12 compressed pages so the extra overhead is <3 bytes per compressed page. Given that the chain length is a function of the class size, I think we can calculate the exact extra memory overhead per-compressed page for each class and get a mean/median over all classes. If the memory overhead is insignificant I'd rather use exisitng lock primitives tbh.