]>
Commit | Line | Data |
---|---|---|
5417ad6e GKH |
1 | From 1bee2addc0c8470c8aaa65ef0599eeae96dd88bc Mon Sep 17 00:00:00 2001 |
2 | From: Coly Li <colyli@suse.de> | |
3 | Date: Thu, 25 Apr 2019 00:48:33 +0800 | |
4 | Subject: bcache: never set KEY_PTRS of journal key to 0 in journal_reclaim() | |
5 | ||
6 | From: Coly Li <colyli@suse.de> | |
7 | ||
8 | commit 1bee2addc0c8470c8aaa65ef0599eeae96dd88bc upstream. | |
9 | ||
10 | In journal_reclaim() ja->cur_idx of each cache will be update to | |
11 | reclaim available journal buckets. Variable 'int n' is used to count how | |
12 | many cache is successfully reclaimed, then n is set to c->journal.key | |
13 | by SET_KEY_PTRS(). Later in journal_write_unlocked(), a for_each_cache() | |
14 | loop will write the jset data onto each cache. | |
15 | ||
16 | The problem is, if all jouranl buckets on each cache is full, the | |
17 | following code in journal_reclaim(), | |
18 | ||
19 | 529 for_each_cache(ca, c, iter) { | |
20 | 530 struct journal_device *ja = &ca->journal; | |
21 | 531 unsigned int next = (ja->cur_idx + 1) % ca->sb.njournal_buckets; | |
22 | 532 | |
23 | 533 /* No space available on this device */ | |
24 | 534 if (next == ja->discard_idx) | |
25 | 535 continue; | |
26 | 536 | |
27 | 537 ja->cur_idx = next; | |
28 | 538 k->ptr[n++] = MAKE_PTR(0, | |
29 | 539 bucket_to_sector(c, ca->sb.d[ja->cur_idx]), | |
30 | 540 ca->sb.nr_this_dev); | |
31 | 541 } | |
32 | 542 | |
33 | 543 bkey_init(k); | |
34 | 544 SET_KEY_PTRS(k, n); | |
35 | ||
36 | If there is no available bucket to reclaim, the if() condition at line | |
37 | 534 will always true, and n remains 0. Then at line 544, SET_KEY_PTRS() | |
38 | will set KEY_PTRS field of c->journal.key to 0. | |
39 | ||
40 | Setting KEY_PTRS field of c->journal.key to 0 is wrong. Because in | |
41 | journal_write_unlocked() the journal data is written in following loop, | |
42 | ||
43 | 649 for (i = 0; i < KEY_PTRS(k); i++) { | |
44 | 650-671 submit journal data to cache device | |
45 | 672 } | |
46 | ||
47 | If KEY_PTRS field is set to 0 in jouranl_reclaim(), the journal data | |
48 | won't be written to cache device here. If system crahed or rebooted | |
49 | before bkeys of the lost journal entries written into btree nodes, data | |
50 | corruption will be reported during bcache reload after rebooting the | |
51 | system. | |
52 | ||
53 | Indeed there is only one cache in a cache set, there is no need to set | |
54 | KEY_PTRS field in journal_reclaim() at all. But in order to keep the | |
55 | for_each_cache() logic consistent for now, this patch fixes the above | |
56 | problem by not setting 0 KEY_PTRS of journal key, if there is no bucket | |
57 | available to reclaim. | |
58 | ||
59 | Signed-off-by: Coly Li <colyli@suse.de> | |
60 | Reviewed-by: Hannes Reinecke <hare@suse.com> | |
61 | Cc: stable@vger.kernel.org | |
62 | Signed-off-by: Jens Axboe <axboe@kernel.dk> | |
63 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
64 | ||
65 | --- | |
66 | drivers/md/bcache/journal.c | 11 +++++++---- | |
67 | 1 file changed, 7 insertions(+), 4 deletions(-) | |
68 | ||
69 | --- a/drivers/md/bcache/journal.c | |
70 | +++ b/drivers/md/bcache/journal.c | |
71 | @@ -513,11 +513,11 @@ static void journal_reclaim(struct cache | |
72 | ca->sb.nr_this_dev); | |
73 | } | |
74 | ||
75 | - bkey_init(k); | |
76 | - SET_KEY_PTRS(k, n); | |
77 | - | |
78 | - if (n) | |
79 | + if (n) { | |
80 | + bkey_init(k); | |
81 | + SET_KEY_PTRS(k, n); | |
82 | c->journal.blocks_free = c->sb.bucket_size >> c->block_bits; | |
83 | + } | |
84 | out: | |
85 | if (!journal_full(&c->journal)) | |
86 | __closure_wake_up(&c->journal.wait); | |
87 | @@ -641,6 +641,9 @@ static void journal_write_unlocked(struc | |
88 | ca->journal.seq[ca->journal.cur_idx] = w->data->seq; | |
89 | } | |
90 | ||
91 | + /* If KEY_PTRS(k) == 0, this jset gets lost in air */ | |
92 | + BUG_ON(i == 0); | |
93 | + | |
94 | atomic_dec_bug(&fifo_back(&c->journal.pin)); | |
95 | bch_journal_next(&c->journal); | |
96 | journal_reclaim(c); |