| Message ID | 1556541059-21234-1-git-send-email-fangguoju@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bcache: fix memleak when error occurred before journal replay | expand |
Oh, Shenghui submitted a patch to fix this bug days ago, so please ignore this. On 2019/4/29 20:30, Guoju Fang wrote: > A list of struct journal_replay is allocated when register cache device > and will be freed when journal replay complete. It will cause memory > leaks if some error occurred before journal replay. > > Signed-off-by: Guoju Fang <fangguoju@gmail.com> > --- > drivers/md/bcache/super.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index a697a3a..e4289291 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -1782,6 +1782,7 @@ static void run_cache_set(struct cache_set *c) > struct cache *ca; > struct closure cl; > unsigned int i; > + LIST_HEAD(journal); > > closure_init_stack(&cl); > > @@ -1790,7 +1791,6 @@ static void run_cache_set(struct cache_set *c) > set_gc_sectors(c); > > if (CACHE_SYNC(&c->sb)) { > - LIST_HEAD(journal); > struct bkey *k; > struct jset *j; > > @@ -1820,25 +1820,25 @@ static void run_cache_set(struct cache_set *c) > > err = "bad btree root"; > if (__bch_btree_ptr_invalid(c, k)) > - goto err; > + goto free_journal; > > err = "error reading btree root"; > c->root = bch_btree_node_get(c, NULL, k, > j->btree_level, > true, NULL); > if (IS_ERR_OR_NULL(c->root)) > - goto err; > + goto free_journal; > > list_del_init(&c->root->list); > rw_unlock(true, c->root); > > err = uuid_read(c, j, &cl); > if (err) > - goto err; > + goto free_journal; > > err = "error in recovery"; > if (bch_btree_check(c)) > - goto err; > + goto free_journal; > > bch_journal_mark(c, &journal); > bch_initial_gc_finish(c); > @@ -1854,7 +1854,7 @@ static void run_cache_set(struct cache_set *c) > err = "error starting allocator thread"; > for_each_cache(ca, c, i) > if (bch_cache_allocator_start(ca)) > - goto err; > + goto free_journal; > > /* > * First place it's safe to allocate: btree_check() and > @@ -1938,6 +1938,14 @@ static void run_cache_set(struct cache_set *c) > > set_bit(CACHE_SET_RUNNING, &c->flags); > return; > + > +free_journal: > + while (!list_empty(&journal)) { > + struct journal_replay *jr = list_first_entry(&journal, > + struct journal_replay, list); > + list_del(&jr->list); > + kfree(jr); > + } > err: > closure_sync(&cl); > /* XXX: test this, it's broken */
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index a697a3a..e4289291 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -1782,6 +1782,7 @@ static void run_cache_set(struct cache_set *c) struct cache *ca; struct closure cl; unsigned int i; + LIST_HEAD(journal); closure_init_stack(&cl); @@ -1790,7 +1791,6 @@ static void run_cache_set(struct cache_set *c) set_gc_sectors(c); if (CACHE_SYNC(&c->sb)) { - LIST_HEAD(journal); struct bkey *k; struct jset *j; @@ -1820,25 +1820,25 @@ static void run_cache_set(struct cache_set *c) err = "bad btree root"; if (__bch_btree_ptr_invalid(c, k)) - goto err; + goto free_journal; err = "error reading btree root"; c->root = bch_btree_node_get(c, NULL, k, j->btree_level, true, NULL); if (IS_ERR_OR_NULL(c->root)) - goto err; + goto free_journal; list_del_init(&c->root->list); rw_unlock(true, c->root); err = uuid_read(c, j, &cl); if (err) - goto err; + goto free_journal; err = "error in recovery"; if (bch_btree_check(c)) - goto err; + goto free_journal; bch_journal_mark(c, &journal); bch_initial_gc_finish(c); @@ -1854,7 +1854,7 @@ static void run_cache_set(struct cache_set *c) err = "error starting allocator thread"; for_each_cache(ca, c, i) if (bch_cache_allocator_start(ca)) - goto err; + goto free_journal; /* * First place it's safe to allocate: btree_check() and @@ -1938,6 +1938,14 @@ static void run_cache_set(struct cache_set *c) set_bit(CACHE_SET_RUNNING, &c->flags); return; + +free_journal: + while (!list_empty(&journal)) { + struct journal_replay *jr = list_first_entry(&journal, + struct journal_replay, list); + list_del(&jr->list); + kfree(jr); + } err: closure_sync(&cl); /* XXX: test this, it's broken */
A list of struct journal_replay is allocated when register cache device and will be freed when journal replay complete. It will cause memory leaks if some error occurred before journal replay. Signed-off-by: Guoju Fang <fangguoju@gmail.com> --- drivers/md/bcache/super.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)