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 54897C77B70 for ; Mon, 17 Apr 2023 11:54:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D53548E0003; Mon, 17 Apr 2023 07:54:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D04868E0001; Mon, 17 Apr 2023 07:54:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BCC2C8E0003; Mon, 17 Apr 2023 07:54:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id ADDBD8E0001 for ; Mon, 17 Apr 2023 07:54:27 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 63BED160532 for ; Mon, 17 Apr 2023 11:54:27 +0000 (UTC) X-FDA: 80690725374.04.01BD2A1 Received: from mail-ed1-f45.google.com (mail-ed1-f45.google.com [209.85.208.45]) by imf17.hostedemail.com (Postfix) with ESMTP id 98F5340015 for ; Mon, 17 Apr 2023 11:54:25 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b="W3Z/9kFr"; spf=pass (imf17.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.45 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681732465; 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=cItKBO+T06PzHqKskJtbZMH9b9s+HGyW1napsrYk6R4=; b=mLkhqJb2LXkhfYllQb7JAjTr7lN5x25b+BwzfiWr8W96BFHFn8eNwconUkkIuth4QOQAwt ncaafLagQIMp/AyHpaMBWchXqqJYlL55ow0+QVQVCcAx/C4yQ+Uu20P6ke4Ho6PlO2ma0E 6MOprcEi9Jck0/Tt1AHQCKKD7dLDkO8= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b="W3Z/9kFr"; spf=pass (imf17.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.45 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681732465; a=rsa-sha256; cv=none; b=fdpvoRvay8bSTc47iC1aUh9Cug6UVRp5bxwxcuiPZWthAoeV0pwNDmck/MXS8CmCO4ffzd TF31BSF2Bq6jCMdMTrDiJPMbQtwNA0JrI7aQzT19KjYHY2p635dGABI4licrrFCxxIWbAw avBuca/WYwUbESwGJmBZBQtmsg+n2aY= Received: by mail-ed1-f45.google.com with SMTP id 4fb4d7f45d1cf-5055141a8fdso3348841a12.3 for ; Mon, 17 Apr 2023 04:54:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1681732464; x=1684324464; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=cItKBO+T06PzHqKskJtbZMH9b9s+HGyW1napsrYk6R4=; b=W3Z/9kFrMfz7ubLB3nwJYFEqJi3JzIX6K1/YcqvsQaT9VNbvVGsG/EtwdytaO8NF3j i4+tNUBAtQ0awyXQEzCMD5OXpasdzkW2wRwhnUzB9zLdK/2MO1GJczhwDaMEqNgDJfLW dB3RZGXSgQABzVEzvMzqKFZ31q/xx9ESkUzB3tEtC8fiBfwfOPqKR4uL/98kYdnmr8C3 VunkYTYoahczoIPLmbYhtNiJlJyKF/jECvd0FbXZibXwlG//2PymGGfPtBmMemmxlu3I fQSyOr7KK1PaQrqkYlRquknleQD5KrDxXSwfOy5Vsxlwz05jFcswa81GGFJiD1UPje9C FxNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681732464; x=1684324464; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cItKBO+T06PzHqKskJtbZMH9b9s+HGyW1napsrYk6R4=; b=fW7eh5r4kBvj/DsvP4lvaR7sQF2Ten/P88rsvMKpHFtKUJUX4yOgtj6XDigE0VNo7F Lo7jdLR+DLTGqE5EQCeDv0NVbqobIBRHtPgF066ekEbRJo5B+unYYow5LpyMZtm7m+Pc CQ9Pev1fcxah2DEIMH/BzGg7jVk35ms6xP5REdD5ZzPztlEz6WBLZJmooJT6fPet7ojb v3sd2Y19vkeWy8UY/bYi6+y0A5uD0iJL+RUefUWuRJbKoj1FyodCIU9hLgDELfjhvqzJ YMWx876SJnzJJ1nEIgAVM/cMeuE4+C0nOUKq8WDdEGP9r3SAHk6BuwGpK2pp+FOPpNnj vXkA== X-Gm-Message-State: AAQBX9f7n0hcmlMwgFyDwyxnHHvh4T4RGJPFXTChcvxgiVvTlv2JtLNf LyWj7YuOm6BiVoScilkqZ+FA3m88AmtIR76bjrGVHw== X-Google-Smtp-Source: AKy350Z5VaMi4DZV2r44mU5qRMW+oXl5ZyrylP1sAAkuIU6UdJPateLg8U1655UGKVpVO4v1dWEvw6PoKWZttvBbZVw= X-Received: by 2002:a50:9b0e:0:b0:504:7684:a23c with SMTP id o14-20020a509b0e000000b005047684a23cmr7159964edi.8.1681732463533; Mon, 17 Apr 2023 04:54:23 -0700 (PDT) MIME-Version: 1.0 References: <20230417110259.1737315-1-senozhatsky@chromium.org> In-Reply-To: <20230417110259.1737315-1-senozhatsky@chromium.org> From: Yosry Ahmed Date: Mon, 17 Apr 2023 04:53:47 -0700 Message-ID: Subject: Re: [PATCH] zsmalloc: allow only one active pool compaction context To: Sergey Senozhatsky Cc: Minchan Kim , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: jcwoaqjf4ksxn4cqkyu4nzjgkghe9asz X-Rspamd-Queue-Id: 98F5340015 X-HE-Tag: 1681732465-754142 X-HE-Meta: U2FsdGVkX1+irzmEJndU6g6Oa9rX88Ckn8f2zINJY9Jhr9eYHFJFy2nfreryMQo4Mnyxdai7Ar9Z6OeZh+sSxMp8pOmoZ7g4BHsyS7It1P8byMXR6refiIRSrw9FxpgJXhi8KXxsLkvbu/2AsikmTSPSvyLYXX6cnlaRdMtGwO+Jozj5bWfB3WPgMKhe8fAARxhpurBggtmHLoy2UUHjH9R92d6I4Zuqkh4Q47Er53elmBjEZXzuNJxY/wlAHEZBRFKFi2QR+sE/mAcTWQy3hOO7KjopMEyY45gGvYzaOna9/cTK2DEDDpkqrintzy2F6WmPW/YtgjflDfOtJjfwxRVDY1ePhroYp8YrN+6iYlyKvzw9ZhjUs3/Y+FaGWvrKfZ3OmSsRoBk+puT/li0wFPdZBxekGw25mnWhLzwWUnVoLdfayd+Eh95sylXYMuSb6E3eRi9ouMdvChacqildPCj88Y6JpF64WDgqHdcQIHPYVlK832FzkLmCWZPes9jMtUz9IG6Bwp0QoUBgwVr3jU43ptB0iv4Q9p7pir0nzbJCVqeNKfQ507RQV9eTCMFfSujl0XE6bKbOUyM2cmB2gKbi4gTxpAMrvlmjOoTRY4cfAjGbKQCeDQHljOUNH8vmLp3rU+VjZ74qNX+eORpsdhg+ODQq22fGmFvlyRiROlWpeJTZHVX7q/piejqG7yTsu7eN1XfafSl7vwO+SaD0OqoERe6BvAI6Se9hqll3pBeUTlbie+mHHipNzw5Ql8pwXRtXjXHEYB4jnfqWnM7IvdSXAC7Fo4fBTYhWTe0uxMSt4VoHOf6PI9PId96CDVXGyp+A6BGjcCMNXw2FAtjUZN60IWydaIU9v8NENIA/qtIM8Uwg3+Av4DT0xz5FwpqbV8wM5Dtt0CKWzi+dy4WVlAz8FvASpye1I7AZGy7/ey3iejwKVMOXz5gYzd1gn4874AulaJ5u5YPPu4zQd3B Sg6Bh9Nc pKARWtGcwoaJsH5VtG9hmyHjGlz94xAyWliohuO3qhhg4R1YhWpxiwbhWz+mEdrAepcZbXeRmHXplSDNiTAW9nUPqBx8S6KIL//Hhl6n+0iilNut9rgH2uMTkOno9R0KMvXBcxulVd9tqUIzBmC3PMMem5nqfQb7p5roz/SkNiMl+8UKw6tBW45qeqp5FKYioIvekctHRZda5eREgttPq946ab7XLApjzeQatbf2l/Hxl2PWJLSCQp4zMFvowcoocAigT21JWji1FmzWUKEMoHozjyg5Bzd7Fn8PgWUL0n5+xlUqmq41BYkPBP3g1zCyRKqFSPTR97SMc0BQd9Ew3NyD+LAiyJAuuvDkexmIpZ7iP4/SGp1gHZgx6I6T6ilyZRtVVC4B1PRjY5cW3ioV2Dzv8qpH6WXsNRLLj 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: Hi Sergey, On Mon, Apr 17, 2023 at 4:03=E2=80=AFAM Sergey Senozhatsky wrote: > > zsmalloc pool can be compacted concurrently by many contexts, > e.g. > > cc1 handle_mm_fault() > do_anonymous_page() > __alloc_pages_slowpath() > try_to_free_pages() > do_try_to_free_pages( > lru_gen_shrink_node() > shrink_slab() > do_shrink_slab() > zs_shrinker_scan() > zs_compact() > > This creates unnecessary contention as all those processes > compete for access to the same classes. A single compaction > process is enough. Moreover contention that is created by > multiple compaction processes impact other zsmalloc functions, > e.g. zs_malloc(), since zsmalloc uses "global" pool->lock to > synchronize access to pool. > > Introduce pool compaction mutex and permit only one compaction > context at a time. I am not sure what's the best practice here, but if the only use of the mutex is a trylock, do we need a mutex here? It seems like a simple atomic would do the trick. Perhaps something like: static atomic_t ongoing_compaction =3D ATOMIC_INIT(0); ... if (atomic_xchg(&ongoing_compaction, 1)) return; .... atomic_set(&ongoing_compaction, 0); FWIW, I am suggesting this because it was recently brought up when one of my patches was reviewed that locks should protect data not code paths [1], so I am trying to apply what I learned there :) [1]https://lore.kernel.org/lkml/20230323172732.GE739026@cmpxchg.org/ > > /proc/lock-stat after make -j$((`nproc`+1)) linux kernel for > &pool->lock#3: > > Base Patched > -------------------------------------- > con-bounces 9797655 8125860 > contentions 11131185 9242153 > waittime-min 0.09 0.10 > waittime-max 4171695.76 3926258.74 > waittime-total 506197629.16 417061026.20 > waittime-avg 45.48 45.13 > acq-bounces 13809103 11383480 > acquisitions 21145155 18049364 > holdtime-min 0.06 0.07 > holdtime-max 7379928.80 3926274.89 > holdtime-total 46273950.89 37279624.53 > holdtime-avg 2.19 2.07 > > Signed-off-by: Sergey Senozhatsky > --- > mm/zsmalloc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > index cc81dfba05a0..0e036ec56c3c 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -264,6 +264,7 @@ struct zs_pool { > struct work_struct free_work; > #endif > spinlock_t lock; > + struct mutex compact_lock; > }; > > struct zspage { > @@ -2274,6 +2275,9 @@ unsigned long zs_compact(struct zs_pool *pool) > struct size_class *class; > unsigned long pages_freed =3D 0; > > + if (!mutex_trylock(&pool->compact_lock)) > + return 0; > + > for (i =3D ZS_SIZE_CLASSES - 1; i >=3D 0; i--) { > class =3D pool->size_class[i]; > if (class->index !=3D i) > @@ -2281,6 +2285,7 @@ unsigned long zs_compact(struct zs_pool *pool) > pages_freed +=3D __zs_compact(pool, class); > } > atomic_long_add(pages_freed, &pool->stats.pages_compacted); > + mutex_unlock(&pool->compact_lock); > > return pages_freed; > } > @@ -2388,6 +2393,7 @@ struct zs_pool *zs_create_pool(const char *name) > > init_deferred_free(pool); > spin_lock_init(&pool->lock); > + mutex_init(&pool->compact_lock); > > pool->name =3D kstrdup(name, GFP_KERNEL); > if (!pool->name) > -- > 2.40.0.634.g4ca3ef3211-goog >