Re: [PATCH v4 1/4] mm: Optimize mprotect() for MM_CP_PROT_NUMA by batch-skipping PTEs

[Dev Jain <dev.jain@arm.com>]

On 30/06/25 5:21 pm, Lorenzo Stoakes wrote:
> On Mon, Jun 30, 2025 at 05:10:36PM +0530, Dev Jain wrote:
>> On 30/06/25 4:55 pm, Lorenzo Stoakes wrote:
>>> On Sat, Jun 28, 2025 at 05:04:32PM +0530, Dev Jain wrote:
>>>> In case of prot_numa, there are various cases in which we can skip to the
>>>> next iteration. Since the skip condition is based on the folio and not
>>>> the PTEs, we can skip a PTE batch. Additionally refactor all of this
>>>> into a new function to clean up the existing code.
>>> Hmm, is this a completely new concept for this series?
>>>
>>> Please try not to introduce brand new things to a series midway through.
>>>
>>> This seems to be adding a whole ton of questionable logic for an edge case.
>>>
>>> Can we maybe just drop this for this series please?
>> I refactored this into a new function on David's suggestion:
>>
>> https://lore.kernel.org/all/912757c0-8a75-4307-a0bd-8755f6135b5a@redhat.com/
>>
>> Maybe you are saying, having a refactoring patch first and then the "skip a
>> PTE batch" second, I'll be happy to do that, that would be cleaner.
> OK apologies then, my mistake.
>
> So essentially you were doing this explicitly for the prot numa case and it just
> wasn't clear in subject line before, ok.
>
> Yes please separate the two out!
>
>> This series was (ofcourse, IMHO) pretty stable at v3, and there were comments
>> coming on David's series, so I guessed that he will have to post a v2 anyways
>> after mine gets merged. My guess could have been wrong. For the khugepaged
>> batching series, I have sent the migration race patch separately exactly
>> because of his series, so that in that case the rebasing burden is mine.
> This stuff can be difficult to align on, but I'd suggest that when there's
> another series in the interim that is fundamentally changing a function
> signature that will make your code not compile it's probably best to hold off.

Good point about changing the function signature. Anyways my bad, should have
been more careful, apologies.

>
> And that's why I'm suggesting this here.
>
>>> I know you like to rush out a dozen series at once, but once again I'm asking
>>> maybe please hold off?
>> Lorenzo : ) Except for the mremap series which you pointed out, I make it a point
>> to never repost in the same week, unless it is an obvious single patch, and even
>> in that case I give 2-3 days for the reviews to settle. I posted
>> v3 of this series more than a month ago, so it makes total sense to post this.
>> Also I have seen many people spamming the list with the next versions on literally
>> the same day, not that I am using this as a precedent. The mistake I made here
>> is that on Saturday I saw David's series but then forgot that I am using the
>> same infrastructure he is changing and went ahead posting this. I suddenly
>> remembered this during the khugepaged series and dropped the first two patches
>> for that.
> I'm not saying you shot this out shortly after a previous version, I'm saying
> you have a whole bunch of series at once (I know as I'm cc'd on them all I think
> :), and I'm politely asking you to maybe not send another that causes a known
> conflict.
>
> Whether or not you do so is up to you, it was a request.

Yup my bad for forgetting about the conflict, apologies!

>
>>> I seem to remember David asked you for the same thing because of this, but maybe
>>> I'm misremembering.
>> I don't recollect that happening, maybe I am wrong.
> Yeah maybe I'm misremembering, apologies if so!