On Wed, Dec 18, 2024 at 12:32:20PM +0100, Michal Hocko wrote:
> I like this proposal better. I am still not convinced that we really
> need internal __GFP_TRYLOCK though.
>
> If we reduce try_alloc_pages to the gfp usage we are at the following
>
> On Tue 17-12-24 19:07:14, alexei.starovoitov@xxxxxxxxx wrote:
> [...]
> > +struct page *try_alloc_pages_noprof(int nid, unsigned int order)
> > +{
> > + gfp_t alloc_gfp = __GFP_NOWARN | __GFP_ZERO |
> > + __GFP_NOMEMALLOC | __GFP_TRYLOCK;
> > + unsigned int alloc_flags = ALLOC_TRYLOCK;
> [...]
> > + prepare_alloc_pages(alloc_gfp, order, nid, NULL, &ac,
> > + &alloc_gfp, &alloc_flags);
> [...]
> > + page = get_page_from_freelist(alloc_gfp, order, alloc_flags, &ac);
> > +
> > + /* Unlike regular alloc_pages() there is no __alloc_pages_slowpath(). */
> > +
> > + trace_mm_page_alloc(page, order, alloc_gfp & ~__GFP_TRYLOCK, ac.migratetype);
> > + kmsan_alloc_page(page, order, alloc_gfp);
> [...]
>
> From those that care about __GFP_TRYLOCK only kmsan_alloc_page doesn't
> have alloc_flags. Those could make the locking decision based on
> ALLOC_TRYLOCK.
>
> I am not familiar with kmsan internals and my main question is whether
> this specific usecase really needs a dedicated reentrant
> kmsan_alloc_page rather than rely on gfp flag to be sufficient.
> Currently kmsan_in_runtime bails out early in some contexts. The
> associated comment about hooks is not completely clear to me though.
> Memory allocation down the road is one of those but it is not really
> clear to me whether this is the only one.
Is the suggestion that just introduce and use ALLOC_TRYLOCK without the
need of __GFP_TRYLOCK?
Regarding KMSAN, the __GFP_ZERO would bypass it. Maybe a comment to
explain that.
