Messages in this thread | | | Subject | Re: [PATCH v2 6/9] mm: free user PTE page table pages | From | Qi Zheng <> | Date | Thu, 2 Sep 2021 15:04:53 +0800 |
| |
On 2021/9/2 AM1:55, Jason Gunthorpe wrote: > On Wed, Sep 01, 2021 at 07:49:23PM +0200, David Hildenbrand wrote: >> On 01.09.21 19:10, Jason Gunthorpe wrote: >>> On Wed, Sep 01, 2021 at 06:19:03PM +0200, David Hildenbrand wrote: >>> >>>>> I wouldn't think it works everywhere, bit it works in a lot of places, >>>>> and it is a heck of a lot better than what is proposed here. I'd >>>>> rather see the places that can use it be moved, and the few places >>>>> that can't be opencoded. >>>> >>>> Well, I used ptep_get_map_lock() and friends. But hacking directly into >>>> ptep_map_lock() and friends wasn't possible due to all the corner cases. >>> >>> Sure, I'm not surprised you can't get every single case, but that just >>> suggest we need two API families, today's to support the special cases >>> and a different one for the other regular simple cases. >>> >>> A new function family pte_try_map/_locked() and paired unmap that can >>> internally do the recounting and THP trickery and convert the easy >>> callsites. >>> >>> Very rough counting suggest at least half of the pte_offset_map_lock() >>> call sites can trivially use the simpler API. >>> >>> The other cases can stay as is and get open coded refcounts, or maybe >>> someone will have a better idea once they are more clearly identified. >>> >>> But I don't think we should take a performance hit of additional >>> atomics in cases like GUP where this is trivially delt with by using a >>> better API. >> >> Right, but as I said in the cover letter, we can happily optimize once we >> have the basic infrastructure in place and properly reviewed. Getting rid of >> some unnecessary atomics by introducing additional fancy helpers falls under >> that category. > > I'm not sure I agree given how big and wide this patch series is. It > would be easier to review if it was touching less places. The helpers > are not fancy, it is a logical re-arrangement of existing code that > shrinks the LOC of this series and makes it more reviewable. > > Or stated another way, a niche feature like this try much harder not > to add more complexity everywhere.
Totally agree, I will rework this patch series based on you and David's suggestions.
Thank you very much, Qi
> > Jason >
| |