From: "Darrick J. Wong" <djwong@kernel.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 3/3] xfs: replace kmem_alloc_large() with kvmalloc()
Date: Fri, 25 Jun 2021 19:06:11 -0700 [thread overview]
Message-ID: <20210626020611.GI13784@locust> (raw)
In-Reply-To: <20210625023029.1472466-4-david@fromorbit.com>
On Fri, Jun 25, 2021 at 12:30:29PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> There is no reason for this wrapper existing anymore. All the places
> that use KM_NOFS allocation are within transaction contexts and
> hence covered by memalloc_nofs_save/restore contexts. Hence we don't
> need any special handling of vmalloc for large IOs anymore and
> so special casing this code isn't necessary.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Looks pretty straightforward,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/kmem.c | 39 -----------------------------------
> fs/xfs/kmem.h | 1 -
> fs/xfs/libxfs/xfs_attr_leaf.c | 2 +-
> fs/xfs/scrub/attr.c | 14 +++++++------
> fs/xfs/scrub/attr.h | 3 ---
> fs/xfs/xfs_log.c | 4 ++--
> fs/xfs/xfs_log_cil.c | 10 ++++++++-
> fs/xfs/xfs_log_recover.c | 2 +-
> fs/xfs/xfs_trace.h | 1 -
> 9 files changed, 21 insertions(+), 55 deletions(-)
>
> diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
> index 3f2979fd2f2b..6f49bf39183c 100644
> --- a/fs/xfs/kmem.c
> +++ b/fs/xfs/kmem.c
> @@ -29,42 +29,3 @@ kmem_alloc(size_t size, xfs_km_flags_t flags)
> congestion_wait(BLK_RW_ASYNC, HZ/50);
> } while (1);
> }
> -
> -
> -/*
> - * __vmalloc() will allocate data pages and auxiliary structures (e.g.
> - * pagetables) with GFP_KERNEL, yet we may be under GFP_NOFS context here. Hence
> - * we need to tell memory reclaim that we are in such a context via
> - * PF_MEMALLOC_NOFS to prevent memory reclaim re-entering the filesystem here
> - * and potentially deadlocking.
> - */
> -static void *
> -__kmem_vmalloc(size_t size, xfs_km_flags_t flags)
> -{
> - unsigned nofs_flag = 0;
> - void *ptr;
> - gfp_t lflags = kmem_flags_convert(flags);
> -
> - if (flags & KM_NOFS)
> - nofs_flag = memalloc_nofs_save();
> -
> - ptr = __vmalloc(size, lflags);
> -
> - if (flags & KM_NOFS)
> - memalloc_nofs_restore(nofs_flag);
> -
> - return ptr;
> -}
> -
> -void *
> -kmem_alloc_large(size_t size, xfs_km_flags_t flags)
> -{
> - void *ptr;
> -
> - trace_kmem_alloc_large(size, flags, _RET_IP_);
> -
> - ptr = kmem_alloc(size, flags | KM_MAYFAIL);
> - if (ptr)
> - return ptr;
> - return __kmem_vmalloc(size, flags);
> -}
> diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
> index 9ff20047f8b8..54da6d717a06 100644
> --- a/fs/xfs/kmem.h
> +++ b/fs/xfs/kmem.h
> @@ -57,7 +57,6 @@ kmem_flags_convert(xfs_km_flags_t flags)
> }
>
> extern void *kmem_alloc(size_t, xfs_km_flags_t);
> -extern void *kmem_alloc_large(size_t size, xfs_km_flags_t);
> static inline void kmem_free(const void *ptr)
> {
> kvfree(ptr);
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index b910bd209949..16d64872acc0 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -489,7 +489,7 @@ xfs_attr_copy_value(
> }
>
> if (!args->value) {
> - args->value = kmem_alloc_large(valuelen, KM_NOLOCKDEP);
> + args->value = kvmalloc(valuelen, GFP_KERNEL | __GFP_NOLOCKDEP);
> if (!args->value)
> return -ENOMEM;
> }
> diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
> index 552af0cf8482..6c36af6dbd35 100644
> --- a/fs/xfs/scrub/attr.c
> +++ b/fs/xfs/scrub/attr.c
> @@ -25,11 +25,11 @@
> * reallocating the buffer if necessary. Buffer contents are not preserved
> * across a reallocation.
> */
> -int
> +static int
> xchk_setup_xattr_buf(
> struct xfs_scrub *sc,
> size_t value_size,
> - xfs_km_flags_t flags)
> + gfp_t flags)
> {
> size_t sz;
> struct xchk_xattr_buf *ab = sc->buf;
> @@ -57,7 +57,7 @@ xchk_setup_xattr_buf(
> * Don't zero the buffer upon allocation to avoid runtime overhead.
> * All users must be careful never to read uninitialized contents.
> */
> - ab = kmem_alloc_large(sizeof(*ab) + sz, flags);
> + ab = kvmalloc(sizeof(*ab) + sz, flags);
> if (!ab)
> return -ENOMEM;
>
> @@ -79,7 +79,7 @@ xchk_setup_xattr(
> * without the inode lock held, which means we can sleep.
> */
> if (sc->flags & XCHK_TRY_HARDER) {
> - error = xchk_setup_xattr_buf(sc, XATTR_SIZE_MAX, 0);
> + error = xchk_setup_xattr_buf(sc, XATTR_SIZE_MAX, GFP_KERNEL);
> if (error)
> return error;
> }
> @@ -138,7 +138,8 @@ xchk_xattr_listent(
> * doesn't work, we overload the seen_enough variable to convey
> * the error message back to the main scrub function.
> */
> - error = xchk_setup_xattr_buf(sx->sc, valuelen, KM_MAYFAIL);
> + error = xchk_setup_xattr_buf(sx->sc, valuelen,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (error == -ENOMEM)
> error = -EDEADLOCK;
> if (error) {
> @@ -323,7 +324,8 @@ xchk_xattr_block(
> return 0;
>
> /* Allocate memory for block usage checking. */
> - error = xchk_setup_xattr_buf(ds->sc, 0, KM_MAYFAIL);
> + error = xchk_setup_xattr_buf(ds->sc, 0,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (error == -ENOMEM)
> return -EDEADLOCK;
> if (error)
> diff --git a/fs/xfs/scrub/attr.h b/fs/xfs/scrub/attr.h
> index 13a1d2e8424d..1719e1c4da59 100644
> --- a/fs/xfs/scrub/attr.h
> +++ b/fs/xfs/scrub/attr.h
> @@ -65,7 +65,4 @@ xchk_xattr_dstmap(
> BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
> }
>
> -int xchk_setup_xattr_buf(struct xfs_scrub *sc, size_t value_size,
> - xfs_km_flags_t flags);
> -
> #endif /* __XFS_SCRUB_ATTR_H__ */
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 404970a4343c..6cc51b0cb99e 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1462,8 +1462,8 @@ xlog_alloc_log(
> iclog->ic_prev = prev_iclog;
> prev_iclog = iclog;
>
> - iclog->ic_data = kmem_alloc_large(log->l_iclog_size,
> - KM_MAYFAIL | KM_ZERO);
> + iclog->ic_data = kvzalloc(log->l_iclog_size,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!iclog->ic_data)
> goto out_free_iclog;
> #ifdef DEBUG
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 3c2b1205944d..bf9d747352df 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -185,7 +185,15 @@ xlog_cil_alloc_shadow_bufs(
> */
> kmem_free(lip->li_lv_shadow);
>
> - lv = kmem_alloc_large(buf_size, KM_NOFS);
> + /*
> + * We are in transaction context, which means this
> + * allocation will pick up GFP_NOFS from the
> + * memalloc_nofs_save/restore context the transaction
> + * holds. This means we can use GFP_KERNEL here so the
> + * generic kvmalloc() code will run vmalloc on
> + * contiguous page allocation failure as we require.
> + */
> + lv = kvmalloc(buf_size, GFP_KERNEL);
> memset(lv, 0, xlog_cil_iovec_space(niovecs));
>
> lv->lv_item = lip;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index cc559815e08f..1a291bf50776 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -106,7 +106,7 @@ xlog_alloc_buffer(
> if (nbblks > 1 && log->l_sectBBsize > 1)
> nbblks += log->l_sectBBsize;
> nbblks = round_up(nbblks, log->l_sectBBsize);
> - return kmem_alloc_large(BBTOB(nbblks), KM_MAYFAIL | KM_ZERO);
> + return kvzalloc(BBTOB(nbblks), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> }
>
> /*
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 6865e838a71b..38f2f67303f7 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3689,7 +3689,6 @@ DEFINE_EVENT(xfs_kmem_class, name, \
> TP_PROTO(ssize_t size, int flags, unsigned long caller_ip), \
> TP_ARGS(size, flags, caller_ip))
> DEFINE_KMEM_EVENT(kmem_alloc);
> -DEFINE_KMEM_EVENT(kmem_alloc_large);
>
> TRACE_EVENT(xfs_check_new_dalign,
> TP_PROTO(struct xfs_mount *mp, int new_dalign, xfs_ino_t calc_rootino),
> --
> 2.31.1
>
next prev parent reply other threads:[~2021-06-26 2:06 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-25 2:30 [PATCH 0/3] mm, xfs: memory allocation improvements Dave Chinner
2021-06-25 2:30 ` [PATCH 1/3] mm: Add kvrealloc() Dave Chinner
2021-06-25 2:40 ` Miaohe Lin
2021-06-25 5:05 ` Dave Chinner
2021-06-25 3:54 ` Matthew Wilcox
2021-06-25 5:02 ` Dave Chinner
2021-06-25 2:30 ` [PATCH 2/3] xfs: remove kmem_alloc_io() Dave Chinner
2021-06-26 2:01 ` Darrick J. Wong
2021-06-27 22:09 ` Dave Chinner
2021-06-27 23:14 ` Darrick J. Wong
2021-06-25 2:30 ` [PATCH 3/3] xfs: replace kmem_alloc_large() with kvmalloc() Dave Chinner
2021-06-26 2:06 ` Darrick J. Wong [this message]
-- strict thread matches above, loose matches on Subject: below --
2021-06-30 6:14 [PATCH 0/3 v2] mm, xfs: memory allocation improvements Dave Chinner
2021-06-30 6:14 ` [PATCH 3/3] xfs: replace kmem_alloc_large() with kvmalloc() Dave Chinner
2021-07-14 2:34 [PATCH 0/3 v3] xfs, mm: memory allocation improvements Dave Chinner
2021-07-14 2:34 ` [PATCH 3/3] xfs: replace kmem_alloc_large() with kvmalloc() Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210626020611.GI13784@locust \
--to=djwong@kernel.org \
--cc=david@fromorbit.com \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).