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!
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
