lkml.org 
[lkml]   [2018]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: dm bufio: Reduce dm_bufio_lock contention


On Tue, 19 Jun 2018, Michal Hocko wrote:

> On Mon 18-06-18 18:11:26, Mikulas Patocka wrote:
> [...]
> > I grepped the kernel for __GFP_NORETRY and triaged them. I found 16 cases
> > without a fallback - those are bugs that make various functions randomly
> > return -ENOMEM.
>
> Well, maybe those are just optimistic attempts to allocate memory and
> have a fallback somewhere else. So I would be careful calling them
> outright bugs. But maybe you are right.

I was trying to find the fallback code when I triaged them and maked as
"BUG" those cases where I didn't find it. You can search harder and
perhaps you'll find something that I didn't.

> > Most of the callers provide callback.
> >
> > There is another strange flag - __GFP_RETRY_MAYFAIL - it provides two
> > different functions - if the allocation is larger than
> > PAGE_ALLOC_COSTLY_ORDER, it retries the allocation as if it were smaller.
> > If the allocations is smaller than PAGE_ALLOC_COSTLY_ORDER,
> > __GFP_RETRY_MAYFAIL will avoid the oom killer (larger order allocations
> > don't trigger the oom killer at all).
>
> Well, the primary purpose of this flag is to provide a consistent
> failure behavior for all requests regardless of the size.
>
> > So, perhaps __GFP_RETRY_MAYFAIL could be used instead of __GFP_NORETRY in
> > the cases where the caller wants to avoid trigerring the oom killer (the
> > problem is that __GFP_NORETRY causes random failure even in no-oom
> > situations but __GFP_RETRY_MAYFAIL doesn't).
>
> myabe yes.
>
> > So my suggestion is - fix these obvious bugs when someone allocates memory
> > with __GFP_NORETRY without any fallback - and then, __GFP_NORETRY could be
> > just changed to return NULL instead of sleeping.
>
> No real objection to fixing wrong __GFP_NORETRY usage. But __GFP_NORETRY
> can sleep. Nothing will really change in that regards. It does a
> reclaim and that _might_ sleep.
>
> But seriously, isn't the best way around the throttling issue to use
> PF_LESS_THROTTLE?

Yes - it could be done by setting PF_LESS_THROTTLE. But I think it would
be better to change it just in one place than to add PF_LESS_THROTTLE to
every block device driver (because adding it to every block driver results
in more code).

What about this patch? If __GFP_NORETRY and __GFP_FS is not set (i.e. the
request comes from a block device driver or a filesystem), we should not
sleep.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org

Index: linux-2.6/mm/vmscan.c
===================================================================
--- linux-2.6.orig/mm/vmscan.c
+++ linux-2.6/mm/vmscan.c
@@ -2674,6 +2674,7 @@ static bool shrink_node(pg_data_t *pgdat
* the LRU too quickly.
*/
if (!sc->hibernation_mode && !current_is_kswapd() &&
+ (sc->gfp_mask & (__GFP_NORETRY | __GFP_FS)) != __GFP_NORETRY &&
current_may_throttle() && pgdat_memcg_congested(pgdat, root))
wait_iff_congested(BLK_RW_ASYNC, HZ/10);

\
 
 \ /
  Last update: 2018-06-22 03:17    [W:0.100 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site