Sorry for the unclear expression.
The log might look like this before we did a recovery :
| mb1 | mb2 | mb3 | | | |
last_checkpoint = mb1'postion, last_cp_seq = mb1'seq
After we did a recovery(we would write a empty meta block emb at log tail):
| mb1 | mb2 | mb3 | emb | | |
last_checkpoint = emb'position, last_cp_seq = mb1'seq + 11
Then we write two meta blocks and suppose crash happens:
| mb1 | mb2 | mb3 | emb | mb4 | mb5 |
last_checkpoint = emb'position, last_cp_seq = mb1'seq + 11
Now we did another recovery after restart and suppose mb4 was invalid:
| mb1 | mb2 | mb3 | emb | mb4 | mb5 |
last_checkpoint = emb'position, last_cp_seq = mb1'seq + 11
Since mb4 was invalid, we would stop recovering mb5 which should be discarded.
After recovery, log_start points to mb4 and we wouldn't write a empty meta block
because condition "ctx.seq > log->last_cp_seq + 1" doesn't satisfy. If we are going to
write a valid meta block and crash happens again, the new meta block will fall into
position of mb4 and recovery process may do a recovery to mb5 since it's seq
is matched.
What I try to say is that if the first meta block ,not only the mid one, we written was
invalid, the log recovery could bring problem here too . I think the condition for
write a empty meta block should like this:
- if (ctx.seq > log->last_cp_seq + 1) {
+ if (ctx.seq > log->last_cp_seq） {
------------------ Original ------------------
From: "Shaohua Li"<shli@xxxxxxxxxx>;
Date: Thu, Oct 27, 2016 02:35 AM
To: "Zhengyuan Liu"<liuzhengyuan@xxxxxxxxxx>;
Cc: "Song Liu"<songliubraving@xxxxxx>; "linux-raid"<linux-raid@xxxxxxxxxxxxxxx>; "liuzhengyuang521"<liuzhengyuang521@xxxxxxxxx>;
Subject: Re: [PATCH] md/raid5: write an empty meta-block when creatinglogsuper-block
On Tue, Oct 25, 2016 at 08:43:50PM +0800, Zhengyuan Liu wrote:
> After discussion with my colleague, I think there is still a problem that
> may happen very unlikely.The superblock should point to the last meta
> block we have written after log reclaim or point to the emtpy meta block
> after log recovery, just consider we write some meta block behind the
> superblock position and suppose crash happens. If the first meta block we
> have written neighboring the superblock position is invalid, ctx.seq would
> also equal to last_cp_seq+1 after we did a recovery . So the safest way is
> we always write an empty meta block at ctx.pos no matter how much
> ctx.req is more than last_cp_seq after we did a recovery.
> How do you think, Shaohua? If it is necessary, I'd revert this patch and
> resend one.
I didn't get the point. Could you please elaborate it again?
Thanks,
Shaohua
>
> ------------------ Original ------------------
> From: "Shaohua Li"<shli@xxxxxxxxxx>;
> Date: Tue, Oct 25, 2016 05:23 AM
> To: "Zhengyuan Liu"<liuzhengyuan@xxxxxxxxxx>;
> Cc: "shli"<shli@xxxxxx>; "Song Liu"<songliubraving@xxxxxx>; "linux-raid"<linux-raid@xxxxxxxxxxxxxxx>; "liuzhengyuang521"<liuzhengyuang521@xxxxxxxxx>;
> Subject: Re: [PATCH] md/raid5: write an empty meta-block when creating logsuper-block
>
> On Mon, Oct 24, 2016 at 04:15:59PM +0800, Zhengyuan Liu wrote:
> > If superblock points to an invalid meta block, r5l_load_log will set
> > create_super with true and create an new superblock, this runtime path
> > would always happen if we do no writing I/O to this array since it was
> > created. Writing an empty meta block could avoid this unnecessary
> > action at the first time we created log superblock.
> >
> > Another reason is for the corretness of log recovery. Currently we have
> > bellow code to guarantee log revocery to be correct.
> >
> > if (ctx.seq > log->last_cp_seq + 1) {
> > int ret;
> >
> > ret = r5l_log_write_empty_meta_block(log, ctx.pos, ctx.seq + 10);
> > if (ret)
> > return ret;
> > log->seq = ctx.seq + 11;
> > log->log_start = r5l_ring_add(log, ctx.pos, BLOCK_SECTORS);
> > r5l_write_super(log, ctx.pos);
> > } else {
> > log->log_start = ctx.pos;
> > log->seq = ctx.seq;
> > }
> >
> > If we just created a array with a journal device, log->log_start and
> > log->last_checkpoint should all be 0, then we write three meta block
> > which are valid except mid one and supposed crash happened. The ctx.seq
> > would equal to log->last_cp_seq + 1 and log->log_start would be set to
> > position of mid invalid meta block after we did a recovery, this will
> > lead to problems which could be avoided with this patch.
>
> This would be very unlikely, but better to fix. Applied, thanks!��.n��������+%������w��{.n�����{����w��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
