On Fri, Jul 19, 2024 at 9:30 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > On Fri 19-07-24 21:02:10, Barry Song wrote: > > what about an earlier WARN_ON, for example, even before we begin to > > try the allocation? > > Page allocator is a hot path and adding checks for something that > shouldn't really even exist is not a great addition there IMHO. I would argue adding checks for something that shouldn't really even exist is precisely the point of having those checks. These checks help ensure the integrity and robustness of the system by catching unexpected conditions. I agree that the page allocator is a hot path, but adding one line might not cause noticeable overhead, especially considering that alloc_pages() already contains a few hundred lines of code. Regardless, let me try to summarize the discussions. Unless anyone has better ideas, v2 series might start with the following actions: 1. Update the documentation for GFP_NOFAIL to explicitly state that non-wait GFP_NOFAIL is not legal and should be avoided. This will provide a basis for other subsystems to explicitly reject anyone using GFP_ATOMIC | GFP_NOFAIL, etc. 2. Add BUG_ON() at the existing points where we return NULL for GFP_NOFAIL - __alloc_pages_slowpath() , kmalloc, vmalloc to avoid exposing security vulnerabilities. 3. Raise a bug report to the vdpa maintainer, requesting that they either drop GFP_ATOMIC or GFP_NOFAIL based on whether their context is atomic. If GFP_NOFAIL is dropped, ask for an explicit check on the return value. Christoph, Michal, Hailong, Vlastimil, if you have any better ideas or objections, please let me know :-) > > -- > Michal Hocko > SUSE Labs Thanks Barry
