Messages in this thread | | | From | Uladzislau Rezki <> | Date | Tue, 26 Oct 2021 17:48:32 +0200 | Subject | Re: [PATCH 2/4] mm/vmalloc: add support for __GFP_NOFAIL |
| |
> From: Michal Hocko <mhocko@suse.com> > > Dave Chinner has mentioned that some of the xfs code would benefit from > kvmalloc support for __GFP_NOFAIL because they have allocations that > cannot fail and they do not fit into a single page. > > The larg part of the vmalloc implementation already complies with the > given gfp flags so there is no work for those to be done. The area > and page table allocations are an exception to that. Implement a retry > loop for those. > > Add a short sleep before retrying. 1 jiffy is a completely random > timeout. Ideally the retry would wait for an explicit event - e.g. > a change to the vmalloc space change if the failure was caused by > the space fragmentation or depletion. But there are multiple different > reasons to retry and this could become much more complex. Keep the retry > simple for now and just sleep to prevent from hogging CPUs. > > Signed-off-by: Michal Hocko <mhocko@suse.com> > --- > mm/vmalloc.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index c6cc77d2f366..602649919a9d 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -2941,8 +2941,12 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, > else if ((gfp_mask & (__GFP_FS | __GFP_IO)) == 0) > flags = memalloc_noio_save(); > > - ret = vmap_pages_range(addr, addr + size, prot, area->pages, > + do { > + ret = vmap_pages_range(addr, addr + size, prot, area->pages, > page_shift); > + if (ret < 0) > + schedule_timeout_uninterruptible(1); > + } while ((gfp_mask & __GFP_NOFAIL) && (ret < 0)); >
1. After that change a below code:
<snip> if (ret < 0) { warn_alloc(orig_gfp_mask, NULL, "vmalloc error: size %lu, failed to map pages", area->nr_pages * PAGE_SIZE); goto fail; } <snip>
does not make any sense anymore.
2. Can we combine two places where we handle __GFP_NOFAIL into one place? That would look like as more sorted out.
-- Uladzislau Rezki
| |