From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 3/5] xfs: remove kmem_zalloc() wrapper
Date: Wed, 20 Nov 2019 15:08:28 -0800 [thread overview]
Message-ID: <20191120230828.GU6219@magnolia> (raw)
In-Reply-To: <20191120224404.dbuegv5ejmydrlb6@orion>
On Wed, Nov 20, 2019 at 11:44:04PM +0100, Carlos Maiolino wrote:
> On Wed, Nov 20, 2019 at 01:41:45PM -0800, Darrick J. Wong wrote:
> > 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.
>
> Sure, thanks guys, I can focus on that then after this series. Do you guys
> prefer me to 'finish' this series (i.e. removing KM_* flags), or fixing the
> wrong contexts before the next patches for KM_* -> GFP_* stuff?
Dunno, but it's all 5.6 material at this point, so you could possibly do
both. I'm guessing that continuing the KM_ removal will be easier to
review and therefore should go first.
--D
> Cheers.
>
> >
> > --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
> >
>
> --
> Carlos
>
next prev parent reply other threads:[~2019-11-20 23:09 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
2019-11-20 22:44 ` Carlos Maiolino
2019-11-20 23:08 ` Darrick J. Wong [this message]
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=20191120230828.GU6219@magnolia \
--to=darrick.wong@oracle.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.