On Tue, Feb 27, 2024 at 11:17 AM Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote: > > On Tue, Feb 27, 2024 at 09:31:33AM -0800, Namhyung Kim wrote: > > I can see some other differences like machine__findnew_thread() > > which I think is due to the locking change. Maybe we can fix the > > problem before moving the code and let the code move simple. > > I was going to suggest that, agreed. > > We may start doing a refactoring, then find a bug, at that point we > first fix the problem them go back to refactoring. Sure I do this all the time. Your typical complaint on the v+1 patch set is to move the bug fixes to the front of the changes. On the v+2 patch set the bug fixes get applied but not the rest of the patch series, etc. Here we are refactoring code for an rb-tree implementation of threads and worrying about its correctness. There's no indication it's not correct, it is largely copy and paste, there is also good evidence in the locking disciple it is more correct. The next patch deletes that implementation, replacing it with a hash table. Were I not trying to break things apart I could squash those 2 patches together, but I've tried to do the right thing. Now we're trying to micro correct, break apart, etc. a state that gets deleted. A reviewer could equally criticise this being 2 changes rather than 1, and the cognitive load of having to look at code that gets deleted. At some point it is a judgement call, and I think this patch is actually the right size. I think what is missing here is some motivation in the commit message to the findnew refactoring and so I'll add that. Thanks, Ian
