Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1423604 > unrolled thread

Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Dave Chinner <david@fromorbit.com> - 2016-06-16 02:30 +0200
    Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Michal Hocko <mhocko@kernel.org> - 2016-06-16 10:10 +0200
      Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Michal Hocko <mhocko@kernel.org> - 2016-06-16 13:30 +0200
        Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Johannes Weiner <hannes@cmpxchg.org> - 2016-06-17 20:30 +0200
          Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Vlastimil Babka <vbabka@suse.cz> - 2016-06-17 22:40 +0200
            Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Johannes Weiner <hannes@cmpxchg.org> - 2016-06-17 23:50 +0200
              Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Michal Hocko <mhocko@kernel.org> - 2016-06-20 12:00 +0200
                Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Johannes Weiner <hannes@cmpxchg.org> - 2016-06-21 07:00 +0200
                  Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Vlastimil Babka <vbabka@suse.cz> - 2016-06-21 11:30 +0200
                  Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD Michal Hocko <mhocko@kernel.org> - 2016-06-21 19:10 +0200

#1423604 — Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD

On Mon, Jun 06, 2016 at 01:32:16PM +0200, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> KM_MAYFAIL didn't have any suitable GFP_FOO counterpart until recently
> so it relied on the default page allocator behavior for the given set
> of flags. This means that small allocations actually never failed.
> 
> Now that we have __GFP_RETRY_HARD flags which works independently on the
> allocation request size we can map KM_MAYFAIL to it. The allocator will
> try as hard as it can to fulfill the request but fails eventually if
> the progress cannot be made.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  fs/xfs/kmem.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
> index 689f746224e7..34e6b062ce0e 100644
> --- a/fs/xfs/kmem.h
> +++ b/fs/xfs/kmem.h
> @@ -54,6 +54,9 @@ kmem_flags_convert(xfs_km_flags_t flags)
>  			lflags &= ~__GFP_FS;
>  	}
>  
> +	if (flags & KM_MAYFAIL)
> +		lflags |= __GFP_RETRY_HARD;
> +

I don't understand. KM_MAYFAIL means "caller handles
allocation failure, so retry on failure is not required." To then
map KM_MAYFAIL to a flag that implies the allocation will internally
retry to try exceptionally hard to prevent failure seems wrong.

IOWs, KM_MAYFAIL means XFS is just using for normal allocator
behaviour here, so I'm not sure what problem this change is actually
solving and it's not clear from the description....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [next] | [standalone]

#1423774

On Thu 16-06-16 10:23:02, Dave Chinner wrote:
> On Mon, Jun 06, 2016 at 01:32:16PM +0200, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > KM_MAYFAIL didn't have any suitable GFP_FOO counterpart until recently
> > so it relied on the default page allocator behavior for the given set
> > of flags. This means that small allocations actually never failed.
> > 
> > Now that we have __GFP_RETRY_HARD flags which works independently on the
> > allocation request size we can map KM_MAYFAIL to it. The allocator will
> > try as hard as it can to fulfill the request but fails eventually if
> > the progress cannot be made.
> > 
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> > ---
> >  fs/xfs/kmem.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
> > index 689f746224e7..34e6b062ce0e 100644
> > --- a/fs/xfs/kmem.h
> > +++ b/fs/xfs/kmem.h
> > @@ -54,6 +54,9 @@ kmem_flags_convert(xfs_km_flags_t flags)
> >  			lflags &= ~__GFP_FS;
> >  	}
> >  
> > +	if (flags & KM_MAYFAIL)
> > +		lflags |= __GFP_RETRY_HARD;
> > +
> 
> I don't understand. KM_MAYFAIL means "caller handles
> allocation failure, so retry on failure is not required." To then
> map KM_MAYFAIL to a flag that implies the allocation will internally
> retry to try exceptionally hard to prevent failure seems wrong.

The primary point, which I've tried to describe in the changelog, is
that the default allocator behavior is to retry endlessly for small
orders. You can override this by using __GFP_NORETRY which doesn't retry
at all and fails quite early. My understanding of KM_MAYFAIL is that
it can cope with allocation failures. The lack of __GFP_NORETRY made me
think that the failure should be prevented as much as possible.
__GFP_RETRY_HARD is semantically somwhere in the middle between
__GFP_NORETRY and __GFP_NOFAIL semantic independently on the allocation
size.

Does that make more sense now?
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1423967

On Thu 16-06-16 10:03:55, Michal Hocko wrote:
> On Thu 16-06-16 10:23:02, Dave Chinner wrote:
> > On Mon, Jun 06, 2016 at 01:32:16PM +0200, Michal Hocko wrote:
> > > From: Michal Hocko <mhocko@suse.com>
> > > 
> > > KM_MAYFAIL didn't have any suitable GFP_FOO counterpart until recently
> > > so it relied on the default page allocator behavior for the given set
> > > of flags. This means that small allocations actually never failed.
> > > 
> > > Now that we have __GFP_RETRY_HARD flags which works independently on the
> > > allocation request size we can map KM_MAYFAIL to it. The allocator will
> > > try as hard as it can to fulfill the request but fails eventually if
> > > the progress cannot be made.
> > > 
> > > Signed-off-by: Michal Hocko <mhocko@suse.com>
> > > ---
> > >  fs/xfs/kmem.h | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
> > > index 689f746224e7..34e6b062ce0e 100644
> > > --- a/fs/xfs/kmem.h
> > > +++ b/fs/xfs/kmem.h
> > > @@ -54,6 +54,9 @@ kmem_flags_convert(xfs_km_flags_t flags)
> > >  			lflags &= ~__GFP_FS;
> > >  	}
> > >  
> > > +	if (flags & KM_MAYFAIL)
> > > +		lflags |= __GFP_RETRY_HARD;
> > > +
> > 
> > I don't understand. KM_MAYFAIL means "caller handles
> > allocation failure, so retry on failure is not required." To then
> > map KM_MAYFAIL to a flag that implies the allocation will internally
> > retry to try exceptionally hard to prevent failure seems wrong.
> 
> The primary point, which I've tried to describe in the changelog, is
> that the default allocator behavior is to retry endlessly for small
> orders. You can override this by using __GFP_NORETRY which doesn't retry
> at all and fails quite early. My understanding of KM_MAYFAIL is that
> it can cope with allocation failures. The lack of __GFP_NORETRY made me
> think that the failure should be prevented as much as possible.
> __GFP_RETRY_HARD is semantically somwhere in the middle between
> __GFP_NORETRY and __GFP_NOFAIL semantic independently on the allocation
> size.
> 
> Does that make more sense now?

I would add the following explanation into the code:
diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
index 34e6b062ce0e..10708f065191 100644
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -54,6 +54,13 @@ kmem_flags_convert(xfs_km_flags_t flags)
 			lflags &= ~__GFP_FS;
 	}
 
+	/*
+	 * Default page/slab allocator behavior is to retry for ever
+	 * for small allocations. We can override this behavior by using
+	 * __GFP_RETRY_HARD which will tell the allocator to retry as long
+	 * as it is feasible but rather fail than retry for ever for all
+	 * request sizes.
+	 */
 	if (flags & KM_MAYFAIL)
 		lflags |= __GFP_RETRY_HARD;
 

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1425378

On Thu, Jun 16, 2016 at 01:26:06PM +0200, Michal Hocko wrote:
> @@ -54,6 +54,13 @@ kmem_flags_convert(xfs_km_flags_t flags)
>  			lflags &= ~__GFP_FS;
>  	}
>  
> +	/*
> +	 * Default page/slab allocator behavior is to retry for ever
> +	 * for small allocations. We can override this behavior by using
> +	 * __GFP_RETRY_HARD which will tell the allocator to retry as long
> +	 * as it is feasible but rather fail than retry for ever for all
> +	 * request sizes.
> +	 */
>  	if (flags & KM_MAYFAIL)
>  		lflags |= __GFP_RETRY_HARD;

I think this example shows that __GFP_RETRY_HARD is not a good flag
because it conflates two seemingly unrelated semantics; the comment
doesn't quite make up for that.

When the flag is set,

- it allows costly orders to invoke the OOM killer and retry
- it allows !costly orders to fail

While 1. is obvious from the name, 2. is not. Even if we don't want
full-on fine-grained naming for every reclaim methodology and retry
behavior, those two things just shouldn't be tied together.

I don't see us failing !costly order per default anytime soon, and
they are common, so adding a __GFP_MAYFAIL to explicitely override
that behavior seems like a good idea to me. That would make the XFS
callsite here perfectly obvious.

And you can still combine it with __GFP_REPEAT.

For a generic allocation site like this, __GFP_MAYFAIL | __GFP_REPEAT
does the right thing for all orders, and it's self-explanatory: try
hard, allow falling back.

Whether we want a __GFP_REPEAT or __GFP_TRY_HARD at all is a different
topic. In the long term, it might be better to provide best-effort per
default and simply annotate MAYFAIL/NORETRY callsites that want to
give up earlier. Because as I mentioned at LSFMM, it's much easier to
identify callsites that have a convenient fallback than callsites that
need to "try harder." Everybody thinks their allocations are oh so
important. The former is much more specific and uses obvious criteria.

Either way, __GFP_MAYFAIL should be on its own.

[toc] | [prev] | [next] | [standalone]

#1425459

On 17.6.2016 20:22, Johannes Weiner wrote:
> On Thu, Jun 16, 2016 at 01:26:06PM +0200, Michal Hocko wrote:
>> @@ -54,6 +54,13 @@ kmem_flags_convert(xfs_km_flags_t flags)
>>  			lflags &= ~__GFP_FS;
>>  	}
>>  
>> +	/*
>> +	 * Default page/slab allocator behavior is to retry for ever
>> +	 * for small allocations. We can override this behavior by using
>> +	 * __GFP_RETRY_HARD which will tell the allocator to retry as long
>> +	 * as it is feasible but rather fail than retry for ever for all
>> +	 * request sizes.
>> +	 */
>>  	if (flags & KM_MAYFAIL)
>>  		lflags |= __GFP_RETRY_HARD;
> 
> I think this example shows that __GFP_RETRY_HARD is not a good flag
> because it conflates two seemingly unrelated semantics; the comment
> doesn't quite make up for that.
> 
> When the flag is set,
> 
> - it allows costly orders to invoke the OOM killer and retry

No, it's not allowing the OOM killer for costly orders, only non-costly, AFAIK.
Mainly it allows more aggressive compaction (especially after my series [1]).

> - it allows !costly orders to fail
> 
> While 1. is obvious from the name, 2. is not. Even if we don't want
> full-on fine-grained naming for every reclaim methodology and retry
> behavior, those two things just shouldn't be tied together.

Well, if allocation is not allowed to fail, it's like trying "indefinitely hard"
already. Telling it it should "try hard" then doesn't make any sense without
also being able to fail.

> I don't see us failing !costly order per default anytime soon, and
> they are common, so adding a __GFP_MAYFAIL to explicitely override
> that behavior seems like a good idea to me. That would make the XFS
> callsite here perfectly obvious.
> 
> And you can still combine it with __GFP_REPEAT.

But that would mean the following meaningful combinations for non-costly orders
(assuming e.g. GFP_KERNEL which allows reclaim/compaction in the first place).

__GFP_NORETRY - that one is well understood hopefully, and implicitly mayfail

__GFP_MAYFAIL - ???
__GFP_MAYFAIL | __GFP_REPEAT - ???

Which one of the last two tries harder? How specifically? Will they differ by
(not) allowing OOM? Won't that be just extra confusing?

> For a generic allocation site like this, __GFP_MAYFAIL | __GFP_REPEAT
> does the right thing for all orders, and it's self-explanatory: try
> hard, allow falling back.
> 
> Whether we want a __GFP_REPEAT or __GFP_TRY_HARD at all is a different
> topic. In the long term, it might be better to provide best-effort per
> default and simply annotate MAYFAIL/NORETRY callsites that want to
> give up earlier. Because as I mentioned at LSFMM, it's much easier to
> identify callsites that have a convenient fallback than callsites that
> need to "try harder." Everybody thinks their allocations are oh so
> important. The former is much more specific and uses obvious criteria.

For higher-order allocations, best-effort might also mean significant system
disruption, not just latency of the allocation itself. One example is hugeltbfs
allocations (echo X > .../nr_hugepages) where the admin is willing to pay this
cost. But to do that by default and rely on everyone else passing NORETRY
wouldn't go far. So I think the TRY_HARD kind of flag makes sense.

> Either way, __GFP_MAYFAIL should be on its own.

[1] https://lwn.net/Articles/689154/

[toc] | [prev] | [next] | [standalone]

#1425504

On Fri, Jun 17, 2016 at 10:30:06PM +0200, Vlastimil Babka wrote:
> On 17.6.2016 20:22, Johannes Weiner wrote:
> > On Thu, Jun 16, 2016 at 01:26:06PM +0200, Michal Hocko wrote:
> >> @@ -54,6 +54,13 @@ kmem_flags_convert(xfs_km_flags_t flags)
> >>  			lflags &= ~__GFP_FS;
> >>  	}
> >>  
> >> +	/*
> >> +	 * Default page/slab allocator behavior is to retry for ever
> >> +	 * for small allocations. We can override this behavior by using
> >> +	 * __GFP_RETRY_HARD which will tell the allocator to retry as long
> >> +	 * as it is feasible but rather fail than retry for ever for all
> >> +	 * request sizes.
> >> +	 */
> >>  	if (flags & KM_MAYFAIL)
> >>  		lflags |= __GFP_RETRY_HARD;
> > 
> > I think this example shows that __GFP_RETRY_HARD is not a good flag
> > because it conflates two seemingly unrelated semantics; the comment
> > doesn't quite make up for that.
> > 
> > When the flag is set,
> > 
> > - it allows costly orders to invoke the OOM killer and retry
> 
> No, it's not allowing the OOM killer for costly orders, only non-costly, AFAIK.
> Mainly it allows more aggressive compaction (especially after my series [1]).

Ah, you're right. It calls into the may_oom function but that skips
actual killing for costly orders.

> > - it allows !costly orders to fail
> > 
> > While 1. is obvious from the name, 2. is not. Even if we don't want
> > full-on fine-grained naming for every reclaim methodology and retry
> > behavior, those two things just shouldn't be tied together.
> 
> Well, if allocation is not allowed to fail, it's like trying "indefinitely hard"
> already. Telling it it should "try hard" then doesn't make any sense without
> also being able to fail.

I can see that argument, but it's really anything but obvious at the
callsite. Dave's response to Michal's patch was a good demonstration.
And I don't think adding comments fixes an unintuitive interface.

> > I don't see us failing !costly order per default anytime soon, and
> > they are common, so adding a __GFP_MAYFAIL to explicitely override
> > that behavior seems like a good idea to me. That would make the XFS
> > callsite here perfectly obvious.
> > 
> > And you can still combine it with __GFP_REPEAT.
> 
> But that would mean the following meaningful combinations for non-costly orders
> (assuming e.g. GFP_KERNEL which allows reclaim/compaction in the first place).

I would ignore order here. Part of what makes this interface
unintuitive is when we expect different flags to be passed for
different orders, especially because the orders are often
variable. Michal's __GFP_RETRY_HARD is an improvement in the sense
that it ignores the order and tries to do the right thing regardless
of it. The interface should really be about the intent at the
callsite, not about implementation details of the allocator.

But adding TRY_HARD to express "this can fail" isn't intuitive.

> __GFP_NORETRY - that one is well understood hopefully, and implicitly mayfail

Yeah. Never OOM, never retry etc. The callsite can fall back, and
prefers that over OOM kills and disruptive allocation latencies.

> __GFP_MAYFAIL - ???

May OOM for certain orders and retry a few times, but still fail. The
callsite can fall back, but it wouldn't come for free. E.g. it might
have to abort an explicitely requested user operation.

This is the default for costly orders, so it has an effect only on
non-costly orders. But that's where I would separate interface from
implementation: you'd use it e.g. in callsites where you have variable
orders but always the same fallback. XFS does that extensively.

> __GFP_MAYFAIL | __GFP_REPEAT - ???
> 
> Which one of the last two tries harder? How specifically? Will they differ by
> (not) allowing OOM? Won't that be just extra confusing?

Adding __GFP_REPEAT would always be additive. This combination would
mean: try the hardest not to fail, but don't lock up in cases when the
order happens to be !costly.

Again, I'm not too thrilled about that flag as it's so damn vague. But
that's more about how we communicate latency/success expectations. My
concern is exclusively about its implication of MAYFAIL.

> > For a generic allocation site like this, __GFP_MAYFAIL | __GFP_REPEAT
> > does the right thing for all orders, and it's self-explanatory: try
> > hard, allow falling back.
> > 
> > Whether we want a __GFP_REPEAT or __GFP_TRY_HARD at all is a different
> > topic. In the long term, it might be better to provide best-effort per
> > default and simply annotate MAYFAIL/NORETRY callsites that want to
> > give up earlier. Because as I mentioned at LSFMM, it's much easier to
> > identify callsites that have a convenient fallback than callsites that
> > need to "try harder." Everybody thinks their allocations are oh so
> > important. The former is much more specific and uses obvious criteria.
> 
> For higher-order allocations, best-effort might also mean significant system
> disruption, not just latency of the allocation itself. One example is hugeltbfs
> allocations (echo X > .../nr_hugepages) where the admin is willing to pay this
> cost. But to do that by default and rely on everyone else passing NORETRY
> wouldn't go far. So I think the TRY_HARD kind of flag makes sense.

I think whether the best-effort behavior should be opt-in or opt-out,
or how fine-grained the latency/success control over the allocator
should be is a different topic. I'd prefer defaulting to reliability
and annotating low-latency requirements, but I can see TRY_HARD work
too. It just shouldn't imply MAY_FAIL.

[toc] | [prev] | [next] | [standalone]

#1426451

On Fri 17-06-16 17:39:31, Johannes Weiner wrote:
> On Fri, Jun 17, 2016 at 10:30:06PM +0200, Vlastimil Babka wrote:
> > On 17.6.2016 20:22, Johannes Weiner wrote:
[...]
> > > - it allows !costly orders to fail
> > > 
> > > While 1. is obvious from the name, 2. is not. Even if we don't want
> > > full-on fine-grained naming for every reclaim methodology and retry
> > > behavior, those two things just shouldn't be tied together.
> > 
> > Well, if allocation is not allowed to fail, it's like trying "indefinitely hard"
> > already. Telling it it should "try hard" then doesn't make any sense without
> > also being able to fail.
> 
> I can see that argument, but it's really anything but obvious at the
> callsite. Dave's response to Michal's patch was a good demonstration.
> And I don't think adding comments fixes an unintuitive interface.

Yeah, I am aware of that. And it is unfortunate but a side effect of our
!costly vs. costly difference in the default behavior. What I wanted
to achieve was to have overrides for the default behavior (whatever it
is). We already have two such flags and having something semantically in
the middle sounds like a consistent way to me.

> > > I don't see us failing !costly order per default anytime soon, and
> > > they are common, so adding a __GFP_MAYFAIL to explicitely override
> > > that behavior seems like a good idea to me. That would make the XFS
> > > callsite here perfectly obvious.
> > > 
> > > And you can still combine it with __GFP_REPEAT.
> > 
> > But that would mean the following meaningful combinations for non-costly orders
> > (assuming e.g. GFP_KERNEL which allows reclaim/compaction in the first place).
> 
> I would ignore order here. Part of what makes this interface
> unintuitive is when we expect different flags to be passed for
> different orders, especially because the orders are often
> variable. Michal's __GFP_RETRY_HARD is an improvement in the sense
> that it ignores the order and tries to do the right thing regardless
> of it. The interface should really be about the intent at the
> callsite, not about implementation details of the allocator.
>
> But adding TRY_HARD to express "this can fail" isn't intuitive.

I am all for a better name but everything else I could come up with was
just more confusing. Take __GFP_MAYFAIL as an example. How it would be
any less confusing? Aren't all the requests which do not have
__GFP_NOFAIL automatically MAYFAIL? RETRY_HARD was an attempt to tell
you can retry as hard as you find reasonable but fail eventually which
should fit quite nicely between NORETRY and NOFAIL.

> > __GFP_NORETRY - that one is well understood hopefully, and implicitly mayfail
> 
> Yeah. Never OOM, never retry etc. The callsite can fall back, and
> prefers that over OOM kills and disruptive allocation latencies.
> 
> > __GFP_MAYFAIL - ???
> 
> May OOM for certain orders and retry a few times, but still fail. The
> callsite can fall back, but it wouldn't come for free. E.g. it might
> have to abort an explicitely requested user operation.
> 
> This is the default for costly orders, so it has an effect only on
> non-costly orders. But that's where I would separate interface from
> implementation: you'd use it e.g. in callsites where you have variable
> orders but always the same fallback. XFS does that extensively.
> 
> > __GFP_MAYFAIL | __GFP_REPEAT - ???
> > 
> > Which one of the last two tries harder? How specifically? Will they differ by
> > (not) allowing OOM? Won't that be just extra confusing?
> 
> Adding __GFP_REPEAT would always be additive. This combination would
> mean: try the hardest not to fail, but don't lock up in cases when the
> order happens to be !costly.
> 
> Again, I'm not too thrilled about that flag as it's so damn vague. But
> that's more about how we communicate latency/success expectations. My
> concern is exclusively about its implication of MAYFAIL.

Our gfp flags space is quite full and additing a new flag while we keep
one with a vague meaning doesn't sound very well to me. So I really
think we should just ditch __GFP_REPEAT. Whether __GFP_MAYFAIL is a
better name for the new flag I dunno. It feels confusing to me but if
that is a general agreement I don't have a big problem with that.

> > > For a generic allocation site like this, __GFP_MAYFAIL | __GFP_REPEAT
> > > does the right thing for all orders, and it's self-explanatory: try
> > > hard, allow falling back.
> > > 
> > > Whether we want a __GFP_REPEAT or __GFP_TRY_HARD at all is a different
> > > topic. In the long term, it might be better to provide best-effort per
> > > default and simply annotate MAYFAIL/NORETRY callsites that want to
> > > give up earlier. Because as I mentioned at LSFMM, it's much easier to
> > > identify callsites that have a convenient fallback than callsites that
> > > need to "try harder." Everybody thinks their allocations are oh so
> > > important. The former is much more specific and uses obvious criteria.
> > 
> > For higher-order allocations, best-effort might also mean significant system
> > disruption, not just latency of the allocation itself. One example is hugeltbfs
> > allocations (echo X > .../nr_hugepages) where the admin is willing to pay this
> > cost. But to do that by default and rely on everyone else passing NORETRY
> > wouldn't go far. So I think the TRY_HARD kind of flag makes sense.
> 
> I think whether the best-effort behavior should be opt-in or opt-out,
> or how fine-grained the latency/success control over the allocator
> should be is a different topic. I'd prefer defaulting to reliability
> and annotating low-latency requirements, but I can see TRY_HARD work
> too. It just shouldn't imply MAY_FAIL.

It is always hard to change the default behavior without breaking
anything. Up to now we had opt-in and as you can see there are not that
many users who really wanted to have higher reliability. I guess this is
because they just do not care and didn't see too many failures. The
opt-out has also a disadvantage that we would need to provide a flag
to tell to try less hard and all we have is NORETRY and that is way too
easy. So to me it sounds like the opt-in fits better with the current
usage.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1427294

On Mon, Jun 20, 2016 at 10:08:56AM +0200, Michal Hocko wrote:
> On Fri 17-06-16 17:39:31, Johannes Weiner wrote:
> > On Fri, Jun 17, 2016 at 10:30:06PM +0200, Vlastimil Babka wrote:
> > > On 17.6.2016 20:22, Johannes Weiner wrote:
> [...]
> > > > - it allows !costly orders to fail
> > > > 
> > > > While 1. is obvious from the name, 2. is not. Even if we don't want
> > > > full-on fine-grained naming for every reclaim methodology and retry
> > > > behavior, those two things just shouldn't be tied together.
> > > 
> > > Well, if allocation is not allowed to fail, it's like trying "indefinitely hard"
> > > already. Telling it it should "try hard" then doesn't make any sense without
> > > also being able to fail.
> > 
> > I can see that argument, but it's really anything but obvious at the
> > callsite. Dave's response to Michal's patch was a good demonstration.
> > And I don't think adding comments fixes an unintuitive interface.
> 
> Yeah, I am aware of that. And it is unfortunate but a side effect of our
> !costly vs. costly difference in the default behavior. What I wanted
> to achieve was to have overrides for the default behavior (whatever it
> is). We already have two such flags and having something semantically in
> the middle sounds like a consistent way to me.

The "whatever it is" is the problem I'm having. It's one flag that
does two entirely orthogonal things, and it's quite reasonable for
somebody to want to change one behavior without the other.

"I can handle allocation failures, even when they are !costly"

has really nothing to do with

"I am ready to pay high a allocation latency to make costly
allocations succeed."

So while I understand that you want an effort-flag leveled somewhere
between NORETRY and NOFAIL, this looks more like a theoretical thing
than what existing callsites actually would want to use.

> > I think whether the best-effort behavior should be opt-in or opt-out,
> > or how fine-grained the latency/success control over the allocator
> > should be is a different topic. I'd prefer defaulting to reliability
> > and annotating low-latency requirements, but I can see TRY_HARD work
> > too. It just shouldn't imply MAY_FAIL.
> 
> It is always hard to change the default behavior without breaking
> anything. Up to now we had opt-in and as you can see there are not that
> many users who really wanted to have higher reliability. I guess this is
> because they just do not care and didn't see too many failures. The
> opt-out has also a disadvantage that we would need to provide a flag
> to tell to try less hard and all we have is NORETRY and that is way too
> easy. So to me it sounds like the opt-in fits better with the current
> usage.

For costly allocations, the presence of __GFP_NORETRY is exactly the
same as the absence of __GFP_REPEAT. So if we made __GFP_REPEAT the
default (and deleted the flag), the opt-outs would use __GFP_NORETRY
to restore their original behavior.

As for changing the default - remember that we currently warn about
allocation failures as if they were bugs, unless they are explicitely
allocated with the __GFP_NOWARN flag. We can assume that the current
__GFP_NOWARN sites are 1) commonly failing but 2) prefer to fall back
rather than incurring latency (otherwise they would have added the
__GFP_REPEAT flag). These sites would be a good list of candidates to
annotate with __GFP_NORETRY. If we made __GFP_REPEAT then the default,
the sites that would then try harder are the same sites that would now
emit page allocation failure warnings. These are rare, and the only
times I have seen them is under enough load that latency is shot to
hell anyway. So I'm not really convinced by the regression argument.

But that would *actually* clean up the flags, not make them even more
confusing:

Allocations that can't ever handle failure would use __GFP_NOFAIL.

Callers like XFS would use __GFP_MAYFAIL specifically to disable the
implicit __GFP_NOFAIL of !costly allocations.

Callers that would prefer falling back over killing and looping would
use __GFP_NORETRY.

Wouldn't that cover all usecases and be much more intuitive, both in
the default behavior as well as in the names of the flags?

[toc] | [prev] | [next] | [standalone]

#1427509

On 06/21/2016 06:22 AM, Johannes Weiner wrote:
>>> I think whether the best-effort behavior should be opt-in or opt-out,
>>> or how fine-grained the latency/success control over the allocator
>>> should be is a different topic. I'd prefer defaulting to reliability
>>> and annotating low-latency requirements, but I can see TRY_HARD work
>>> too. It just shouldn't imply MAY_FAIL.
>>
>> It is always hard to change the default behavior without breaking
>> anything. Up to now we had opt-in and as you can see there are not that
>> many users who really wanted to have higher reliability. I guess this is
>> because they just do not care and didn't see too many failures. The
>> opt-out has also a disadvantage that we would need to provide a flag
>> to tell to try less hard and all we have is NORETRY and that is way too
>> easy. So to me it sounds like the opt-in fits better with the current
>> usage.
>
> For costly allocations, the presence of __GFP_NORETRY is exactly the
> same as the absence of __GFP_REPEAT. So if we made __GFP_REPEAT the
> default (and deleted the flag), the opt-outs would use __GFP_NORETRY
> to restore their original behavior.

Just FYI, this argument distorts my idea how to get rid of hacky checks 
for GFP_TRANSHUGE and PF_KTHREAD (patches 05 and 06 in [1]), where I 
observed the mentioned no difference between __GFP_NORETRY presence and 
__GFP_REPEAT absence, and made use of it. Without __GFP_REPEAT I'd have 
two options for khugepaged and madvise(MADV_HUGEPAGE) allocations. 
Either pass __GFP_NORETRY and make them fail more, or don't and then 
they become much more disruptive (if the default becomes best-effort, 
i.e. what __GFP_REPEAT used to do).

[1] http://thread.gmane.org/gmane.linux.kernel.mm/152313

> As for changing the default - remember that we currently warn about
> allocation failures as if they were bugs, unless they are explicitely
> allocated with the __GFP_NOWARN flag. We can assume that the current
> __GFP_NOWARN sites are 1) commonly failing but 2) prefer to fall back
> rather than incurring latency (otherwise they would have added the
> __GFP_REPEAT flag). These sites would be a good list of candidates to
> annotate with __GFP_NORETRY. If we made __GFP_REPEAT then the default,
> the sites that would then try harder are the same sites that would now
> emit page allocation failure warnings. These are rare, and the only
> times I have seen them is under enough load that latency is shot to
> hell anyway. So I'm not really convinced by the regression argument.
>
> But that would *actually* clean up the flags, not make them even more
> confusing:
>
> Allocations that can't ever handle failure would use __GFP_NOFAIL.
>
> Callers like XFS would use __GFP_MAYFAIL specifically to disable the
> implicit __GFP_NOFAIL of !costly allocations.
>
> Callers that would prefer falling back over killing and looping would
> use __GFP_NORETRY.
>
> Wouldn't that cover all usecases and be much more intuitive, both in
> the default behavior as well as in the names of the flags?
>

[toc] | [prev] | [next] | [standalone]

#1427964

On Tue 21-06-16 00:22:49, Johannes Weiner wrote:
[...]
> As for changing the default - remember that we currently warn about
> allocation failures as if they were bugs, unless they are explicitely
> allocated with the __GFP_NOWARN flag. We can assume that the current
> __GFP_NOWARN sites are 1) commonly failing but 2) prefer to fall back
> rather than incurring latency (otherwise they would have added the
> __GFP_REPEAT flag). These sites would be a good list of candidates to
> annotate with __GFP_NORETRY.

This sounds like a good idea at first sight but a brief git grep shows
that many of them are just trying to silence the warning from non
sleeping allocations which are quite likely to fail. This wouldn't
be hard to filter out so we can ignore them.

Then there are things like 8be04b9374e5 ("treewide: Add __GFP_NOWARN
to k.alloc calls with v.alloc fallbacks") where the flag is added to
many places with vmalloc fallbacks. Do we want to weaken them in
favor of the vmalloc in general (note that it is not clear from the size
whether they are costly or !costly)?

I have looked at some random others and they are adding the flag without
any explanation so it is not really clear what was the motivation.

To me it seems like the flag is used quite randomly. I suspect there are
many places which do not have that flag just because nobody bothered to
report the allocation failure which is hard to reproduce.

> If we made __GFP_REPEAT then the default,
> the sites that would then try harder are the same sites that would now
> emit page allocation failure warnings. These are rare, and the only
> times I have seen them is under enough load that latency is shot to
> hell anyway. So I'm not really convinced by the regression argument.

You do not need to be under a heavy load to fail those allocations. It
is sufficient to have the memory fragmented which might be just a matter
of time. I am worried that we have hard to examine number of allocation
requests that might change the overall system behavior because they
might trigger more reclaim/swap and the source of the behavior change
wouldn't be quite obvious. On the other hand we already have some places
already annotated to require a more effort which is the reason I would
find it better to follow up with that.

High order allocations can be really expensive and the current behavior
with the allocation warning has an advantage that we can see the failure
mode and get a bug report with the exact trace (hopefully) without too
much of a background interference. Then the subsystem familiar person
can judge whether that particular allocation is worth more effort or
different fallback.

I am not saying that changing the default behavior for costly
allocations is a no go. I just feel it is too risky and it would be
better to use "override the default because I know what I am doing"
flag. The __GFP_RETRY_HARD might be a terrible name and a better name
would cause less confusion (__GFP_RETRY_MAYFAIL?).

> But that would *actually* clean up the flags, not make them even more
> confusing:
> 
> Allocations that can't ever handle failure would use __GFP_NOFAIL.
> 
> Callers like XFS would use __GFP_MAYFAIL specifically to disable the
> implicit __GFP_NOFAIL of !costly allocations.
>
> Callers that would prefer falling back over killing and looping would
> use __GFP_NORETRY.
> 
> Wouldn't that cover all usecases and be much more intuitive, both in
> the default behavior as well as in the names of the flags?

How do we describe kcompacd vs. page fault THP allocations? We do not
want to cause a lot of reclaim for those but we can wait for compaction
for the first while we would prefer not to for the later.

Thanks!
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web