Re: [PATCH V2 04/11] cxl/mem: Clear events on driver load

Ira Weiny
2022-12-03T21:01:08Z

On Fri, Dec 02, 2022 at 03:34:20PM -0800, Dan Williams wrote:
> Ira Weiny wrote:
> > On Thu, Dec 01, 2022 at 06:48:12PM -0800, Dan Williams wrote:
> > > cxl/mem is cxl_mem.ko, This is cxl/pci.

[snip]

> > > > + cxl_mem_get_event_records(cxlds);
> > > > +
> > >
> > > This hunk likely goes with the first patch that actually implements some
> > > mocked events.
> >
> > If this patch was squashed into the other patches yes. But as a patch which
> > does exactly 1 thing "Clear events on driver load" it works IMO. I could just
> > have well put this patch at the very end.
> >
> > Now that the Get/Clear operations are more settled I'll split this out and
> > squash it as you suggest. Jonathan suggested squashing Get/Clear too but again
> > I really prefer the 1 thing/patch and each of those operations seemed like a
> > good breakdown.
> >
>
> I'll preface this by saying if you ask 3 kernel developers how to split
> a patch series you'll get 5 answers.

Indeed.

> For me though, a patch should be a
> bisectable full-thought. That at each step of a series the kernel is
> incrementally better in a way that makes sense. The kernel that gets Get
> Events likely needs to clear them too to complete 1 full thought about
> enbling Event handling. Otherwise a kernel that just retrieves some
> events until they overflow feels like a POC.

I've squashed it.

Ira