On Thu, Aug 22, 2024 at 3:04 AM Usama Arif <usamaarif642@xxxxxxxxx> wrote: > > > > On 20/08/2024 17:30, Barry Song wrote: > > > Hi Usama, > > thanks! I can't judge if we need this partially_mapped flag. but if we > > need, the code > > looks correct to me. I'd like to leave this to David and other experts to ack. > > > > Thanks for the reviews! > > > an alternative approach might be two lists? one for entirely_mapped, > > the other one > > for split_deferred. also seems ugly ? > > > > That was my very first prototype! I shifted to using a bool which I sent in v1, and then a bit in _flags_1 as David suggested. I believe a bit in _flags_1 is the best way forward, as it leaves the most space in folio for future work. > Thanks for the explanation; I missed the earlier discussion. > > On the other hand, when we want to extend your patchset to mTHP other than PMD- > > order, will the only deferred_list create huge lock contention while > > adding or removing > > folios from it? > > > > Yes, I would imagine so. the deferred_split_queue is per memcg/node, so that helps. > Right. > Also, this work is tied to khugepaged. So would need some thought when doing it for mTHP. > > I would imagine doing underused shrinker for mTHP would be less beneficial compared to doing it for 2M THP. But probably needs experimentation. > Agreed. I suspect this will be also useful for larger mTHPs like 1M and 512KB. While lock contention might be a concern, it shouldn't be a barrier to proceeding with your patchset, which only supports PMD-order. > Thanks > Thanks Barry