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 ADA74C0218F for ; Mon, 3 Feb 2025 03:14:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 150666B007B; Sun, 2 Feb 2025 22:14:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 100596B0083; Sun, 2 Feb 2025 22:14:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F095A6B0085; Sun, 2 Feb 2025 22:13:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id D31016B007B for ; Sun, 2 Feb 2025 22:13:59 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 5AD0CB4819 for ; Mon, 3 Feb 2025 03:13:59 +0000 (UTC) X-FDA: 83077164198.02.ACE0AD1 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by imf22.hostedemail.com (Postfix) with ESMTP id 613C0C0006 for ; Mon, 3 Feb 2025 03:13:57 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=IOotrWbu; spf=pass (imf22.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.177 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=1738552437; 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=gYP5JZw9enwSAcD80b42q4URYMXFfjhtW+2lwITmu/8=; b=tx8tmXcD2TPq3VQuXPecOODXkiaXnBMfA0lo4U35J24fjaziT+ncgX8jtdHy/cwcML+gsw M3c3M/SmVr6Xuw4zzjtIEW0zZC2/6dhbN2bib1apfOby5xhW+oZvqIjTei0W19kJlfF88Q Y7xwaiUdcNW2kanmGTlehbW+wHmp3kA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738552437; a=rsa-sha256; cv=none; b=gte1RYtA74YT7s+TijmRmpMT96MYHw89E/R8eVPi5jgqYPecFfpIKHDTWARwxQFomuS0iZ aaUdGcDJSj6vh6Wd+wtVem8a0WrOlwCRBcwYbeJwk+4K9MJwk8aF6ydovW2Ka/sAnCzAY+ jdBBkh3CNLh8m8vN5dZfSZejO3Qsl/8= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=IOotrWbu; spf=pass (imf22.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.177 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org; dmarc=pass (policy=none) header.from=chromium.org Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-218c8aca5f1so74407035ad.0 for ; Sun, 02 Feb 2025 19:13:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1738552436; x=1739157236; 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=gYP5JZw9enwSAcD80b42q4URYMXFfjhtW+2lwITmu/8=; b=IOotrWbuRJXK2bubXPSPZGtEI/oEMHrj7e/rsZd4aYtfOzcDrzYAL0rh0giguUOfNp 1HWk8DkB2G67qV9fv/1vCiyhqsjai4j+evHdkIJGpL7NdrzV+paK0UKuPJ0zbGtgaKsN p3xcOxf3Qtuy36vlU57BCdHnrHrCJU8RPhyXo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738552436; x=1739157236; 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=gYP5JZw9enwSAcD80b42q4URYMXFfjhtW+2lwITmu/8=; b=Amhb7bKrmbFWFs3uedmcJc4JzyPk/7n9ZKRQvoce6AbP2cBoCTRVVBg8MjOcjPWAjo d7y4X42HNh2lp9KJIDNyYzHlCbBNUnsaGpuabwK5qeCXlxrjZBDa3W6pyfnh2CBeeh/p 6BHx0/BvweyJ7ZoidBLGNvP3liqivjvGTdJ/maqo/HqhTwNWCD2+NNGoZp2Tzwgjm/OS mz87sbmprAuSPlyzLZlqx9z1m8sNhL0fz0YbBJqnDX9r9BOPT+kxXS96aJiBn8HV9dmU oLF5HXDx3TC6686YNO9uB2ik5C/C2PjB6oFI0nQq09ntTxAhVWTYTV/H5FbtOuKtqIZn 12sA== X-Forwarded-Encrypted: i=1; AJvYcCXMZ+wfHy1CKFK9SJCYINomSoOM+PWX2WSdeh/MQrW/BPe4Mae4tV34rMEQbiAuoQFS1ynG/ORe7w==@kvack.org X-Gm-Message-State: AOJu0YyDTDjKi0k1zXZfJbeFh+lP+oKBqEEKrQjd9D0MuZtpTTUOmqP8 Cdx7dzC0iIE1d+zI7fv+IQQqCNYyNdqWH8jpRYleBTPYOIf0YOo9xFtpj5W8yw== X-Gm-Gg: ASbGnctq/CQh0lm2zkpHfOSjlcGQIvvdxWId7Lq33Zp3qzZ0CFLY6U/+jVFy7dmN1rX UtH5elyqe5EewrKhvXY75SQcYB1BKtfadsRoNKQog6XkD8J8CnrgbMdqWoQmmm4Ad3mN7xV9j8c yDXS2pCaNz1rQ58UV1OIU/DHvJUAWxWst6qTLsDjMnRiGcG6qJ5gcwbJbJJM5dTQ8ruCYHqGP/c LXLKgtiaLiVMgP8eALpp98sG9IVtB4TUqhdMQochVyS7oVyg1CC35zn/czC6Cw2vIifOmnV6tQy gybTlSfugog7MAvY X-Google-Smtp-Source: AGHT+IHMO7w1Wk5ujysTU2+lwr2NDUbJbqgHPbRD6QwvxnV19TA5HwYvsQ/c36OpS88s6BWDcglyHA== X-Received: by 2002:aa7:930b:0:b0:726:8366:40ca with SMTP id d2e1a72fcca58-72fd0bbd6dcmr28760348b3a.1.1738552435960; Sun, 02 Feb 2025 19:13:55 -0800 (PST) Received: from google.com ([2401:fa00:8f:203:f3c0:b0:250f:9437]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72fe6a1a742sm7251708b3a.165.2025.02.02.19.13.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 02 Feb 2025 19:13:55 -0800 (PST) Date: Mon, 3 Feb 2025 12:13:49 +0900 From: Sergey Senozhatsky To: Yosry Ahmed Cc: Sergey Senozhatsky , 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-Stat-Signature: 9t68umy1z9ocbrmajyf9469bum6f4tyc X-Rspam-User: X-Rspamd-Queue-Id: 613C0C0006 X-Rspamd-Server: rspam03 X-HE-Tag: 1738552437-542225 X-HE-Meta: U2FsdGVkX1+BLXVNFSwcpvoziLo3E/9PyTVpnAos+eTKwr428LSeJPJMXDCK+DPzZXFdMuO7IyhGwZrvNqbXsV9dV/M0boVNIBetzh7u1sT23HQUyGVDXLUhSBdUeWqqKtxnkkcxe8JX/wn+aHvi4E0SUNIyHKc5spzFMA8ogFHW7uLbQFyUw5VNsnsrrbCLqGgj6V+QM/cgMx+ENumDv9RjfPwErkk0LXqU8KwsJIi77d5uoP8+X5cRjMOgwHsMhSIaq0dKgpWx5YFK/Bvtiuu1jiQtK+DZ42dGUsMlMIHa0diIJS6ful2q564zNHfsVXxKzkSXtgwmKwcZvSBrCgTW2YeEVJAlomVmRvwV3VVBR5HU5c0lNjzngLmirw/piuT+U1OkvyoAxxy4WECb5svhT4kwB4uxUgCSYXVdsoJXLzw63NYivwzFONiLK3tBAFeTINMuJxXgmf9UzGFWvGLhUREuY4+NocasveHaKb/It9n5+SFUWDhh+zuzaIqa9r2w5N+bOql6BN4EUFpfIFW2sb4EwcMpW+wg5RwFehrM7PUIAKzmg5YgcBOj2x+zwpr3pjDiIEkMMRFwU3y+Rm7GqKYRNuCiS9DQ4/sRjTtgAmi+81V/i19vASV1FKDnBsujGBY5keQD2/FZs76PznGB9LOXcS3PRqd7g9u/8jDP2FfAfzZVR5tqaH+XlkJXlSnOtPIH9rxcdkyvjmHLx0C3A1Yaz3I0QnyYpATf6Oz4rnTME1WjhS32VAly6mLAs0MSGbRM1Krt0nc5znXJJ9JvfNPRCYfFmbeemDBbWR6soRASJ9s4iAe5F3GyUb11i0WpoazvlFmreS5u05Xqh23yOf4OjnOEHxY4EPC44xgL3oRMDfM6r3JZdx/qwk9c76khseOCI6fV3egsROM/MRNRjPA9e3CjmTWv2RwiDqQiZjDxcOvQ8brgiU6OkYLQpCQG2jrAEStRJyGmQ3Q jObHBEp5 52m4EUuHL8M/AzV1/k6+JpMagckw+roJ9RmllgPJjOZWrAmbRgiJCGdsUqqUaNHcvn3D2btnp6VDI+Eip2uBlThRjnU1YQCRT5Zk5ZFN+wWkJi2r6ICGSZ27RVHmZVeX0WcjxZHlc0xzsLCLVCIpdPhhQ8rHhwEuIUyDnxHGiSv1+86TT3ymvPs6MQu7z30mvmyf326wxubYb+W+46WlJDqp5+6Ag84gPcUF4N4f3IVHdsu5t69h+lVoob2Yq6MMNEAJT/aHESZAyEj5msVgMFuktYp8VFy2mAQWFMYJXZaCiBD1EcRTP50yfTcdPBHLH75rzQI3jPgmStTpkzBh2xqxBzapzt2+hZc/9bKgz72uedc8= 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/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(); } > Just my 2c. Perhaps we can sprinkle some lockdep on it. For instance: --- mm/zsmalloc.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index 956445f4d554..06b1d8ca9e89 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -308,6 +308,10 @@ struct zspage { struct list_head list; /* fullness list */ struct zs_pool *pool; atomic_t lock; + +#ifdef CONFIG_DEBUG_LOCK_ALLOC + struct lockdep_map lockdep_map; +#endif }; struct mapping_area { @@ -319,6 +323,12 @@ struct mapping_area { static void zspage_lock_init(struct zspage *zspage) { +#ifdef CONFIG_DEBUG_LOCK_ALLOC + static struct lock_class_key key; + + lockdep_init_map(&zspage->lockdep_map, "zsmalloc-page", &key, 0); +#endif + atomic_set(&zspage->lock, ZS_PAGE_UNLOCKED); } @@ -344,11 +354,19 @@ static void zspage_read_lock(struct zspage *zspage) continue; } } while (!atomic_try_cmpxchg(lock, &old, old + 1)); + +#ifdef CONFIG_DEBUG_LOCK_ALLOC + rwsem_acquire_read(&zspage->lockdep_map, 0, 0, _RET_IP_); +#endif } static void zspage_read_unlock(struct zspage *zspage) { atomic_dec(&zspage->lock); + +#ifdef CONFIG_DEBUG_LOCK_ALLOC + rwsem_release(&zspage->lockdep_map, _RET_IP_); +#endif } static bool zspage_try_write_lock(struct zspage *zspage) @@ -357,8 +375,12 @@ static bool zspage_try_write_lock(struct zspage *zspage) int old = ZS_PAGE_UNLOCKED; preempt_disable(); - if (atomic_try_cmpxchg(lock, &old, ZS_PAGE_WRLOCKED)) + if (atomic_try_cmpxchg(lock, &old, ZS_PAGE_WRLOCKED)) { +#ifdef CONFIG_DEBUG_LOCK_ALLOC + rwsem_acquire(&zspage->lockdep_map, 0, 0, _RET_IP_); +#endif return true; + } preempt_enable(); return false; -- 2.48.1.362.g079036d154-goog