On 2019/11/13 4:03 下午, Coly Li wrote:
> Fifo structure journal.pin is implemented by a cycle buffer, if the back
> index reaches highest location of the cycle buffer, it will be swapped
> to 0. Once the swapping happens, it means a smaller fifo index might be
> associated to a newer journal entry. So the btree node with oldest
> journal entry won't be selected in bch_btree_leaf_dirty() to reference
> the dirty B+tree leaf node. This problem may cause bcache journal won't
> protect unflushed oldest B+tree dirty leaf node in power failure, and
> this B+tree leaf node is possible to beinconsistent after reboot from
> power failure.
>
> This patch fixes the fifo index comparing logic in journal_pin_cmp(),
> to avoid potential corrupted B+tree leaf node when the back index of
> journal pin is swapped.
>
> Signed-off-by: Coly Li <colyli@xxxxxxx>
Hi Jens,
Guoju Fang talked to me today, he told me this change was unnecessary
and I was over-thought.
Then I realize fifo_idx() uses a mask to handle the array index overflow
condition, so the index swap in journal_pin_cmp() won't happen. And yes,
Guoju and Kent are correct.
Since you already applied this patch, can you please to remove this
patch from your for-next branch ? This single patch does not break
thing, but it is unecessary at this moment.
Thanks.
Coly Li
> ---
> drivers/md/bcache/btree.c | 26 ++++++++++++++++++++++++++
> drivers/md/bcache/journal.h | 4 ----
> 2 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
> index ba434d9ac720..00523cd1db80 100644
> --- a/drivers/md/bcache/btree.c
> +++ b/drivers/md/bcache/btree.c
> @@ -528,6 +528,32 @@ static void btree_node_write_work(struct work_struct *w)
> mutex_unlock(&b->write_lock);
> }
>
> +/* return true if journal pin 'l' is newer than 'r' */
> +static bool journal_pin_cmp(struct cache_set *c,
> + atomic_t *l,
> + atomic_t *r)
> +{
> + int l_idx, r_idx, f_idx, b_idx;
> + bool ret = false;
> +
> + l_idx = fifo_idx(&(c)->journal.pin, (l));
> + r_idx = fifo_idx(&(c)->journal.pin, (r));
> + f_idx = (c)->journal.pin.front;
> + b_idx = (c)->journal.pin.back;
> +
> + if (l_idx > r_idx)
> + ret = true;
> + /* in case fifo back pointer is swapped */
> + if (b_idx < f_idx) {
> + if (l_idx <= b_idx && r_idx >= f_idx)
> + ret = true;
> + else if (l_idx >= f_idx && r_idx <= b_idx)
> + ret = false;
> + }
> +
> + return ret;
> +}
> +
> static void bch_btree_leaf_dirty(struct btree *b, atomic_t *journal_ref)
> {
> struct bset *i = btree_bset_last(b);
> diff --git a/drivers/md/bcache/journal.h b/drivers/md/bcache/journal.h
> index f2ea34d5f431..06b3eaab7d16 100644
> --- a/drivers/md/bcache/journal.h
> +++ b/drivers/md/bcache/journal.h
> @@ -157,10 +157,6 @@ struct journal_device {
> };
>
> #define BTREE_FLUSH_NR 8
> -
> -#define journal_pin_cmp(c, l, r) \
> - (fifo_idx(&(c)->journal.pin, (l)) > fifo_idx(&(c)->journal.pin, (r)))
> -
> #define JOURNAL_PIN 20000
>
> #define journal_full(j) \
>
