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 46C62C0218A for ; Thu, 30 Jan 2025 04:01:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6CCDF2800B7; Wed, 29 Jan 2025 23:01:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 67CC62800B5; Wed, 29 Jan 2025 23:01:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 544032800B7; Wed, 29 Jan 2025 23:01:25 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 31FE72800B5 for ; Wed, 29 Jan 2025 23:01:25 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8E00AB030F for ; Thu, 30 Jan 2025 04:01:24 +0000 (UTC) X-FDA: 83062768488.18.848EFFB Received: from mail-pl1-f181.google.com (mail-pl1-f181.google.com [209.85.214.181]) by imf15.hostedemail.com (Postfix) with ESMTP id A049BA000C for ; Thu, 30 Jan 2025 04:01:22 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=RKFUxbbI; spf=pass (imf15.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.181 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=1738209682; 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=0hVhgDYWdgmBmFX69W+KC+RzCnVk6gFSJciJit7shNE=; b=1ANJatJnZofBHL4+/6C6yohDV4TUKQ5/ulE5EVJaiAHoqitUD1v1gBw3xnca2OBEnVrf9K ugWwFMFG21Rr+PaooYITP/lgSxrOnN6FhyhY/dqDUczJLgxueYDYDr/gUZxLZ8OhavXvWW CwKDP/89dNmcmeIPJ7l8vOItbbCcWtA= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=RKFUxbbI; spf=pass (imf15.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.181 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738209682; a=rsa-sha256; cv=none; b=Pr7rOn8PHXWlgjsnqV7B1whKaEJkQeH2ZdSsMz8hSP1+XQhIvS4wwlPE+dnUfYnhsmo2kj hNiUXOxJwLuhJxoNeR/n284ed464TCYRzRKM9n7syWhC9oehrFlcx7PgsyJZ50BqJJ4E96 /G5RwIjugd31HpikqAQRfyw5LKZiJfM= Received: by mail-pl1-f181.google.com with SMTP id d9443c01a7336-21bc1512a63so6055115ad.1 for ; Wed, 29 Jan 2025 20:01:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1738209681; x=1738814481; 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=0hVhgDYWdgmBmFX69W+KC+RzCnVk6gFSJciJit7shNE=; b=RKFUxbbIWZejxqVBu50qb9L7PZfhL+Vcv2Iu01vvcGMSjy7bQIc9i4gsH70K7Pt1oE vRCSeTEZQ69UEfpB1CH6V5vFwu74PjNJunVv7D8k5Gf63z4DqxFLYhkgkKN8UkM4LbEX ABp6L7Fw7007Q+WZyKwvh8NCpP+ZkAmaMWuQc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738209681; x=1738814481; 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=0hVhgDYWdgmBmFX69W+KC+RzCnVk6gFSJciJit7shNE=; b=kKZS6A/qiwgPDCpMNM7hMi3zBbpJpoS16Jo3Zu83axP90fMrrm0Yy18LlcYZctlWfu 7S68LG+yZQQO22pQfJ5fajcU3JpeCfAPb8rH81W3LunzhYogsCPCgiA+OHoPO40Pn6Ux 9iQj2VvKUrxZyZqXpqFXhauzz3pOCRPtsHsNQdVUX1XssqdaQkDUrtDZCBrHUoopyF1/ jTCe7QStijBT5KR5XklGVLowVdvKe0KLWNzKuqjfLUAtkNmkPtJV3fOsuwZrmsqYx+Gw jpEiWIa8s3iPbDA92z9xmQPJ4asE7nDMTdSWzUhH5UdrWBnhKxUwMrh6zkT7EG3fVJ9a XsCw== X-Forwarded-Encrypted: i=1; AJvYcCVBCM96KfOcZVJCwTr5L4vglE3EqPck8NNo97G27YWYEbRmQn0ethBiY4F/CrVsa0DmQSCMXqIwFA==@kvack.org X-Gm-Message-State: AOJu0YxbuwkMtyuNsAI8XNcfZEcnrH4/UqMSQLNI9Mw+IPEvGin5vK+G 9ZWhMXITQmlJwU6bouVVg74DInXOCycbhbQfJKZYC7dapz5ooxgSK3XR/VCChQ== X-Gm-Gg: ASbGncvM1IiKPqlLqYafjcaeGy7TAvMMnp3NmjbdFbE1HjZK9NeQFaJpDj9q8RCuWBe jCTgCukIY+zlSXbfbzA8qPMI7IYDIiwm0SzsuCS2U+52jEz06jb1NvLtg92Zpo+pzYnVsbGmBH9 sx0JaWl8gNEBEY0i8adOleQJPlQ2mz19ZCpZlEwxFnV4mnmf9MJXIQZSSTq5Q80apSxyG3jM0bH ZQiANm2cOJRqJQeI5bB7/VYMs43E4X9FHmCtrvRjb99G0vMQk20oT6UFQGgWu9xZRb2ji6thwkW qptU0DqJqIo/eYuSgF4= X-Google-Smtp-Source: AGHT+IHv4h2aDNStCJ9Jiihz/9zu58ILtvcW39t0Uh/3SEdXBTJpO2f+RHWrNht/wapJ7ehu3KMDyg== X-Received: by 2002:a05:6a21:348b:b0:1e1:bdae:e045 with SMTP id adf61e73a8af0-1ed7a535e98mr9805762637.23.1738209681278; Wed, 29 Jan 2025 20:01:21 -0800 (PST) Received: from google.com ([2401:fa00:8f:203:d794:9e7a:5186:857e]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72fe6424f8csm341096b3a.44.2025.01.29.20.01.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:01:20 -0800 (PST) Date: Thu, 30 Jan 2025 13:01:15 +0900 From: Sergey Senozhatsky To: Yosry Ahmed Cc: Sergey Senozhatsky , Andrew Morton , Minchan Kim , Johannes Weiner , Nhat Pham , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv1 1/6] zsmalloc: factor out pool locking helpers Message-ID: References: <20250129064853.2210753-1-senozhatsky@chromium.org> <20250129064853.2210753-2-senozhatsky@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: A049BA000C X-Stat-Signature: 9ytzr9sh7x74n97agk86njb5ekymy65r X-Rspam-User: X-HE-Tag: 1738209682-985031 X-HE-Meta: U2FsdGVkX1+dlRDqp4ZXaRabpAOpafFa3t3dhn4MBv7wmqxH5NYNol+HhBG623ldcPWcNf0MTdevqwuzEZ+J3CSkfilg5KZ/syU9G6sjI2J3bIdFGxa/otFyPvtPuKoecTO5/2Y0DdzM51uF5RJNysJCnN2S+SntnA2U2W781kZsO9c6/qUwSD+oCDII3wOkrEumk+Cek3s/jgJQrpB9/oue0DKeIajnBTbf+RzRsRNz1Y/RI4rZBq+9pPxinOecq0pSzqjI8dgQH9Cnk5auvjJ1HxwNyh8LodTXIisCOJ67SpUXpGcRlze1doOp1RZw9BWEhLoJL8ZM/OnX0P+i8d/R5y0oFAME6NnVgWVYUvbeAZ7F89ghAK7uPDYPRzTrwG3OReAaBPrOYNtFOe/JCtkXzhEYfx2xznM5NbrFd9tJ+xqVLfnhzGXwenkkAUQBXnBtRO4Mr5pGVVf+kToA1w5j2hVqy1L8CKdpqgrkVFsnGbGVXtch23F9qrHxtR7ekM3ALW2LuhsvF/jMF7VkRDytWhvef/v1k5uPUIS7XpaYYVzUpUeeo1hrWulEdKVvgtNwtOWNfYDdKoilvvpBNzD7mZgkxpql8AaSWAhHflnbb+qI68T4k6M4kkP/JlSJdXOVDgRJItOJx5MXaDqEjO0uoSrJISaFPe5w8LmS24kljlZNAAKhcFFahrRk/bCE4m9GUI1xqDJFgCjmnBF/sdCTbRbi00Y1Ns9RJFrMNVJ53IwyjGSPBZEBVGRCGrwXdMshHKyKQmqSKy9CB0Amoi9FMeuvkWwfevA8pylkpnthv8Be9MClN9ijFV23MDsWos8WlinGMWty2RDZzqCxgn8UYpZSongqvAcOx/x5QOxOP8EZ6Vm3RvktnbCHa6Gj0DnI84ubjjdCNkzJ2jodHcdTGBnkgu4fP0MrZIu58GkvJErG0xkHm34UMPv7wbWhGP1zN3fyScr5z7b5qL6 jc3dXIyb UJz7oqvc1c8QehoJ5D5DauYN+1Zt65OdH0n7kQaRwux0Dm7cfl45kmi7NMq55wk6gnQTkkQvV2jzCHpbQo4ZIjZ5gaVsi07AsLqjpIcx5FXNwurVfl39I6geuYY7mTNwmb9XchsKiTGSxcZG2GaW/q+0GeCi3NiHl9kPsc6E9CHBQQRNyz5Bo21BN3hfuf0XXq1QD0Cs5jMuvgtLTGwTOvOytB/n4w8JIiNyETxndK19YC4qg68uWRsTZdCuBB9nYPjnAifUI+8ayUAHoI/Fz6hrbK9tjvigmviaVEPUhYGd663PPhenVZTZOAzAYUVh4lt6h1Ju27t1hUdGW0ra/6asVaHW7nymYyhLOCj6cNJFu1zk= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000384, 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/29 16:59), Yosry Ahmed wrote: > On Wed, Jan 29, 2025 at 03:43:47PM +0900, Sergey Senozhatsky wrote: > > We currently have a mix of migrate_{read,write}_lock() helpers > > that lock zspages, but it's zs_pool that actually has a ->migrate_lock > > access to which is opene-coded. Factor out pool migrate locking > > into helpers, zspage migration locking API will be renamed to > > reduce confusion. > > But why did we remove "migrate" from the helpers' names if this is the > real migrate lock? It seems like the real problem is how the zspage lock > helpers are named, which is addressed later. So this is deliberately. I guess, just like with page-faults in zs_map_object(), this naming comes from the time when we had fewer locks and less functionality. These days we have 3 zsmalloc locks that can "prevent migration" at different points. Even size class spin-lock. But it's not only about migration anymore. We also now have compaction (defragmentation) that these locks synchronize. So I want to have a reader-write naming scheme. > > struct zs_pool { > > - const char *name; > > + /* protect page/zspage migration */ > > + rwlock_t migrate_lock; > > > > struct size_class *size_class[ZS_SIZE_CLASSES]; > > struct kmem_cache *handle_cachep; > > @@ -213,6 +214,7 @@ struct zs_pool { > > atomic_long_t pages_allocated; > > > > struct zs_pool_stats stats; > > + atomic_t compaction_in_progress; > > > > /* Compact classes */ > > struct shrinker *shrinker; > > @@ -223,11 +225,35 @@ struct zs_pool { > > #ifdef CONFIG_COMPACTION > > struct work_struct free_work; > > #endif > > - /* protect page/zspage migration */ > > - rwlock_t migrate_lock; > > - atomic_t compaction_in_progress; > > + > > + const char *name; > > Are the struct moves relevant to the patch or just to improve > readability? Relevance is relative :) That moved from the patch which also converted the lock to rwsem. I'll move this to a separate patch. > I am generally scared to move hot members around unnecessarily > due to cache-line sharing problems -- although I don't know if > these are really hot. Size classes are basically static - once we init the array it becomes RO. Having migrate_lock at the bottom forces us to jump over a big RO zs_pool region, besides we never use ->name (unlike migrate_lock) so having it at offset 0 is unnecessary. zs_pool_stats and compaction_in_progress are modified only during compaction, which we do not run in parallel (only one CPU can do compaction at a time), so we can do something like --- struct zs_pool { - const char *name; + /* protect page/zspage migration */ + rwlock_t migrate_lock; struct size_class *size_class[ZS_SIZE_CLASSES]; - struct kmem_cache *handle_cachep; - struct kmem_cache *zspage_cachep; atomic_long_t pages_allocated; - struct zs_pool_stats stats; + struct kmem_cache *handle_cachep; + struct kmem_cache *zspage_cachep; /* Compact classes */ struct shrinker *shrinker; @@ -223,9 +223,9 @@ struct zs_pool { #ifdef CONFIG_COMPACTION struct work_struct free_work; #endif - /* protect page/zspage migration */ - rwlock_t migrate_lock; + const char *name; atomic_t compaction_in_progress; + struct zs_pool_stats stats; };