Messages in this thread | | | From | Uladzislau Rezki <> | Date | Thu, 4 Nov 2021 12:14:14 +0100 | Subject | Re: [PATCH] mm/vmalloc: Eliminate an extra orig_gfp_mask |
| |
> [Cc Vasily] > > On Wed 03-11-21 21:07:03, Uladzislau Rezki wrote: > > That extra variable has been introduced just for keeping an original > > passed gfp_mask because it is updated with __GFP_NOWARN on entry, thus > > error handling messages were broken. > > I am not sure what you mean by "error handling messages were broken" > part. > We slightly discussed it in another thread :) There was __GFP_NOWARN added on entry(unconditionally), what leads to ignoring all our internal error messages by the warn_alloc(). I have checked the linux-next and saw that Vasily sent a patch fixing it:
<snip> Author: Vasily Averin <vvs@virtuozzo.com> Date: Thu Oct 21 15:07:26 2021 +1100
mm/vmalloc: repair warn_alloc()s in __vmalloc_area_node()
Commit f255935b9767 ("mm: cleanup the gfp_mask handling in __vmalloc_area_node") added __GFP_NOWARN to gfp_mask unconditionally however it disabled all output inside warn_alloc() call. This patch saves original gfp_mask and provides it to all warn_alloc() calls. <snip>
> It is true that the current Linus tree has a broken allocation failure > reporting but that is not a fault of orig_gfp_mask. In fact patch which > is fixing that "mm/vmalloc: repair warn_alloc()s in > __vmalloc_area_node()" currently in akpm tree is adding the additional > mask. > > > Instead we can keep an original gfp_mask without modifying it and add > > an extra __GFP_NOWARN flag together with gfp_mask as a parameter to > > the vm_area_alloc_pages() function. It will make it less confused. > > I would tend to agree that this is a better approach. There is already > nested_gfp mask and one more doesn't add to the readbility. > Agree, that is why i decided to send a patch. Because i find that extra gfp variable as odd one and confusing. I paid an attention on it during our discussion about __GFP_NOFAIL. But on my tree it was not fixed at all and after checking the linux-next i saw a fix.
> > Maybe we should just drop the above patch and just go with one which > doesn't introduce the intermediate step and an additional gfp mask. > That we can do if all agree on.
Thanks!
-- Vlad Rezki
| |