* Jeff Xu jeffxu@chromium.org [240814 23:46]:
On Wed, Aug 14, 2024 at 12:55 PM Liam R. Howlett Liam.Howlett@oracle.com wrote:
The majority of the comments to V2 are mine, you only told us that splitting a sealed vma is wrong (after I asked you directly to answer) and then you made a comment about testing of the patch set. Besides the direct responses to me, your comment was "wait for me to test".
Please share this link for " Besides the direct responses to me, your comment was "wait for me to test". Or pop up that email by responding to it, to remind me. Thanks.
[1].
You are holding us hostage by asking for more testing but not sharing what is and is not valid for mseal() - or even answering questions on tests you run.
https://docs.kernel.org/process/submitting-patches.html#don-t-get-discourage...
If you are implying that I'm impatient, I can assure you that is not the feeling driving these emails.
You are just trying to push a patch through that changes the exact code that you said you would test but didn't say how, and you said the testing of another patch was insufficient but didn't say why. Then you send out this fix.
These patches should be rejected in favour of fixing the feature like it should have been written in the first place.
This is not ture.
Yes, it is.
Without removing arch_unmap, it is impossible to implement in-loop.
arch_unmap() is going away, besides..
arch_unmap() could fail today and leave the ppc vdso pointing to NULL, mseal() would introduce a even less likely case of this happening. I asked you about this in v10 [2]. I elaborated in my response, but I doubt you got that far in the email.
And I have mentioned this during initial discussion of mseal patch, as well as when Pedro expressed the interest on in-loop approach. If you like reference, I can find the links for you.
So the main concern is that ppc is going to mseal the vdso, then fail to unmap it?
It would have been better to put a check in the arch_unmap() code in ppc to avoid that - but it will never happen.
I'm glad that arch_unmap is removed now and resulting in much cleaner code,
If you care at all about cleaner code, please move the mseal check to where it should be - or stop getting in the way of others moving it.
it has always been a question/mysterial to me ever since I read that code.
You could have also looked into what arch_unmap() did, or why it was where it is today. If you had, you would have found that arch_unmap() could be moved lower in the function and allowed in-loop approach - but you didn't bother to find out what it was about.
Liam
...
[1]. https://lore.kernel.org/all/CALmYWFs0v07z5vheDt1h3hD+3--yr6Va0ZuQeaATo+-8MuR... [2]. https://lore.kernel.org/lkml/3rpmzsxiwo5t2uq7xy5inizbtaasotjtzocxbayw5ntgk5a...