Message-ID: <ZxwHHWbjIbGh1RU8@x1>
Date: Fri, 25 Oct 2024 18:01:17 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	Ravi Bangoria <ravi.bangoria@....com>,
	Weilin Wang <weilin.wang@...el.com>,
	Yoshihiro Furudera <fj5100bi@...itsu.com>,
	James Clark <james.clark@...aro.org>,
	Athira Jajeev <atrajeev@...ux.vnet.ibm.com>,
	Howard Chu <howardchu95@...il.com>,
	Oliver Upton <oliver.upton@...ux.dev>,
	Changbin Du <changbin.du@...wei.com>, Ze Gao <zegao2021@...il.com>,
	Junhao He <hejunhao3@...wei.com>, linux-kernel@...r.kernel.org,
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v6 0/5] Hwmon PMUs

On Fri, Oct 25, 2024 at 11:26:26AM -0700, Ian Rogers wrote:
> On Fri, Oct 25, 2024 at 10:30 AM Namhyung Kim <namhyung@...nel.org> wrote:
> > On Thu, Oct 24, 2024 at 06:33:27PM -0700, Ian Rogers wrote:
> > > So I think moving the enum declarations into one patch is okay. But as
> > > the enum values have no bearing on hardware constants, or something
> > > outside of the code that uses them it smells strange to me. Ultimately
> > > this is going to do little to the lines of code count but damage
> > > readability. I'm not sure why we're doing this given the kernel model
> > > for adding a driver is to add it as a large chunk. For example, here
> > > is adding the intel PT driver:
> > > https://lore.kernel.org/all/1422614392-114498-1-git-send-email-alexander.shishkin@linux.intel.com/T/#u

> > Maybe others can understand a big patch easily, but I'm not.
 
> My understanding is that we make small patches so that the codebase is
> more bisectable. When there is something new, like a driver or here a

That is super important, having patches being super small and doing just
one thing helps in bisecting problems.

If two things are done in one patch, and one of them causes a problem,
then bisection is a very effective way of finding out what exactly
caused a problem.

But bisection is not the only benefit from breaking down larger patches
into smaller ones.

We want to have more people joining our ranks, doing low level tooling
and kernel work.

Writing new functionality in a series of patches, growing in complexity
is a way to reduce the cognitive load on understantind how something
works.

As much as trying to emulate how the kernel community works is a good
model as that community has been producing a lot of good code in a
frantic, athletic pace, and as much as I can agree with you that adding
a new piece of code will not affect bisectability as its new code, I
think having it broken down in multiple patches benefits revieweing.

Reviewing is something we should do more, but its very taxing.

One would rather try to write as much code as possible, leaving to
others the reviewing part.

But its a balancing act.

Whatever we can do to help reviewers, like taking into account what they
say they would prefer as a way to submit our work, even if it isn't
exactly of our liking, is one such thing.

So if Namhyung says that it would be best for you to try to break down
your patches into smaller ones, like I did say to you in the past, even
taking the trouble to do it myself, in the process introducing problems,
later fixed, I think you should try to do what he says.

He is the maintainer, try to address his comments.

- Arnaldo

Powered by blists - more mailing lists