Re: [PATCH 4/7] mm: introduce memalloc_nofs_{save,restore} API

From: David Sterba
Date: Thu Mar 09 2017 - 06:49:38 EST


On Tue, Mar 07, 2017 at 04:09:56PM +0100, Michal Hocko wrote:
> On Mon 06-03-17 13:22:14, Andrew Morton wrote:
> > On Mon, 6 Mar 2017 14:14:05 +0100 Michal Hocko <mhocko@xxxxxxxxxx> wrote:
> [...]
> > > --- a/include/linux/gfp.h
> > > +++ b/include/linux/gfp.h
> > > @@ -210,8 +210,16 @@ struct vm_area_struct;
> > > *
> > > * GFP_NOIO will use direct reclaim to discard clean pages or slab pages
> > > * that do not require the starting of any physical IO.
> > > + * Please try to avoid using this flag directly and instead use
> > > + * memalloc_noio_{save,restore} to mark the whole scope which cannot
> > > + * perform any IO with a short explanation why. All allocation requests
> > > + * will inherit GFP_NOIO implicitly.
> > > *
> > > * GFP_NOFS will use direct reclaim but will not use any filesystem interfaces.
> > > + * Please try to avoid using this flag directly and instead use
> > > + * memalloc_nofs_{save,restore} to mark the whole scope which cannot/shouldn't
> > > + * recurse into the FS layer with a short explanation why. All allocation
> > > + * requests will inherit GFP_NOFS implicitly.
> >
> > I wonder if these are worth a checkpatch rule.
>
> I am not really sure, to be honest. This may easilly end up people
> replacing
>
> do_alloc(GFP_NOFS)
>
> with
>
> memalloc_nofs_save()
> do_alloc(GFP_KERNEL)
> memalloc_nofs_restore()
>
> which doesn't make any sense of course. From my experience, people tend
> to do stupid things just to silent checkpatch warnings very often.
> Moreover I believe we need to do the transition to the new api first
> before we can push back on the explicit GFP_NOFS usage. Maybe then we
> can think about the a checkpatch warning.

I agree will all your objections against adding that to checkpatch, at
this point it's less harmful to use GFP_NOFS.