On Sat 20-07-24 08:36:16, Barry Song wrote: > 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. We do not add stuff like that into hot path. > 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. No objection to that. > 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. Let's see how this goes. > 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. GPF_ATOMIC is likely used because of write_lock(&domain->bounce_lock) vduse_domain_remove_user_bounce_pages is iself called with spin lock held from vduse_domain_release. So you would need some pre-allocation. -- Michal Hocko SUSE Labs
