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@xxxxxxxxxx>
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.
> 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@xxxxxxxxxxxxx
