Re: [PATCH v4 1/2] x86/mem_sharing: make fork_reset more configurable

[George Dunlap <dunlapg@xxxxxxxxx>]

On Mon, Apr 25, 2022 at 12:29 PM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:

On Mon, Apr 25, 2022, 3:49 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:

On 22.04.2022 16:07, Tamas K Lengyel wrote:
> On Wed, Apr 13, 2022 at 9:43 AM Tamas K Lengyel <tamas.lengyel@xxxxxxxxx> wrote:
>>
>> Allow specify distinct parts of the fork VM to be reset. This is useful when a
>> fuzzing operation involves mapping in only a handful of pages that are known
>> ahead of time. Throwing these pages away just to be re-copied immediately is
>> expensive, thus allowing to specify partial resets can speed things up.
>>
>> Also allow resetting to be initiated from vm_event responses as an
>> optimization.
>>
>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
>
> Patch ping. Could I get a Reviewed-by if there are no objections?

Hmm, this is a little difficult. I'd be willing to give an ack, but that's
meaningless for most of the code here. Besides a stylistic issue I did
point out which I'm not happy with, I'm afraid I'm not good enough at
mem-sharing and forking. Therefore I wouldn't want to offer an R-b.
Considering the VM event interaction, maybe the BitDefender guys could
take a stab?

Of course you'd then still need a tool stack side ack.

So my take is that noone cares about mem_sharing, which is fine, its an obscure experiment subsystem.

My take is slightly different; it's more that the project is large enough that it's difficult to se where the needs are.  If Roger or Andy or I or Wei or anyone see a thread with you & Jan going back and forth, it's natural for us to assume that you & Jan have it in hand, and there's no need for us to read through the thread.  Jan dislikes asking specific people for a review, but many of the rest of us have sort of gotten in the habit of doing so, as a way to solve the "visibility" issue.  The only other way I can think of to solve the problem is to have a robot try to assign tasks to people -- a method that has received skepticism, and would also require a non-negligible amount of tooling to be written.

 

But the only path I see as maintainer to get anything in-tree is if I hand the task of writing the patch to a coworker who then sends it in so that I can ack it. This is clearly disfunctional and is to the detriment of the project overall. We need to get some rules in place to avoid situations like this that clearly lead to no development and no improvement and a huge incentive to forgot about upstreaming. With no substantive objections but no acks a maintainer should be able to get changes in-tree. That's part of what I would consider maintaining a codebase to be!

Another possibility would be to ask your colleague actually do a Reviewed-by.  The first time or two they might not be "of suitable stature in the community", but I don't think it should take long to establish such a stature, if they were doing the review in earnest.

I do agree that it seems like in this situation, the bar seems too high for you to get your own code checked in.  I'd be open to the argument that we should change the text of the check-in policy in MAINTAINERS to allow maintainer modifications with only an Acked-by.

 

Anyway, to be realistic I don't expect that option to materialize so I'm very close to just stop all contributions to the project. It's dishartening.

I can understand why you'd be disheartened if you thought you just couldn't get any code checked in even as maintainer.  However, there are lots of escalation paths open to you: you could email the community manager (me); you could make a wider appeal on IRC for reviewers; you could raise the general issue at the community call; you could send a patch proposing changes to the check-in procedure described in MAINTAINERS.

 -George