Received: by 2002:a05:7412:8d10:b0:f3:1519:9f41 with SMTP id bj16csp6537700rdb; Fri, 15 Dec 2023 01:04:24 -0800 (PST) X-Google-Smtp-Source: AGHT+IFJlzoDUlztJGROvY6JTSgDbaZRGI8LqfMxXqhufb4Hvzk9RWU1GarUniv8Qd2QYxbSSwoo X-Received: by 2002:ac8:5913:0:b0:425:4043:762e with SMTP id 19-20020ac85913000000b004254043762emr16249943qty.86.1702631063913; Fri, 15 Dec 2023 01:04:23 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702631063; cv=none; d=google.com; s=arc-20160816; b=R1LMded/BsNEKffMzreZhP8Hj+d+A5JL+RmKgekx8EOVolPPssv9gqsWlbPQfZTviw mFu4wKyWxUNOuIqy6GK/hGm8PwAE/nrf4LEJcLAoLYEA4XIOdN78DO8eso18yRox/nN5 7Aw4WSAFQ+KSdduTRcJA3T0LFjQf+Q0FzmhTGCSzzexI+W087VN3fVJaUosFgOuTJSHg JUXJPeFTWX9wBpBGLR1IRMW0k/9vB85SZGH61uNrZgEIuu0RhsRCzYnw5mDvYUSrG1rG N4uFxsgRWBY4OEHVbRmZCA3caG2Dj5/QX5QL1XczCzzcytjO+mQuj9GrAdcr9gyCqDWK yLRQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:list-unsubscribe:list-subscribe :list-id:precedence:dkim-signature; bh=DaIyphWUSQyUpejH9hCl/lG3GBHHtOu7lr9wkQ40dik=; fh=6yEqBb9DWJhifLHlkqaL4I56FZSAMUnpEkreom+8ieA=; b=un8advsB2lA2kQFMR6dHGB0/VIZ4TCADPR0ogPbfeh0SesgDPaX9BVKcnKNdhgk+Rk wS9ru1Bcemfv0NP/lQSN65prciOMlA5NyCQjLeklgCpkcAcT3bTXWICNJnii+B4YaKjA Lh09uH2RfIk6xsvz3KdoMnGBwZoZ9cPAfdWdQM1pQonuI6To9qDm4CgnuUaRysbbXZdQ VZLIHZd9iqDTzDDStE1AodmXDYAR5awim5BfQcZaxeNtesWBZSs8z2lo52fTWL5xFA0P v8xTEbUQ1D8s83yfwdsWzTrmplRu3aU8+b8gDVtxwRaKBfyBucc7mEjX7vFlQW6P8AVT oIrA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=c+z5cJjN; spf=pass (google.com: domain of linux-kernel+bounces-654-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-654-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id t22-20020ac87616000000b00425894bdbb5si16023861qtq.433.2023.12.15.01.04.23 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Dec 2023 01:04:23 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-654-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=c+z5cJjN; spf=pass (google.com: domain of linux-kernel+bounces-654-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-654-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 814AF1C22864 for ; Fri, 15 Dec 2023 09:04:23 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 12796171A6; Fri, 15 Dec 2023 09:04:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="c+z5cJjN" X-Original-To: linux-kernel@vger.kernel.org Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BE6C219445 for ; Fri, 15 Dec 2023 09:04:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f45.google.com with SMTP id ffacd0b85a97d-3333131e08dso370135f8f.2 for ; Fri, 15 Dec 2023 01:04:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702631054; x=1703235854; darn=vger.kernel.org; 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=DaIyphWUSQyUpejH9hCl/lG3GBHHtOu7lr9wkQ40dik=; b=c+z5cJjN7mA1m/CKR/55oF5vTusQoFVBMrtN9DryLV5ONYa9kirJ4hqum0064GdLFk AyjNn8mcg7FuGQSmFNxhYPHdiLGO39ps7TPzmHHBwnt1lEnP85GfwWsInLckNMP2xLvw Gekntf6F3ciMAudkpWfGFnnGTtgnfp3ozxlqYW8v+a+6BhwQCk+SmqJIGiqJeiw4PNc9 2zKXrjA6b475Fm5rExtxOxhV6HmgptO1GwezPvQtCCge5DVIKIthIhwbsBzFjf19Fkda uzh6/V/hEvyKdxbIciA7ic6oLp8Y0wuwhpCDb5F+oqu3U5qu/nl1jHxHwvrEfW41WV3T Ezeg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702631054; x=1703235854; 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=DaIyphWUSQyUpejH9hCl/lG3GBHHtOu7lr9wkQ40dik=; b=jrqStDfb/3I6YcMV26Ai0cE68nZA5GzK5K/So/XEjhow7vW5w9LWN1EIB0NhKefZTQ +OuluTbEXCFfdkvklIZwMHb4wJmQnAe25qoqWxutfqy4oH4m917bze3b4V8o9LQXG4P/ myq0CICsUQinW/Xs7puwgpZjibkPceaQdGwxwjG0A7ewaMP3PGYp/6BmZXBC4RR9GsxR KWtCrtCg9xcurzM6P3qaV7RVi7D0EXB2omqNM5PZum0ofW3cZfIqefsyIlXz4XIAAuMz beqiDNXfms1BlqAMm2aqCkb9R7FnLbJoTJoHBRBleFmLmRa8jtZsjfzY1LGwPRSUfaHH dU1g== X-Gm-Message-State: AOJu0YzbY+1DREZ4aA0nYVrAZ82MrBygW63rKz/gThpyuXJVgspHR8Vj /X1qNXNPSiee8ndfi6i1JUOhPP0Mqn5ulWvkHEi/HnGI9bl3SQ== X-Received: by 2002:adf:e343:0:b0:333:39d4:ff80 with SMTP id n3-20020adfe343000000b0033339d4ff80mr5948727wrj.90.1702631053688; Fri, 15 Dec 2023 01:04:13 -0800 (PST) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20231211052850.3513230-1-debug.penguin32@gmail.com> In-Reply-To: From: Ronald Monthero Date: Fri, 15 Dec 2023 19:03:37 +1000 Message-ID: Subject: Re: [PATCH] mm/zswap: Improve with alloc_workqueue() call To: Nhat Pham Cc: sjenning@redhat.com, akpm@linux-foundation.org, Dan Streetman , Vitaly Wool , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Dec 14, 2023 at 10:28=E2=80=AFAM Nhat Pham wrot= e: > .. < snipped > > I should have been clearer. I'm not against the change per-se - I > agree that we should replace create_workqueue() with > alloc_workqueue(). What I meant was, IIUC, there are two behavioral > changes with this new workqueue creation: > > a) We're replacing a bounded workqueue (which as you noted, is fixed > by create_workqueue()) with an unbounded one (WQ_UNBOUND). This seems > fine to me - I doubt locality buys us much here. Yes the workqueue attribute change per se but the existing functionality remains seamless and the change is more a forward looking change. imo under a memory pressure scenario an unbound workqueue might workaround the scenario better as the number of backing pools is dynamic. And with the WQ_UNBOUND attribute the scheduler is more open to exercise some improvisations in any demanding scenarios for offloading cpu time slicing for workers, ie if any other worker of the same primary cpu had to be served due to workers with WQ_HIGHPRI and WQ_CPU_INTENSIVE.Although normal and highpri worker-pools don't interact with each other, the target cpu atleast need not be the same if our worker for zswap is WQ_UNBOUND. Also noting that the existing wq of zwap worker has the WQ_MEM_RECLAIM attribute, so is there a rescue worker for zswap during a memory pressure scenario ? Quoting: "All work items which might be used on code paths that handle memory reclaim are required to be queued on wq's that have a rescue-worker reserved for execution under memory pressure. Else it is possible that the worker-pool deadlocks waiting for execution contexts to free up" Also additional thought if adding WQ_FREEZABLE attribute while creating the zswap worker make sense in scenarios to handle freeze and unfreeze of specific cgroups or file system wide freeze and unfreeze scenarios ? Does zswap worker participate in freeze/unfreeze code path scenarios ? > b) create_workqueue() limits the number of concurrent per-cpu > execution contexts at 1 (i.e only one single global reclaimer), > whereas after this patch this is set to the default value. This seems > fine to me too - I don't remember us taking advantage of the previous > concurrency limitation. Also, in practice, the task_struct is > one-to-one with the zswap_pool's anyway, and most of the time, there > is just a single pool being used. (But it begs the question - what's > the point of using 0 instead of 1 here?) Nothing in particular but I left it at default 0, which can go upto 256 ( @maxactive per cpu). But if zswap worker is always intended to only have 1 active worker per cpu= , then that's fine with 1, otherwise a default setting might be flexible for scaling. just a thought, does having a higher value help for larger memory systems = ? > Both seem fine (to me anyway - other reviewers feel free to take a > look and fact-check everything). I just feel like this should be > explicitly noted in the changelog, IMHO, in case we are mistaken and > need to revisit this :) Either way, not a NACK from me. Thanks Nhat, for checking. Above are my thoughts, I could be missing some info or incorrect on certain fronts so I would seek clarifications. Also thanks in advance to other experts/maintainers, please share feedback and suggestions. BR, ronald