On Mon, Dec 9, 2019 at 4:38 PM Bob Peterson <[email protected]> wrote: > This patch adds a new slab for gfs2 transactions. That allows us to > have an initialization function and protect against some errors.
The added checks in gfs2_trans_free actually have nothing to do with > > Signed-off-by: Bob Peterson <[email protected]> > --- > fs/gfs2/glops.c | 1 + > fs/gfs2/log.c | 14 +++++++++++--- > fs/gfs2/lops.c | 4 ++++ > fs/gfs2/main.c | 23 +++++++++++++++++++++++ > fs/gfs2/trans.c | 28 ++++++++++++++++++++++------ > fs/gfs2/trans.h | 1 + > fs/gfs2/util.c | 1 + > fs/gfs2/util.h | 1 + > 8 files changed, 64 insertions(+), 9 deletions(-) > > diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c > index d8f9ee756c0e..e0219b0e2229 100644 > --- a/fs/gfs2/glops.c > +++ b/fs/gfs2/glops.c > @@ -89,6 +89,7 @@ static void gfs2_ail_empty_gl(struct gfs2_glock *gl) > memset(&tr, 0, sizeof(tr)); > INIT_LIST_HEAD(&tr.tr_buf); > INIT_LIST_HEAD(&tr.tr_databuf); > + set_bit(TR_ATTACHED, &tr.tr_flags); /* prevent gfs2_trans_end free */ > tr.tr_revokes = atomic_read(&gl->gl_ail_count); > > if (!tr.tr_revokes) > diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c > index f543899c4d70..2b4217883b72 100644 > --- a/fs/gfs2/log.c > +++ b/fs/gfs2/log.c > @@ -30,6 +30,7 @@ > #include "util.h" > #include "dir.h" > #include "trace_gfs2.h" > +#include "trans.h" > > static void gfs2_log_shutdown(struct gfs2_sbd *sdp); > > @@ -331,7 +332,7 @@ static void ail2_empty(struct gfs2_sbd *sdp, unsigned int > new_tail) > list_del(&tr->tr_list); > gfs2_assert_warn(sdp, list_empty(&tr->tr_ail1_list)); > gfs2_assert_warn(sdp, list_empty(&tr->tr_ail2_list)); > - kfree(tr); > + gfs2_trans_free(sdp, tr); > } > > spin_unlock(&sdp->sd_ail_lock); > @@ -876,7 +877,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct > gfs2_glock *gl, u32 flags) > trace_gfs2_log_flush(sdp, 0, flags); > up_write(&sdp->sd_log_flush_lock); > > - kfree(tr); > + gfs2_trans_free(sdp, tr); > } > > /** > @@ -895,6 +896,12 @@ static void gfs2_merge_trans(struct gfs2_trans *old, > struct gfs2_trans *new) > old->tr_num_databuf_rm += new->tr_num_databuf_rm; > old->tr_num_revoke += new->tr_num_revoke; > > + new->tr_num_buf_new = 0; > + new->tr_num_buf_rm = 0; > + new->tr_num_databuf_new = 0; > + new->tr_num_databuf_rm = 0; > + new->tr_num_revoke = 0; > + > list_splice_tail_init(&new->tr_databuf, &old->tr_databuf); > list_splice_tail_init(&new->tr_buf, &old->tr_buf); > } > @@ -904,6 +911,7 @@ static void log_refund(struct gfs2_sbd *sdp, struct > gfs2_trans *tr) > unsigned int reserved; > unsigned int unused; > unsigned int maxres; > + unsigned int new_revokes = tr->tr_num_revoke; > > gfs2_log_lock(sdp); > > @@ -915,7 +923,7 @@ static void log_refund(struct gfs2_sbd *sdp, struct > gfs2_trans *tr) > set_bit(TR_ATTACHED, &tr->tr_flags); > } > > - sdp->sd_log_commited_revoke += tr->tr_num_revoke; > + sdp->sd_log_commited_revoke += new_revokes; > reserved = calc_reserved(sdp); > maxres = sdp->sd_log_blks_reserved + tr->tr_reserved; > gfs2_assert_withdraw(sdp, maxres >= reserved); > diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c > index 2f4c478e7bb7..99b366a6e4e6 100644 > --- a/fs/gfs2/lops.c > +++ b/fs/gfs2/lops.c > @@ -730,6 +730,8 @@ static void buf_lo_after_commit(struct gfs2_sbd *sdp, > struct gfs2_trans *tr) > list_del_init(&bd->bd_list); > gfs2_unpin(sdp, bd->bd_bh, tr); > } > + tr->tr_num_buf_new = 0; > + tr->tr_num_buf_rm = 0; > } > > static void buf_lo_before_scan(struct gfs2_jdesc *jd, > @@ -1079,6 +1081,8 @@ static void databuf_lo_after_commit(struct gfs2_sbd > *sdp, struct gfs2_trans *tr) > list_del_init(&bd->bd_list); > gfs2_unpin(sdp, bd->bd_bh, tr); > } > + tr->tr_num_databuf_new = 0; > + tr->tr_num_databuf_rm = 0; > } > > > diff --git a/fs/gfs2/main.c b/fs/gfs2/main.c > index a1a295b739fb..5ff114bbcede 100644 > --- a/fs/gfs2/main.c > +++ b/fs/gfs2/main.c > @@ -68,6 +68,19 @@ static void gfs2_init_gl_aspace_once(void *foo) > address_space_init_once(mapping); > } > > +static void gfs2_init_tr_once(void *foo) > +{ > + struct gfs2_trans *tr = foo; > + > + memset(tr, 0, sizeof(struct gfs2_trans)); > + INIT_LIST_HEAD(&tr->tr_databuf); > + INIT_LIST_HEAD(&tr->tr_buf); > + INIT_LIST_HEAD(&tr->tr_ail1_list); > + INIT_LIST_HEAD(&tr->tr_ail2_list); > + INIT_LIST_HEAD(&tr->tr_list); > + tr->tr_ip = 0; > +} > + > /** > * init_gfs2_fs - Register GFS2 as a filesystem > * > @@ -143,6 +156,13 @@ static int __init init_gfs2_fs(void) > if (!gfs2_qadata_cachep) > goto fail_cachep7; > > + gfs2_trans_cachep = kmem_cache_create("gfs2_trans", > + sizeof(struct gfs2_trans), > + 0, SLAB_CONSISTENCY_CHECKS| > + SLAB_POISON, > gfs2_init_tr_once); I don't think SLAB_CONSISTENCY_CHECKS should be enabled permanently. It can always be enabled at boot time via the slub_debug kernel command line parameter, though. > + if (!gfs2_trans_cachep) > + goto fail_cachep8; > + > error = register_shrinker(&gfs2_qd_shrinker); > if (error) > goto fail_shrinker; > @@ -194,6 +214,8 @@ static int __init init_gfs2_fs(void) > fail_fs1: > unregister_shrinker(&gfs2_qd_shrinker); > fail_shrinker: > + kmem_cache_destroy(gfs2_trans_cachep); > +fail_cachep8: > kmem_cache_destroy(gfs2_qadata_cachep); > fail_cachep7: > kmem_cache_destroy(gfs2_quotad_cachep); > @@ -236,6 +258,7 @@ static void __exit exit_gfs2_fs(void) > rcu_barrier(); > > mempool_destroy(gfs2_page_pool); > + kmem_cache_destroy(gfs2_trans_cachep); > kmem_cache_destroy(gfs2_qadata_cachep); > kmem_cache_destroy(gfs2_quotad_cachep); > kmem_cache_destroy(gfs2_rgrpd_cachep); > diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c > index 9d4227330de4..50f902a23c4d 100644 > --- a/fs/gfs2/trans.c > +++ b/fs/gfs2/trans.c > @@ -37,7 +37,7 @@ int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int > blocks, > if (!test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags)) > return -EROFS; > > - tr = kzalloc(sizeof(struct gfs2_trans), GFP_NOFS); > + tr = kmem_cache_alloc(gfs2_trans_cachep, GFP_NOFS); > if (!tr) > return -ENOMEM; > > @@ -45,14 +45,19 @@ int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int > blocks, > tr->tr_blocks = blocks; > tr->tr_revokes = revokes; > tr->tr_reserved = 1; > + tr->tr_num_revoke = 0; > + > + clear_bit(TR_TOUCHED, &tr->tr_flags); > + clear_bit(TR_ATTACHED, &tr->tr_flags); > + > set_bit(TR_ALLOCED, &tr->tr_flags); > if (blocks) > tr->tr_reserved += 6 + blocks; > if (revokes) > tr->tr_reserved += gfs2_struct2blk(sdp, revokes, > sizeof(u64)); > - INIT_LIST_HEAD(&tr->tr_databuf); > - INIT_LIST_HEAD(&tr->tr_buf); > + gfs2_assert_warn(sdp, list_empty(&tr->tr_databuf)); > + gfs2_assert_warn(sdp, list_empty(&tr->tr_buf)); These asserts don't seem to make sense; this must have been checked in gfs2_trans_free already. > > sb_start_intwrite(sdp->sd_vfs); > > @@ -66,7 +71,7 @@ int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int > blocks, > > fail: > sb_end_intwrite(sdp->sd_vfs); > - kfree(tr); > + kmem_cache_free(gfs2_trans_cachep, tr); > > return error; > } > @@ -94,7 +99,7 @@ void gfs2_trans_end(struct gfs2_sbd *sdp) > if (!test_bit(TR_TOUCHED, &tr->tr_flags)) { > gfs2_log_release(sdp, tr->tr_reserved); > if (alloced) { > - kfree(tr); > + gfs2_trans_free(sdp, tr); > sb_end_intwrite(sdp->sd_vfs); > } > return; > @@ -110,7 +115,7 @@ void gfs2_trans_end(struct gfs2_sbd *sdp) > > gfs2_log_commit(sdp, tr); > if (alloced && !test_bit(TR_ATTACHED, &tr->tr_flags)) > - kfree(tr); > + gfs2_trans_free(sdp, tr); > up_read(&sdp->sd_log_flush_lock); > > if (sdp->sd_vfs->s_flags & SB_SYNCHRONOUS) > @@ -273,3 +278,14 @@ void gfs2_trans_remove_revoke(struct gfs2_sbd *sdp, u64 > blkno, unsigned int len) > gfs2_log_unlock(sdp); > } > > +void gfs2_trans_free(struct gfs2_sbd *sdp, struct gfs2_trans *tr) > +{ > + if (tr == NULL) > + return; > + > + gfs2_assert_warn(sdp, list_empty(&tr->tr_ail1_list)); > + gfs2_assert_warn(sdp, list_empty(&tr->tr_ail2_list)); > + gfs2_assert_warn(sdp, list_empty(&tr->tr_databuf)); > + gfs2_assert_warn(sdp, list_empty(&tr->tr_buf)); > + kmem_cache_free(gfs2_trans_cachep, tr); > +} > diff --git a/fs/gfs2/trans.h b/fs/gfs2/trans.h > index 6071334de035..83199ce5a5c5 100644 > --- a/fs/gfs2/trans.h > +++ b/fs/gfs2/trans.h > @@ -42,5 +42,6 @@ extern void gfs2_trans_add_data(struct gfs2_glock *gl, > struct buffer_head *bh); > extern void gfs2_trans_add_meta(struct gfs2_glock *gl, struct buffer_head > *bh); > extern void gfs2_trans_add_revoke(struct gfs2_sbd *sdp, struct gfs2_bufdata > *bd); > extern void gfs2_trans_remove_revoke(struct gfs2_sbd *sdp, u64 blkno, > unsigned int len); > +extern void gfs2_trans_free(struct gfs2_sbd *sdp, struct gfs2_trans *tr); > > #endif /* __TRANS_DOT_H__ */ > diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c > index 690a2f575709..04141d0d8e17 100644 > --- a/fs/gfs2/util.c > +++ b/fs/gfs2/util.c > @@ -32,6 +32,7 @@ struct kmem_cache *gfs2_bufdata_cachep __read_mostly; > struct kmem_cache *gfs2_rgrpd_cachep __read_mostly; > struct kmem_cache *gfs2_quotad_cachep __read_mostly; > struct kmem_cache *gfs2_qadata_cachep __read_mostly; > +struct kmem_cache *gfs2_trans_cachep __read_mostly; > mempool_t *gfs2_page_pool __read_mostly; > > void gfs2_assert_i(struct gfs2_sbd *sdp) > diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h > index f23cced3809e..479d351c24a7 100644 > --- a/fs/gfs2/util.h > +++ b/fs/gfs2/util.h > @@ -154,6 +154,7 @@ extern struct kmem_cache *gfs2_bufdata_cachep; > extern struct kmem_cache *gfs2_rgrpd_cachep; > extern struct kmem_cache *gfs2_quotad_cachep; > extern struct kmem_cache *gfs2_qadata_cachep; > +extern struct kmem_cache *gfs2_trans_cachep; > extern mempool_t *gfs2_page_pool; > extern struct workqueue_struct *gfs2_control_wq; > > -- > 2.23.0 >
