On Wed, Oct 13, 2021 at 4:15 PM Roman Gushchin <guro@xxxxxx> wrote: > [...] > > > > > > Isn't it a bit too aggressive? > > > > > > How about > > > if (WARN_ON_ONCE(gfp & __GFP_ACCOUNT)) > > > > We actually know that kvmalloc(__GFP_ACCOUNT) users exist and can > > trigger bulk page allocator through vmalloc, so I don't think the > > warning would be any helpful. > > > > > gfp &= ~__GFP_ACCOUNT; > > > > Bulk allocator is best effort, so callers have adequate fallbacks. > > Transparently disabling accounting would be unexpected. > > I see... > > Shouldn't we then move this check to an upper level? > > E.g.: > > if (!(gfp & __GFP_ACCOUNT)) > call_into_bulk_allocator(); > else > call_into_per_page_allocator(); > If we add this check in the upper level (e.g. in vm_area_alloc_pages() ) then I think we would need WARN_ON_ONCE(gfp & __GFP_ACCOUNT) in the bulk allocator to detect future users. At the moment I am more inclined towards this patch's approach. Let's say in future we find there is a __GFP_ACCOUNT allocation which can benefit from bulk allocator and we decide to add such support in bulk allocator then we would not need to change the bulk allocator callers at that time just the bulk allocator.
