Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Oleksandr Natalenko <oleksandr@natalenko.name>
To:     Michal Hocko <mhocko@suse.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Matthew Wilcox <willy@infradead.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Miaohe Lin <linmiaohe@huawei.com>,
        Mel Gorman <mgorman@techsingularity.net>,
        Stephen Rothwell <sfr@canb.auug.org.au>
Subject: Re: [RFC PATCH 1/1] mm: provide one common K(x) macro
Date:   Wed, 01 Sep 2021 12:50:40 +0200
Message-ID: <5529272.KFOknHQvy8@natalenko.name>
In-Reply-To: <YS9WiF6enhSo8sYc@dhcp22.suse.cz>
References: <20210901092149.994791-1-oleksandr@natalenko.name> <20210901092149.994791-2-oleksandr@natalenko.name> <YS9WiF6enhSo8sYc@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hello.

On st=C5=99eda 1. z=C3=A1=C5=99=C3=AD 2021 12:31:36 CEST Michal Hocko wrote:
> On Wed 01-09-21 11:21:49, Oleksandr Natalenko wrote:
> > There are various places where the K(x) macro is defined. This commit
> > gets rid of multiple definitions and provides a common one.
> >=20
> > This doesn't solve open-coding this macro in various other places. This
> > should be addressed by another subsequent commit.
>=20
> Why is this an improvement? You are adding a header file for a single
> macro which sounds like an overkill.

I agree a separate header file is an overkill for just one #define, hence
still looking for a suggestion on a better place for it.

> The overall net outcome is added
> lines of code.

Not always. There are some long statements like:

```
seq_printf(seq, ",size=3D%luk",
        sbinfo->max_blocks << (PAGE_SHIFT - 10));
```

that are split into two lines. With the macro those take one line only:

```
seq_printf(seq, ",size=3D%luk", K(sbinfo->max_blocks));
```

As of now (counting unposted open-coding replacements) the grand total is:

```
31 files changed, 104 insertions(+), 90 deletions(-)
```

which is not that horrible.

> It is not like K() or any of its variant is adding a
> maintenance burden due to code duplication. So why do we want to change
> the existing state?

=46or me it's about readability. Compare, for instance:

```
seq_put_decimal_ull_width(m, str, (val) << (PAGE_SHIFT-10), 8)
```

and

```
seq_put_decimal_ull_width(m, str, K(val), 8)
```

It's a small yet visible difference.

Thanks.

=2D-=20
Oleksandr Natalenko (post-factum)