Skip to content

Commit 231f91a

Browse files
Dave ChinnerDarrick J. Wong
authored andcommitted
xfs: xfs_buf cache destroy isn't RCU safe
Darrick and Sachin Sant reported that xfs/435 and xfs/436 would report an non-empty xfs_buf slab on module remove. This isn't easily to reproduce, but is clearly a side effect of converting the buffer caceh to RUC freeing and lockless lookups. Sachin bisected and Darrick hit it when testing the patchset directly. Turns out that the xfs_buf slab is not destroyed when all the other XFS slab caches are destroyed. Instead, it's got it's own little wrapper function that gets called separately, and so it doesn't have an rcu_barrier() call in it that is needed to drain all the rcu callbacks before the slab is destroyed. Fix it by removing the xfs_buf_init/terminate wrappers that just allocate and destroy the xfs_buf slab, and move them to the same place that all the other slab caches are set up and destroyed. Reported-and-tested-by: Sachin Sant <[email protected]> Fixes: 298f342 ("xfs: lockless buffer lookup") Signed-off-by: Dave Chinner <[email protected]> Reviewed-by: Darrick J. Wong <[email protected]> Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]>
1 parent 3f52e01 commit 231f91a

File tree

3 files changed

+16
-37
lines changed

3 files changed

+16
-37
lines changed

fs/xfs/xfs_buf.c

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
#include "xfs_error.h"
2222
#include "xfs_ag.h"
2323

24-
static struct kmem_cache *xfs_buf_cache;
24+
struct kmem_cache *xfs_buf_cache;
2525

2626
/*
2727
* Locking orders
@@ -2308,29 +2308,6 @@ xfs_buf_delwri_pushbuf(
23082308
return error;
23092309
}
23102310

2311-
int __init
2312-
xfs_buf_init(void)
2313-
{
2314-
xfs_buf_cache = kmem_cache_create("xfs_buf", sizeof(struct xfs_buf), 0,
2315-
SLAB_HWCACHE_ALIGN |
2316-
SLAB_RECLAIM_ACCOUNT |
2317-
SLAB_MEM_SPREAD,
2318-
NULL);
2319-
if (!xfs_buf_cache)
2320-
goto out;
2321-
2322-
return 0;
2323-
2324-
out:
2325-
return -ENOMEM;
2326-
}
2327-
2328-
void
2329-
xfs_buf_terminate(void)
2330-
{
2331-
kmem_cache_destroy(xfs_buf_cache);
2332-
}
2333-
23342311
void xfs_buf_set_ref(struct xfs_buf *bp, int lru_ref)
23352312
{
23362313
/*

fs/xfs/xfs_buf.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#include <linux/uio.h>
1616
#include <linux/list_lru.h>
1717

18+
extern struct kmem_cache *xfs_buf_cache;
19+
1820
/*
1921
* Base types
2022
*/
@@ -307,10 +309,6 @@ extern int xfs_buf_delwri_submit(struct list_head *);
307309
extern int xfs_buf_delwri_submit_nowait(struct list_head *);
308310
extern int xfs_buf_delwri_pushbuf(struct xfs_buf *, struct list_head *);
309311

310-
/* Buffer Daemon Setup Routines */
311-
extern int xfs_buf_init(void);
312-
extern void xfs_buf_terminate(void);
313-
314312
static inline xfs_daddr_t xfs_buf_daddr(struct xfs_buf *bp)
315313
{
316314
return bp->b_maps[0].bm_bn;

fs/xfs/xfs_super.c

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1967,11 +1967,19 @@ xfs_init_caches(void)
19671967
{
19681968
int error;
19691969

1970+
xfs_buf_cache = kmem_cache_create("xfs_buf", sizeof(struct xfs_buf), 0,
1971+
SLAB_HWCACHE_ALIGN |
1972+
SLAB_RECLAIM_ACCOUNT |
1973+
SLAB_MEM_SPREAD,
1974+
NULL);
1975+
if (!xfs_buf_cache)
1976+
goto out;
1977+
19701978
xfs_log_ticket_cache = kmem_cache_create("xfs_log_ticket",
19711979
sizeof(struct xlog_ticket),
19721980
0, 0, NULL);
19731981
if (!xfs_log_ticket_cache)
1974-
goto out;
1982+
goto out_destroy_buf_cache;
19751983

19761984
error = xfs_btree_init_cur_caches();
19771985
if (error)
@@ -2145,6 +2153,8 @@ xfs_init_caches(void)
21452153
xfs_btree_destroy_cur_caches();
21462154
out_destroy_log_ticket_cache:
21472155
kmem_cache_destroy(xfs_log_ticket_cache);
2156+
out_destroy_buf_cache:
2157+
kmem_cache_destroy(xfs_buf_cache);
21482158
out:
21492159
return -ENOMEM;
21502160
}
@@ -2178,6 +2188,7 @@ xfs_destroy_caches(void)
21782188
xfs_defer_destroy_item_caches();
21792189
xfs_btree_destroy_cur_caches();
21802190
kmem_cache_destroy(xfs_log_ticket_cache);
2191+
kmem_cache_destroy(xfs_buf_cache);
21812192
}
21822193

21832194
STATIC int __init
@@ -2283,13 +2294,9 @@ init_xfs_fs(void)
22832294
if (error)
22842295
goto out_destroy_wq;
22852296

2286-
error = xfs_buf_init();
2287-
if (error)
2288-
goto out_mru_cache_uninit;
2289-
22902297
error = xfs_init_procfs();
22912298
if (error)
2292-
goto out_buf_terminate;
2299+
goto out_mru_cache_uninit;
22932300

22942301
error = xfs_sysctl_register();
22952302
if (error)
@@ -2346,8 +2353,6 @@ init_xfs_fs(void)
23462353
xfs_sysctl_unregister();
23472354
out_cleanup_procfs:
23482355
xfs_cleanup_procfs();
2349-
out_buf_terminate:
2350-
xfs_buf_terminate();
23512356
out_mru_cache_uninit:
23522357
xfs_mru_cache_uninit();
23532358
out_destroy_wq:
@@ -2373,7 +2378,6 @@ exit_xfs_fs(void)
23732378
kset_unregister(xfs_kset);
23742379
xfs_sysctl_unregister();
23752380
xfs_cleanup_procfs();
2376-
xfs_buf_terminate();
23772381
xfs_mru_cache_uninit();
23782382
xfs_destroy_workqueues();
23792383
xfs_destroy_caches();

0 commit comments

Comments
 (0)