* [PATCH V2 1/5] xfs: remove kmem_zone_zalloc()
2019-11-20 10:44 [PATCH 0/5] Remove/convert more kmem_* wrappers Carlos Maiolino
@ 2019-11-20 10:44 ` Carlos Maiolino
2019-11-20 10:44 ` [PATCH V2 2/5] xfs: Remove kmem_zone_alloc() wrapper Carlos Maiolino
` (4 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Carlos Maiolino @ 2019-11-20 10:44 UTC (permalink / raw)
To: linux-xfs
Use kmem_cache_zalloc() directly.
We can use __GFP_NOFAIL to loop infinitely on a memory allocation request, which
also has an implied congestion_wait()
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V2:
- shortens a >80 characters line left on V1
- rephrase commit log
fs/xfs/kmem.h | 6 ------
fs/xfs/libxfs/xfs_alloc_btree.c | 2 +-
fs/xfs/libxfs/xfs_bmap.c | 3 ++-
fs/xfs/libxfs/xfs_bmap_btree.c | 2 +-
fs/xfs/libxfs/xfs_da_btree.c | 2 +-
fs/xfs/libxfs/xfs_ialloc_btree.c | 2 +-
fs/xfs/libxfs/xfs_inode_fork.c | 6 +++---
fs/xfs/libxfs/xfs_refcount_btree.c | 2 +-
fs/xfs/libxfs/xfs_rmap_btree.c | 2 +-
fs/xfs/xfs_bmap_item.c | 4 ++--
fs/xfs/xfs_buf.c | 2 +-
fs/xfs/xfs_buf_item.c | 2 +-
fs/xfs/xfs_dquot.c | 2 +-
fs/xfs/xfs_extfree_item.c | 6 ++++--
fs/xfs/xfs_icreate_item.c | 2 +-
fs/xfs/xfs_inode_item.c | 3 ++-
fs/xfs/xfs_log.c | 7 ++++---
fs/xfs/xfs_log_cil.c | 2 +-
fs/xfs/xfs_log_priv.h | 2 +-
fs/xfs/xfs_refcount_item.c | 5 +++--
fs/xfs/xfs_rmap_item.c | 5 +++--
fs/xfs/xfs_trans.c | 4 ++--
fs/xfs/xfs_trans_dquot.c | 3 ++-
23 files changed, 39 insertions(+), 37 deletions(-)
diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
index ccdc72519339..3242de676808 100644
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -78,12 +78,6 @@ kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
extern void *kmem_zone_alloc(kmem_zone_t *, xfs_km_flags_t);
-static inline void *
-kmem_zone_zalloc(kmem_zone_t *zone, xfs_km_flags_t flags)
-{
- return kmem_zone_alloc(zone, flags | KM_ZERO);
-}
-
static inline struct page *
kmem_to_page(void *addr)
{
diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index 279694d73e4e..0867c1fad11b 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -487,7 +487,7 @@ xfs_allocbt_init_cursor(
ASSERT(btnum == XFS_BTNUM_BNO || btnum == XFS_BTNUM_CNT);
- cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+ cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
cur->bc_tp = tp;
cur->bc_mp = mp;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index b7cc2f9eae7b..9fbdca183465 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1104,7 +1104,8 @@ xfs_bmap_add_attrfork(
if (error)
goto trans_cancel;
ASSERT(ip->i_afp == NULL);
- ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, 0);
+ ip->i_afp = kmem_cache_zalloc(xfs_ifork_zone,
+ GFP_KERNEL | __GFP_NOFAIL);
ip->i_afp->if_flags = XFS_IFEXTENTS;
logflags = 0;
switch (ip->i_d.di_format) {
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index ffe608d2a2d9..77fe4ae671e5 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -552,7 +552,7 @@ xfs_bmbt_init_cursor(
struct xfs_btree_cur *cur;
ASSERT(whichfork != XFS_COW_FORK);
- cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+ cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
cur->bc_tp = tp;
cur->bc_mp = mp;
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 7ae82d91f776..10a96e64b2ec 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -81,7 +81,7 @@ kmem_zone_t *xfs_da_state_zone; /* anchor for state struct zone */
xfs_da_state_t *
xfs_da_state_alloc(void)
{
- return kmem_zone_zalloc(xfs_da_state_zone, KM_NOFS);
+ return kmem_cache_zalloc(xfs_da_state_zone, GFP_NOFS | __GFP_NOFAIL);
}
/*
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index b82992f795aa..5366a874b076 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -413,7 +413,7 @@ xfs_inobt_init_cursor(
struct xfs_agi *agi = XFS_BUF_TO_AGI(agbp);
struct xfs_btree_cur *cur;
- cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+ cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
cur->bc_tp = tp;
cur->bc_mp = mp;
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 296677958212..ceb322c7105e 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -98,7 +98,7 @@ xfs_iformat_fork(
return 0;
ASSERT(ip->i_afp == NULL);
- ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_NOFS);
+ ip->i_afp = kmem_cache_zalloc(xfs_ifork_zone, GFP_NOFS | __GFP_NOFAIL);
switch (dip->di_aformat) {
case XFS_DINODE_FMT_LOCAL:
@@ -688,8 +688,8 @@ xfs_ifork_init_cow(
if (ip->i_cowfp)
return;
- ip->i_cowfp = kmem_zone_zalloc(xfs_ifork_zone,
- KM_NOFS);
+ ip->i_cowfp = kmem_cache_zalloc(xfs_ifork_zone,
+ GFP_NOFS | __GFP_NOFAIL);
ip->i_cowfp->if_flags = XFS_IFEXTENTS;
ip->i_cformat = XFS_DINODE_FMT_EXTENTS;
ip->i_cnextents = 0;
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 38529dbacd55..bb86988780ea 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -325,7 +325,7 @@ xfs_refcountbt_init_cursor(
ASSERT(agno != NULLAGNUMBER);
ASSERT(agno < mp->m_sb.sb_agcount);
- cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+ cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
cur->bc_tp = tp;
cur->bc_mp = mp;
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index fc78efa52c94..8d84dd98e8d3 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -461,7 +461,7 @@ xfs_rmapbt_init_cursor(
struct xfs_agf *agf = XFS_BUF_TO_AGF(agbp);
struct xfs_btree_cur *cur;
- cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+ cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
cur->bc_tp = tp;
cur->bc_mp = mp;
/* Overlapping btree; 2 keys per pointer. */
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index a89e10519f05..cda582d4bde0 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -141,7 +141,7 @@ xfs_bui_init(
{
struct xfs_bui_log_item *buip;
- buip = kmem_zone_zalloc(xfs_bui_zone, 0);
+ buip = kmem_cache_zalloc(xfs_bui_zone, GFP_KERNEL | __GFP_NOFAIL);
xfs_log_item_init(mp, &buip->bui_item, XFS_LI_BUI, &xfs_bui_item_ops);
buip->bui_format.bui_nextents = XFS_BUI_MAX_FAST_EXTENTS;
@@ -218,7 +218,7 @@ xfs_trans_get_bud(
{
struct xfs_bud_log_item *budp;
- budp = kmem_zone_zalloc(xfs_bud_zone, 0);
+ budp = kmem_cache_zalloc(xfs_bud_zone, GFP_KERNEL | __GFP_NOFAIL);
xfs_log_item_init(tp->t_mountp, &budp->bud_item, XFS_LI_BUD,
&xfs_bud_item_ops);
budp->bud_buip = buip;
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 80ef2fc8bb77..0d2c41c6639d 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -209,7 +209,7 @@ _xfs_buf_alloc(
int error;
int i;
- bp = kmem_zone_zalloc(xfs_buf_zone, KM_NOFS);
+ bp = kmem_cache_zalloc(xfs_buf_zone, GFP_NOFS | __GFP_NOFAIL);
if (unlikely(!bp))
return NULL;
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 8dc3330f1797..dc39b2d1b351 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -747,7 +747,7 @@ xfs_buf_item_init(
return 0;
}
- bip = kmem_zone_zalloc(xfs_buf_item_zone, 0);
+ bip = kmem_cache_zalloc(xfs_buf_item_zone, GFP_KERNEL | __GFP_NOFAIL);
xfs_log_item_init(mp, &bip->bli_item, XFS_LI_BUF, &xfs_buf_item_ops);
bip->bli_buf = bp;
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index a073281c3bd7..13c013918cd7 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -440,7 +440,7 @@ xfs_dquot_alloc(
{
struct xfs_dquot *dqp;
- dqp = kmem_zone_zalloc(xfs_qm_dqzone, 0);
+ dqp = kmem_cache_zalloc(xfs_qm_dqzone, GFP_KERNEL | __GFP_NOFAIL);
dqp->dq_flags = type;
dqp->q_core.d_id = cpu_to_be32(id);
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 29b3a90aee91..c3b8804aa396 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -165,7 +165,8 @@ xfs_efi_init(
((nextents - 1) * sizeof(xfs_extent_t)));
efip = kmem_zalloc(size, 0);
} else {
- efip = kmem_zone_zalloc(xfs_efi_zone, 0);
+ efip = kmem_cache_zalloc(xfs_efi_zone,
+ GFP_KERNEL | __GFP_NOFAIL);
}
xfs_log_item_init(mp, &efip->efi_item, XFS_LI_EFI, &xfs_efi_item_ops);
@@ -336,7 +337,8 @@ xfs_trans_get_efd(
(nextents - 1) * sizeof(struct xfs_extent),
0);
} else {
- efdp = kmem_zone_zalloc(xfs_efd_zone, 0);
+ efdp = kmem_cache_zalloc(xfs_efd_zone,
+ GFP_KERNEL | __GFP_NOFAIL);
}
xfs_log_item_init(tp->t_mountp, &efdp->efd_item, XFS_LI_EFD,
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index 490fee22b878..85bbf9dbe095 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -89,7 +89,7 @@ xfs_icreate_log(
{
struct xfs_icreate_item *icp;
- icp = kmem_zone_zalloc(xfs_icreate_zone, 0);
+ icp = kmem_cache_zalloc(xfs_icreate_zone, GFP_KERNEL | __GFP_NOFAIL);
xfs_log_item_init(tp->t_mountp, &icp->ic_item, XFS_LI_ICREATE,
&xfs_icreate_item_ops);
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index c8461a5515f1..6f8a7226d41d 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -652,7 +652,8 @@ xfs_inode_item_init(
struct xfs_inode_log_item *iip;
ASSERT(ip->i_itemp == NULL);
- iip = ip->i_itemp = kmem_zone_zalloc(xfs_ili_zone, 0);
+ iip = ip->i_itemp = kmem_cache_zalloc(xfs_ili_zone,
+ GFP_KERNEL | __GFP_NOFAIL);
iip->ili_inode = ip;
xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE,
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index e8349b0d7c51..9ed7869d879f 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -454,7 +454,8 @@ xfs_log_reserve(
XFS_STATS_INC(mp, xs_try_logspace);
ASSERT(*ticp == NULL);
- tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, 0);
+ tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent,
+ GFP_KERNEL | __GFP_NOFAIL);
*ticp = tic;
xlog_grant_push_ail(log, tic->t_cnt ? tic->t_unit_res * tic->t_cnt
@@ -3587,12 +3588,12 @@ xlog_ticket_alloc(
int cnt,
char client,
bool permanent,
- xfs_km_flags_t alloc_flags)
+ gfp_t alloc_flags)
{
struct xlog_ticket *tic;
int unit_res;
- tic = kmem_zone_zalloc(xfs_log_ticket_zone, alloc_flags);
+ tic = kmem_cache_zalloc(xfs_log_ticket_zone, alloc_flags);
if (!tic)
return NULL;
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 23d70836a2b7..9953f2f040ab 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -38,7 +38,7 @@ xlog_cil_ticket_alloc(
struct xlog_ticket *tic;
tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0,
- KM_NOFS);
+ GFP_NOFS | __GFP_NOFAIL);
/*
* set the current reservation to zero so we know to steal the basic
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index c47aa2ca6dc7..54c95fee9dc4 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -427,7 +427,7 @@ xlog_ticket_alloc(
int count,
char client,
bool permanent,
- xfs_km_flags_t alloc_flags);
+ gfp_t alloc_flags);
static inline void
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 0ac598f55339..76b39f2a0260 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -146,7 +146,8 @@ xfs_cui_init(
cuip = kmem_zalloc(xfs_cui_log_item_sizeof(nextents),
0);
else
- cuip = kmem_zone_zalloc(xfs_cui_zone, 0);
+ cuip = kmem_cache_zalloc(xfs_cui_zone,
+ GFP_KERNEL | __GFP_NOFAIL);
xfs_log_item_init(mp, &cuip->cui_item, XFS_LI_CUI, &xfs_cui_item_ops);
cuip->cui_format.cui_nextents = nextents;
@@ -223,7 +224,7 @@ xfs_trans_get_cud(
{
struct xfs_cud_log_item *cudp;
- cudp = kmem_zone_zalloc(xfs_cud_zone, 0);
+ cudp = kmem_cache_zalloc(xfs_cud_zone, GFP_KERNEL | __GFP_NOFAIL);
xfs_log_item_init(tp->t_mountp, &cudp->cud_item, XFS_LI_CUD,
&xfs_cud_item_ops);
cudp->cud_cuip = cuip;
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index a0a02d862ddd..6aeb6745d007 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -144,7 +144,8 @@ xfs_rui_init(
if (nextents > XFS_RUI_MAX_FAST_EXTENTS)
ruip = kmem_zalloc(xfs_rui_log_item_sizeof(nextents), 0);
else
- ruip = kmem_zone_zalloc(xfs_rui_zone, 0);
+ ruip = kmem_cache_zalloc(xfs_rui_zone,
+ GFP_KERNEL | __GFP_NOFAIL);
xfs_log_item_init(mp, &ruip->rui_item, XFS_LI_RUI, &xfs_rui_item_ops);
ruip->rui_format.rui_nextents = nextents;
@@ -246,7 +247,7 @@ xfs_trans_get_rud(
{
struct xfs_rud_log_item *rudp;
- rudp = kmem_zone_zalloc(xfs_rud_zone, 0);
+ rudp = kmem_cache_zalloc(xfs_rud_zone, GFP_KERNEL | __GFP_NOFAIL);
xfs_log_item_init(tp->t_mountp, &rudp->rud_item, XFS_LI_RUD,
&xfs_rud_item_ops);
rudp->rud_ruip = ruip;
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 3b208f9a865c..29f34492d5f4 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -90,7 +90,7 @@ xfs_trans_dup(
trace_xfs_trans_dup(tp, _RET_IP_);
- ntp = kmem_zone_zalloc(xfs_trans_zone, 0);
+ ntp = kmem_cache_zalloc(xfs_trans_zone, GFP_KERNEL | __GFP_NOFAIL);
/*
* Initialize the new transaction structure.
@@ -263,7 +263,7 @@ xfs_trans_alloc(
* GFP_NOFS allocation context so that we avoid lockdep false positives
* by doing GFP_KERNEL allocations inside sb_start_intwrite().
*/
- tp = kmem_zone_zalloc(xfs_trans_zone, 0);
+ tp = kmem_cache_zalloc(xfs_trans_zone, GFP_KERNEL | __GFP_NOFAIL);
if (!(flags & XFS_TRANS_NO_WRITECOUNT))
sb_start_intwrite(mp->m_super);
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index ff1c326826d3..69e8f6d049aa 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -863,7 +863,8 @@ STATIC void
xfs_trans_alloc_dqinfo(
xfs_trans_t *tp)
{
- tp->t_dqinfo = kmem_zone_zalloc(xfs_qm_dqtrxzone, 0);
+ tp->t_dqinfo = kmem_cache_zalloc(xfs_qm_dqtrxzone,
+ GFP_KERNEL | __GFP_NOFAIL);
}
void
--
2.23.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V2 2/5] xfs: Remove kmem_zone_alloc() wrapper
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 ` 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
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Carlos Maiolino @ 2019-11-20 10:44 UTC (permalink / raw)
To: linux-xfs
__GFP_NOFAIL can be used for an infinite retry + congestion_wait, so we
can use kmem_cache_alloc() directly.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V2:
- Rephrase commit log to explain why it's ok to remove the retry
loop from kmem_zone_alloc().
fs/xfs/kmem.c | 21 ---------------------
fs/xfs/kmem.h | 2 --
fs/xfs/libxfs/xfs_alloc.c | 3 ++-
fs/xfs/libxfs/xfs_bmap.c | 3 ++-
fs/xfs/xfs_icache.c | 2 +-
fs/xfs/xfs_trace.h | 1 -
6 files changed, 5 insertions(+), 27 deletions(-)
diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
index 1da94237a8cf..2644fdaa0549 100644
--- a/fs/xfs/kmem.c
+++ b/fs/xfs/kmem.c
@@ -115,24 +115,3 @@ kmem_realloc(const void *old, size_t newsize, xfs_km_flags_t flags)
congestion_wait(BLK_RW_ASYNC, HZ/50);
} while (1);
}
-
-void *
-kmem_zone_alloc(kmem_zone_t *zone, xfs_km_flags_t flags)
-{
- int retries = 0;
- gfp_t lflags = kmem_flags_convert(flags);
- void *ptr;
-
- trace_kmem_zone_alloc(kmem_cache_size(zone), flags, _RET_IP_);
- do {
- ptr = kmem_cache_alloc(zone, lflags);
- if (ptr || (flags & KM_MAYFAIL))
- return ptr;
- if (!(++retries % 100))
- xfs_err(NULL,
- "%s(%u) possible memory allocation deadlock in %s (mode:0x%x)",
- current->comm, current->pid,
- __func__, lflags);
- congestion_wait(BLK_RW_ASYNC, HZ/50);
- } while (1);
-}
diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
index 3242de676808..7e4ad73771ce 100644
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -76,8 +76,6 @@ kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
#define kmem_zone kmem_cache
#define kmem_zone_t struct kmem_cache
-extern void *kmem_zone_alloc(kmem_zone_t *, xfs_km_flags_t);
-
static inline struct page *
kmem_to_page(void *addr)
{
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 675613c7bacb..42cae87bdd2d 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -2351,7 +2351,8 @@ xfs_defer_agfl_block(
ASSERT(xfs_bmap_free_item_zone != NULL);
ASSERT(oinfo != NULL);
- new = kmem_zone_alloc(xfs_bmap_free_item_zone, 0);
+ new = kmem_cache_alloc(xfs_bmap_free_item_zone,
+ GFP_KERNEL | __GFP_NOFAIL);
new->xefi_startblock = XFS_AGB_TO_FSB(mp, agno, agbno);
new->xefi_blockcount = 1;
new->xefi_oinfo = *oinfo;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9fbdca183465..37596e49b92e 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -554,7 +554,8 @@ __xfs_bmap_add_free(
#endif
ASSERT(xfs_bmap_free_item_zone != NULL);
- new = kmem_zone_alloc(xfs_bmap_free_item_zone, 0);
+ new = kmem_cache_alloc(xfs_bmap_free_item_zone,
+ GFP_KERNEL | __GFP_NOFAIL);
new->xefi_startblock = bno;
new->xefi_blockcount = (xfs_extlen_t)len;
if (oinfo)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 950e8a51ec66..985f48e3795f 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -40,7 +40,7 @@ xfs_inode_alloc(
* KM_MAYFAIL and return NULL here on ENOMEM. Set the
* code up to do this anyway.
*/
- ip = kmem_zone_alloc(xfs_inode_zone, 0);
+ ip = kmem_cache_alloc(xfs_inode_zone, GFP_KERNEL | __GFP_NOFAIL);
if (!ip)
return NULL;
if (inode_init_always(mp->m_super, VFS_I(ip))) {
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index c13bb3655e48..192f499ccd7e 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -3571,7 +3571,6 @@ DEFINE_KMEM_EVENT(kmem_alloc);
DEFINE_KMEM_EVENT(kmem_alloc_io);
DEFINE_KMEM_EVENT(kmem_alloc_large);
DEFINE_KMEM_EVENT(kmem_realloc);
-DEFINE_KMEM_EVENT(kmem_zone_alloc);
#endif /* _TRACE_XFS_H */
--
2.23.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH V2 2/5] xfs: Remove kmem_zone_alloc() wrapper
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
0 siblings, 0 replies; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-20 18:58 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Nov 20, 2019 at 11:44:22AM +0100, Carlos Maiolino wrote:
> __GFP_NOFAIL can be used for an infinite retry + congestion_wait, so we
> can use kmem_cache_alloc() directly.
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> V2:
> - Rephrase commit log to explain why it's ok to remove the retry
> loop from kmem_zone_alloc().
>
> fs/xfs/kmem.c | 21 ---------------------
> fs/xfs/kmem.h | 2 --
> fs/xfs/libxfs/xfs_alloc.c | 3 ++-
> fs/xfs/libxfs/xfs_bmap.c | 3 ++-
> fs/xfs/xfs_icache.c | 2 +-
> fs/xfs/xfs_trace.h | 1 -
> 6 files changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
> index 1da94237a8cf..2644fdaa0549 100644
> --- a/fs/xfs/kmem.c
> +++ b/fs/xfs/kmem.c
> @@ -115,24 +115,3 @@ kmem_realloc(const void *old, size_t newsize, xfs_km_flags_t flags)
> congestion_wait(BLK_RW_ASYNC, HZ/50);
> } while (1);
> }
> -
> -void *
> -kmem_zone_alloc(kmem_zone_t *zone, xfs_km_flags_t flags)
> -{
> - int retries = 0;
> - gfp_t lflags = kmem_flags_convert(flags);
> - void *ptr;
> -
> - trace_kmem_zone_alloc(kmem_cache_size(zone), flags, _RET_IP_);
> - do {
> - ptr = kmem_cache_alloc(zone, lflags);
> - if (ptr || (flags & KM_MAYFAIL))
> - return ptr;
> - if (!(++retries % 100))
> - xfs_err(NULL,
> - "%s(%u) possible memory allocation deadlock in %s (mode:0x%x)",
> - current->comm, current->pid,
> - __func__, lflags);
> - congestion_wait(BLK_RW_ASYNC, HZ/50);
> - } while (1);
> -}
> diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
> index 3242de676808..7e4ad73771ce 100644
> --- a/fs/xfs/kmem.h
> +++ b/fs/xfs/kmem.h
> @@ -76,8 +76,6 @@ kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
> #define kmem_zone kmem_cache
> #define kmem_zone_t struct kmem_cache
>
> -extern void *kmem_zone_alloc(kmem_zone_t *, xfs_km_flags_t);
> -
> static inline struct page *
> kmem_to_page(void *addr)
> {
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 675613c7bacb..42cae87bdd2d 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2351,7 +2351,8 @@ xfs_defer_agfl_block(
> ASSERT(xfs_bmap_free_item_zone != NULL);
> ASSERT(oinfo != NULL);
>
> - new = kmem_zone_alloc(xfs_bmap_free_item_zone, 0);
> + new = kmem_cache_alloc(xfs_bmap_free_item_zone,
> + GFP_KERNEL | __GFP_NOFAIL);
> new->xefi_startblock = XFS_AGB_TO_FSB(mp, agno, agbno);
> new->xefi_blockcount = 1;
> new->xefi_oinfo = *oinfo;
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 9fbdca183465..37596e49b92e 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -554,7 +554,8 @@ __xfs_bmap_add_free(
> #endif
> ASSERT(xfs_bmap_free_item_zone != NULL);
>
> - new = kmem_zone_alloc(xfs_bmap_free_item_zone, 0);
> + new = kmem_cache_alloc(xfs_bmap_free_item_zone,
> + GFP_KERNEL | __GFP_NOFAIL);
> new->xefi_startblock = bno;
> new->xefi_blockcount = (xfs_extlen_t)len;
> if (oinfo)
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 950e8a51ec66..985f48e3795f 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -40,7 +40,7 @@ xfs_inode_alloc(
> * KM_MAYFAIL and return NULL here on ENOMEM. Set the
> * code up to do this anyway.
> */
> - ip = kmem_zone_alloc(xfs_inode_zone, 0);
> + ip = kmem_cache_alloc(xfs_inode_zone, GFP_KERNEL | __GFP_NOFAIL);
> if (!ip)
> return NULL;
> if (inode_init_always(mp->m_super, VFS_I(ip))) {
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index c13bb3655e48..192f499ccd7e 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3571,7 +3571,6 @@ DEFINE_KMEM_EVENT(kmem_alloc);
> DEFINE_KMEM_EVENT(kmem_alloc_io);
> DEFINE_KMEM_EVENT(kmem_alloc_large);
> DEFINE_KMEM_EVENT(kmem_realloc);
> -DEFINE_KMEM_EVENT(kmem_zone_alloc);
>
> #endif /* _TRACE_XFS_H */
>
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/5] xfs: remove kmem_zalloc() wrapper
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 10:44 ` Carlos Maiolino
2019-11-20 19:00 ` Darrick J. Wong
2019-11-20 21:24 ` Dave Chinner
2019-11-20 10:44 ` [PATCH 4/5] xfs: Remove kmem_realloc Carlos Maiolino
` (2 subsequent siblings)
5 siblings, 2 replies; 19+ messages in thread
From: Carlos Maiolino @ 2019-11-20 10:44 UTC (permalink / raw)
To: linux-xfs
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>
---
V2:
- Fix comment on xfs_dir_lookup()
fs/xfs/kmem.h | 6 ------
fs/xfs/libxfs/xfs_attr_leaf.c | 3 ++-
fs/xfs/libxfs/xfs_da_btree.c | 10 ++++------
fs/xfs/libxfs/xfs_dir2.c | 27 +++++++++++++--------------
fs/xfs/libxfs/xfs_iext_tree.c | 12 ++++++++----
| 4 ++--
fs/xfs/scrub/fscounters.c | 3 ++-
fs/xfs/xfs_buf.c | 6 +++---
fs/xfs/xfs_buf_item.c | 4 ++--
fs/xfs/xfs_dquot_item.c | 3 ++-
fs/xfs/xfs_error.c | 4 ++--
fs/xfs/xfs_extent_busy.c | 3 ++-
fs/xfs/xfs_extfree_item.c | 6 +++---
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_itable.c | 8 ++++----
fs/xfs/xfs_iwalk.c | 3 ++-
fs/xfs/xfs_log.c | 5 +++--
fs/xfs/xfs_log_cil.c | 6 +++---
fs/xfs/xfs_log_recover.c | 12 ++++++------
fs/xfs/xfs_mount.c | 3 ++-
fs/xfs/xfs_mru_cache.c | 5 +++--
fs/xfs/xfs_qm.c | 3 ++-
fs/xfs/xfs_refcount_item.c | 4 ++--
fs/xfs/xfs_rmap_item.c | 3 ++-
fs/xfs/xfs_trans_ail.c | 3 ++-
25 files changed, 77 insertions(+), 71 deletions(-)
diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
index 7e4ad73771ce..b9ee67fa747b 100644
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -57,12 +57,6 @@ extern void *kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags);
extern void *kmem_alloc_large(size_t size, xfs_km_flags_t);
extern void *kmem_realloc(const void *, size_t, xfs_km_flags_t);
-static inline void *
-kmem_zalloc(size_t size, xfs_km_flags_t flags)
-{
- return kmem_alloc(size, flags | KM_ZERO);
-}
-
static inline void *
kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
{
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);
/*
* Copy the header into the temp leaf so that all the stuff
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 10a96e64b2ec..29c25d1b3b76 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2534,10 +2534,8 @@ xfs_buf_map_from_irec(
ASSERT(nirecs >= 1);
if (nirecs > 1) {
- map = kmem_zalloc(nirecs * sizeof(struct xfs_buf_map),
- KM_NOFS);
- if (!map)
- return -ENOMEM;
+ map = kzalloc(nirecs * sizeof(struct xfs_buf_map),
+ GFP_NOFS | __GFP_NOFAIL);
*mapp = map;
}
@@ -2593,8 +2591,8 @@ xfs_dabuf_map(
* Optimize the one-block case.
*/
if (nfsb != 1)
- irecs = kmem_zalloc(sizeof(irec) * nfsb,
- KM_NOFS);
+ irecs = kzalloc(sizeof(irec) * nfsb,
+ GFP_NOFS | __GFP_NOFAIL);
nirecs = nfsb;
error = xfs_bmapi_read(dp, (xfs_fileoff_t)bno, nfsb, irecs,
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index efd7cec65259..c2deda036271 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -104,10 +104,10 @@ xfs_da_mount(
ASSERT(mp->m_sb.sb_versionnum & XFS_SB_VERSION_DIRV2BIT);
ASSERT(xfs_dir2_dirblock_bytes(&mp->m_sb) <= XFS_MAX_BLOCKSIZE);
- mp->m_dir_geo = kmem_zalloc(sizeof(struct xfs_da_geometry),
- KM_MAYFAIL);
- mp->m_attr_geo = kmem_zalloc(sizeof(struct xfs_da_geometry),
- KM_MAYFAIL);
+ mp->m_dir_geo = kzalloc(sizeof(struct xfs_da_geometry),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
+ mp->m_attr_geo = kzalloc(sizeof(struct xfs_da_geometry),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!mp->m_dir_geo || !mp->m_attr_geo) {
kfree(mp->m_dir_geo);
kfree(mp->m_attr_geo);
@@ -234,7 +234,7 @@ xfs_dir_init(
if (error)
return error;
- args = kmem_zalloc(sizeof(*args), KM_NOFS);
+ args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
if (!args)
return -ENOMEM;
@@ -271,7 +271,7 @@ xfs_dir_createname(
XFS_STATS_INC(dp->i_mount, xs_dir_create);
}
- args = kmem_zalloc(sizeof(*args), KM_NOFS);
+ args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
if (!args)
return -ENOMEM;
@@ -363,14 +363,13 @@ xfs_dir_lookup(
XFS_STATS_INC(dp->i_mount, xs_dir_lookup);
/*
- * We need to use KM_NOFS here so that lockdep will not throw false
+ * We need to use GFP_NOFS here so that lockdep will not throw false
* positive deadlock warnings on a non-transactional lookup path. It is
- * safe to recurse into inode recalim in that case, but lockdep can't
- * easily be taught about it. Hence KM_NOFS avoids having to add more
- * lockdep Doing this avoids having to add a bunch of lockdep class
- * annotations into the reclaim path for the ilock.
+ * safe to recurse into inode reclaim in that case, but lockdep can't
+ * easily be taught about it. Hence GFP_NOFS avoids having to add more
+ * lockdep class annotations into the reclaim path for the ilock.
*/
- args = kmem_zalloc(sizeof(*args), KM_NOFS);
+ args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
args->geo = dp->i_mount->m_dir_geo;
args->name = name->name;
args->namelen = name->len;
@@ -439,7 +438,7 @@ xfs_dir_removename(
ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
XFS_STATS_INC(dp->i_mount, xs_dir_remove);
- args = kmem_zalloc(sizeof(*args), KM_NOFS);
+ args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
if (!args)
return -ENOMEM;
@@ -500,7 +499,7 @@ xfs_dir_replace(
if (rval)
return rval;
- args = kmem_zalloc(sizeof(*args), KM_NOFS);
+ args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
if (!args)
return -ENOMEM;
diff --git a/fs/xfs/libxfs/xfs_iext_tree.c b/fs/xfs/libxfs/xfs_iext_tree.c
index a7ba30cd81da..e75e7c021187 100644
--- a/fs/xfs/libxfs/xfs_iext_tree.c
+++ b/fs/xfs/libxfs/xfs_iext_tree.c
@@ -398,7 +398,8 @@ static void
xfs_iext_grow(
struct xfs_ifork *ifp)
{
- struct xfs_iext_node *node = kmem_zalloc(NODE_SIZE, KM_NOFS);
+ struct xfs_iext_node *node = kzalloc(NODE_SIZE,
+ GFP_NOFS | __GFP_NOFAIL);
int i;
if (ifp->if_height == 1) {
@@ -454,7 +455,8 @@ xfs_iext_split_node(
int *nr_entries)
{
struct xfs_iext_node *node = *nodep;
- struct xfs_iext_node *new = kmem_zalloc(NODE_SIZE, KM_NOFS);
+ struct xfs_iext_node *new = kzalloc(NODE_SIZE,
+ GFP_NOFS | __GFP_NOFAIL);
const int nr_move = KEYS_PER_NODE / 2;
int nr_keep = nr_move + (KEYS_PER_NODE & 1);
int i = 0;
@@ -542,7 +544,8 @@ xfs_iext_split_leaf(
int *nr_entries)
{
struct xfs_iext_leaf *leaf = cur->leaf;
- struct xfs_iext_leaf *new = kmem_zalloc(NODE_SIZE, KM_NOFS);
+ struct xfs_iext_leaf *new = kzalloc(NODE_SIZE,
+ GFP_NOFS | __GFP_NOFAIL);
const int nr_move = RECS_PER_LEAF / 2;
int nr_keep = nr_move + (RECS_PER_LEAF & 1);
int i;
@@ -583,7 +586,8 @@ xfs_iext_alloc_root(
{
ASSERT(ifp->if_bytes == 0);
- ifp->if_u1.if_root = kmem_zalloc(sizeof(struct xfs_iext_rec), KM_NOFS);
+ ifp->if_u1.if_root = kzalloc(sizeof(struct xfs_iext_rec),
+ GFP_NOFS | __GFP_NOFAIL);
ifp->if_height = 1;
/* now that we have a node step into it */
--git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 6f7126f6d25c..5533c0d38333 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -720,8 +720,8 @@ xchk_agfl(
memset(&sai, 0, sizeof(sai));
sai.sc = sc;
sai.sz_entries = agflcount;
- sai.entries = kmem_zalloc(sizeof(xfs_agblock_t) * agflcount,
- KM_MAYFAIL);
+ sai.entries = kzalloc(sizeof(xfs_agblock_t) * agflcount,
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!sai.entries) {
error = -ENOMEM;
goto out;
diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
index 7251c66a82c9..bb036c5a6f21 100644
--- a/fs/xfs/scrub/fscounters.c
+++ b/fs/xfs/scrub/fscounters.c
@@ -125,7 +125,8 @@ xchk_setup_fscounters(
struct xchk_fscounters *fsc;
int error;
- sc->buf = kmem_zalloc(sizeof(struct xchk_fscounters), 0);
+ sc->buf = kzalloc(sizeof(struct xchk_fscounters),
+ GFP_KERNEL | __GFP_NOFAIL);
if (!sc->buf)
return -ENOMEM;
fsc = sc->buf;
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 0d2c41c6639d..c70122fbc2a8 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -178,8 +178,8 @@ xfs_buf_get_maps(
return 0;
}
- bp->b_maps = kmem_zalloc(map_count * sizeof(struct xfs_buf_map),
- KM_NOFS);
+ bp->b_maps = kzalloc(map_count * sizeof(struct xfs_buf_map),
+ GFP_NOFS | __GFP_NOFAIL);
if (!bp->b_maps)
return -ENOMEM;
return 0;
@@ -1749,7 +1749,7 @@ xfs_alloc_buftarg(
{
xfs_buftarg_t *btp;
- btp = kmem_zalloc(sizeof(*btp), KM_NOFS);
+ btp = kzalloc(sizeof(*btp), GFP_NOFS | __GFP_NOFAIL);
btp->bt_mount = mp;
btp->bt_dev = bdev->bd_dev;
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;
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);
xfs_log_item_init(mp, &qf->qql_item, XFS_LI_QUOTAOFF, start ?
&xfs_qm_qoffend_logitem_ops : &xfs_qm_qoff_logitem_ops);
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 4c0883380d7c..0e0ef2e15e2e 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -212,8 +212,8 @@ int
xfs_errortag_init(
struct xfs_mount *mp)
{
- mp->m_errortag = kmem_zalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
- KM_MAYFAIL);
+ mp->m_errortag = kzalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!mp->m_errortag)
return -ENOMEM;
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);
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);
@@ -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);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index e1121ed7cbb5..297b2a73f285 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2024,7 +2024,7 @@ xfs_iunlink_add_backref(
if (XFS_TEST_ERROR(false, pag->pag_mount, XFS_ERRTAG_IUNLINK_FALLBACK))
return 0;
- iu = kmem_zalloc(sizeof(*iu), KM_NOFS);
+ iu = kzalloc(sizeof(*iu), GFP_NOFS | __GFP_NOFAIL);
iu->iu_agino = prev_agino;
iu->iu_next_unlinked = this_agino;
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 36bf47f11117..ec5d590469fc 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -168,8 +168,8 @@ xfs_bulkstat_one(
ASSERT(breq->icount == 1);
- bc.buf = kmem_zalloc(sizeof(struct xfs_bulkstat),
- KM_MAYFAIL);
+ bc.buf = kzalloc(sizeof(struct xfs_bulkstat),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!bc.buf)
return -ENOMEM;
@@ -242,8 +242,8 @@ xfs_bulkstat(
if (xfs_bulkstat_already_done(breq->mp, breq->startino))
return 0;
- bc.buf = kmem_zalloc(sizeof(struct xfs_bulkstat),
- KM_MAYFAIL);
+ bc.buf = kzalloc(sizeof(struct xfs_bulkstat),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!bc.buf)
return -ENOMEM;
diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
index 67e98f9023d2..e6006423e140 100644
--- a/fs/xfs/xfs_iwalk.c
+++ b/fs/xfs/xfs_iwalk.c
@@ -616,7 +616,8 @@ xfs_iwalk_threaded(
if (xfs_pwork_ctl_want_abort(&pctl))
break;
- iwag = kmem_zalloc(sizeof(struct xfs_iwalk_ag), 0);
+ iwag = kzalloc(sizeof(struct xfs_iwalk_ag),
+ GFP_KERNEL | __GFP_NOFAIL);
iwag->mp = mp;
iwag->iwalk_fn = iwalk_fn;
iwag->data = data;
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 9ed7869d879f..152c87865241 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1412,7 +1412,7 @@ xlog_alloc_log(
int error = -ENOMEM;
uint log2_size = 0;
- log = kmem_zalloc(sizeof(struct xlog), KM_MAYFAIL);
+ log = kzalloc(sizeof(struct xlog), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!log) {
xfs_warn(mp, "Log allocation failed: No memory!");
goto out;
@@ -1482,7 +1482,8 @@ xlog_alloc_log(
size_t bvec_size = howmany(log->l_iclog_size, PAGE_SIZE) *
sizeof(struct bio_vec);
- iclog = kmem_zalloc(sizeof(*iclog) + bvec_size, KM_MAYFAIL);
+ iclog = kzalloc(sizeof(*iclog) + bvec_size,
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!iclog)
goto out_free_iclog;
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 9953f2f040ab..de5c892c72b0 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -660,7 +660,7 @@ xlog_cil_push(
if (!cil)
return 0;
- new_ctx = kmem_zalloc(sizeof(*new_ctx), KM_NOFS);
+ new_ctx = kzalloc(sizeof(*new_ctx), GFP_NOFS | __GFP_NOFAIL);
new_ctx->ticket = xlog_cil_ticket_alloc(log);
down_write(&cil->xc_ctx_lock);
@@ -1179,11 +1179,11 @@ xlog_cil_init(
struct xfs_cil *cil;
struct xfs_cil_ctx *ctx;
- cil = kmem_zalloc(sizeof(*cil), KM_MAYFAIL);
+ cil = kzalloc(sizeof(*cil), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!cil)
return -ENOMEM;
- ctx = kmem_zalloc(sizeof(*ctx), KM_MAYFAIL);
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!ctx) {
kfree(cil);
return -ENOMEM;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 4167e1326f62..cb02deb5dedc 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4171,7 +4171,7 @@ xlog_recover_add_item(
{
xlog_recover_item_t *item;
- item = kmem_zalloc(sizeof(xlog_recover_item_t), 0);
+ item = kzalloc(sizeof(xlog_recover_item_t), GFP_KERNEL | __GFP_NOFAIL);
INIT_LIST_HEAD(&item->ri_list);
list_add_tail(&item->ri_list, head);
}
@@ -4298,8 +4298,8 @@ xlog_recover_add_to_trans(
item->ri_total = in_f->ilf_size;
item->ri_buf =
- kmem_zalloc(item->ri_total * sizeof(xfs_log_iovec_t),
- 0);
+ kzalloc(item->ri_total * sizeof(xfs_log_iovec_t),
+ GFP_KERNEL | __GFP_NOFAIL);
}
if (item->ri_total <= item->ri_cnt) {
@@ -4442,7 +4442,7 @@ xlog_recover_ophdr_to_trans(
* This is a new transaction so allocate a new recovery container to
* hold the recovery ops that will follow.
*/
- trans = kmem_zalloc(sizeof(struct xlog_recover), 0);
+ trans = kzalloc(sizeof(struct xlog_recover), GFP_KERNEL | __GFP_NOFAIL);
trans->r_log_tid = tid;
trans->r_lsn = be64_to_cpu(rhead->h_lsn);
INIT_LIST_HEAD(&trans->r_itemq);
@@ -5561,9 +5561,9 @@ xlog_do_log_recovery(
* First do a pass to find all of the cancelled buf log items.
* Store them in the buf_cancel_table for use in the second pass.
*/
- log->l_buf_cancel_table = kmem_zalloc(XLOG_BC_TABLE_SIZE *
+ log->l_buf_cancel_table = kzalloc(XLOG_BC_TABLE_SIZE *
sizeof(struct list_head),
- 0);
+ GFP_KERNEL | __GFP_NOFAIL);
for (i = 0; i < XLOG_BC_TABLE_SIZE; i++)
INIT_LIST_HEAD(&log->l_buf_cancel_table[i]);
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 53ddb058b11a..c352567c8ef5 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -194,7 +194,8 @@ xfs_initialize_perag(
continue;
}
- pag = kmem_zalloc(sizeof(*pag), KM_MAYFAIL);
+ pag = kzalloc(sizeof(*pag),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!pag)
goto out_unwind_new_pags;
pag->pag_agno = index;
diff --git a/fs/xfs/xfs_mru_cache.c b/fs/xfs/xfs_mru_cache.c
index 6ef0a71d7681..f24014759c57 100644
--- a/fs/xfs/xfs_mru_cache.c
+++ b/fs/xfs/xfs_mru_cache.c
@@ -333,12 +333,13 @@ xfs_mru_cache_create(
if (!(grp_time = msecs_to_jiffies(lifetime_ms) / grp_count))
return -EINVAL;
- if (!(mru = kmem_zalloc(sizeof(*mru), 0)))
+ if (!(mru = kzalloc(sizeof(*mru), GFP_KERNEL | __GFP_NOFAIL)))
return -ENOMEM;
/* An extra list is needed to avoid reaping up to a grp_time early. */
mru->grp_count = grp_count + 1;
- mru->lists = kmem_zalloc(mru->grp_count * sizeof(*mru->lists), 0);
+ mru->lists = kzalloc(mru->grp_count * sizeof(*mru->lists),
+ GFP_KERNEL | __GFP_NOFAIL);
if (!mru->lists) {
err = -ENOMEM;
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 06c92dc61a03..a2664afa10c3 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -643,7 +643,8 @@ xfs_qm_init_quotainfo(
ASSERT(XFS_IS_QUOTA_RUNNING(mp));
- qinf = mp->m_quotainfo = kmem_zalloc(sizeof(xfs_quotainfo_t), 0);
+ qinf = mp->m_quotainfo = kzalloc(sizeof(xfs_quotainfo_t),
+ GFP_KERNEL | __GFP_NOFAIL);
error = list_lru_init(&qinf->qi_lru);
if (error)
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);
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);
diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 589918d11041..2e3515df1bb5 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -824,7 +824,8 @@ xfs_trans_ail_init(
{
struct xfs_ail *ailp;
- ailp = kmem_zalloc(sizeof(struct xfs_ail), KM_MAYFAIL);
+ ailp = kzalloc(sizeof(struct xfs_ail),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!ailp)
return -ENOMEM;
--
2.23.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 3/5] xfs: remove kmem_zalloc() wrapper
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
1 sibling, 0 replies; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-20 19:00 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
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>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> V2:
> - Fix comment on xfs_dir_lookup()
>
> fs/xfs/kmem.h | 6 ------
> fs/xfs/libxfs/xfs_attr_leaf.c | 3 ++-
> fs/xfs/libxfs/xfs_da_btree.c | 10 ++++------
> fs/xfs/libxfs/xfs_dir2.c | 27 +++++++++++++--------------
> fs/xfs/libxfs/xfs_iext_tree.c | 12 ++++++++----
> fs/xfs/scrub/agheader.c | 4 ++--
> fs/xfs/scrub/fscounters.c | 3 ++-
> fs/xfs/xfs_buf.c | 6 +++---
> fs/xfs/xfs_buf_item.c | 4 ++--
> fs/xfs/xfs_dquot_item.c | 3 ++-
> fs/xfs/xfs_error.c | 4 ++--
> fs/xfs/xfs_extent_busy.c | 3 ++-
> fs/xfs/xfs_extfree_item.c | 6 +++---
> fs/xfs/xfs_inode.c | 2 +-
> fs/xfs/xfs_itable.c | 8 ++++----
> fs/xfs/xfs_iwalk.c | 3 ++-
> fs/xfs/xfs_log.c | 5 +++--
> fs/xfs/xfs_log_cil.c | 6 +++---
> fs/xfs/xfs_log_recover.c | 12 ++++++------
> fs/xfs/xfs_mount.c | 3 ++-
> fs/xfs/xfs_mru_cache.c | 5 +++--
> fs/xfs/xfs_qm.c | 3 ++-
> fs/xfs/xfs_refcount_item.c | 4 ++--
> fs/xfs/xfs_rmap_item.c | 3 ++-
> fs/xfs/xfs_trans_ail.c | 3 ++-
> 25 files changed, 77 insertions(+), 71 deletions(-)
>
> diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
> index 7e4ad73771ce..b9ee67fa747b 100644
> --- a/fs/xfs/kmem.h
> +++ b/fs/xfs/kmem.h
> @@ -57,12 +57,6 @@ extern void *kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags);
> extern void *kmem_alloc_large(size_t size, xfs_km_flags_t);
> extern void *kmem_realloc(const void *, size_t, xfs_km_flags_t);
>
> -static inline void *
> -kmem_zalloc(size_t size, xfs_km_flags_t flags)
> -{
> - return kmem_alloc(size, flags | KM_ZERO);
> -}
> -
> static inline void *
> kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
> {
> 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);
>
> /*
> * Copy the header into the temp leaf so that all the stuff
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 10a96e64b2ec..29c25d1b3b76 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2534,10 +2534,8 @@ xfs_buf_map_from_irec(
> ASSERT(nirecs >= 1);
>
> if (nirecs > 1) {
> - map = kmem_zalloc(nirecs * sizeof(struct xfs_buf_map),
> - KM_NOFS);
> - if (!map)
> - return -ENOMEM;
> + map = kzalloc(nirecs * sizeof(struct xfs_buf_map),
> + GFP_NOFS | __GFP_NOFAIL);
> *mapp = map;
> }
>
> @@ -2593,8 +2591,8 @@ xfs_dabuf_map(
> * Optimize the one-block case.
> */
> if (nfsb != 1)
> - irecs = kmem_zalloc(sizeof(irec) * nfsb,
> - KM_NOFS);
> + irecs = kzalloc(sizeof(irec) * nfsb,
> + GFP_NOFS | __GFP_NOFAIL);
>
> nirecs = nfsb;
> error = xfs_bmapi_read(dp, (xfs_fileoff_t)bno, nfsb, irecs,
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index efd7cec65259..c2deda036271 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -104,10 +104,10 @@ xfs_da_mount(
> ASSERT(mp->m_sb.sb_versionnum & XFS_SB_VERSION_DIRV2BIT);
> ASSERT(xfs_dir2_dirblock_bytes(&mp->m_sb) <= XFS_MAX_BLOCKSIZE);
>
> - mp->m_dir_geo = kmem_zalloc(sizeof(struct xfs_da_geometry),
> - KM_MAYFAIL);
> - mp->m_attr_geo = kmem_zalloc(sizeof(struct xfs_da_geometry),
> - KM_MAYFAIL);
> + mp->m_dir_geo = kzalloc(sizeof(struct xfs_da_geometry),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> + mp->m_attr_geo = kzalloc(sizeof(struct xfs_da_geometry),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!mp->m_dir_geo || !mp->m_attr_geo) {
> kfree(mp->m_dir_geo);
> kfree(mp->m_attr_geo);
> @@ -234,7 +234,7 @@ xfs_dir_init(
> if (error)
> return error;
>
> - args = kmem_zalloc(sizeof(*args), KM_NOFS);
> + args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
> if (!args)
> return -ENOMEM;
>
> @@ -271,7 +271,7 @@ xfs_dir_createname(
> XFS_STATS_INC(dp->i_mount, xs_dir_create);
> }
>
> - args = kmem_zalloc(sizeof(*args), KM_NOFS);
> + args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
> if (!args)
> return -ENOMEM;
>
> @@ -363,14 +363,13 @@ xfs_dir_lookup(
> XFS_STATS_INC(dp->i_mount, xs_dir_lookup);
>
> /*
> - * We need to use KM_NOFS here so that lockdep will not throw false
> + * We need to use GFP_NOFS here so that lockdep will not throw false
> * positive deadlock warnings on a non-transactional lookup path. It is
> - * safe to recurse into inode recalim in that case, but lockdep can't
> - * easily be taught about it. Hence KM_NOFS avoids having to add more
> - * lockdep Doing this avoids having to add a bunch of lockdep class
> - * annotations into the reclaim path for the ilock.
> + * safe to recurse into inode reclaim in that case, but lockdep can't
> + * easily be taught about it. Hence GFP_NOFS avoids having to add more
> + * lockdep class annotations into the reclaim path for the ilock.
> */
> - args = kmem_zalloc(sizeof(*args), KM_NOFS);
> + args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
> args->geo = dp->i_mount->m_dir_geo;
> args->name = name->name;
> args->namelen = name->len;
> @@ -439,7 +438,7 @@ xfs_dir_removename(
> ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
> XFS_STATS_INC(dp->i_mount, xs_dir_remove);
>
> - args = kmem_zalloc(sizeof(*args), KM_NOFS);
> + args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
> if (!args)
> return -ENOMEM;
>
> @@ -500,7 +499,7 @@ xfs_dir_replace(
> if (rval)
> return rval;
>
> - args = kmem_zalloc(sizeof(*args), KM_NOFS);
> + args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
> if (!args)
> return -ENOMEM;
>
> diff --git a/fs/xfs/libxfs/xfs_iext_tree.c b/fs/xfs/libxfs/xfs_iext_tree.c
> index a7ba30cd81da..e75e7c021187 100644
> --- a/fs/xfs/libxfs/xfs_iext_tree.c
> +++ b/fs/xfs/libxfs/xfs_iext_tree.c
> @@ -398,7 +398,8 @@ static void
> xfs_iext_grow(
> struct xfs_ifork *ifp)
> {
> - struct xfs_iext_node *node = kmem_zalloc(NODE_SIZE, KM_NOFS);
> + struct xfs_iext_node *node = kzalloc(NODE_SIZE,
> + GFP_NOFS | __GFP_NOFAIL);
> int i;
>
> if (ifp->if_height == 1) {
> @@ -454,7 +455,8 @@ xfs_iext_split_node(
> int *nr_entries)
> {
> struct xfs_iext_node *node = *nodep;
> - struct xfs_iext_node *new = kmem_zalloc(NODE_SIZE, KM_NOFS);
> + struct xfs_iext_node *new = kzalloc(NODE_SIZE,
> + GFP_NOFS | __GFP_NOFAIL);
> const int nr_move = KEYS_PER_NODE / 2;
> int nr_keep = nr_move + (KEYS_PER_NODE & 1);
> int i = 0;
> @@ -542,7 +544,8 @@ xfs_iext_split_leaf(
> int *nr_entries)
> {
> struct xfs_iext_leaf *leaf = cur->leaf;
> - struct xfs_iext_leaf *new = kmem_zalloc(NODE_SIZE, KM_NOFS);
> + struct xfs_iext_leaf *new = kzalloc(NODE_SIZE,
> + GFP_NOFS | __GFP_NOFAIL);
> const int nr_move = RECS_PER_LEAF / 2;
> int nr_keep = nr_move + (RECS_PER_LEAF & 1);
> int i;
> @@ -583,7 +586,8 @@ xfs_iext_alloc_root(
> {
> ASSERT(ifp->if_bytes == 0);
>
> - ifp->if_u1.if_root = kmem_zalloc(sizeof(struct xfs_iext_rec), KM_NOFS);
> + ifp->if_u1.if_root = kzalloc(sizeof(struct xfs_iext_rec),
> + GFP_NOFS | __GFP_NOFAIL);
> ifp->if_height = 1;
>
> /* now that we have a node step into it */
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index 6f7126f6d25c..5533c0d38333 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
> @@ -720,8 +720,8 @@ xchk_agfl(
> memset(&sai, 0, sizeof(sai));
> sai.sc = sc;
> sai.sz_entries = agflcount;
> - sai.entries = kmem_zalloc(sizeof(xfs_agblock_t) * agflcount,
> - KM_MAYFAIL);
> + sai.entries = kzalloc(sizeof(xfs_agblock_t) * agflcount,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!sai.entries) {
> error = -ENOMEM;
> goto out;
> diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
> index 7251c66a82c9..bb036c5a6f21 100644
> --- a/fs/xfs/scrub/fscounters.c
> +++ b/fs/xfs/scrub/fscounters.c
> @@ -125,7 +125,8 @@ xchk_setup_fscounters(
> struct xchk_fscounters *fsc;
> int error;
>
> - sc->buf = kmem_zalloc(sizeof(struct xchk_fscounters), 0);
> + sc->buf = kzalloc(sizeof(struct xchk_fscounters),
> + GFP_KERNEL | __GFP_NOFAIL);
> if (!sc->buf)
> return -ENOMEM;
> fsc = sc->buf;
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 0d2c41c6639d..c70122fbc2a8 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -178,8 +178,8 @@ xfs_buf_get_maps(
> return 0;
> }
>
> - bp->b_maps = kmem_zalloc(map_count * sizeof(struct xfs_buf_map),
> - KM_NOFS);
> + bp->b_maps = kzalloc(map_count * sizeof(struct xfs_buf_map),
> + GFP_NOFS | __GFP_NOFAIL);
> if (!bp->b_maps)
> return -ENOMEM;
> return 0;
> @@ -1749,7 +1749,7 @@ xfs_alloc_buftarg(
> {
> xfs_buftarg_t *btp;
>
> - btp = kmem_zalloc(sizeof(*btp), KM_NOFS);
> + btp = kzalloc(sizeof(*btp), GFP_NOFS | __GFP_NOFAIL);
>
> btp->bt_mount = mp;
> btp->bt_dev = bdev->bd_dev;
> 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;
> 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);
>
> xfs_log_item_init(mp, &qf->qql_item, XFS_LI_QUOTAOFF, start ?
> &xfs_qm_qoffend_logitem_ops : &xfs_qm_qoff_logitem_ops);
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 4c0883380d7c..0e0ef2e15e2e 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -212,8 +212,8 @@ int
> xfs_errortag_init(
> struct xfs_mount *mp)
> {
> - mp->m_errortag = kmem_zalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
> - KM_MAYFAIL);
> + mp->m_errortag = kzalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!mp->m_errortag)
> return -ENOMEM;
>
> 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);
> 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);
> @@ -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);
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index e1121ed7cbb5..297b2a73f285 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2024,7 +2024,7 @@ xfs_iunlink_add_backref(
> if (XFS_TEST_ERROR(false, pag->pag_mount, XFS_ERRTAG_IUNLINK_FALLBACK))
> return 0;
>
> - iu = kmem_zalloc(sizeof(*iu), KM_NOFS);
> + iu = kzalloc(sizeof(*iu), GFP_NOFS | __GFP_NOFAIL);
> iu->iu_agino = prev_agino;
> iu->iu_next_unlinked = this_agino;
>
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 36bf47f11117..ec5d590469fc 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -168,8 +168,8 @@ xfs_bulkstat_one(
>
> ASSERT(breq->icount == 1);
>
> - bc.buf = kmem_zalloc(sizeof(struct xfs_bulkstat),
> - KM_MAYFAIL);
> + bc.buf = kzalloc(sizeof(struct xfs_bulkstat),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!bc.buf)
> return -ENOMEM;
>
> @@ -242,8 +242,8 @@ xfs_bulkstat(
> if (xfs_bulkstat_already_done(breq->mp, breq->startino))
> return 0;
>
> - bc.buf = kmem_zalloc(sizeof(struct xfs_bulkstat),
> - KM_MAYFAIL);
> + bc.buf = kzalloc(sizeof(struct xfs_bulkstat),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!bc.buf)
> return -ENOMEM;
>
> diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
> index 67e98f9023d2..e6006423e140 100644
> --- a/fs/xfs/xfs_iwalk.c
> +++ b/fs/xfs/xfs_iwalk.c
> @@ -616,7 +616,8 @@ xfs_iwalk_threaded(
> if (xfs_pwork_ctl_want_abort(&pctl))
> break;
>
> - iwag = kmem_zalloc(sizeof(struct xfs_iwalk_ag), 0);
> + iwag = kzalloc(sizeof(struct xfs_iwalk_ag),
> + GFP_KERNEL | __GFP_NOFAIL);
> iwag->mp = mp;
> iwag->iwalk_fn = iwalk_fn;
> iwag->data = data;
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 9ed7869d879f..152c87865241 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1412,7 +1412,7 @@ xlog_alloc_log(
> int error = -ENOMEM;
> uint log2_size = 0;
>
> - log = kmem_zalloc(sizeof(struct xlog), KM_MAYFAIL);
> + log = kzalloc(sizeof(struct xlog), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!log) {
> xfs_warn(mp, "Log allocation failed: No memory!");
> goto out;
> @@ -1482,7 +1482,8 @@ xlog_alloc_log(
> size_t bvec_size = howmany(log->l_iclog_size, PAGE_SIZE) *
> sizeof(struct bio_vec);
>
> - iclog = kmem_zalloc(sizeof(*iclog) + bvec_size, KM_MAYFAIL);
> + iclog = kzalloc(sizeof(*iclog) + bvec_size,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!iclog)
> goto out_free_iclog;
>
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 9953f2f040ab..de5c892c72b0 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -660,7 +660,7 @@ xlog_cil_push(
> if (!cil)
> return 0;
>
> - new_ctx = kmem_zalloc(sizeof(*new_ctx), KM_NOFS);
> + new_ctx = kzalloc(sizeof(*new_ctx), GFP_NOFS | __GFP_NOFAIL);
> new_ctx->ticket = xlog_cil_ticket_alloc(log);
>
> down_write(&cil->xc_ctx_lock);
> @@ -1179,11 +1179,11 @@ xlog_cil_init(
> struct xfs_cil *cil;
> struct xfs_cil_ctx *ctx;
>
> - cil = kmem_zalloc(sizeof(*cil), KM_MAYFAIL);
> + cil = kzalloc(sizeof(*cil), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!cil)
> return -ENOMEM;
>
> - ctx = kmem_zalloc(sizeof(*ctx), KM_MAYFAIL);
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!ctx) {
> kfree(cil);
> return -ENOMEM;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 4167e1326f62..cb02deb5dedc 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -4171,7 +4171,7 @@ xlog_recover_add_item(
> {
> xlog_recover_item_t *item;
>
> - item = kmem_zalloc(sizeof(xlog_recover_item_t), 0);
> + item = kzalloc(sizeof(xlog_recover_item_t), GFP_KERNEL | __GFP_NOFAIL);
> INIT_LIST_HEAD(&item->ri_list);
> list_add_tail(&item->ri_list, head);
> }
> @@ -4298,8 +4298,8 @@ xlog_recover_add_to_trans(
>
> item->ri_total = in_f->ilf_size;
> item->ri_buf =
> - kmem_zalloc(item->ri_total * sizeof(xfs_log_iovec_t),
> - 0);
> + kzalloc(item->ri_total * sizeof(xfs_log_iovec_t),
> + GFP_KERNEL | __GFP_NOFAIL);
> }
>
> if (item->ri_total <= item->ri_cnt) {
> @@ -4442,7 +4442,7 @@ xlog_recover_ophdr_to_trans(
> * This is a new transaction so allocate a new recovery container to
> * hold the recovery ops that will follow.
> */
> - trans = kmem_zalloc(sizeof(struct xlog_recover), 0);
> + trans = kzalloc(sizeof(struct xlog_recover), GFP_KERNEL | __GFP_NOFAIL);
> trans->r_log_tid = tid;
> trans->r_lsn = be64_to_cpu(rhead->h_lsn);
> INIT_LIST_HEAD(&trans->r_itemq);
> @@ -5561,9 +5561,9 @@ xlog_do_log_recovery(
> * First do a pass to find all of the cancelled buf log items.
> * Store them in the buf_cancel_table for use in the second pass.
> */
> - log->l_buf_cancel_table = kmem_zalloc(XLOG_BC_TABLE_SIZE *
> + log->l_buf_cancel_table = kzalloc(XLOG_BC_TABLE_SIZE *
> sizeof(struct list_head),
> - 0);
> + GFP_KERNEL | __GFP_NOFAIL);
> for (i = 0; i < XLOG_BC_TABLE_SIZE; i++)
> INIT_LIST_HEAD(&log->l_buf_cancel_table[i]);
>
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 53ddb058b11a..c352567c8ef5 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -194,7 +194,8 @@ xfs_initialize_perag(
> continue;
> }
>
> - pag = kmem_zalloc(sizeof(*pag), KM_MAYFAIL);
> + pag = kzalloc(sizeof(*pag),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!pag)
> goto out_unwind_new_pags;
> pag->pag_agno = index;
> diff --git a/fs/xfs/xfs_mru_cache.c b/fs/xfs/xfs_mru_cache.c
> index 6ef0a71d7681..f24014759c57 100644
> --- a/fs/xfs/xfs_mru_cache.c
> +++ b/fs/xfs/xfs_mru_cache.c
> @@ -333,12 +333,13 @@ xfs_mru_cache_create(
> if (!(grp_time = msecs_to_jiffies(lifetime_ms) / grp_count))
> return -EINVAL;
>
> - if (!(mru = kmem_zalloc(sizeof(*mru), 0)))
> + if (!(mru = kzalloc(sizeof(*mru), GFP_KERNEL | __GFP_NOFAIL)))
> return -ENOMEM;
>
> /* An extra list is needed to avoid reaping up to a grp_time early. */
> mru->grp_count = grp_count + 1;
> - mru->lists = kmem_zalloc(mru->grp_count * sizeof(*mru->lists), 0);
> + mru->lists = kzalloc(mru->grp_count * sizeof(*mru->lists),
> + GFP_KERNEL | __GFP_NOFAIL);
>
> if (!mru->lists) {
> err = -ENOMEM;
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 06c92dc61a03..a2664afa10c3 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -643,7 +643,8 @@ xfs_qm_init_quotainfo(
>
> ASSERT(XFS_IS_QUOTA_RUNNING(mp));
>
> - qinf = mp->m_quotainfo = kmem_zalloc(sizeof(xfs_quotainfo_t), 0);
> + qinf = mp->m_quotainfo = kzalloc(sizeof(xfs_quotainfo_t),
> + GFP_KERNEL | __GFP_NOFAIL);
>
> error = list_lru_init(&qinf->qi_lru);
> if (error)
> 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);
> 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);
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 589918d11041..2e3515df1bb5 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -824,7 +824,8 @@ xfs_trans_ail_init(
> {
> struct xfs_ail *ailp;
>
> - ailp = kmem_zalloc(sizeof(struct xfs_ail), KM_MAYFAIL);
> + ailp = kzalloc(sizeof(struct xfs_ail),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!ailp)
> return -ENOMEM;
>
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3/5] xfs: remove kmem_zalloc() wrapper
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
1 sibling, 1 reply; 19+ messages in thread
From: Dave Chinner @ 2019-11-20 21:24 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
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.
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3/5] xfs: remove kmem_zalloc() wrapper
2019-11-20 21:24 ` Dave Chinner
@ 2019-11-20 21:41 ` Darrick J. Wong
2019-11-20 22:44 ` Carlos Maiolino
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-20 21:41 UTC (permalink / raw)
To: Dave Chinner; +Cc: Carlos Maiolino, linux-xfs
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3/5] xfs: remove kmem_zalloc() wrapper
2019-11-20 21:41 ` Darrick J. Wong
@ 2019-11-20 22:44 ` Carlos Maiolino
2019-11-20 23:08 ` Darrick J. Wong
0 siblings, 1 reply; 19+ messages in thread
From: Carlos Maiolino @ 2019-11-20 22:44 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Dave Chinner, linux-xfs
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?
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3/5] xfs: remove kmem_zalloc() wrapper
2019-11-20 22:44 ` Carlos Maiolino
@ 2019-11-20 23:08 ` Darrick J. Wong
0 siblings, 0 replies; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-20 23:08 UTC (permalink / raw)
To: Dave Chinner, linux-xfs
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
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/5] xfs: Remove kmem_realloc
2019-11-20 10:44 [PATCH 0/5] Remove/convert more kmem_* wrappers Carlos Maiolino
` (2 preceding siblings ...)
2019-11-20 10:44 ` [PATCH 3/5] xfs: remove kmem_zalloc() wrapper Carlos Maiolino
@ 2019-11-20 10:44 ` 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
2020-05-14 9:26 ` [PATCH 0/5] Remove/convert more kmem_* wrappers Dave Chinner
5 siblings, 1 reply; 19+ messages in thread
From: Carlos Maiolino @ 2019-11-20 10:44 UTC (permalink / raw)
To: linux-xfs
We can use krealloc() with __GFP_NOFAIL directly
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V2:
- Fix small conflict in kmem.h due removal of kmem_free()
- Small comment update on xfs_iroot_realloc()
fs/xfs/kmem.c | 22 ----------------------
fs/xfs/kmem.h | 1 -
fs/xfs/libxfs/xfs_iext_tree.c | 2 +-
fs/xfs/libxfs/xfs_inode_fork.c | 10 +++++-----
fs/xfs/xfs_log_recover.c | 2 +-
fs/xfs/xfs_mount.c | 4 ++--
6 files changed, 9 insertions(+), 32 deletions(-)
diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
index 2644fdaa0549..6e10e565632c 100644
--- a/fs/xfs/kmem.c
+++ b/fs/xfs/kmem.c
@@ -93,25 +93,3 @@ kmem_alloc_large(size_t size, xfs_km_flags_t flags)
return ptr;
return __kmem_vmalloc(size, flags);
}
-
-void *
-kmem_realloc(const void *old, size_t newsize, xfs_km_flags_t flags)
-{
- int retries = 0;
- gfp_t lflags = kmem_flags_convert(flags);
- void *ptr;
-
- trace_kmem_realloc(newsize, flags, _RET_IP_);
-
- do {
- ptr = krealloc(old, newsize, lflags);
- if (ptr || (flags & KM_MAYFAIL))
- return ptr;
- if (!(++retries % 100))
- xfs_err(NULL,
- "%s(%u) possible memory allocation deadlock size %zu in %s (mode:0x%x)",
- current->comm, current->pid,
- newsize, __func__, lflags);
- congestion_wait(BLK_RW_ASYNC, HZ/50);
- } while (1);
-}
diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
index b9ee67fa747b..a18c27c99721 100644
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -55,7 +55,6 @@ kmem_flags_convert(xfs_km_flags_t flags)
extern void *kmem_alloc(size_t, xfs_km_flags_t);
extern void *kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags);
extern void *kmem_alloc_large(size_t size, xfs_km_flags_t);
-extern void *kmem_realloc(const void *, size_t, xfs_km_flags_t);
static inline void *
kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
diff --git a/fs/xfs/libxfs/xfs_iext_tree.c b/fs/xfs/libxfs/xfs_iext_tree.c
index e75e7c021187..78c9f6c7a36a 100644
--- a/fs/xfs/libxfs/xfs_iext_tree.c
+++ b/fs/xfs/libxfs/xfs_iext_tree.c
@@ -607,7 +607,7 @@ xfs_iext_realloc_root(
if (new_size / sizeof(struct xfs_iext_rec) == RECS_PER_LEAF)
new_size = NODE_SIZE;
- new = kmem_realloc(ifp->if_u1.if_root, new_size, KM_NOFS);
+ new = krealloc(ifp->if_u1.if_root, new_size, GFP_NOFS | __GFP_NOFAIL);
memset(new + ifp->if_bytes, 0, new_size - ifp->if_bytes);
ifp->if_u1.if_root = new;
cur->leaf = new;
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index ceb322c7105e..82799dddf97d 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -380,15 +380,15 @@ xfs_iroot_realloc(
/*
* If there is already an existing if_broot, then we need
- * to realloc() it and shift the pointers to their new
+ * to krealloc() it and shift the pointers to their new
* location. The records don't change location because
* they are kept butted up against the btree block header.
*/
cur_max = xfs_bmbt_maxrecs(mp, ifp->if_broot_bytes, 0);
new_max = cur_max + rec_diff;
new_size = XFS_BMAP_BROOT_SPACE_CALC(mp, new_max);
- ifp->if_broot = kmem_realloc(ifp->if_broot, new_size,
- KM_NOFS);
+ ifp->if_broot = krealloc(ifp->if_broot, new_size,
+ GFP_NOFS | __GFP_NOFAIL);
op = (char *)XFS_BMAP_BROOT_PTR_ADDR(mp, ifp->if_broot, 1,
ifp->if_broot_bytes);
np = (char *)XFS_BMAP_BROOT_PTR_ADDR(mp, ifp->if_broot, 1,
@@ -497,8 +497,8 @@ xfs_idata_realloc(
* in size so that it can be logged and stay on word boundaries.
* We enforce that here.
*/
- ifp->if_u1.if_data = kmem_realloc(ifp->if_u1.if_data,
- roundup(new_size, 4), KM_NOFS);
+ ifp->if_u1.if_data = krealloc(ifp->if_u1.if_data, roundup(new_size, 4),
+ GFP_NOFS | __GFP_NOFAIL);
ifp->if_bytes = new_size;
}
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index cb02deb5dedc..5423171e0b7d 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4211,7 +4211,7 @@ xlog_recover_add_to_cont_trans(
old_ptr = item->ri_buf[item->ri_cnt-1].i_addr;
old_len = item->ri_buf[item->ri_cnt-1].i_len;
- ptr = kmem_realloc(old_ptr, len + old_len, 0);
+ ptr = krealloc(old_ptr, len + old_len, GFP_KERNEL | __GFP_NOFAIL);
memcpy(&ptr[old_len], dp, len);
item->ri_buf[item->ri_cnt-1].i_len += len;
item->ri_buf[item->ri_cnt-1].i_addr = ptr;
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index c352567c8ef5..12a1cdf8e292 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -80,9 +80,9 @@ xfs_uuid_mount(
}
if (hole < 0) {
- xfs_uuid_table = kmem_realloc(xfs_uuid_table,
+ xfs_uuid_table = krealloc(xfs_uuid_table,
(xfs_uuid_table_size + 1) * sizeof(*xfs_uuid_table),
- 0);
+ GFP_KERNEL | __GFP_NOFAIL);
hole = xfs_uuid_table_size++;
}
xfs_uuid_table[hole] = *uuid;
--
2.23.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 4/5] xfs: Remove kmem_realloc
2019-11-20 10:44 ` [PATCH 4/5] xfs: Remove kmem_realloc Carlos Maiolino
@ 2019-11-20 19:00 ` Darrick J. Wong
0 siblings, 0 replies; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-20 19:00 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Nov 20, 2019 at 11:44:24AM +0100, Carlos Maiolino wrote:
> We can use krealloc() with __GFP_NOFAIL directly
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> V2:
> - Fix small conflict in kmem.h due removal of kmem_free()
> - Small comment update on xfs_iroot_realloc()
>
> fs/xfs/kmem.c | 22 ----------------------
> fs/xfs/kmem.h | 1 -
> fs/xfs/libxfs/xfs_iext_tree.c | 2 +-
> fs/xfs/libxfs/xfs_inode_fork.c | 10 +++++-----
> fs/xfs/xfs_log_recover.c | 2 +-
> fs/xfs/xfs_mount.c | 4 ++--
> 6 files changed, 9 insertions(+), 32 deletions(-)
>
> diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
> index 2644fdaa0549..6e10e565632c 100644
> --- a/fs/xfs/kmem.c
> +++ b/fs/xfs/kmem.c
> @@ -93,25 +93,3 @@ kmem_alloc_large(size_t size, xfs_km_flags_t flags)
> return ptr;
> return __kmem_vmalloc(size, flags);
> }
> -
> -void *
> -kmem_realloc(const void *old, size_t newsize, xfs_km_flags_t flags)
> -{
> - int retries = 0;
> - gfp_t lflags = kmem_flags_convert(flags);
> - void *ptr;
> -
> - trace_kmem_realloc(newsize, flags, _RET_IP_);
> -
> - do {
> - ptr = krealloc(old, newsize, lflags);
> - if (ptr || (flags & KM_MAYFAIL))
> - return ptr;
> - if (!(++retries % 100))
> - xfs_err(NULL,
> - "%s(%u) possible memory allocation deadlock size %zu in %s (mode:0x%x)",
> - current->comm, current->pid,
> - newsize, __func__, lflags);
> - congestion_wait(BLK_RW_ASYNC, HZ/50);
> - } while (1);
> -}
> diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
> index b9ee67fa747b..a18c27c99721 100644
> --- a/fs/xfs/kmem.h
> +++ b/fs/xfs/kmem.h
> @@ -55,7 +55,6 @@ kmem_flags_convert(xfs_km_flags_t flags)
> extern void *kmem_alloc(size_t, xfs_km_flags_t);
> extern void *kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags);
> extern void *kmem_alloc_large(size_t size, xfs_km_flags_t);
> -extern void *kmem_realloc(const void *, size_t, xfs_km_flags_t);
>
> static inline void *
> kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
> diff --git a/fs/xfs/libxfs/xfs_iext_tree.c b/fs/xfs/libxfs/xfs_iext_tree.c
> index e75e7c021187..78c9f6c7a36a 100644
> --- a/fs/xfs/libxfs/xfs_iext_tree.c
> +++ b/fs/xfs/libxfs/xfs_iext_tree.c
> @@ -607,7 +607,7 @@ xfs_iext_realloc_root(
> if (new_size / sizeof(struct xfs_iext_rec) == RECS_PER_LEAF)
> new_size = NODE_SIZE;
>
> - new = kmem_realloc(ifp->if_u1.if_root, new_size, KM_NOFS);
> + new = krealloc(ifp->if_u1.if_root, new_size, GFP_NOFS | __GFP_NOFAIL);
> memset(new + ifp->if_bytes, 0, new_size - ifp->if_bytes);
> ifp->if_u1.if_root = new;
> cur->leaf = new;
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index ceb322c7105e..82799dddf97d 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -380,15 +380,15 @@ xfs_iroot_realloc(
>
> /*
> * If there is already an existing if_broot, then we need
> - * to realloc() it and shift the pointers to their new
> + * to krealloc() it and shift the pointers to their new
> * location. The records don't change location because
> * they are kept butted up against the btree block header.
> */
> cur_max = xfs_bmbt_maxrecs(mp, ifp->if_broot_bytes, 0);
> new_max = cur_max + rec_diff;
> new_size = XFS_BMAP_BROOT_SPACE_CALC(mp, new_max);
> - ifp->if_broot = kmem_realloc(ifp->if_broot, new_size,
> - KM_NOFS);
> + ifp->if_broot = krealloc(ifp->if_broot, new_size,
> + GFP_NOFS | __GFP_NOFAIL);
> op = (char *)XFS_BMAP_BROOT_PTR_ADDR(mp, ifp->if_broot, 1,
> ifp->if_broot_bytes);
> np = (char *)XFS_BMAP_BROOT_PTR_ADDR(mp, ifp->if_broot, 1,
> @@ -497,8 +497,8 @@ xfs_idata_realloc(
> * in size so that it can be logged and stay on word boundaries.
> * We enforce that here.
> */
> - ifp->if_u1.if_data = kmem_realloc(ifp->if_u1.if_data,
> - roundup(new_size, 4), KM_NOFS);
> + ifp->if_u1.if_data = krealloc(ifp->if_u1.if_data, roundup(new_size, 4),
> + GFP_NOFS | __GFP_NOFAIL);
> ifp->if_bytes = new_size;
> }
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index cb02deb5dedc..5423171e0b7d 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -4211,7 +4211,7 @@ xlog_recover_add_to_cont_trans(
> old_ptr = item->ri_buf[item->ri_cnt-1].i_addr;
> old_len = item->ri_buf[item->ri_cnt-1].i_len;
>
> - ptr = kmem_realloc(old_ptr, len + old_len, 0);
> + ptr = krealloc(old_ptr, len + old_len, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(&ptr[old_len], dp, len);
> item->ri_buf[item->ri_cnt-1].i_len += len;
> item->ri_buf[item->ri_cnt-1].i_addr = ptr;
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index c352567c8ef5..12a1cdf8e292 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -80,9 +80,9 @@ xfs_uuid_mount(
> }
>
> if (hole < 0) {
> - xfs_uuid_table = kmem_realloc(xfs_uuid_table,
> + xfs_uuid_table = krealloc(xfs_uuid_table,
> (xfs_uuid_table_size + 1) * sizeof(*xfs_uuid_table),
> - 0);
> + GFP_KERNEL | __GFP_NOFAIL);
> hole = xfs_uuid_table_size++;
> }
> xfs_uuid_table[hole] = *uuid;
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/5] xfs: Convert kmem_alloc() users
2019-11-20 10:44 [PATCH 0/5] Remove/convert more kmem_* wrappers Carlos Maiolino
` (3 preceding siblings ...)
2019-11-20 10:44 ` [PATCH 4/5] xfs: Remove kmem_realloc Carlos Maiolino
@ 2019-11-20 10:44 ` Carlos Maiolino
2019-11-20 19:00 ` Darrick J. Wong
2019-11-22 15:57 ` Darrick J. Wong
2020-05-14 9:26 ` [PATCH 0/5] Remove/convert more kmem_* wrappers Dave Chinner
5 siblings, 2 replies; 19+ messages in thread
From: Carlos Maiolino @ 2019-11-20 10:44 UTC (permalink / raw)
To: linux-xfs
Use kmalloc() directly.
There is no logic change on kmem_alloc() since it's being removed soon, but for
now, kmem_alloc_io() and kmem_alloc_large() still have use for kmem_alloc() due
their fallback to vmalloc() and also the alignment check, so we can't completely
remove it here.
But, there is no need to export kmem_alloc() to the whole XFS driver anymore, so,
convert kmem_alloc() into a static, local function __kmem_alloc().
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V2:
- Rephrase commit log
fs/xfs/kmem.c | 8 ++++----
fs/xfs/kmem.h | 1 -
fs/xfs/libxfs/xfs_attr_leaf.c | 6 +++---
fs/xfs/libxfs/xfs_bmap.c | 2 +-
fs/xfs/libxfs/xfs_da_btree.c | 4 +++-
fs/xfs/libxfs/xfs_defer.c | 4 ++--
fs/xfs/libxfs/xfs_dir2.c | 2 +-
fs/xfs/libxfs/xfs_dir2_block.c | 2 +-
fs/xfs/libxfs/xfs_dir2_sf.c | 8 ++++----
fs/xfs/libxfs/xfs_inode_fork.c | 10 ++++++----
fs/xfs/libxfs/xfs_refcount.c | 9 +++++----
fs/xfs/libxfs/xfs_rmap.c | 2 +-
fs/xfs/scrub/bitmap.c | 7 ++++---
fs/xfs/scrub/btree.c | 4 ++--
fs/xfs/scrub/refcount.c | 4 ++--
fs/xfs/xfs_attr_inactive.c | 2 +-
fs/xfs/xfs_attr_list.c | 2 +-
fs/xfs/xfs_buf.c | 5 +++--
fs/xfs/xfs_filestream.c | 2 +-
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_iwalk.c | 2 +-
fs/xfs/xfs_log_recover.c | 7 ++++---
fs/xfs/xfs_qm.c | 3 ++-
fs/xfs/xfs_rtalloc.c | 2 +-
fs/xfs/xfs_super.c | 2 +-
25 files changed, 55 insertions(+), 47 deletions(-)
diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
index 6e10e565632c..79467813d810 100644
--- a/fs/xfs/kmem.c
+++ b/fs/xfs/kmem.c
@@ -8,8 +8,8 @@
#include "xfs_message.h"
#include "xfs_trace.h"
-void *
-kmem_alloc(size_t size, xfs_km_flags_t flags)
+static void *
+__kmem_alloc(size_t size, xfs_km_flags_t flags)
{
int retries = 0;
gfp_t lflags = kmem_flags_convert(flags);
@@ -72,7 +72,7 @@ kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags)
if (WARN_ON_ONCE(align_mask >= PAGE_SIZE))
align_mask = PAGE_SIZE - 1;
- ptr = kmem_alloc(size, flags | KM_MAYFAIL);
+ ptr = __kmem_alloc(size, flags | KM_MAYFAIL);
if (ptr) {
if (!((uintptr_t)ptr & align_mask))
return ptr;
@@ -88,7 +88,7 @@ kmem_alloc_large(size_t size, xfs_km_flags_t flags)
trace_kmem_alloc_large(size, flags, _RET_IP_);
- ptr = kmem_alloc(size, flags | KM_MAYFAIL);
+ 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 a18c27c99721..78a54839430a 100644
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -52,7 +52,6 @@ kmem_flags_convert(xfs_km_flags_t flags)
return lflags;
}
-extern void *kmem_alloc(size_t, xfs_km_flags_t);
extern void *kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags);
extern void *kmem_alloc_large(size_t size, xfs_km_flags_t);
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 67de68584224..807950eca17a 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -885,7 +885,7 @@ xfs_attr_shortform_to_leaf(
ifp = dp->i_afp;
sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
size = be16_to_cpu(sf->hdr.totsize);
- tmpbuffer = kmem_alloc(size, 0);
+ tmpbuffer = kmalloc(size, GFP_KERNEL | __GFP_NOFAIL);
ASSERT(tmpbuffer != NULL);
memcpy(tmpbuffer, ifp->if_u1.if_data, size);
sf = (xfs_attr_shortform_t *)tmpbuffer;
@@ -1073,7 +1073,7 @@ xfs_attr3_leaf_to_shortform(
trace_xfs_attr_leaf_to_sf(args);
- tmpbuffer = kmem_alloc(args->geo->blksize, 0);
+ tmpbuffer = kmalloc(args->geo->blksize, GFP_KERNEL | __GFP_NOFAIL);
if (!tmpbuffer)
return -ENOMEM;
@@ -1534,7 +1534,7 @@ xfs_attr3_leaf_compact(
trace_xfs_attr_leaf_compact(args);
- tmpbuffer = kmem_alloc(args->geo->blksize, 0);
+ tmpbuffer = kmalloc(args->geo->blksize, GFP_KERNEL | __GFP_NOFAIL);
memcpy(tmpbuffer, bp->b_addr, args->geo->blksize);
memset(bp->b_addr, 0, args->geo->blksize);
leaf_src = (xfs_attr_leafblock_t *)tmpbuffer;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 37596e49b92e..fc5bed95bd44 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -6045,7 +6045,7 @@ __xfs_bmap_add(
bmap->br_blockcount,
bmap->br_state);
- bi = kmem_alloc(sizeof(struct xfs_bmap_intent), KM_NOFS);
+ bi = kmalloc(sizeof(struct xfs_bmap_intent), GFP_NOFS | __GFP_NOFAIL);
INIT_LIST_HEAD(&bi->bi_list);
bi->bi_type = type;
bi->bi_owner = ip;
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 29c25d1b3b76..efe84c636bd3 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2152,7 +2152,9 @@ xfs_da_grow_inode_int(
* If we didn't get it and the block might work if fragmented,
* try without the CONTIG flag. Loop until we get it all.
*/
- mapp = kmem_alloc(sizeof(*mapp) * count, 0);
+ mapp = kmalloc(sizeof(*mapp) * count,
+ GFP_KERNEL | __GFP_NOFAIL);
+
for (b = *bno, mapi = 0; b < *bno + count; ) {
nmap = min(XFS_BMAP_MAX_NMAP, count);
c = (int)(*bno + count - b);
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 27c3d150068a..7dd16f208b82 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -516,8 +516,8 @@ xfs_defer_add(
dfp = NULL;
}
if (!dfp) {
- dfp = kmem_alloc(sizeof(struct xfs_defer_pending),
- KM_NOFS);
+ dfp = kmalloc(sizeof(struct xfs_defer_pending),
+ GFP_NOFS | __GFP_NOFAIL);
dfp->dfp_type = type;
dfp->dfp_intent = NULL;
dfp->dfp_done = NULL;
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index c2deda036271..4777356b4f83 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -331,7 +331,7 @@ xfs_dir_cilookup_result(
!(args->op_flags & XFS_DA_OP_CILOOKUP))
return -EEXIST;
- args->value = kmem_alloc(len, KM_NOFS | KM_MAYFAIL);
+ args->value = kmalloc(len, GFP_NOFS | __GFP_RETRY_MAYFAIL);
if (!args->value)
return -ENOMEM;
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 766f282b706a..54ae07a432e4 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -1083,7 +1083,7 @@ xfs_dir2_sf_to_block(
* Copy the directory into a temporary buffer.
* Then pitch the incore inode data so we can make extents.
*/
- sfp = kmem_alloc(ifp->if_bytes, 0);
+ sfp = kmalloc(ifp->if_bytes, GFP_KERNEL | __GFP_NOFAIL);
memcpy(sfp, oldsfp, ifp->if_bytes);
xfs_idata_realloc(dp, -ifp->if_bytes, XFS_DATA_FORK);
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index f4de4e7b10ef..43d72aebb9cf 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -276,7 +276,7 @@ xfs_dir2_block_to_sf(
* format the data into. Once we have formatted the data, we can free
* the block and copy the formatted data into the inode literal area.
*/
- sfp = kmem_alloc(mp->m_sb.sb_inodesize, 0);
+ sfp = kmalloc(mp->m_sb.sb_inodesize, GFP_KERNEL | __GFP_NOFAIL);
memcpy(sfp, sfhp, xfs_dir2_sf_hdr_size(sfhp->i8count));
/*
@@ -530,7 +530,7 @@ xfs_dir2_sf_addname_hard(
*/
sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
old_isize = (int)dp->i_d.di_size;
- buf = kmem_alloc(old_isize, 0);
+ buf = kmalloc(old_isize, GFP_KERNEL | __GFP_NOFAIL);
oldsfp = (xfs_dir2_sf_hdr_t *)buf;
memcpy(oldsfp, sfp, old_isize);
/*
@@ -1162,7 +1162,7 @@ xfs_dir2_sf_toino4(
* Don't want xfs_idata_realloc copying the data here.
*/
oldsize = dp->i_df.if_bytes;
- buf = kmem_alloc(oldsize, 0);
+ buf = kmalloc(oldsize, GFP_KERNEL | __GFP_NOFAIL);
oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
ASSERT(oldsfp->i8count == 1);
memcpy(buf, oldsfp, oldsize);
@@ -1235,7 +1235,7 @@ xfs_dir2_sf_toino8(
* Don't want xfs_idata_realloc copying the data here.
*/
oldsize = dp->i_df.if_bytes;
- buf = kmem_alloc(oldsize, 0);
+ buf = kmalloc(oldsize, GFP_KERNEL | __GFP_NOFAIL);
oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
ASSERT(oldsfp->i8count == 0);
memcpy(buf, oldsfp, oldsize);
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 82799dddf97d..62c305654657 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -153,7 +153,8 @@ xfs_init_local_fork(
if (size) {
real_size = roundup(mem_size, 4);
- ifp->if_u1.if_data = kmem_alloc(real_size, KM_NOFS);
+ ifp->if_u1.if_data = kmalloc(real_size,
+ GFP_NOFS | __GFP_NOFAIL);
memcpy(ifp->if_u1.if_data, data, size);
if (zero_terminate)
ifp->if_u1.if_data[size] = '\0';
@@ -308,7 +309,7 @@ xfs_iformat_btree(
}
ifp->if_broot_bytes = size;
- ifp->if_broot = kmem_alloc(size, KM_NOFS);
+ ifp->if_broot = kmalloc(size, GFP_NOFS | __GFP_NOFAIL);
ASSERT(ifp->if_broot != NULL);
/*
* Copy and convert from the on-disk structure
@@ -373,7 +374,8 @@ xfs_iroot_realloc(
*/
if (ifp->if_broot_bytes == 0) {
new_size = XFS_BMAP_BROOT_SPACE_CALC(mp, rec_diff);
- ifp->if_broot = kmem_alloc(new_size, KM_NOFS);
+ ifp->if_broot = kmalloc(new_size,
+ GFP_NOFS | __GFP_NOFAIL);
ifp->if_broot_bytes = (int)new_size;
return;
}
@@ -414,7 +416,7 @@ xfs_iroot_realloc(
else
new_size = 0;
if (new_size > 0) {
- new_broot = kmem_alloc(new_size, KM_NOFS);
+ new_broot = kmalloc(new_size, GFP_NOFS | __GFP_NOFAIL);
/*
* First copy over the btree block header.
*/
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 07894c53e753..6a89443da50a 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -1188,8 +1188,8 @@ __xfs_refcount_add(
type, XFS_FSB_TO_AGBNO(tp->t_mountp, startblock),
blockcount);
- ri = kmem_alloc(sizeof(struct xfs_refcount_intent),
- KM_NOFS);
+ ri = kmalloc(sizeof(struct xfs_refcount_intent),
+ GFP_NOFS | __GFP_NOFAIL);
INIT_LIST_HEAD(&ri->ri_list);
ri->ri_type = type;
ri->ri_startblock = startblock;
@@ -1584,7 +1584,7 @@ struct xfs_refcount_recovery {
/* Stuff an extent on the recovery list. */
STATIC int
xfs_refcount_recover_extent(
- struct xfs_btree_cur *cur,
+ struct xfs_btree_cur *cur,
union xfs_btree_rec *rec,
void *priv)
{
@@ -1596,7 +1596,8 @@ xfs_refcount_recover_extent(
return -EFSCORRUPTED;
}
- rr = kmem_alloc(sizeof(struct xfs_refcount_recovery), 0);
+ rr = kmalloc(sizeof(struct xfs_refcount_recovery),
+ GFP_KERNEL | __GFP_NOFAIL);
xfs_refcount_btrec_to_irec(rec, &rr->rr_rrec);
list_add_tail(&rr->rr_list, debris);
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index 38e9414878b3..0e1e8cbb8862 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -2286,7 +2286,7 @@ __xfs_rmap_add(
bmap->br_blockcount,
bmap->br_state);
- ri = kmem_alloc(sizeof(struct xfs_rmap_intent), KM_NOFS);
+ ri = kmalloc(sizeof(struct xfs_rmap_intent), GFP_NOFS | __GFP_NOFAIL);
INIT_LIST_HEAD(&ri->ri_list);
ri->ri_type = type;
ri->ri_owner = owner;
diff --git a/fs/xfs/scrub/bitmap.c b/fs/xfs/scrub/bitmap.c
index cabde1c4c235..5787d4f74e71 100644
--- a/fs/xfs/scrub/bitmap.c
+++ b/fs/xfs/scrub/bitmap.c
@@ -25,7 +25,8 @@ xfs_bitmap_set(
{
struct xfs_bitmap_range *bmr;
- bmr = kmem_alloc(sizeof(struct xfs_bitmap_range), KM_MAYFAIL);
+ bmr = kmalloc(sizeof(struct xfs_bitmap_range),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!bmr)
return -ENOMEM;
@@ -181,8 +182,8 @@ xfs_bitmap_disunion(
* Deleting from the middle: add the new right extent
* and then shrink the left extent.
*/
- new_br = kmem_alloc(sizeof(struct xfs_bitmap_range),
- KM_MAYFAIL);
+ new_br = kmalloc(sizeof(struct xfs_bitmap_range),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!new_br) {
error = -ENOMEM;
goto out;
diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index bed40b605076..857f813681ed 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -429,8 +429,8 @@ xchk_btree_check_owner(
* later scanning.
*/
if (cur->bc_btnum == XFS_BTNUM_BNO || cur->bc_btnum == XFS_BTNUM_RMAP) {
- co = kmem_alloc(sizeof(struct check_owner),
- KM_MAYFAIL);
+ co = kmalloc(sizeof(struct check_owner),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!co)
return -ENOMEM;
co->level = level;
diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
index 985724e81ebf..f5c2e320e416 100644
--- a/fs/xfs/scrub/refcount.c
+++ b/fs/xfs/scrub/refcount.c
@@ -125,8 +125,8 @@ xchk_refcountbt_rmap_check(
* is healthy each rmap_irec we see will be in agbno order
* so we don't need insertion sort here.
*/
- frag = kmem_alloc(sizeof(struct xchk_refcnt_frag),
- KM_MAYFAIL);
+ frag = kmalloc(sizeof(struct xchk_refcnt_frag),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!frag)
return -ENOMEM;
memcpy(&frag->rm, rec, sizeof(frag->rm));
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index 8351b3b611ac..42d7d8cbdb6e 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -148,7 +148,7 @@ xfs_attr3_leaf_inactive(
* Allocate storage for a list of all the "remote" value extents.
*/
size = count * sizeof(xfs_attr_inactive_list_t);
- list = kmem_alloc(size, 0);
+ list = kmalloc(size, GFP_KERNEL | __GFP_NOFAIL);
/*
* Identify each of the "remote" value extents.
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index e1d1c4eb9e69..2a475ca6e353 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -116,7 +116,7 @@ xfs_attr_shortform_list(
* It didn't all fit, so we have to sort everything on hashval.
*/
sbsize = sf->hdr.count * sizeof(*sbuf);
- sbp = sbuf = kmem_alloc(sbsize, KM_NOFS);
+ sbp = sbuf = kmalloc(sbsize, GFP_NOFS | __GFP_NOFAIL);
/*
* Scan the attribute list for the rest of the entries, storing
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index c70122fbc2a8..7428fe6a322c 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -274,8 +274,9 @@ _xfs_buf_get_pages(
if (page_count <= XB_PAGES) {
bp->b_pages = bp->b_page_array;
} else {
- bp->b_pages = kmem_alloc(sizeof(struct page *) *
- page_count, KM_NOFS);
+ bp->b_pages = kmalloc(sizeof(struct page *) *
+ page_count,
+ GFP_NOFS | __GFP_NOFAIL);
if (bp->b_pages == NULL)
return -ENOMEM;
}
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index 9778e4e69e07..38b634cef1ed 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -247,7 +247,7 @@ xfs_filestream_pick_ag(
return 0;
err = -ENOMEM;
- item = kmem_alloc(sizeof(*item), KM_MAYFAIL);
+ item = kmalloc(sizeof(*item), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (!item)
goto out_put_ag;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 297b2a73f285..1d1fe67ad237 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3493,7 +3493,7 @@ xfs_iflush_cluster(
pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
cilist_size = igeo->inodes_per_cluster * sizeof(struct xfs_inode *);
- cilist = kmem_alloc(cilist_size, KM_MAYFAIL|KM_NOFS);
+ cilist = kmalloc(cilist_size, GFP_NOFS | __GFP_RETRY_MAYFAIL);
if (!cilist)
goto out_put;
diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
index e6006423e140..aa6bc0555d21 100644
--- a/fs/xfs/xfs_iwalk.c
+++ b/fs/xfs/xfs_iwalk.c
@@ -152,7 +152,7 @@ xfs_iwalk_alloc(
/* Allocate a prefetch buffer for inobt records. */
size = iwag->sz_recs * sizeof(struct xfs_inobt_rec_incore);
- iwag->recs = kmem_alloc(size, KM_MAYFAIL);
+ iwag->recs = kmalloc(size, GFP_KERNEL | __GFP_RETRY_MAYFAIL);
if (iwag->recs == NULL)
return -ENOMEM;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 5423171e0b7d..7bb53fbf32f6 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1962,7 +1962,7 @@ xlog_recover_buffer_pass1(
}
}
- bcp = kmem_alloc(sizeof(struct xfs_buf_cancel), 0);
+ bcp = kmalloc(sizeof(struct xfs_buf_cancel), GFP_KERNEL | __GFP_NOFAIL);
bcp->bc_blkno = buf_f->blf_blkno;
bcp->bc_len = buf_f->blf_len;
bcp->bc_refcount = 1;
@@ -2932,7 +2932,8 @@ xlog_recover_inode_pass2(
if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
in_f = item->ri_buf[0].i_addr;
} else {
- in_f = kmem_alloc(sizeof(struct xfs_inode_log_format), 0);
+ in_f = kmalloc(sizeof(struct xfs_inode_log_format),
+ GFP_KERNEL | __GFP_NOFAIL);
need_free = 1;
error = xfs_inode_item_format_convert(&item->ri_buf[0], in_f);
if (error)
@@ -4271,7 +4272,7 @@ xlog_recover_add_to_trans(
return 0;
}
- ptr = kmem_alloc(len, 0);
+ ptr = kmalloc(len, GFP_KERNEL | __GFP_NOFAIL);
memcpy(ptr, dp, len);
in_f = (struct xfs_inode_log_format *)ptr;
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index a2664afa10c3..2993af4a9935 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -988,7 +988,8 @@ xfs_qm_reset_dqcounts_buf(
if (qip->i_d.di_nblocks == 0)
return 0;
- map = kmem_alloc(XFS_DQITER_MAP_SIZE * sizeof(*map), 0);
+ map = kmalloc(XFS_DQITER_MAP_SIZE * sizeof(*map),
+ GFP_KERNEL | __GFP_NOFAIL);
lblkno = 0;
maxlblkcnt = XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 7f03b4ab3452..dfd419d402ea 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -962,7 +962,7 @@ xfs_growfs_rt(
/*
* Allocate a new (fake) mount/sb.
*/
- nmp = kmem_alloc(sizeof(*nmp), 0);
+ nmp = kmalloc(sizeof(*nmp), GFP_KERNEL | __GFP_NOFAIL);
/*
* Loop over the bitmap blocks.
* We will do everything one bitmap block at a time.
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index cc1933dc652f..eee831681e9c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1739,7 +1739,7 @@ static int xfs_init_fs_context(
{
struct xfs_mount *mp;
- mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
+ mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL | __GFP_NOFAIL);
if (!mp)
return -ENOMEM;
--
2.23.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 5/5] xfs: Convert kmem_alloc() users
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
1 sibling, 0 replies; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-20 19:00 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Nov 20, 2019 at 11:44:25AM +0100, Carlos Maiolino wrote:
> Use kmalloc() directly.
>
> There is no logic change on kmem_alloc() since it's being removed soon, but for
> now, kmem_alloc_io() and kmem_alloc_large() still have use for kmem_alloc() due
> their fallback to vmalloc() and also the alignment check, so we can't completely
> remove it here.
> But, there is no need to export kmem_alloc() to the whole XFS driver anymore, so,
> convert kmem_alloc() into a static, local function __kmem_alloc().
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Looks fine, I'll give it a spin,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> V2:
> - Rephrase commit log
>
> fs/xfs/kmem.c | 8 ++++----
> fs/xfs/kmem.h | 1 -
> fs/xfs/libxfs/xfs_attr_leaf.c | 6 +++---
> fs/xfs/libxfs/xfs_bmap.c | 2 +-
> fs/xfs/libxfs/xfs_da_btree.c | 4 +++-
> fs/xfs/libxfs/xfs_defer.c | 4 ++--
> fs/xfs/libxfs/xfs_dir2.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_block.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_sf.c | 8 ++++----
> fs/xfs/libxfs/xfs_inode_fork.c | 10 ++++++----
> fs/xfs/libxfs/xfs_refcount.c | 9 +++++----
> fs/xfs/libxfs/xfs_rmap.c | 2 +-
> fs/xfs/scrub/bitmap.c | 7 ++++---
> fs/xfs/scrub/btree.c | 4 ++--
> fs/xfs/scrub/refcount.c | 4 ++--
> fs/xfs/xfs_attr_inactive.c | 2 +-
> fs/xfs/xfs_attr_list.c | 2 +-
> fs/xfs/xfs_buf.c | 5 +++--
> fs/xfs/xfs_filestream.c | 2 +-
> fs/xfs/xfs_inode.c | 2 +-
> fs/xfs/xfs_iwalk.c | 2 +-
> fs/xfs/xfs_log_recover.c | 7 ++++---
> fs/xfs/xfs_qm.c | 3 ++-
> fs/xfs/xfs_rtalloc.c | 2 +-
> fs/xfs/xfs_super.c | 2 +-
> 25 files changed, 55 insertions(+), 47 deletions(-)
>
> diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
> index 6e10e565632c..79467813d810 100644
> --- a/fs/xfs/kmem.c
> +++ b/fs/xfs/kmem.c
> @@ -8,8 +8,8 @@
> #include "xfs_message.h"
> #include "xfs_trace.h"
>
> -void *
> -kmem_alloc(size_t size, xfs_km_flags_t flags)
> +static void *
> +__kmem_alloc(size_t size, xfs_km_flags_t flags)
> {
> int retries = 0;
> gfp_t lflags = kmem_flags_convert(flags);
> @@ -72,7 +72,7 @@ kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags)
> if (WARN_ON_ONCE(align_mask >= PAGE_SIZE))
> align_mask = PAGE_SIZE - 1;
>
> - ptr = kmem_alloc(size, flags | KM_MAYFAIL);
> + ptr = __kmem_alloc(size, flags | KM_MAYFAIL);
> if (ptr) {
> if (!((uintptr_t)ptr & align_mask))
> return ptr;
> @@ -88,7 +88,7 @@ kmem_alloc_large(size_t size, xfs_km_flags_t flags)
>
> trace_kmem_alloc_large(size, flags, _RET_IP_);
>
> - ptr = kmem_alloc(size, flags | KM_MAYFAIL);
> + 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 a18c27c99721..78a54839430a 100644
> --- a/fs/xfs/kmem.h
> +++ b/fs/xfs/kmem.h
> @@ -52,7 +52,6 @@ kmem_flags_convert(xfs_km_flags_t flags)
> return lflags;
> }
>
> -extern void *kmem_alloc(size_t, xfs_km_flags_t);
> extern void *kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags);
> extern void *kmem_alloc_large(size_t size, xfs_km_flags_t);
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 67de68584224..807950eca17a 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -885,7 +885,7 @@ xfs_attr_shortform_to_leaf(
> ifp = dp->i_afp;
> sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> size = be16_to_cpu(sf->hdr.totsize);
> - tmpbuffer = kmem_alloc(size, 0);
> + tmpbuffer = kmalloc(size, GFP_KERNEL | __GFP_NOFAIL);
> ASSERT(tmpbuffer != NULL);
> memcpy(tmpbuffer, ifp->if_u1.if_data, size);
> sf = (xfs_attr_shortform_t *)tmpbuffer;
> @@ -1073,7 +1073,7 @@ xfs_attr3_leaf_to_shortform(
>
> trace_xfs_attr_leaf_to_sf(args);
>
> - tmpbuffer = kmem_alloc(args->geo->blksize, 0);
> + tmpbuffer = kmalloc(args->geo->blksize, GFP_KERNEL | __GFP_NOFAIL);
> if (!tmpbuffer)
> return -ENOMEM;
>
> @@ -1534,7 +1534,7 @@ xfs_attr3_leaf_compact(
>
> trace_xfs_attr_leaf_compact(args);
>
> - tmpbuffer = kmem_alloc(args->geo->blksize, 0);
> + tmpbuffer = kmalloc(args->geo->blksize, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(tmpbuffer, bp->b_addr, args->geo->blksize);
> memset(bp->b_addr, 0, args->geo->blksize);
> leaf_src = (xfs_attr_leafblock_t *)tmpbuffer;
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 37596e49b92e..fc5bed95bd44 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -6045,7 +6045,7 @@ __xfs_bmap_add(
> bmap->br_blockcount,
> bmap->br_state);
>
> - bi = kmem_alloc(sizeof(struct xfs_bmap_intent), KM_NOFS);
> + bi = kmalloc(sizeof(struct xfs_bmap_intent), GFP_NOFS | __GFP_NOFAIL);
> INIT_LIST_HEAD(&bi->bi_list);
> bi->bi_type = type;
> bi->bi_owner = ip;
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 29c25d1b3b76..efe84c636bd3 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2152,7 +2152,9 @@ xfs_da_grow_inode_int(
> * If we didn't get it and the block might work if fragmented,
> * try without the CONTIG flag. Loop until we get it all.
> */
> - mapp = kmem_alloc(sizeof(*mapp) * count, 0);
> + mapp = kmalloc(sizeof(*mapp) * count,
> + GFP_KERNEL | __GFP_NOFAIL);
> +
> for (b = *bno, mapi = 0; b < *bno + count; ) {
> nmap = min(XFS_BMAP_MAX_NMAP, count);
> c = (int)(*bno + count - b);
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 27c3d150068a..7dd16f208b82 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -516,8 +516,8 @@ xfs_defer_add(
> dfp = NULL;
> }
> if (!dfp) {
> - dfp = kmem_alloc(sizeof(struct xfs_defer_pending),
> - KM_NOFS);
> + dfp = kmalloc(sizeof(struct xfs_defer_pending),
> + GFP_NOFS | __GFP_NOFAIL);
> dfp->dfp_type = type;
> dfp->dfp_intent = NULL;
> dfp->dfp_done = NULL;
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index c2deda036271..4777356b4f83 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -331,7 +331,7 @@ xfs_dir_cilookup_result(
> !(args->op_flags & XFS_DA_OP_CILOOKUP))
> return -EEXIST;
>
> - args->value = kmem_alloc(len, KM_NOFS | KM_MAYFAIL);
> + args->value = kmalloc(len, GFP_NOFS | __GFP_RETRY_MAYFAIL);
> if (!args->value)
> return -ENOMEM;
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 766f282b706a..54ae07a432e4 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -1083,7 +1083,7 @@ xfs_dir2_sf_to_block(
> * Copy the directory into a temporary buffer.
> * Then pitch the incore inode data so we can make extents.
> */
> - sfp = kmem_alloc(ifp->if_bytes, 0);
> + sfp = kmalloc(ifp->if_bytes, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(sfp, oldsfp, ifp->if_bytes);
>
> xfs_idata_realloc(dp, -ifp->if_bytes, XFS_DATA_FORK);
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index f4de4e7b10ef..43d72aebb9cf 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -276,7 +276,7 @@ xfs_dir2_block_to_sf(
> * format the data into. Once we have formatted the data, we can free
> * the block and copy the formatted data into the inode literal area.
> */
> - sfp = kmem_alloc(mp->m_sb.sb_inodesize, 0);
> + sfp = kmalloc(mp->m_sb.sb_inodesize, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(sfp, sfhp, xfs_dir2_sf_hdr_size(sfhp->i8count));
>
> /*
> @@ -530,7 +530,7 @@ xfs_dir2_sf_addname_hard(
> */
> sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> old_isize = (int)dp->i_d.di_size;
> - buf = kmem_alloc(old_isize, 0);
> + buf = kmalloc(old_isize, GFP_KERNEL | __GFP_NOFAIL);
> oldsfp = (xfs_dir2_sf_hdr_t *)buf;
> memcpy(oldsfp, sfp, old_isize);
> /*
> @@ -1162,7 +1162,7 @@ xfs_dir2_sf_toino4(
> * Don't want xfs_idata_realloc copying the data here.
> */
> oldsize = dp->i_df.if_bytes;
> - buf = kmem_alloc(oldsize, 0);
> + buf = kmalloc(oldsize, GFP_KERNEL | __GFP_NOFAIL);
> oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> ASSERT(oldsfp->i8count == 1);
> memcpy(buf, oldsfp, oldsize);
> @@ -1235,7 +1235,7 @@ xfs_dir2_sf_toino8(
> * Don't want xfs_idata_realloc copying the data here.
> */
> oldsize = dp->i_df.if_bytes;
> - buf = kmem_alloc(oldsize, 0);
> + buf = kmalloc(oldsize, GFP_KERNEL | __GFP_NOFAIL);
> oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> ASSERT(oldsfp->i8count == 0);
> memcpy(buf, oldsfp, oldsize);
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 82799dddf97d..62c305654657 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -153,7 +153,8 @@ xfs_init_local_fork(
>
> if (size) {
> real_size = roundup(mem_size, 4);
> - ifp->if_u1.if_data = kmem_alloc(real_size, KM_NOFS);
> + ifp->if_u1.if_data = kmalloc(real_size,
> + GFP_NOFS | __GFP_NOFAIL);
> memcpy(ifp->if_u1.if_data, data, size);
> if (zero_terminate)
> ifp->if_u1.if_data[size] = '\0';
> @@ -308,7 +309,7 @@ xfs_iformat_btree(
> }
>
> ifp->if_broot_bytes = size;
> - ifp->if_broot = kmem_alloc(size, KM_NOFS);
> + ifp->if_broot = kmalloc(size, GFP_NOFS | __GFP_NOFAIL);
> ASSERT(ifp->if_broot != NULL);
> /*
> * Copy and convert from the on-disk structure
> @@ -373,7 +374,8 @@ xfs_iroot_realloc(
> */
> if (ifp->if_broot_bytes == 0) {
> new_size = XFS_BMAP_BROOT_SPACE_CALC(mp, rec_diff);
> - ifp->if_broot = kmem_alloc(new_size, KM_NOFS);
> + ifp->if_broot = kmalloc(new_size,
> + GFP_NOFS | __GFP_NOFAIL);
> ifp->if_broot_bytes = (int)new_size;
> return;
> }
> @@ -414,7 +416,7 @@ xfs_iroot_realloc(
> else
> new_size = 0;
> if (new_size > 0) {
> - new_broot = kmem_alloc(new_size, KM_NOFS);
> + new_broot = kmalloc(new_size, GFP_NOFS | __GFP_NOFAIL);
> /*
> * First copy over the btree block header.
> */
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 07894c53e753..6a89443da50a 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -1188,8 +1188,8 @@ __xfs_refcount_add(
> type, XFS_FSB_TO_AGBNO(tp->t_mountp, startblock),
> blockcount);
>
> - ri = kmem_alloc(sizeof(struct xfs_refcount_intent),
> - KM_NOFS);
> + ri = kmalloc(sizeof(struct xfs_refcount_intent),
> + GFP_NOFS | __GFP_NOFAIL);
> INIT_LIST_HEAD(&ri->ri_list);
> ri->ri_type = type;
> ri->ri_startblock = startblock;
> @@ -1584,7 +1584,7 @@ struct xfs_refcount_recovery {
> /* Stuff an extent on the recovery list. */
> STATIC int
> xfs_refcount_recover_extent(
> - struct xfs_btree_cur *cur,
> + struct xfs_btree_cur *cur,
> union xfs_btree_rec *rec,
> void *priv)
> {
> @@ -1596,7 +1596,8 @@ xfs_refcount_recover_extent(
> return -EFSCORRUPTED;
> }
>
> - rr = kmem_alloc(sizeof(struct xfs_refcount_recovery), 0);
> + rr = kmalloc(sizeof(struct xfs_refcount_recovery),
> + GFP_KERNEL | __GFP_NOFAIL);
> xfs_refcount_btrec_to_irec(rec, &rr->rr_rrec);
> list_add_tail(&rr->rr_list, debris);
>
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index 38e9414878b3..0e1e8cbb8862 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -2286,7 +2286,7 @@ __xfs_rmap_add(
> bmap->br_blockcount,
> bmap->br_state);
>
> - ri = kmem_alloc(sizeof(struct xfs_rmap_intent), KM_NOFS);
> + ri = kmalloc(sizeof(struct xfs_rmap_intent), GFP_NOFS | __GFP_NOFAIL);
> INIT_LIST_HEAD(&ri->ri_list);
> ri->ri_type = type;
> ri->ri_owner = owner;
> diff --git a/fs/xfs/scrub/bitmap.c b/fs/xfs/scrub/bitmap.c
> index cabde1c4c235..5787d4f74e71 100644
> --- a/fs/xfs/scrub/bitmap.c
> +++ b/fs/xfs/scrub/bitmap.c
> @@ -25,7 +25,8 @@ xfs_bitmap_set(
> {
> struct xfs_bitmap_range *bmr;
>
> - bmr = kmem_alloc(sizeof(struct xfs_bitmap_range), KM_MAYFAIL);
> + bmr = kmalloc(sizeof(struct xfs_bitmap_range),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!bmr)
> return -ENOMEM;
>
> @@ -181,8 +182,8 @@ xfs_bitmap_disunion(
> * Deleting from the middle: add the new right extent
> * and then shrink the left extent.
> */
> - new_br = kmem_alloc(sizeof(struct xfs_bitmap_range),
> - KM_MAYFAIL);
> + new_br = kmalloc(sizeof(struct xfs_bitmap_range),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!new_br) {
> error = -ENOMEM;
> goto out;
> diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
> index bed40b605076..857f813681ed 100644
> --- a/fs/xfs/scrub/btree.c
> +++ b/fs/xfs/scrub/btree.c
> @@ -429,8 +429,8 @@ xchk_btree_check_owner(
> * later scanning.
> */
> if (cur->bc_btnum == XFS_BTNUM_BNO || cur->bc_btnum == XFS_BTNUM_RMAP) {
> - co = kmem_alloc(sizeof(struct check_owner),
> - KM_MAYFAIL);
> + co = kmalloc(sizeof(struct check_owner),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!co)
> return -ENOMEM;
> co->level = level;
> diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
> index 985724e81ebf..f5c2e320e416 100644
> --- a/fs/xfs/scrub/refcount.c
> +++ b/fs/xfs/scrub/refcount.c
> @@ -125,8 +125,8 @@ xchk_refcountbt_rmap_check(
> * is healthy each rmap_irec we see will be in agbno order
> * so we don't need insertion sort here.
> */
> - frag = kmem_alloc(sizeof(struct xchk_refcnt_frag),
> - KM_MAYFAIL);
> + frag = kmalloc(sizeof(struct xchk_refcnt_frag),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!frag)
> return -ENOMEM;
> memcpy(&frag->rm, rec, sizeof(frag->rm));
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index 8351b3b611ac..42d7d8cbdb6e 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -148,7 +148,7 @@ xfs_attr3_leaf_inactive(
> * Allocate storage for a list of all the "remote" value extents.
> */
> size = count * sizeof(xfs_attr_inactive_list_t);
> - list = kmem_alloc(size, 0);
> + list = kmalloc(size, GFP_KERNEL | __GFP_NOFAIL);
>
> /*
> * Identify each of the "remote" value extents.
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index e1d1c4eb9e69..2a475ca6e353 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -116,7 +116,7 @@ xfs_attr_shortform_list(
> * It didn't all fit, so we have to sort everything on hashval.
> */
> sbsize = sf->hdr.count * sizeof(*sbuf);
> - sbp = sbuf = kmem_alloc(sbsize, KM_NOFS);
> + sbp = sbuf = kmalloc(sbsize, GFP_NOFS | __GFP_NOFAIL);
>
> /*
> * Scan the attribute list for the rest of the entries, storing
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index c70122fbc2a8..7428fe6a322c 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -274,8 +274,9 @@ _xfs_buf_get_pages(
> if (page_count <= XB_PAGES) {
> bp->b_pages = bp->b_page_array;
> } else {
> - bp->b_pages = kmem_alloc(sizeof(struct page *) *
> - page_count, KM_NOFS);
> + bp->b_pages = kmalloc(sizeof(struct page *) *
> + page_count,
> + GFP_NOFS | __GFP_NOFAIL);
> if (bp->b_pages == NULL)
> return -ENOMEM;
> }
> diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
> index 9778e4e69e07..38b634cef1ed 100644
> --- a/fs/xfs/xfs_filestream.c
> +++ b/fs/xfs/xfs_filestream.c
> @@ -247,7 +247,7 @@ xfs_filestream_pick_ag(
> return 0;
>
> err = -ENOMEM;
> - item = kmem_alloc(sizeof(*item), KM_MAYFAIL);
> + item = kmalloc(sizeof(*item), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!item)
> goto out_put_ag;
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 297b2a73f285..1d1fe67ad237 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3493,7 +3493,7 @@ xfs_iflush_cluster(
> pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
>
> cilist_size = igeo->inodes_per_cluster * sizeof(struct xfs_inode *);
> - cilist = kmem_alloc(cilist_size, KM_MAYFAIL|KM_NOFS);
> + cilist = kmalloc(cilist_size, GFP_NOFS | __GFP_RETRY_MAYFAIL);
> if (!cilist)
> goto out_put;
>
> diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
> index e6006423e140..aa6bc0555d21 100644
> --- a/fs/xfs/xfs_iwalk.c
> +++ b/fs/xfs/xfs_iwalk.c
> @@ -152,7 +152,7 @@ xfs_iwalk_alloc(
>
> /* Allocate a prefetch buffer for inobt records. */
> size = iwag->sz_recs * sizeof(struct xfs_inobt_rec_incore);
> - iwag->recs = kmem_alloc(size, KM_MAYFAIL);
> + iwag->recs = kmalloc(size, GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (iwag->recs == NULL)
> return -ENOMEM;
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 5423171e0b7d..7bb53fbf32f6 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1962,7 +1962,7 @@ xlog_recover_buffer_pass1(
> }
> }
>
> - bcp = kmem_alloc(sizeof(struct xfs_buf_cancel), 0);
> + bcp = kmalloc(sizeof(struct xfs_buf_cancel), GFP_KERNEL | __GFP_NOFAIL);
> bcp->bc_blkno = buf_f->blf_blkno;
> bcp->bc_len = buf_f->blf_len;
> bcp->bc_refcount = 1;
> @@ -2932,7 +2932,8 @@ xlog_recover_inode_pass2(
> if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
> in_f = item->ri_buf[0].i_addr;
> } else {
> - in_f = kmem_alloc(sizeof(struct xfs_inode_log_format), 0);
> + in_f = kmalloc(sizeof(struct xfs_inode_log_format),
> + GFP_KERNEL | __GFP_NOFAIL);
> need_free = 1;
> error = xfs_inode_item_format_convert(&item->ri_buf[0], in_f);
> if (error)
> @@ -4271,7 +4272,7 @@ xlog_recover_add_to_trans(
> return 0;
> }
>
> - ptr = kmem_alloc(len, 0);
> + ptr = kmalloc(len, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(ptr, dp, len);
> in_f = (struct xfs_inode_log_format *)ptr;
>
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index a2664afa10c3..2993af4a9935 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -988,7 +988,8 @@ xfs_qm_reset_dqcounts_buf(
> if (qip->i_d.di_nblocks == 0)
> return 0;
>
> - map = kmem_alloc(XFS_DQITER_MAP_SIZE * sizeof(*map), 0);
> + map = kmalloc(XFS_DQITER_MAP_SIZE * sizeof(*map),
> + GFP_KERNEL | __GFP_NOFAIL);
>
> lblkno = 0;
> maxlblkcnt = XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 7f03b4ab3452..dfd419d402ea 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -962,7 +962,7 @@ xfs_growfs_rt(
> /*
> * Allocate a new (fake) mount/sb.
> */
> - nmp = kmem_alloc(sizeof(*nmp), 0);
> + nmp = kmalloc(sizeof(*nmp), GFP_KERNEL | __GFP_NOFAIL);
> /*
> * Loop over the bitmap blocks.
> * We will do everything one bitmap block at a time.
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index cc1933dc652f..eee831681e9c 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1739,7 +1739,7 @@ static int xfs_init_fs_context(
> {
> struct xfs_mount *mp;
>
> - mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
> + mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL | __GFP_NOFAIL);
> if (!mp)
> return -ENOMEM;
>
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 5/5] xfs: Convert kmem_alloc() users
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
1 sibling, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-22 15:57 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Nov 20, 2019 at 11:44:25AM +0100, Carlos Maiolino wrote:
> Use kmalloc() directly.
>
> There is no logic change on kmem_alloc() since it's being removed soon, but for
> now, kmem_alloc_io() and kmem_alloc_large() still have use for kmem_alloc() due
> their fallback to vmalloc() and also the alignment check, so we can't completely
> remove it here.
> But, there is no need to export kmem_alloc() to the whole XFS driver anymore, so,
> convert kmem_alloc() into a static, local function __kmem_alloc().
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> V2:
> - Rephrase commit log
>
> fs/xfs/kmem.c | 8 ++++----
> fs/xfs/kmem.h | 1 -
> fs/xfs/libxfs/xfs_attr_leaf.c | 6 +++---
> fs/xfs/libxfs/xfs_bmap.c | 2 +-
> fs/xfs/libxfs/xfs_da_btree.c | 4 +++-
> fs/xfs/libxfs/xfs_defer.c | 4 ++--
> fs/xfs/libxfs/xfs_dir2.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_block.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_sf.c | 8 ++++----
> fs/xfs/libxfs/xfs_inode_fork.c | 10 ++++++----
> fs/xfs/libxfs/xfs_refcount.c | 9 +++++----
> fs/xfs/libxfs/xfs_rmap.c | 2 +-
> fs/xfs/scrub/bitmap.c | 7 ++++---
> fs/xfs/scrub/btree.c | 4 ++--
> fs/xfs/scrub/refcount.c | 4 ++--
> fs/xfs/xfs_attr_inactive.c | 2 +-
> fs/xfs/xfs_attr_list.c | 2 +-
> fs/xfs/xfs_buf.c | 5 +++--
> fs/xfs/xfs_filestream.c | 2 +-
> fs/xfs/xfs_inode.c | 2 +-
> fs/xfs/xfs_iwalk.c | 2 +-
> fs/xfs/xfs_log_recover.c | 7 ++++---
> fs/xfs/xfs_qm.c | 3 ++-
> fs/xfs/xfs_rtalloc.c | 2 +-
> fs/xfs/xfs_super.c | 2 +-
> 25 files changed, 55 insertions(+), 47 deletions(-)
>
> diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
> index 6e10e565632c..79467813d810 100644
> --- a/fs/xfs/kmem.c
> +++ b/fs/xfs/kmem.c
> @@ -8,8 +8,8 @@
> #include "xfs_message.h"
> #include "xfs_trace.h"
>
> -void *
> -kmem_alloc(size_t size, xfs_km_flags_t flags)
> +static void *
> +__kmem_alloc(size_t size, xfs_km_flags_t flags)
> {
> int retries = 0;
> gfp_t lflags = kmem_flags_convert(flags);
> @@ -72,7 +72,7 @@ kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags)
> if (WARN_ON_ONCE(align_mask >= PAGE_SIZE))
> align_mask = PAGE_SIZE - 1;
>
> - ptr = kmem_alloc(size, flags | KM_MAYFAIL);
> + ptr = __kmem_alloc(size, flags | KM_MAYFAIL);
> if (ptr) {
> if (!((uintptr_t)ptr & align_mask))
> return ptr;
> @@ -88,7 +88,7 @@ kmem_alloc_large(size_t size, xfs_km_flags_t flags)
>
> trace_kmem_alloc_large(size, flags, _RET_IP_);
>
> - ptr = kmem_alloc(size, flags | KM_MAYFAIL);
> + 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 a18c27c99721..78a54839430a 100644
> --- a/fs/xfs/kmem.h
> +++ b/fs/xfs/kmem.h
> @@ -52,7 +52,6 @@ kmem_flags_convert(xfs_km_flags_t flags)
> return lflags;
> }
>
> -extern void *kmem_alloc(size_t, xfs_km_flags_t);
> extern void *kmem_alloc_io(size_t size, int align_mask, xfs_km_flags_t flags);
> extern void *kmem_alloc_large(size_t size, xfs_km_flags_t);
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 67de68584224..807950eca17a 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -885,7 +885,7 @@ xfs_attr_shortform_to_leaf(
> ifp = dp->i_afp;
> sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> size = be16_to_cpu(sf->hdr.totsize);
> - tmpbuffer = kmem_alloc(size, 0);
> + tmpbuffer = kmalloc(size, GFP_KERNEL | __GFP_NOFAIL);
> ASSERT(tmpbuffer != NULL);
> memcpy(tmpbuffer, ifp->if_u1.if_data, size);
> sf = (xfs_attr_shortform_t *)tmpbuffer;
> @@ -1073,7 +1073,7 @@ xfs_attr3_leaf_to_shortform(
>
> trace_xfs_attr_leaf_to_sf(args);
>
> - tmpbuffer = kmem_alloc(args->geo->blksize, 0);
> + tmpbuffer = kmalloc(args->geo->blksize, GFP_KERNEL | __GFP_NOFAIL);
> if (!tmpbuffer)
> return -ENOMEM;
>
> @@ -1534,7 +1534,7 @@ xfs_attr3_leaf_compact(
>
> trace_xfs_attr_leaf_compact(args);
>
> - tmpbuffer = kmem_alloc(args->geo->blksize, 0);
> + tmpbuffer = kmalloc(args->geo->blksize, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(tmpbuffer, bp->b_addr, args->geo->blksize);
> memset(bp->b_addr, 0, args->geo->blksize);
> leaf_src = (xfs_attr_leafblock_t *)tmpbuffer;
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 37596e49b92e..fc5bed95bd44 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -6045,7 +6045,7 @@ __xfs_bmap_add(
> bmap->br_blockcount,
> bmap->br_state);
>
> - bi = kmem_alloc(sizeof(struct xfs_bmap_intent), KM_NOFS);
> + bi = kmalloc(sizeof(struct xfs_bmap_intent), GFP_NOFS | __GFP_NOFAIL);
> INIT_LIST_HEAD(&bi->bi_list);
> bi->bi_type = type;
> bi->bi_owner = ip;
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 29c25d1b3b76..efe84c636bd3 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2152,7 +2152,9 @@ xfs_da_grow_inode_int(
> * If we didn't get it and the block might work if fragmented,
> * try without the CONTIG flag. Loop until we get it all.
> */
> - mapp = kmem_alloc(sizeof(*mapp) * count, 0);
> + mapp = kmalloc(sizeof(*mapp) * count,
> + GFP_KERNEL | __GFP_NOFAIL);
> +
> for (b = *bno, mapi = 0; b < *bno + count; ) {
> nmap = min(XFS_BMAP_MAX_NMAP, count);
> c = (int)(*bno + count - b);
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 27c3d150068a..7dd16f208b82 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -516,8 +516,8 @@ xfs_defer_add(
> dfp = NULL;
> }
> if (!dfp) {
> - dfp = kmem_alloc(sizeof(struct xfs_defer_pending),
> - KM_NOFS);
> + dfp = kmalloc(sizeof(struct xfs_defer_pending),
> + GFP_NOFS | __GFP_NOFAIL);
> dfp->dfp_type = type;
> dfp->dfp_intent = NULL;
> dfp->dfp_done = NULL;
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index c2deda036271..4777356b4f83 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -331,7 +331,7 @@ xfs_dir_cilookup_result(
> !(args->op_flags & XFS_DA_OP_CILOOKUP))
> return -EEXIST;
>
> - args->value = kmem_alloc(len, KM_NOFS | KM_MAYFAIL);
> + args->value = kmalloc(len, GFP_NOFS | __GFP_RETRY_MAYFAIL);
> if (!args->value)
> return -ENOMEM;
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 766f282b706a..54ae07a432e4 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -1083,7 +1083,7 @@ xfs_dir2_sf_to_block(
> * Copy the directory into a temporary buffer.
> * Then pitch the incore inode data so we can make extents.
> */
> - sfp = kmem_alloc(ifp->if_bytes, 0);
> + sfp = kmalloc(ifp->if_bytes, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(sfp, oldsfp, ifp->if_bytes);
>
> xfs_idata_realloc(dp, -ifp->if_bytes, XFS_DATA_FORK);
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index f4de4e7b10ef..43d72aebb9cf 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -276,7 +276,7 @@ xfs_dir2_block_to_sf(
> * format the data into. Once we have formatted the data, we can free
> * the block and copy the formatted data into the inode literal area.
> */
> - sfp = kmem_alloc(mp->m_sb.sb_inodesize, 0);
> + sfp = kmalloc(mp->m_sb.sb_inodesize, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(sfp, sfhp, xfs_dir2_sf_hdr_size(sfhp->i8count));
>
> /*
> @@ -530,7 +530,7 @@ xfs_dir2_sf_addname_hard(
> */
> sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> old_isize = (int)dp->i_d.di_size;
> - buf = kmem_alloc(old_isize, 0);
> + buf = kmalloc(old_isize, GFP_KERNEL | __GFP_NOFAIL);
> oldsfp = (xfs_dir2_sf_hdr_t *)buf;
> memcpy(oldsfp, sfp, old_isize);
> /*
> @@ -1162,7 +1162,7 @@ xfs_dir2_sf_toino4(
> * Don't want xfs_idata_realloc copying the data here.
> */
> oldsize = dp->i_df.if_bytes;
> - buf = kmem_alloc(oldsize, 0);
> + buf = kmalloc(oldsize, GFP_KERNEL | __GFP_NOFAIL);
> oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> ASSERT(oldsfp->i8count == 1);
> memcpy(buf, oldsfp, oldsize);
> @@ -1235,7 +1235,7 @@ xfs_dir2_sf_toino8(
> * Don't want xfs_idata_realloc copying the data here.
> */
> oldsize = dp->i_df.if_bytes;
> - buf = kmem_alloc(oldsize, 0);
> + buf = kmalloc(oldsize, GFP_KERNEL | __GFP_NOFAIL);
> oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> ASSERT(oldsfp->i8count == 0);
> memcpy(buf, oldsfp, oldsize);
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 82799dddf97d..62c305654657 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -153,7 +153,8 @@ xfs_init_local_fork(
>
> if (size) {
> real_size = roundup(mem_size, 4);
> - ifp->if_u1.if_data = kmem_alloc(real_size, KM_NOFS);
> + ifp->if_u1.if_data = kmalloc(real_size,
> + GFP_NOFS | __GFP_NOFAIL);
> memcpy(ifp->if_u1.if_data, data, size);
> if (zero_terminate)
> ifp->if_u1.if_data[size] = '\0';
> @@ -308,7 +309,7 @@ xfs_iformat_btree(
> }
>
> ifp->if_broot_bytes = size;
> - ifp->if_broot = kmem_alloc(size, KM_NOFS);
> + ifp->if_broot = kmalloc(size, GFP_NOFS | __GFP_NOFAIL);
> ASSERT(ifp->if_broot != NULL);
> /*
> * Copy and convert from the on-disk structure
> @@ -373,7 +374,8 @@ xfs_iroot_realloc(
> */
> if (ifp->if_broot_bytes == 0) {
> new_size = XFS_BMAP_BROOT_SPACE_CALC(mp, rec_diff);
> - ifp->if_broot = kmem_alloc(new_size, KM_NOFS);
> + ifp->if_broot = kmalloc(new_size,
> + GFP_NOFS | __GFP_NOFAIL);
> ifp->if_broot_bytes = (int)new_size;
> return;
> }
> @@ -414,7 +416,7 @@ xfs_iroot_realloc(
> else
> new_size = 0;
> if (new_size > 0) {
> - new_broot = kmem_alloc(new_size, KM_NOFS);
> + new_broot = kmalloc(new_size, GFP_NOFS | __GFP_NOFAIL);
> /*
> * First copy over the btree block header.
> */
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 07894c53e753..6a89443da50a 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -1188,8 +1188,8 @@ __xfs_refcount_add(
> type, XFS_FSB_TO_AGBNO(tp->t_mountp, startblock),
> blockcount);
>
> - ri = kmem_alloc(sizeof(struct xfs_refcount_intent),
> - KM_NOFS);
> + ri = kmalloc(sizeof(struct xfs_refcount_intent),
> + GFP_NOFS | __GFP_NOFAIL);
> INIT_LIST_HEAD(&ri->ri_list);
> ri->ri_type = type;
> ri->ri_startblock = startblock;
> @@ -1584,7 +1584,7 @@ struct xfs_refcount_recovery {
> /* Stuff an extent on the recovery list. */
> STATIC int
> xfs_refcount_recover_extent(
> - struct xfs_btree_cur *cur,
> + struct xfs_btree_cur *cur,
> union xfs_btree_rec *rec,
> void *priv)
> {
> @@ -1596,7 +1596,8 @@ xfs_refcount_recover_extent(
> return -EFSCORRUPTED;
> }
>
> - rr = kmem_alloc(sizeof(struct xfs_refcount_recovery), 0);
> + rr = kmalloc(sizeof(struct xfs_refcount_recovery),
> + GFP_KERNEL | __GFP_NOFAIL);
> xfs_refcount_btrec_to_irec(rec, &rr->rr_rrec);
> list_add_tail(&rr->rr_list, debris);
>
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index 38e9414878b3..0e1e8cbb8862 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -2286,7 +2286,7 @@ __xfs_rmap_add(
> bmap->br_blockcount,
> bmap->br_state);
>
> - ri = kmem_alloc(sizeof(struct xfs_rmap_intent), KM_NOFS);
> + ri = kmalloc(sizeof(struct xfs_rmap_intent), GFP_NOFS | __GFP_NOFAIL);
> INIT_LIST_HEAD(&ri->ri_list);
> ri->ri_type = type;
> ri->ri_owner = owner;
> diff --git a/fs/xfs/scrub/bitmap.c b/fs/xfs/scrub/bitmap.c
> index cabde1c4c235..5787d4f74e71 100644
> --- a/fs/xfs/scrub/bitmap.c
> +++ b/fs/xfs/scrub/bitmap.c
> @@ -25,7 +25,8 @@ xfs_bitmap_set(
> {
> struct xfs_bitmap_range *bmr;
>
> - bmr = kmem_alloc(sizeof(struct xfs_bitmap_range), KM_MAYFAIL);
> + bmr = kmalloc(sizeof(struct xfs_bitmap_range),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!bmr)
> return -ENOMEM;
>
> @@ -181,8 +182,8 @@ xfs_bitmap_disunion(
> * Deleting from the middle: add the new right extent
> * and then shrink the left extent.
> */
> - new_br = kmem_alloc(sizeof(struct xfs_bitmap_range),
> - KM_MAYFAIL);
> + new_br = kmalloc(sizeof(struct xfs_bitmap_range),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!new_br) {
> error = -ENOMEM;
> goto out;
> diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
> index bed40b605076..857f813681ed 100644
> --- a/fs/xfs/scrub/btree.c
> +++ b/fs/xfs/scrub/btree.c
> @@ -429,8 +429,8 @@ xchk_btree_check_owner(
> * later scanning.
> */
> if (cur->bc_btnum == XFS_BTNUM_BNO || cur->bc_btnum == XFS_BTNUM_RMAP) {
> - co = kmem_alloc(sizeof(struct check_owner),
> - KM_MAYFAIL);
> + co = kmalloc(sizeof(struct check_owner),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!co)
> return -ENOMEM;
> co->level = level;
> diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
> index 985724e81ebf..f5c2e320e416 100644
> --- a/fs/xfs/scrub/refcount.c
> +++ b/fs/xfs/scrub/refcount.c
> @@ -125,8 +125,8 @@ xchk_refcountbt_rmap_check(
> * is healthy each rmap_irec we see will be in agbno order
> * so we don't need insertion sort here.
> */
> - frag = kmem_alloc(sizeof(struct xchk_refcnt_frag),
> - KM_MAYFAIL);
> + frag = kmalloc(sizeof(struct xchk_refcnt_frag),
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!frag)
> return -ENOMEM;
> memcpy(&frag->rm, rec, sizeof(frag->rm));
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index 8351b3b611ac..42d7d8cbdb6e 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -148,7 +148,7 @@ xfs_attr3_leaf_inactive(
> * Allocate storage for a list of all the "remote" value extents.
> */
> size = count * sizeof(xfs_attr_inactive_list_t);
> - list = kmem_alloc(size, 0);
> + list = kmalloc(size, GFP_KERNEL | __GFP_NOFAIL);
>
> /*
> * Identify each of the "remote" value extents.
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index e1d1c4eb9e69..2a475ca6e353 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -116,7 +116,7 @@ xfs_attr_shortform_list(
> * It didn't all fit, so we have to sort everything on hashval.
> */
> sbsize = sf->hdr.count * sizeof(*sbuf);
> - sbp = sbuf = kmem_alloc(sbsize, KM_NOFS);
> + sbp = sbuf = kmalloc(sbsize, GFP_NOFS | __GFP_NOFAIL);
>
> /*
> * Scan the attribute list for the rest of the entries, storing
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index c70122fbc2a8..7428fe6a322c 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -274,8 +274,9 @@ _xfs_buf_get_pages(
> if (page_count <= XB_PAGES) {
> bp->b_pages = bp->b_page_array;
> } else {
> - bp->b_pages = kmem_alloc(sizeof(struct page *) *
> - page_count, KM_NOFS);
> + bp->b_pages = kmalloc(sizeof(struct page *) *
> + page_count,
> + GFP_NOFS | __GFP_NOFAIL);
> if (bp->b_pages == NULL)
> return -ENOMEM;
> }
> diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
> index 9778e4e69e07..38b634cef1ed 100644
> --- a/fs/xfs/xfs_filestream.c
> +++ b/fs/xfs/xfs_filestream.c
> @@ -247,7 +247,7 @@ xfs_filestream_pick_ag(
> return 0;
>
> err = -ENOMEM;
> - item = kmem_alloc(sizeof(*item), KM_MAYFAIL);
> + item = kmalloc(sizeof(*item), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!item)
> goto out_put_ag;
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 297b2a73f285..1d1fe67ad237 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3493,7 +3493,7 @@ xfs_iflush_cluster(
> pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
>
> cilist_size = igeo->inodes_per_cluster * sizeof(struct xfs_inode *);
> - cilist = kmem_alloc(cilist_size, KM_MAYFAIL|KM_NOFS);
> + cilist = kmalloc(cilist_size, GFP_NOFS | __GFP_RETRY_MAYFAIL);
> if (!cilist)
> goto out_put;
>
> diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
> index e6006423e140..aa6bc0555d21 100644
> --- a/fs/xfs/xfs_iwalk.c
> +++ b/fs/xfs/xfs_iwalk.c
> @@ -152,7 +152,7 @@ xfs_iwalk_alloc(
>
> /* Allocate a prefetch buffer for inobt records. */
> size = iwag->sz_recs * sizeof(struct xfs_inobt_rec_incore);
> - iwag->recs = kmem_alloc(size, KM_MAYFAIL);
> + iwag->recs = kmalloc(size, GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (iwag->recs == NULL)
> return -ENOMEM;
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 5423171e0b7d..7bb53fbf32f6 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1962,7 +1962,7 @@ xlog_recover_buffer_pass1(
> }
> }
>
> - bcp = kmem_alloc(sizeof(struct xfs_buf_cancel), 0);
> + bcp = kmalloc(sizeof(struct xfs_buf_cancel), GFP_KERNEL | __GFP_NOFAIL);
> bcp->bc_blkno = buf_f->blf_blkno;
> bcp->bc_len = buf_f->blf_len;
> bcp->bc_refcount = 1;
> @@ -2932,7 +2932,8 @@ xlog_recover_inode_pass2(
> if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
> in_f = item->ri_buf[0].i_addr;
> } else {
> - in_f = kmem_alloc(sizeof(struct xfs_inode_log_format), 0);
> + in_f = kmalloc(sizeof(struct xfs_inode_log_format),
> + GFP_KERNEL | __GFP_NOFAIL);
> need_free = 1;
> error = xfs_inode_item_format_convert(&item->ri_buf[0], in_f);
> if (error)
> @@ -4271,7 +4272,7 @@ xlog_recover_add_to_trans(
> return 0;
> }
>
> - ptr = kmem_alloc(len, 0);
> + ptr = kmalloc(len, GFP_KERNEL | __GFP_NOFAIL);
> memcpy(ptr, dp, len);
> in_f = (struct xfs_inode_log_format *)ptr;
I noticed that kmalloc is generating warnings with generic/049 when 16k
directories (-n size=16k) are enabled. I /think/ this is because it's
quite possible to write out an xlog_op_header with a length of more than
a single page; log recovery will then try to allocate a huge memory
buffer to recover the transaction; and so we try to do a huge NOFAIL
allocation, which makes the mm unhappy.
The one thing I've noticed with this conversion series is that the flags
translation isn't 100% 1-to-1. Before, kmem_flags_convert didn't
explicitly set __GFP_NOFAIL anywhere; we simply took the default
behavior. IIRC that means that small allocations actually /are/
guaranteed to succeed, but multipage allocations certainly aren't.
This seems to be one place where we could have asked for a lot of
memory, failed to get it, and crashed.
Now that we explicitly set NOFAIL in all the places where we don't also
check for a null return, I think we're just uncovering latent bugs
lurking in the code base. The kernel does actually fulfill the
allocation request, but it's clearly not happy.
--D
Relevant snippet of dmesg; everything else was normal:
XFS (sdd): Mounting V5 Filesystem
XFS (sdd): Starting recovery (logdev: internal)
------------[ cut here ]------------
WARNING: CPU: 1 PID: 459342 at mm/page_alloc.c:3275 get_page_from_freelist+0x434/0x1660
Modules linked in: dm_thin_pool dm_persistent_data dm_bio_prison dm_snapshot dm_bufio dm_flakey xfs libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac bfq ip_set nfnetlink ip6table_filter ip6_tables iptable_filter sch_fq_codel ip_tables x_tables nfsv4 af_packet [last unloaded: scsi_debug]
CPU: 1 PID: 459342 Comm: mount Not tainted 5.4.0-rc3-djw #rc3
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-1ubuntu1 04/01/2014
RIP: 0010:get_page_from_freelist+0x434/0x1660
Code: e6 00 00 00 00 48 89 84 24 a0 00 00 00 0f 84 08 fd ff ff f7 84 24 c0 00 00 00 00 80 00 00 74 0c 83 bc 24 84 00 00 00 01 76 02 <0f> 0b 49 8d 87 10 05 00 00 48 89 c7 48 89 84 24 88 00 00 00 e8 03
RSP: 0018:ffffc900035d3918 EFLAGS: 00010202
RAX: ffff88803fffb680 RBX: 0000000000002968 RCX: ffffea0000c8e108
RDX: ffff88803fffbba8 RSI: ffff88803fffb870 RDI: 0000000000000000
RBP: 0000000000000002 R08: 0000000000000201 R09: 000000000002ff81
R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000048cc0 R14: 0000000000000001 R15: ffff88803fffb680
FS: 00007fcfdf89a080(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055b202ec48c8 CR3: 000000003bfdc005 CR4: 00000000001606a0
Call Trace:
? kvm_clock_read+0x14/0x30
__alloc_pages_nodemask+0x172/0x3a0
kmalloc_order+0x18/0x80
kmalloc_order_trace+0x1d/0x130
xlog_recover_add_to_trans+0x4b/0x340 [xfs]
xlog_recovery_process_trans+0xe9/0xf0 [xfs]
xlog_recover_process_data+0x9e/0x1f0 [xfs]
xlog_do_recovery_pass+0x3a9/0x7c0 [xfs]
xlog_do_log_recovery+0x72/0x150 [xfs]
xlog_do_recover+0x43/0x2a0 [xfs]
xlog_recover+0xdf/0x170 [xfs]
xfs_log_mount+0x2e3/0x300 [xfs]
xfs_mountfs+0x4e7/0x9f0 [xfs]
xfs_fc_fill_super+0x2f8/0x520 [xfs]
? xfs_fs_destroy_inode+0x4f0/0x4f0 [xfs]
get_tree_bdev+0x198/0x270
vfs_get_tree+0x23/0xb0
do_mount+0x87e/0xa20
ksys_mount+0xb6/0xd0
__x64_sys_mount+0x21/0x30
do_syscall_64+0x50/0x180
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7fcfdf15d3ca
Code: 48 8b 0d c1 8a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8e 8a 2c 00 f7 d8 64 89 01 48
RSP: 002b:00007fff0af10a58 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 000055b202ec1970 RCX: 00007fcfdf15d3ca
RDX: 000055b202ec1be0 RSI: 000055b202ec1c20 RDI: 000055b202ec1c00
RBP: 0000000000000000 R08: 000055b202ec1b80 R09: 0000000000000000
R10: 00000000c0ed0000 R11: 0000000000000202 R12: 000055b202ec1c00
R13: 000055b202ec1be0 R14: 0000000000000000 R15: 00007fcfdf67e8a4
irq event stamp: 18398
hardirqs last enabled at (18397): [<ffffffff8123738f>] __slab_alloc.isra.83+0x6f/0x80
hardirqs last disabled at (18398): [<ffffffff81001d8a>] trace_hardirqs_off_thunk+0x1a/0x20
softirqs last enabled at (18158): [<ffffffff81a003af>] __do_softirq+0x3af/0x4a4
softirqs last disabled at (18149): [<ffffffff8106528c>] irq_exit+0xbc/0xe0
---[ end trace 3669c914fa8ccac6 ]---
AFAICT this is because inode buffers are 32K on this system
>
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index a2664afa10c3..2993af4a9935 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -988,7 +988,8 @@ xfs_qm_reset_dqcounts_buf(
> if (qip->i_d.di_nblocks == 0)
> return 0;
>
> - map = kmem_alloc(XFS_DQITER_MAP_SIZE * sizeof(*map), 0);
> + map = kmalloc(XFS_DQITER_MAP_SIZE * sizeof(*map),
> + GFP_KERNEL | __GFP_NOFAIL);
>
> lblkno = 0;
> maxlblkcnt = XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 7f03b4ab3452..dfd419d402ea 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -962,7 +962,7 @@ xfs_growfs_rt(
> /*
> * Allocate a new (fake) mount/sb.
> */
> - nmp = kmem_alloc(sizeof(*nmp), 0);
> + nmp = kmalloc(sizeof(*nmp), GFP_KERNEL | __GFP_NOFAIL);
> /*
> * Loop over the bitmap blocks.
> * We will do everything one bitmap block at a time.
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index cc1933dc652f..eee831681e9c 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1739,7 +1739,7 @@ static int xfs_init_fs_context(
> {
> struct xfs_mount *mp;
>
> - mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
> + mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL | __GFP_NOFAIL);
> if (!mp)
> return -ENOMEM;
>
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 5/5] xfs: Convert kmem_alloc() users
2019-11-22 15:57 ` Darrick J. Wong
@ 2019-11-22 22:30 ` Darrick J. Wong
2019-11-24 22:02 ` Darrick J. Wong
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-22 22:30 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Fri, Nov 22, 2019 at 07:57:56AM -0800, Darrick J. Wong wrote:
> On Wed, Nov 20, 2019 at 11:44:25AM +0100, Carlos Maiolino wrote:
> > Use kmalloc() directly.
<snip all this>
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 5423171e0b7d..7bb53fbf32f6 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -1962,7 +1962,7 @@ xlog_recover_buffer_pass1(
> > }
> > }
> >
> > - bcp = kmem_alloc(sizeof(struct xfs_buf_cancel), 0);
> > + bcp = kmalloc(sizeof(struct xfs_buf_cancel), GFP_KERNEL | __GFP_NOFAIL);
> > bcp->bc_blkno = buf_f->blf_blkno;
> > bcp->bc_len = buf_f->blf_len;
> > bcp->bc_refcount = 1;
> > @@ -2932,7 +2932,8 @@ xlog_recover_inode_pass2(
> > if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
> > in_f = item->ri_buf[0].i_addr;
> > } else {
> > - in_f = kmem_alloc(sizeof(struct xfs_inode_log_format), 0);
> > + in_f = kmalloc(sizeof(struct xfs_inode_log_format),
> > + GFP_KERNEL | __GFP_NOFAIL);
> > need_free = 1;
> > error = xfs_inode_item_format_convert(&item->ri_buf[0], in_f);
> > if (error)
> > @@ -4271,7 +4272,7 @@ xlog_recover_add_to_trans(
> > return 0;
> > }
> >
> > - ptr = kmem_alloc(len, 0);
> > + ptr = kmalloc(len, GFP_KERNEL | __GFP_NOFAIL);
> > memcpy(ptr, dp, len);
> > in_f = (struct xfs_inode_log_format *)ptr;
>
> I noticed that kmalloc is generating warnings with generic/049 when 16k
> directories (-n size=16k) are enabled. I /think/ this is because it's
> quite possible to write out an xlog_op_header with a length of more than
> a single page; log recovery will then try to allocate a huge memory
> buffer to recover the transaction; and so we try to do a huge NOFAIL
> allocation, which makes the mm unhappy.
>
> The one thing I've noticed with this conversion series is that the flags
> translation isn't 100% 1-to-1. Before, kmem_flags_convert didn't
> explicitly set __GFP_NOFAIL anywhere; we simply took the default
> behavior. IIRC that means that small allocations actually /are/
> guaranteed to succeed, but multipage allocations certainly aren't.
> This seems to be one place where we could have asked for a lot of
> memory, failed to get it, and crashed.
>
> Now that we explicitly set NOFAIL in all the places where we don't also
> check for a null return, I think we're just uncovering latent bugs
> lurking in the code base. The kernel does actually fulfill the
> allocation request, but it's clearly not happy.
FWIW I ran with various dirsizes and options and it looks like this is
the only place where we screw this up... patches soon.
--D
> --D
>
> Relevant snippet of dmesg; everything else was normal:
>
> XFS (sdd): Mounting V5 Filesystem
> XFS (sdd): Starting recovery (logdev: internal)
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 459342 at mm/page_alloc.c:3275 get_page_from_freelist+0x434/0x1660
> Modules linked in: dm_thin_pool dm_persistent_data dm_bio_prison dm_snapshot dm_bufio dm_flakey xfs libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac bfq ip_set nfnetlink ip6table_filter ip6_tables iptable_filter sch_fq_codel ip_tables x_tables nfsv4 af_packet [last unloaded: scsi_debug]
> CPU: 1 PID: 459342 Comm: mount Not tainted 5.4.0-rc3-djw #rc3
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-1ubuntu1 04/01/2014
> RIP: 0010:get_page_from_freelist+0x434/0x1660
> Code: e6 00 00 00 00 48 89 84 24 a0 00 00 00 0f 84 08 fd ff ff f7 84 24 c0 00 00 00 00 80 00 00 74 0c 83 bc 24 84 00 00 00 01 76 02 <0f> 0b 49 8d 87 10 05 00 00 48 89 c7 48 89 84 24 88 00 00 00 e8 03
> RSP: 0018:ffffc900035d3918 EFLAGS: 00010202
> RAX: ffff88803fffb680 RBX: 0000000000002968 RCX: ffffea0000c8e108
> RDX: ffff88803fffbba8 RSI: ffff88803fffb870 RDI: 0000000000000000
> RBP: 0000000000000002 R08: 0000000000000201 R09: 000000000002ff81
> R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
> R13: 0000000000048cc0 R14: 0000000000000001 R15: ffff88803fffb680
> FS: 00007fcfdf89a080(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000055b202ec48c8 CR3: 000000003bfdc005 CR4: 00000000001606a0
> Call Trace:
> ? kvm_clock_read+0x14/0x30
> __alloc_pages_nodemask+0x172/0x3a0
> kmalloc_order+0x18/0x80
> kmalloc_order_trace+0x1d/0x130
> xlog_recover_add_to_trans+0x4b/0x340 [xfs]
> xlog_recovery_process_trans+0xe9/0xf0 [xfs]
> xlog_recover_process_data+0x9e/0x1f0 [xfs]
> xlog_do_recovery_pass+0x3a9/0x7c0 [xfs]
> xlog_do_log_recovery+0x72/0x150 [xfs]
> xlog_do_recover+0x43/0x2a0 [xfs]
> xlog_recover+0xdf/0x170 [xfs]
> xfs_log_mount+0x2e3/0x300 [xfs]
> xfs_mountfs+0x4e7/0x9f0 [xfs]
> xfs_fc_fill_super+0x2f8/0x520 [xfs]
> ? xfs_fs_destroy_inode+0x4f0/0x4f0 [xfs]
> get_tree_bdev+0x198/0x270
> vfs_get_tree+0x23/0xb0
> do_mount+0x87e/0xa20
> ksys_mount+0xb6/0xd0
> __x64_sys_mount+0x21/0x30
> do_syscall_64+0x50/0x180
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x7fcfdf15d3ca
> Code: 48 8b 0d c1 8a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8e 8a 2c 00 f7 d8 64 89 01 48
> RSP: 002b:00007fff0af10a58 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
> RAX: ffffffffffffffda RBX: 000055b202ec1970 RCX: 00007fcfdf15d3ca
> RDX: 000055b202ec1be0 RSI: 000055b202ec1c20 RDI: 000055b202ec1c00
> RBP: 0000000000000000 R08: 000055b202ec1b80 R09: 0000000000000000
> R10: 00000000c0ed0000 R11: 0000000000000202 R12: 000055b202ec1c00
> R13: 000055b202ec1be0 R14: 0000000000000000 R15: 00007fcfdf67e8a4
> irq event stamp: 18398
> hardirqs last enabled at (18397): [<ffffffff8123738f>] __slab_alloc.isra.83+0x6f/0x80
> hardirqs last disabled at (18398): [<ffffffff81001d8a>] trace_hardirqs_off_thunk+0x1a/0x20
> softirqs last enabled at (18158): [<ffffffff81a003af>] __do_softirq+0x3af/0x4a4
> softirqs last disabled at (18149): [<ffffffff8106528c>] irq_exit+0xbc/0xe0
> ---[ end trace 3669c914fa8ccac6 ]---
>
> AFAICT this is because inode buffers are 32K on this system
>
> >
> > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > index a2664afa10c3..2993af4a9935 100644
> > --- a/fs/xfs/xfs_qm.c
> > +++ b/fs/xfs/xfs_qm.c
> > @@ -988,7 +988,8 @@ xfs_qm_reset_dqcounts_buf(
> > if (qip->i_d.di_nblocks == 0)
> > return 0;
> >
> > - map = kmem_alloc(XFS_DQITER_MAP_SIZE * sizeof(*map), 0);
> > + map = kmalloc(XFS_DQITER_MAP_SIZE * sizeof(*map),
> > + GFP_KERNEL | __GFP_NOFAIL);
> >
> > lblkno = 0;
> > maxlblkcnt = XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
> > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > index 7f03b4ab3452..dfd419d402ea 100644
> > --- a/fs/xfs/xfs_rtalloc.c
> > +++ b/fs/xfs/xfs_rtalloc.c
> > @@ -962,7 +962,7 @@ xfs_growfs_rt(
> > /*
> > * Allocate a new (fake) mount/sb.
> > */
> > - nmp = kmem_alloc(sizeof(*nmp), 0);
> > + nmp = kmalloc(sizeof(*nmp), GFP_KERNEL | __GFP_NOFAIL);
> > /*
> > * Loop over the bitmap blocks.
> > * We will do everything one bitmap block at a time.
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index cc1933dc652f..eee831681e9c 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -1739,7 +1739,7 @@ static int xfs_init_fs_context(
> > {
> > struct xfs_mount *mp;
> >
> > - mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
> > + mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL | __GFP_NOFAIL);
> > if (!mp)
> > return -ENOMEM;
> >
> > --
> > 2.23.0
> >
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 5/5] xfs: Convert kmem_alloc() users
2019-11-22 22:30 ` Darrick J. Wong
@ 2019-11-24 22:02 ` Darrick J. Wong
2019-11-25 9:28 ` Carlos Maiolino
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2019-11-24 22:02 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Fri, Nov 22, 2019 at 02:30:48PM -0800, Darrick J. Wong wrote:
> On Fri, Nov 22, 2019 at 07:57:56AM -0800, Darrick J. Wong wrote:
> > On Wed, Nov 20, 2019 at 11:44:25AM +0100, Carlos Maiolino wrote:
> > > Use kmalloc() directly.
>
> <snip all this>
>
> > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > > index 5423171e0b7d..7bb53fbf32f6 100644
> > > --- a/fs/xfs/xfs_log_recover.c
> > > +++ b/fs/xfs/xfs_log_recover.c
> > > @@ -1962,7 +1962,7 @@ xlog_recover_buffer_pass1(
> > > }
> > > }
> > >
> > > - bcp = kmem_alloc(sizeof(struct xfs_buf_cancel), 0);
> > > + bcp = kmalloc(sizeof(struct xfs_buf_cancel), GFP_KERNEL | __GFP_NOFAIL);
> > > bcp->bc_blkno = buf_f->blf_blkno;
> > > bcp->bc_len = buf_f->blf_len;
> > > bcp->bc_refcount = 1;
> > > @@ -2932,7 +2932,8 @@ xlog_recover_inode_pass2(
> > > if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
> > > in_f = item->ri_buf[0].i_addr;
> > > } else {
> > > - in_f = kmem_alloc(sizeof(struct xfs_inode_log_format), 0);
> > > + in_f = kmalloc(sizeof(struct xfs_inode_log_format),
> > > + GFP_KERNEL | __GFP_NOFAIL);
> > > need_free = 1;
> > > error = xfs_inode_item_format_convert(&item->ri_buf[0], in_f);
> > > if (error)
> > > @@ -4271,7 +4272,7 @@ xlog_recover_add_to_trans(
> > > return 0;
> > > }
> > >
> > > - ptr = kmem_alloc(len, 0);
> > > + ptr = kmalloc(len, GFP_KERNEL | __GFP_NOFAIL);
> > > memcpy(ptr, dp, len);
> > > in_f = (struct xfs_inode_log_format *)ptr;
> >
> > I noticed that kmalloc is generating warnings with generic/049 when 16k
> > directories (-n size=16k) are enabled. I /think/ this is because it's
> > quite possible to write out an xlog_op_header with a length of more than
> > a single page; log recovery will then try to allocate a huge memory
> > buffer to recover the transaction; and so we try to do a huge NOFAIL
> > allocation, which makes the mm unhappy.
> >
> > The one thing I've noticed with this conversion series is that the flags
> > translation isn't 100% 1-to-1. Before, kmem_flags_convert didn't
> > explicitly set __GFP_NOFAIL anywhere; we simply took the default
> > behavior. IIRC that means that small allocations actually /are/
> > guaranteed to succeed, but multipage allocations certainly aren't.
> > This seems to be one place where we could have asked for a lot of
> > memory, failed to get it, and crashed.
> >
> > Now that we explicitly set NOFAIL in all the places where we don't also
> > check for a null return, I think we're just uncovering latent bugs
> > lurking in the code base. The kernel does actually fulfill the
> > allocation request, but it's clearly not happy.
>
> FWIW I ran with various dirsizes and options and it looks like this is
> the only place where we screw this up... patches soon.
I rescind that statement -- there's enough places in this series where I
can't 100% tell that a k{mzre}alloc call asks for a small enough amount
of memory to qualify for __GFP_NOFAIL.
I really want this cleanup to start with a straightforward removal of
the kmem.c wrappers without any behavior changes. Only after that's
done should we move on to things like adding __GFP_NOFAIL to allocations
or deciding if/where we can substitute kfree for kvfree.
Munging them together means I can't easily tell if something is
seriously broken here (but the WARN_ONs suggest this) and I'd forgotten
that the merge window is opening the week of a major US holiday, so I
choose to defer this series to 5.6.
--D
> --D
>
> > --D
> >
> > Relevant snippet of dmesg; everything else was normal:
> >
> > XFS (sdd): Mounting V5 Filesystem
> > XFS (sdd): Starting recovery (logdev: internal)
> > ------------[ cut here ]------------
> > WARNING: CPU: 1 PID: 459342 at mm/page_alloc.c:3275 get_page_from_freelist+0x434/0x1660
> > Modules linked in: dm_thin_pool dm_persistent_data dm_bio_prison dm_snapshot dm_bufio dm_flakey xfs libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac bfq ip_set nfnetlink ip6table_filter ip6_tables iptable_filter sch_fq_codel ip_tables x_tables nfsv4 af_packet [last unloaded: scsi_debug]
> > CPU: 1 PID: 459342 Comm: mount Not tainted 5.4.0-rc3-djw #rc3
> > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-1ubuntu1 04/01/2014
> > RIP: 0010:get_page_from_freelist+0x434/0x1660
> > Code: e6 00 00 00 00 48 89 84 24 a0 00 00 00 0f 84 08 fd ff ff f7 84 24 c0 00 00 00 00 80 00 00 74 0c 83 bc 24 84 00 00 00 01 76 02 <0f> 0b 49 8d 87 10 05 00 00 48 89 c7 48 89 84 24 88 00 00 00 e8 03
> > RSP: 0018:ffffc900035d3918 EFLAGS: 00010202
> > RAX: ffff88803fffb680 RBX: 0000000000002968 RCX: ffffea0000c8e108
> > RDX: ffff88803fffbba8 RSI: ffff88803fffb870 RDI: 0000000000000000
> > RBP: 0000000000000002 R08: 0000000000000201 R09: 000000000002ff81
> > R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
> > R13: 0000000000048cc0 R14: 0000000000000001 R15: ffff88803fffb680
> > FS: 00007fcfdf89a080(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 000055b202ec48c8 CR3: 000000003bfdc005 CR4: 00000000001606a0
> > Call Trace:
> > ? kvm_clock_read+0x14/0x30
> > __alloc_pages_nodemask+0x172/0x3a0
> > kmalloc_order+0x18/0x80
> > kmalloc_order_trace+0x1d/0x130
> > xlog_recover_add_to_trans+0x4b/0x340 [xfs]
> > xlog_recovery_process_trans+0xe9/0xf0 [xfs]
> > xlog_recover_process_data+0x9e/0x1f0 [xfs]
> > xlog_do_recovery_pass+0x3a9/0x7c0 [xfs]
> > xlog_do_log_recovery+0x72/0x150 [xfs]
> > xlog_do_recover+0x43/0x2a0 [xfs]
> > xlog_recover+0xdf/0x170 [xfs]
> > xfs_log_mount+0x2e3/0x300 [xfs]
> > xfs_mountfs+0x4e7/0x9f0 [xfs]
> > xfs_fc_fill_super+0x2f8/0x520 [xfs]
> > ? xfs_fs_destroy_inode+0x4f0/0x4f0 [xfs]
> > get_tree_bdev+0x198/0x270
> > vfs_get_tree+0x23/0xb0
> > do_mount+0x87e/0xa20
> > ksys_mount+0xb6/0xd0
> > __x64_sys_mount+0x21/0x30
> > do_syscall_64+0x50/0x180
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x7fcfdf15d3ca
> > Code: 48 8b 0d c1 8a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8e 8a 2c 00 f7 d8 64 89 01 48
> > RSP: 002b:00007fff0af10a58 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
> > RAX: ffffffffffffffda RBX: 000055b202ec1970 RCX: 00007fcfdf15d3ca
> > RDX: 000055b202ec1be0 RSI: 000055b202ec1c20 RDI: 000055b202ec1c00
> > RBP: 0000000000000000 R08: 000055b202ec1b80 R09: 0000000000000000
> > R10: 00000000c0ed0000 R11: 0000000000000202 R12: 000055b202ec1c00
> > R13: 000055b202ec1be0 R14: 0000000000000000 R15: 00007fcfdf67e8a4
> > irq event stamp: 18398
> > hardirqs last enabled at (18397): [<ffffffff8123738f>] __slab_alloc.isra.83+0x6f/0x80
> > hardirqs last disabled at (18398): [<ffffffff81001d8a>] trace_hardirqs_off_thunk+0x1a/0x20
> > softirqs last enabled at (18158): [<ffffffff81a003af>] __do_softirq+0x3af/0x4a4
> > softirqs last disabled at (18149): [<ffffffff8106528c>] irq_exit+0xbc/0xe0
> > ---[ end trace 3669c914fa8ccac6 ]---
> >
> > AFAICT this is because inode buffers are 32K on this system
> >
> > >
> > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > > index a2664afa10c3..2993af4a9935 100644
> > > --- a/fs/xfs/xfs_qm.c
> > > +++ b/fs/xfs/xfs_qm.c
> > > @@ -988,7 +988,8 @@ xfs_qm_reset_dqcounts_buf(
> > > if (qip->i_d.di_nblocks == 0)
> > > return 0;
> > >
> > > - map = kmem_alloc(XFS_DQITER_MAP_SIZE * sizeof(*map), 0);
> > > + map = kmalloc(XFS_DQITER_MAP_SIZE * sizeof(*map),
> > > + GFP_KERNEL | __GFP_NOFAIL);
> > >
> > > lblkno = 0;
> > > maxlblkcnt = XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
> > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > > index 7f03b4ab3452..dfd419d402ea 100644
> > > --- a/fs/xfs/xfs_rtalloc.c
> > > +++ b/fs/xfs/xfs_rtalloc.c
> > > @@ -962,7 +962,7 @@ xfs_growfs_rt(
> > > /*
> > > * Allocate a new (fake) mount/sb.
> > > */
> > > - nmp = kmem_alloc(sizeof(*nmp), 0);
> > > + nmp = kmalloc(sizeof(*nmp), GFP_KERNEL | __GFP_NOFAIL);
> > > /*
> > > * Loop over the bitmap blocks.
> > > * We will do everything one bitmap block at a time.
> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > index cc1933dc652f..eee831681e9c 100644
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -1739,7 +1739,7 @@ static int xfs_init_fs_context(
> > > {
> > > struct xfs_mount *mp;
> > >
> > > - mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
> > > + mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL | __GFP_NOFAIL);
> > > if (!mp)
> > > return -ENOMEM;
> > >
> > > --
> > > 2.23.0
> > >
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 5/5] xfs: Convert kmem_alloc() users
2019-11-24 22:02 ` Darrick J. Wong
@ 2019-11-25 9:28 ` Carlos Maiolino
0 siblings, 0 replies; 19+ messages in thread
From: Carlos Maiolino @ 2019-11-25 9:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Sun, Nov 24, 2019 at 02:02:56PM -0800, Darrick J. Wong wrote:
> On Fri, Nov 22, 2019 at 02:30:48PM -0800, Darrick J. Wong wrote:
> > On Fri, Nov 22, 2019 at 07:57:56AM -0800, Darrick J. Wong wrote:
> > > On Wed, Nov 20, 2019 at 11:44:25AM +0100, Carlos Maiolino wrote:
> > > > Use kmalloc() directly.
> >
> > <snip all this>
> >
> > > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > > > index 5423171e0b7d..7bb53fbf32f6 100644
> > > > --- a/fs/xfs/xfs_log_recover.c
> > > > +++ b/fs/xfs/xfs_log_recover.c
> > > > @@ -1962,7 +1962,7 @@ xlog_recover_buffer_pass1(
> > > > }
> > > > }
> > > >
> > > > - bcp = kmem_alloc(sizeof(struct xfs_buf_cancel), 0);
> > > > + bcp = kmalloc(sizeof(struct xfs_buf_cancel), GFP_KERNEL | __GFP_NOFAIL);
> > > > bcp->bc_blkno = buf_f->blf_blkno;
> > > > bcp->bc_len = buf_f->blf_len;
> > > > bcp->bc_refcount = 1;
> > > > @@ -2932,7 +2932,8 @@ xlog_recover_inode_pass2(
> > > > if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
> > > > in_f = item->ri_buf[0].i_addr;
> > > > } else {
> > > > - in_f = kmem_alloc(sizeof(struct xfs_inode_log_format), 0);
> > > > + in_f = kmalloc(sizeof(struct xfs_inode_log_format),
> > > > + GFP_KERNEL | __GFP_NOFAIL);
> > > > need_free = 1;
> > > > error = xfs_inode_item_format_convert(&item->ri_buf[0], in_f);
> > > > if (error)
> > > > @@ -4271,7 +4272,7 @@ xlog_recover_add_to_trans(
> > > > return 0;
> > > > }
> > > >
> > > > - ptr = kmem_alloc(len, 0);
> > > > + ptr = kmalloc(len, GFP_KERNEL | __GFP_NOFAIL);
> > > > memcpy(ptr, dp, len);
> > > > in_f = (struct xfs_inode_log_format *)ptr;
> > >
> > > I noticed that kmalloc is generating warnings with generic/049 when 16k
> > > directories (-n size=16k) are enabled. I /think/ this is because it's
> > > quite possible to write out an xlog_op_header with a length of more than
> > > a single page; log recovery will then try to allocate a huge memory
> > > buffer to recover the transaction; and so we try to do a huge NOFAIL
> > > allocation, which makes the mm unhappy.
> > >
> > > The one thing I've noticed with this conversion series is that the flags
> > > translation isn't 100% 1-to-1. Before, kmem_flags_convert didn't
> > > explicitly set __GFP_NOFAIL anywhere; we simply took the default
> > > behavior. IIRC that means that small allocations actually /are/
> > > guaranteed to succeed, but multipage allocations certainly aren't.
> > > This seems to be one place where we could have asked for a lot of
> > > memory, failed to get it, and crashed.
> > >
> > > Now that we explicitly set NOFAIL in all the places where we don't also
> > > check for a null return, I think we're just uncovering latent bugs
> > > lurking in the code base. The kernel does actually fulfill the
> > > allocation request, but it's clearly not happy.
> >
> > FWIW I ran with various dirsizes and options and it looks like this is
> > the only place where we screw this up... patches soon.
>
> I rescind that statement -- there's enough places in this series where I
> can't 100% tell that a k{mzre}alloc call asks for a small enough amount
> of memory to qualify for __GFP_NOFAIL.
>
> I really want this cleanup to start with a straightforward removal of
> the kmem.c wrappers without any behavior changes. Only after that's
> done should we move on to things like adding __GFP_NOFAIL to allocations
> or deciding if/where we can substitute kfree for kvfree.
>
> Munging them together means I can't easily tell if something is
> seriously broken here (but the WARN_ONs suggest this) and I'd forgotten
> that the merge window is opening the week of a major US holiday, so I
> choose to defer this series to 5.6.
Thanks, this is the best option IMO too. I'll think a bit about it and check
what should we do about it.
>
> --D
>
> > --D
> >
> > > --D
> > >
> > > Relevant snippet of dmesg; everything else was normal:
> > >
> > > XFS (sdd): Mounting V5 Filesystem
> > > XFS (sdd): Starting recovery (logdev: internal)
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 1 PID: 459342 at mm/page_alloc.c:3275 get_page_from_freelist+0x434/0x1660
> > > Modules linked in: dm_thin_pool dm_persistent_data dm_bio_prison dm_snapshot dm_bufio dm_flakey xfs libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac bfq ip_set nfnetlink ip6table_filter ip6_tables iptable_filter sch_fq_codel ip_tables x_tables nfsv4 af_packet [last unloaded: scsi_debug]
> > > CPU: 1 PID: 459342 Comm: mount Not tainted 5.4.0-rc3-djw #rc3
> > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-1ubuntu1 04/01/2014
> > > RIP: 0010:get_page_from_freelist+0x434/0x1660
> > > Code: e6 00 00 00 00 48 89 84 24 a0 00 00 00 0f 84 08 fd ff ff f7 84 24 c0 00 00 00 00 80 00 00 74 0c 83 bc 24 84 00 00 00 01 76 02 <0f> 0b 49 8d 87 10 05 00 00 48 89 c7 48 89 84 24 88 00 00 00 e8 03
> > > RSP: 0018:ffffc900035d3918 EFLAGS: 00010202
> > > RAX: ffff88803fffb680 RBX: 0000000000002968 RCX: ffffea0000c8e108
> > > RDX: ffff88803fffbba8 RSI: ffff88803fffb870 RDI: 0000000000000000
> > > RBP: 0000000000000002 R08: 0000000000000201 R09: 000000000002ff81
> > > R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
> > > R13: 0000000000048cc0 R14: 0000000000000001 R15: ffff88803fffb680
> > > FS: 00007fcfdf89a080(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 000055b202ec48c8 CR3: 000000003bfdc005 CR4: 00000000001606a0
> > > Call Trace:
> > > ? kvm_clock_read+0x14/0x30
> > > __alloc_pages_nodemask+0x172/0x3a0
> > > kmalloc_order+0x18/0x80
> > > kmalloc_order_trace+0x1d/0x130
> > > xlog_recover_add_to_trans+0x4b/0x340 [xfs]
> > > xlog_recovery_process_trans+0xe9/0xf0 [xfs]
> > > xlog_recover_process_data+0x9e/0x1f0 [xfs]
> > > xlog_do_recovery_pass+0x3a9/0x7c0 [xfs]
> > > xlog_do_log_recovery+0x72/0x150 [xfs]
> > > xlog_do_recover+0x43/0x2a0 [xfs]
> > > xlog_recover+0xdf/0x170 [xfs]
> > > xfs_log_mount+0x2e3/0x300 [xfs]
> > > xfs_mountfs+0x4e7/0x9f0 [xfs]
> > > xfs_fc_fill_super+0x2f8/0x520 [xfs]
> > > ? xfs_fs_destroy_inode+0x4f0/0x4f0 [xfs]
> > > get_tree_bdev+0x198/0x270
> > > vfs_get_tree+0x23/0xb0
> > > do_mount+0x87e/0xa20
> > > ksys_mount+0xb6/0xd0
> > > __x64_sys_mount+0x21/0x30
> > > do_syscall_64+0x50/0x180
> > > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > RIP: 0033:0x7fcfdf15d3ca
> > > Code: 48 8b 0d c1 8a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8e 8a 2c 00 f7 d8 64 89 01 48
> > > RSP: 002b:00007fff0af10a58 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
> > > RAX: ffffffffffffffda RBX: 000055b202ec1970 RCX: 00007fcfdf15d3ca
> > > RDX: 000055b202ec1be0 RSI: 000055b202ec1c20 RDI: 000055b202ec1c00
> > > RBP: 0000000000000000 R08: 000055b202ec1b80 R09: 0000000000000000
> > > R10: 00000000c0ed0000 R11: 0000000000000202 R12: 000055b202ec1c00
> > > R13: 000055b202ec1be0 R14: 0000000000000000 R15: 00007fcfdf67e8a4
> > > irq event stamp: 18398
> > > hardirqs last enabled at (18397): [<ffffffff8123738f>] __slab_alloc.isra.83+0x6f/0x80
> > > hardirqs last disabled at (18398): [<ffffffff81001d8a>] trace_hardirqs_off_thunk+0x1a/0x20
> > > softirqs last enabled at (18158): [<ffffffff81a003af>] __do_softirq+0x3af/0x4a4
> > > softirqs last disabled at (18149): [<ffffffff8106528c>] irq_exit+0xbc/0xe0
> > > ---[ end trace 3669c914fa8ccac6 ]---
> > >
> > > AFAICT this is because inode buffers are 32K on this system
> > >
> > > >
> > > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > > > index a2664afa10c3..2993af4a9935 100644
> > > > --- a/fs/xfs/xfs_qm.c
> > > > +++ b/fs/xfs/xfs_qm.c
> > > > @@ -988,7 +988,8 @@ xfs_qm_reset_dqcounts_buf(
> > > > if (qip->i_d.di_nblocks == 0)
> > > > return 0;
> > > >
> > > > - map = kmem_alloc(XFS_DQITER_MAP_SIZE * sizeof(*map), 0);
> > > > + map = kmalloc(XFS_DQITER_MAP_SIZE * sizeof(*map),
> > > > + GFP_KERNEL | __GFP_NOFAIL);
> > > >
> > > > lblkno = 0;
> > > > maxlblkcnt = XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
> > > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > > > index 7f03b4ab3452..dfd419d402ea 100644
> > > > --- a/fs/xfs/xfs_rtalloc.c
> > > > +++ b/fs/xfs/xfs_rtalloc.c
> > > > @@ -962,7 +962,7 @@ xfs_growfs_rt(
> > > > /*
> > > > * Allocate a new (fake) mount/sb.
> > > > */
> > > > - nmp = kmem_alloc(sizeof(*nmp), 0);
> > > > + nmp = kmalloc(sizeof(*nmp), GFP_KERNEL | __GFP_NOFAIL);
> > > > /*
> > > > * Loop over the bitmap blocks.
> > > > * We will do everything one bitmap block at a time.
> > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > > index cc1933dc652f..eee831681e9c 100644
> > > > --- a/fs/xfs/xfs_super.c
> > > > +++ b/fs/xfs/xfs_super.c
> > > > @@ -1739,7 +1739,7 @@ static int xfs_init_fs_context(
> > > > {
> > > > struct xfs_mount *mp;
> > > >
> > > > - mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
> > > > + mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL | __GFP_NOFAIL);
> > > > if (!mp)
> > > > return -ENOMEM;
> > > >
> > > > --
> > > > 2.23.0
> > > >
>
--
Carlos
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] Remove/convert more kmem_* wrappers
2019-11-20 10:44 [PATCH 0/5] Remove/convert more kmem_* wrappers Carlos Maiolino
` (4 preceding siblings ...)
2019-11-20 10:44 ` [PATCH 5/5] xfs: Convert kmem_alloc() users Carlos Maiolino
@ 2020-05-14 9:26 ` Dave Chinner
5 siblings, 0 replies; 19+ messages in thread
From: Dave Chinner @ 2020-05-14 9:26 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Nov 20, 2019 at 11:44:20AM +0100, Carlos Maiolino wrote:
> Hi,
>
> in this new series, we remove most of the remaining kmem_* wrappers.
>
> All of the wrappers being removed in this series can be directly replaced by
> generic kernel kmalloc()/kzalloc() interface.
>
> Only interface kept is kmem_alloc() but has been converted into a local helper.
>
> This series should be applied on top of my previous series aiming to clean up
> our memory allocation interface.
>
>
> Darrick, I believe this is slightly different from what you suggested
> previously, about converting kmem_* interfaces to use GFP flags directly. At
> least I read that as keeping current kmem_* interface, and getting rid of KM_*
> flags now.
>
> But, I believe these patches does not change any allocation logic, and after the
> series we are left with fewer users of KM_* flags users to get rid of, which
> IMHO will be easier. And also I already had the patches mostly done :)
>
> Let me know if this is ok for you.
>
>
> Carlos Maiolino (5):
> xfs: remove kmem_zone_zalloc()
> xfs: Remove kmem_zone_alloc() wrapper
> xfs: remove kmem_zalloc() wrapper
> xfs: Remove kmem_realloc
> xfs: Convert kmem_alloc() users
Hmmm, just noticed that this never got merged. Whatever happened to
this patchset?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 19+ messages in thread