From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Carlos Maiolino <cmaiolino@redhat.com>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 3/5] xfs: remove kmem_zalloc() wrapper
Date: Wed, 20 Nov 2019 13:41:45 -0800 [thread overview]
Message-ID: <20191120214145.GT6219@magnolia> (raw)
In-Reply-To: <20191120212401.GC4614@dread.disaster.area>
On Thu, Nov 21, 2019 at 08:24:01AM +1100, Dave Chinner wrote:
> On Wed, Nov 20, 2019 at 11:44:23AM +0100, Carlos Maiolino wrote:
> > Use kzalloc() directly
> >
> > Special attention goes to function xfs_buf_map_from_irec(). Giving the
> > fact we are not allowed to fail there, I removed the 'if (!map)'
> > conditional from there, I'd just like somebody to double check if it's
> > fine as I believe it is
> >
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
>
> It looks good as a 1:1 translation, but I've noticed a few places we
> actually have the context wrong and have been saved by the fact tehy
> are called in transaction context (hence GFP_NOFS is enforced by
> task flags).
>
> This can be fixed in a separate patch, I've noted the ones I think
> need changing below.
>
> > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> > index 795b9b21b64d..67de68584224 100644
> > --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> > @@ -2253,7 +2253,8 @@ xfs_attr3_leaf_unbalance(
> > struct xfs_attr_leafblock *tmp_leaf;
> > struct xfs_attr3_icleaf_hdr tmphdr;
> >
> > - tmp_leaf = kmem_zalloc(state->args->geo->blksize, 0);
> > + tmp_leaf = kzalloc(state->args->geo->blksize,
> > + GFP_KERNEL | __GFP_NOFAIL);
>
> In a transaction, GFP_NOFS.
As we're discussing on IRC, this is probably correct, but let's do a
straight KM_ -> GFP_ conversion here, warts and all; and then do a
separate series to sort out incorrect flag usage.
--D
> > diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> > index dc39b2d1b351..4fea8e5e70fb 100644
> > --- a/fs/xfs/xfs_buf_item.c
> > +++ b/fs/xfs/xfs_buf_item.c
> > @@ -701,8 +701,8 @@ xfs_buf_item_get_format(
> > return 0;
> > }
> >
> > - bip->bli_formats = kmem_zalloc(count * sizeof(struct xfs_buf_log_format),
> > - 0);
> > + bip->bli_formats = kzalloc(count * sizeof(struct xfs_buf_log_format),
> > + GFP_KERNEL | __GFP_NOFAIL);
> > if (!bip->bli_formats)
> > return -ENOMEM;
> > return 0;
>
> In a transaction, GFP_NOFS.
>
> > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
> > index 1b5e68ccef60..91bd47e8b832 100644
> > --- a/fs/xfs/xfs_dquot_item.c
> > +++ b/fs/xfs/xfs_dquot_item.c
> > @@ -347,7 +347,8 @@ xfs_qm_qoff_logitem_init(
> > {
> > struct xfs_qoff_logitem *qf;
> >
> > - qf = kmem_zalloc(sizeof(struct xfs_qoff_logitem), 0);
> > + qf = kzalloc(sizeof(struct xfs_qoff_logitem),
> > + GFP_KERNEL | __GFP_NOFAIL);
>
> In a transaction, GFP_NOFS.
>
> > diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c
> > index 9f0b99c7b34a..0ce50b47fc28 100644
> > --- a/fs/xfs/xfs_extent_busy.c
> > +++ b/fs/xfs/xfs_extent_busy.c
> > @@ -33,7 +33,8 @@ xfs_extent_busy_insert(
> > struct rb_node **rbp;
> > struct rb_node *parent = NULL;
> >
> > - new = kmem_zalloc(sizeof(struct xfs_extent_busy), 0);
> > + new = kzalloc(sizeof(struct xfs_extent_busy),
> > + GFP_KERNEL | __GFP_NOFAIL);
>
> transaction, GFP_NOFS.
>
> > new->agno = agno;
> > new->bno = bno;
> > new->length = len;
> > diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> > index c3b8804aa396..872312029957 100644
> > --- a/fs/xfs/xfs_extfree_item.c
> > +++ b/fs/xfs/xfs_extfree_item.c
> > @@ -163,7 +163,7 @@ xfs_efi_init(
> > if (nextents > XFS_EFI_MAX_FAST_EXTENTS) {
> > size = (uint)(sizeof(xfs_efi_log_item_t) +
> > ((nextents - 1) * sizeof(xfs_extent_t)));
> > - efip = kmem_zalloc(size, 0);
> > + efip = kzalloc(size, GFP_KERNEL | __GFP_NOFAIL);
> > } else {
> > efip = kmem_cache_zalloc(xfs_efi_zone,
> > GFP_KERNEL | __GFP_NOFAIL);
>
> Both of these GFP_NOFS.
>
> > @@ -333,9 +333,9 @@ xfs_trans_get_efd(
> > ASSERT(nextents > 0);
> >
> > if (nextents > XFS_EFD_MAX_FAST_EXTENTS) {
> > - efdp = kmem_zalloc(sizeof(struct xfs_efd_log_item) +
> > + efdp = kzalloc(sizeof(struct xfs_efd_log_item) +
> > (nextents - 1) * sizeof(struct xfs_extent),
> > - 0);
> > + GFP_KERNEL | __GFP_NOFAIL);
> > } else {
> > efdp = kmem_cache_zalloc(xfs_efd_zone,
> > GFP_KERNEL | __GFP_NOFAIL);
>
> Same here.
>
> Hmmm. I guess I better go look at the kmem_cache_[z]alloc() patches,
> too.
>
> > diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> > index 76b39f2a0260..7ec70a5f1cb0 100644
> > --- a/fs/xfs/xfs_refcount_item.c
> > +++ b/fs/xfs/xfs_refcount_item.c
> > @@ -143,8 +143,8 @@ xfs_cui_init(
> >
> > ASSERT(nextents > 0);
> > if (nextents > XFS_CUI_MAX_FAST_EXTENTS)
> > - cuip = kmem_zalloc(xfs_cui_log_item_sizeof(nextents),
> > - 0);
> > + cuip = kzalloc(xfs_cui_log_item_sizeof(nextents),
> > + GFP_KERNEL | __GFP_NOFAIL);
> > else
> > cuip = kmem_cache_zalloc(xfs_cui_zone,
> > GFP_KERNEL | __GFP_NOFAIL);
>
> Both GFP_NOFS.
>
> > diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> > index 6aeb6745d007..82d822885996 100644
> > --- a/fs/xfs/xfs_rmap_item.c
> > +++ b/fs/xfs/xfs_rmap_item.c
> > @@ -142,7 +142,8 @@ xfs_rui_init(
> >
> > ASSERT(nextents > 0);
> > if (nextents > XFS_RUI_MAX_FAST_EXTENTS)
> > - ruip = kmem_zalloc(xfs_rui_log_item_sizeof(nextents), 0);
> > + ruip = kzalloc(xfs_rui_log_item_sizeof(nextents),
> > + GFP_KERNEL | __GFP_NOFAIL);
> > else
> > ruip = kmem_cache_zalloc(xfs_rui_zone,
> > GFP_KERNEL | __GFP_NOFAIL);
>
> Both GFP_NOFS.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
next prev parent reply other threads:[~2019-11-20 21:41 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-20 10:44 [PATCH 0/5] Remove/convert more kmem_* wrappers Carlos Maiolino
2019-11-20 10:44 ` [PATCH V2 1/5] xfs: remove kmem_zone_zalloc() Carlos Maiolino
2019-11-20 10:44 ` [PATCH V2 2/5] xfs: Remove kmem_zone_alloc() wrapper Carlos Maiolino
2019-11-20 18:58 ` Darrick J. Wong
2019-11-20 10:44 ` [PATCH 3/5] xfs: remove kmem_zalloc() wrapper Carlos Maiolino
2019-11-20 19:00 ` Darrick J. Wong
2019-11-20 21:24 ` Dave Chinner
2019-11-20 21:41 ` Darrick J. Wong [this message]
2019-11-20 22:44 ` Carlos Maiolino
2019-11-20 23:08 ` Darrick J. Wong
2019-11-20 10:44 ` [PATCH 4/5] xfs: Remove kmem_realloc Carlos Maiolino
2019-11-20 19:00 ` Darrick J. Wong
2019-11-20 10:44 ` [PATCH 5/5] xfs: Convert kmem_alloc() users Carlos Maiolino
2019-11-20 19:00 ` Darrick J. Wong
2019-11-22 15:57 ` Darrick J. Wong
2019-11-22 22:30 ` Darrick J. Wong
2019-11-24 22:02 ` Darrick J. Wong
2019-11-25 9:28 ` Carlos Maiolino
2020-05-14 9:26 ` [PATCH 0/5] Remove/convert more kmem_* wrappers 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=20191120214145.GT6219@magnolia \
--to=darrick.wong@oracle.com \
--cc=cmaiolino@redhat.com \
--cc=david@fromorbit.com \
--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).