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 21697C4332F for ; Sat, 11 Nov 2023 18:22:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2DA808D0014; Sat, 11 Nov 2023 13:22:18 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2622E8D0003; Sat, 11 Nov 2023 13:22:18 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0DE668D0014; Sat, 11 Nov 2023 13:22:18 -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 EB25B8D0003 for ; Sat, 11 Nov 2023 13:22:17 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id BC613140108 for ; Sat, 11 Nov 2023 18:22:17 +0000 (UTC) X-FDA: 81446493114.26.C5D2121 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf22.hostedemail.com (Postfix) with ESMTP id D9B35C000F for ; Sat, 11 Nov 2023 18:22:14 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=vFFM9ujb; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf22.hostedemail.com: domain of chrisl@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=chrisl@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1699726935; a=rsa-sha256; cv=none; b=oAKqwJUlXLv3VPodC1CsJtmnbhJRZTObnkaVe0UN84DP4XQjym4GuSCoW21NFTUehD3ggo Gz+h6bieQX5d1e6YbOgYRFXbK+HAcN6d7nFYcGolgPI1fXAy9mLCN1miFXF69weRk9jAcO jyUYlmRPsnqWklvkFH5rnooY+/iw/Cc= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=vFFM9ujb; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf22.hostedemail.com: domain of chrisl@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=chrisl@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1699726935; 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=DsFB+pJaC6L10j+Pf2lb8eDn2SoWQUzyzSNJVNPk3Fk=; b=m5gRJBI6W/yLGHljgvJ6OO3upJvqnP3NXexAqIGftMwmHQ4HUuGWZO2/16nw61IhMaySmy QuDDWt23fFVQOLqCpkO0zdD/hRZGlE3AbjddZflwwPbyyYYgL6wlh5a+aNhJmivxnLWlVr wB3vkxwfQ36Bk2u7Mzq/JWoJajHO2KE= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id D539960B51 for ; Sat, 11 Nov 2023 18:22:13 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E819FC433CB for ; Sat, 11 Nov 2023 18:22:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1699726932; bh=CW5tpNdFHXddseGoEXwpAJxkp57G2fy3axYjwxjjpHo=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=vFFM9ujbE1s/fefcHV6DqhEgEoHriNBsCwjpOnZHkwVE8+synCr/GBR+ygEy5aOdk uVNVHWDJtQY4iR8CF6sngeBVwCiAA0i6vVGOl4mABKBMbsYZbgXVkg0w1/LbVXaBbJ 2rXsFkqhPM2Yy7bz+0PC82rKEQY40aF7gjOoVkRwznrcatM2vVsibme/1/KZRcjggt Hd83SBbpUigXOILQcQVFfOHCbHeIYNxupeGjUsoflSTYyklE7+e5qvw63ITA0IQYmT alWGzeo5COI09BHgQgEH2mIHMBXmr0xhmrdzLlUZCLod2B1q/iTmG+yIVn7L01Zu27 qf1gjFmP4IIxA== Received: by mail-pj1-f41.google.com with SMTP id 98e67ed59e1d1-27ff7fe7fbcso2738276a91.1 for ; Sat, 11 Nov 2023 10:22:12 -0800 (PST) X-Gm-Message-State: AOJu0Yzh420CAAikuZo53lTbcDFXyjZFrasyLcqTMwSEGzkcAUoBRl+g bDuSh8JDEQKXKZ0MEXkdX78WvjqbJOd6uKINiEKEmg== X-Google-Smtp-Source: AGHT+IFRst9RU625qAGHUXiFdwUcqInfH7cBpB6bfU2QT9KP7GwFE+6fAD0QjOLiwzcDhmCQ5NB2iGC3TieCzx5Ci+c= X-Received: by 2002:a17:90a:eac9:b0:27d:9f6:47a3 with SMTP id ev9-20020a17090aeac900b0027d09f647a3mr2745035pjb.31.1699726932330; Sat, 11 Nov 2023 10:22:12 -0800 (PST) MIME-Version: 1.0 References: <20231106231158.380730-1-nphamcs@gmail.com> In-Reply-To: From: Chris Li Date: Sat, 11 Nov 2023 10:22:00 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4] zswap: memcontrol: implement zswap writeback disabling To: Nhat Pham Cc: Andrew Morton , tj@kernel.org, lizefan.x@bytedance.com, Johannes Weiner , Domenico Cerasuolo , Yosry Ahmed , Seth Jennings , Dan Streetman , Vitaly Wool , mhocko@kernel.org, roman.gushchin@linux.dev, Shakeel Butt , muchun.song@linux.dev, Hugh Dickins , corbet@lwn.net, Konrad Rzeszutek Wilk , senozhatsky@chromium.org, rppt@kernel.org, linux-mm , kernel-team@meta.com, LKML , linux-doc@vger.kernel.org, david@ixit.cz Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: D9B35C000F X-Stat-Signature: i33m34jbshg3qhaahytgyc74br3p1uf5 X-HE-Tag: 1699726934-127636 X-HE-Meta: U2FsdGVkX19t90Nqs4t9ERP1U1qQpmDVad2V0aXqyeyIteoYgw1QIaqHVtuS8w01Q62i0kjwe908+6rN6SYnNlifR/Sq2CzjUzKsqxvewZ8m/+yZbWUamgztyzSwSomY3+LYOgAkGRKNEQNE9/cYWmCLVzEFPHHLhAe6xrhLabQ/YhFK5ghzK5gAthUdoEp1o3W+JaJahpjG9YSmBCEZxxjWxdGIOIAT+w7IA2sNN+hMjFn3eVyUG8sJdCkIvVDbOqaF5v5DqEBUBEHz29yQ+iIyS43zQugGdsWDl5IrI0Y9gb2sUBULQPPqGPsfIkyeqdzBIjuLWV0l/5dUKHGesZ3oBK6hEvvpThKQwZldMIFeBSRbrRV0NWqzj3EfRyVJcB8ZUJWVKOtPd+mDr7qq7VW4xh4mQYrvI69JGS4ZwAw+yOlrcB4cK0qfl+E0/6m/KAaD1TBBuYvTXYS7MGCN5aV09WpUJsCSUOKDgEyUmY94N3C3VOl0XjTRWQallQlHoXbEi723AYaeMc+MF+dtVpbRlUsXwn8ZPRq0639/oGsQqfDo3m25DS+Sx/bq4jNghPJZ0iuVtGtMozQgPwYikaAtEm/aWlvyhrsaQU+/SkyXFpYXxgVOqOPHQJa56jO+BMz334AjxxAj8scx3ZR6oEC8ElqxOXn0T/y7dm1TLlE3um0iDD6s6q9GgmGeT1nP4SdkK7fYvRZEY3MlnKslUlslXs9FF5nDhzhBQ4uA6Eao1h/+Mr1z3ZM8MUftxvvNWREhhv6wnf7eXf3FhjOwSv98vSTH5VHQMZHP7IvL8NRwNL1wbrFiQ2LHGVOCKYhxfSFDi/QZcp+u0oMJ6C39d+zOcYzkInKp7RVOARMMbaZT/OVNSG2+J3kBMGQmoG/sHA1XS87nCqLkLpbeanBc2fXb5nZrwX5ava+d7e5xHKTIUJTtarlb0KkD+FMUDcjXwa9XoLMzJ6TiDZEQTg2 KRkHPvKi CddjDjZGjTdTZOSx39Dd8a7P/F4AT+g1D/BIwVwATp0DVV5m6O1qLjlGrh1a3jYKlwrD2It2xZYisKopzaSGPlPGMeXhYvvpUswC7aATRtTgzCubjxSktNnpEZ0gSaomz7KUywwon3UGhffOYJ5+RpgjpeY3eqWUDlM5OEH2hYha4aUEhdhG8egpWFePPAL+AoNlbZqEBq+597KPCi/jcSxagzKn4XCulKT1f5zdvdl/YKtkqw/KjVXvIiA== 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 Fri, Nov 10, 2023 at 4:10=E2=80=AFPM Nhat Pham wrote= : > > I notice the bool is between two integers. > > mem_cgroup structure has a few bool sprinkle in different locations. > > Arrange them together might save a few padding bytes. We can also > > consider using bit fields. > > It is a very minor point, the condition also exists before your change. > > This sounds like an optimization worthy of its own patch. Two random > thoughts however: Sure. I consider this a very minor point as well. > > a) Can this be done at the compiler level? I believe you can reduce > the padding required by sorting the fields of a struct by its size, corre= ct? > That sounds like a job that a compiler should do for us... According to the C standard, the struct member should be layered out in the order it was declared. There are too many codes that assume the first member has the same address of the struct. Consider we use struct for DMA descriptor as well, where the memory layout needs to match the underlying hardware. Re-ordering the members will be really bad there. There are gcc extensions to do structure member randomization. But the randomization layout is determined by the randomization seed. The compiler actually doesn't have the flexibility to rearrange the member orders to reduce the padding either. > > b) Re: the bitfield idea, some of the fields are CONFIG-dependent (well > like this one). Might be a bit hairier to do it... You can declare the bit field under preprocessor condition as well, just like a normal declare. Can you clarify why it is more hairier? The bitfield does not have a pointer address associated with it, the compiler can actually move the bit field bits around. You get the compiler to do it for you in this case. > > > > > > #endif /* _LINUX_ZSWAP_H */ > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > index e43b5aba8efc..9cb3ea912cbe 100644 > > > --- a/mm/memcontrol.c > > > +++ b/mm/memcontrol.c > > > @@ -5545,6 +5545,11 @@ mem_cgroup_css_alloc(struct cgroup_subsys_stat= e *parent_css) > > > WRITE_ONCE(memcg->soft_limit, PAGE_COUNTER_MAX); > > > #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_ZSWAP) > > > memcg->zswap_max =3D PAGE_COUNTER_MAX; > > > + > > > + if (parent) > > > + WRITE_ONCE(memcg->zswap_writeback, READ_ONCE(parent->= zswap_writeback)); > > > + else > > > + WRITE_ONCE(memcg->zswap_writeback, true); > > > > You can combine this two WRITE_ONCE to one > > > > bool writeback =3D !parent || READ_ONCE(parent->zswap_writeback); > > WRITE_ONCE(memcg->zswap_writeback, writeback); > > > > Yeah I originally did something similar, but then decided to do the if-el= se > instead. Honest no strong preference here - just felt that the if-else wa= s > cleaner at that moment. One WRITE_ONCE will produce slightly better machine code as less memory store instructions. Normally the compiler is allowed to do the common expression elimination to merge the write. However here it has explicite WRITE_ONCE, so the compiler has to place two memory stores instructions, because you have two WRITE_ONCE. My suggestion will only have one memory store instruction. I agree it is micro optimization. Chris