Re: [PATCH v1 07/14] fs: Add aop_flags parameter to create_page_buffers()

Matthew Wilcox
2022-02-16T18:34:28Z

On Wed, Feb 16, 2022 at 10:30:33AM -0800, Stefan Roesch wrote:
> On 2/14/22 10:14 AM, Matthew Wilcox wrote:
> > On Mon, Feb 14, 2022 at 09:43:56AM -0800, Stefan Roesch wrote:
> >> This adds the aop_flags parameter to the create_page_buffers function.
> >> When AOP_FLAGS_NOWAIT parameter is set, the atomic allocation flag is
> >> set. The AOP_FLAGS_NOWAIT flag is set, when async buffered writes are
> >> enabled.
> >
> > Why is this better than passing in gfp flags directly?
> >
>
> I don't think that gfp flags are a great fit here. We only want to pass in
> a nowait flag and this does not map nicely to a gfp flag.

... what? The only thing you do with this flag is use it to choose
some gfp flags. Pass those gfp flags in directly.

> >> + gfp_t gfp = GFP_NOFS | __GFP_ACCOUNT;
> >> +
> >> + if (aop_flags & AOP_FLAGS_NOWAIT) {
> >> + gfp |= GFP_ATOMIC | __GFP_NOWARN;
> >> + gfp &= ~__GFP_DIRECT_RECLAIM;
> >> + } else {
> >> + gfp |= __GFP_NOFAIL;
> >> + }

The flags you've chosen here are also bonkers, but I'm not sure that
it's worth explaining to you why if you're this resistant to making
obvious corrections to your patches.