On Fri, 15 Jun 2018, Michal Hocko wrote:
> On Fri 15-06-18 08:47:52, Mikulas Patocka wrote:
> >
> >
> > On Fri, 15 Jun 2018, Michal Hocko wrote:
> >
> > > On Fri 15-06-18 07:35:07, Mikulas Patocka wrote:
> > > >
> > > > Because mempool uses it. Mempool uses allocations with "GFP_NOIO |
> > > > __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN". An so dm-bufio uses
> > > > these flags too. dm-bufio is just a big mempool.
> > >
> > > This doesn't answer my question though. Somebody else is doing it is not
> > > an explanation. Prior to your 41c73a49df31 there was no GFP_NOIO
> > > allocation AFAICS. So why do you really need it now? Why cannot you
> >
> > dm-bufio always used "GFP_NOIO | __GFP_NORETRY | __GFP_NOMEMALLOC |
> > __GFP_NOWARN" since the kernel 3.2 when it was introduced.
> >
> > In the kernel 4.10, dm-bufio was changed so that it does GFP_NOWAIT
> > allocation, then drops the lock and does GFP_NOIO with the dropped lock
> > (because someone was likely experiencing the same issue that is reported
> > in this thread) - there are two commits that change it - 9ea61cac0 and
> > 41c73a49df31.
>
> OK, I see. Is there any fundamental reason why this path has to do one
> round of GFP_IO or it can keep NOWAIT, drop the lock, sleep and retry
> again?
If the process is woken up, there was some buffer added to the freelist,
or refcount of some buffer was dropped to 0. In this case, we don't want
to drop the lock and use GFP_NOIO, because the freed buffer may disappear
when we drop the lock.
> [...]
> > > is the same class of problem, honestly, I dunno. And I've already said
> > > that stalling __GFP_NORETRY might be a good way around that but that
> > > needs much more consideration and existing users examination. I am not
> > > aware anybody has done that. Doing changes like that based on a single
> > > user is certainly risky.
> >
> > Why don't you set any rules how these flags should be used?
>
> It is really hard to change rules during the game. You basically have to
> examine all existing users and that is well beyond my time scope. I've
> tried that where it was possible. E.g. __GFP_REPEAT and turned it into a
> well defined semantic. __GFP_NORETRY is a bigger beast.
>
> Anyway, I believe that it would be much safer to look at the problem
> from a highlevel perspective. You seem to be focused on __GFP_NORETRY
> little bit too much IMHO. We are not throttling callers which explicitly
> do not want to or cannot - see current_may_throttle. Is it possible that
> both your md and mempool allocators can either (ab)use PF_LESS_THROTTLE
> or use other means? E.g. do you have backing_dev_info at that time?
> --
> Michal Hocko
> SUSE Labs
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.
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).
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).
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.
arch/arm/mm/dma-mapping.c - fallback to a smaller size without __GFP_NORETRY
arch/mips/mm/dma-default.c - says that it uses __GFP_NORETRY to avoid the
oom killer, provides no fallback - it seems to be a BUG
arch/sparc/mm/tsb.c - fallback to a smaller size without __GFP_NORETRY
arch/x86/include/asm/floppy.h - __GFP_NORETRY doesn't seem to serve any
purpose, it may cause random failures during initialization, can be
removed - BUG
arch/powerpc/mm/mmu_context_iommu.c - uses it just during moving pages,
there's no problem with failure
arch/powerpc/platforms/pseries/cmm.c - a vm balloon driver, no problem
with failure
block/bio.c - falls back to mempool
block/blk-mq.c - errorneous use of __GFP_NORETRY during initialization, it
falls back to a smaller size, but doesn't drop the __GFP_NORETRY flag
(BUG)
drivers/gpu/drm/i915/i915_gem.c - it starts with __GFP_NORETRY and on
failure, it ORs it with __GFP_RETRY_MAYFAIL (which of these conflicting
flags wins?)
drivers/gpu/drm/i915/i915_gem_gtt.c - __GFP_NORETRY is used during
initialization (BUG), it shouldn't be used
drivers/gpu/drm/i915/i915_gem_execbuffer.c - fallback to a smaller size without
__GFP_NORETRY
drivers/gpu/drm/i915/i915_gem_internal.c - fallback to a smaller size without
__GFP_NORETRY
size
drivers/gpu/drm/i915/i915_gem_userptr.c - seems to provide fallback
drivers/gpu/drm/i915/i915_gpu_error.c - fallback to a smaller size without
__GFP_NORETRY
drivers/gpu/drm/etnaviv/etnaviv_dump.c - coredump on error path, no
problem if it fails
drivers/gpu/drm/ttm/ttm_page_alloc.c - uses __GFP_NORETRY for transparent
hugepages, no problem with failure
drivers/gpu/drm/ttm/ttm_page_alloc_dma.c - uses __GFP_NORETRY for
transparent hugepages, no problem with failure
drivers/gpu/drm/msm/msm_gem_submit.c - uses __GFP_NORETRY to process ioctl
and lacks a fallback - it is a BUG - __GFP_NORETRY should be dropped
drivers/hv/hv_balloon.c - a vm balloon driver, no problem with failure
drivers/crypto/chelsio/chtls/chtls_io.c - fallback to a smaller size without
__GFP_NORETRY
drivers/xen/balloon.c - a vm balloon driver, no problem with failure
drivers/mtd/mtdcore.c - fallback to a smaller size without __GFP_NORETRY
drivers/md/dm-verity-target.c - skips prefetch on failure, no problem
drivers/md/dm-writecache.c - falls back to a smaller i/os on failure
drivers/md/dm-bufio.c - reserves some buffers on creation and falls back
to them
drivers/md/dm-integrity.c - falls back to sector-by-sector verification
drivers/md/dm-kcopyd.c - falls back to reserved pages
drivers/md/dm-crypt.c - falls back to mempool
drivers/iommu/dma-iommu.c - fallback to a smaller size without __GFP_NORETRY
drivers/mmc/core/mmc_test.c - fallback to a smaller size, but doesn't drop
__GFP_NORETRY - BUG
drivers/staging/android/ion/ion_system_heap.c - fallback to a smaller size
without __GFP_NORETRY
fs/cachefiles/ - uses __GFP_NORETRY extensively - since this is just a
cache, so failure supposedly shouldn't be problem - but it's hard to
verify the whole driver that it handles failures properly
fs/xfs/xfs_buf.c - uses __GFP_NORETRY only on readahead
fs/fscache/cookie.c - no fallback, but if it fails, the caller will just
invalidate the cache entry - no problem
fs/fscache/page.c - like above - failure will just inhibit caching
fs/nfs/write.c - fails only if allowed by the arugment never_fail
include/linux/kexec.h - no fallback - it seems to be a BUG
include/linux/pagemap.h - uses __GFP_NORETRY only on readahead
kernel/bpf/syscall.c - it says it need __GFP_NORETRY to avoid oom killer -
provides no fallback, so it seems to be BUG
kernel/events/ring_buffer.c - falls back to a smaller size, but doesn't drop
__GFP_NORETRY - BUG
kernel/groups.c - falls back to vmalloc (should it use kvmalloc?)
kernel/trace/ring_buffer.c - no fallback - BUG
kernel/trace/trace.c - no fallback - BUG
kernel/power/swap.c - falls back, but there is useless WARN_ON_ONCE(1) on
the fallback path
lib/debugobjects.c - turns off debugging on alloaction failure
lib/rhashtable.c - uses __GFP_NORETRY only with GFP_ATOMIC - __GFP_NORETRY
is useless
mm/hugetlb.c - no problem if hugepage allocation fails
mm/mempool.c - falls back to mempool
mm/kmemleak.c - disables kmemleak on allocation failure
mm/page_alloc.c/__page_frag_cache_refill - fallback to a smaller size without
__GFP_NORETRY
mm/zswap.c/zswap_pool_create - used during pool creation - probably a BUG
mm/zswap.c/zswap_frontswap_store - used when compressing a page - probably
ok to fail
mm/memcontrol.c - I don't know if it is a bug
mm/shmem.c - used during hugepage allocation - ok to fail
mm/slub.c - fallback to a smaller size without __GFP_NORETRY
mm/util.c - fallback to vmalloc
net/packet/af_packet.c - fallback without __GFP_NORETRY
net/xdp/xsk_queue.c - no fallback - BUG
net/core/skbuff.c - fallback to a smaller size without __GFP_NORETRY
net/core/sock.c - fallback to a smaller size without __GFP_NORETRY
net/netlink/af_netlink.c - it's ok to fail when decreasing the size of skb
net/smc/smc_core.c - falls back to a smaller size, but doesn't drop
__GFP_NORETRY - BUG
net/netfilter/x_tables.c - __GFP_NORETRY is used to avoid the oom killer
- provides no fallback - it seems to be a BUG
security/integrity/ima/ima_crypto.c - fallback to a smaller size without
__GFP_NORETRY
sound/core/memalloc.c - __GFP_NORETRY is used to avoid the oom killer
- provides no fallback - it seems to be a BUG
Mikulas
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
