Re: [PATCH 1/2] raid5-cache: update superblock at shutdown/reboot

[Wols Lists <antlists@youngman.org.uk>]

On 17/11/16 05:18, NeilBrown wrote:
> On Thu, Nov 17 2016, Shaohua Li wrote:
> 
>> Currently raid5-cache update superblock in .quiesce. But since at
>> shutdown/reboot, .quiesce is called with reconfig mutex locked,
>> superblock isn't guaranteed to be called in reclaim thread (see
>> 8e018c21da3). This will make assemble do unnecessary journal recovery.
>> It doesn't corrupt data but is annoying.  This adds an extra hook to
>> guarantee journal is flushed to raid disks.  And since this hook is
>> called before superblock update, this will guarantee we have a uptodate
>> superblock in shutdown/reboot
> 
> Hi.
> I don't quite follow some of the reasoning here.
> In particular, the ->stop_writes() that you have implemented
> does almost exactly the same thing as r5l_quiesce(1).
> So why not simply call ->quiesce(mddev, 1) in __md_stop_writes()??
> You probably need to also call ->quiesce(mddev, 0) to keep things
> balanced.
> 
> Also you have introduced a static mutex (which isn't my favourite sort
> of thing) without giving any explanation why in the changelog comment.
> So I cannot easily see if that addition is at all justified.
> 
> Thanks,
> NeilBrown
> 
I need to be careful I don't ruffle any feathers here ...

But this is saying to me this is a nice feature that hasn't been
properly spec'd and thought through. Don't get me wrong, I know that -
in typical linux fashion - people have been adding things, and raid has
"just growed" topsy fashion. So it's incredibly difficult to spec a new
feature when you don't have a spec for the stuff you're building it on.

Anyways, what I'm saying is, it seems to me this caching stuff (it's a
new feature, iirc) would be great for trying to write out a proper spec
of what's meant to be going on. It'll roll over into spec'ing the stuff
it relies on ...

And yes, I *AM* volunteering to do the work - as I said elsewhere, I
want to put a load of kerneldoc into the raid source, and get to
understand it all, but the downside is you'll get a lot of newbie-ish
questions from me trying to get to grips with what's going on. I'm an
experienced C programmer but kernel style is alien to me - you know the
disconnect when you're reading something, you can read the words easily,
but you can't decipher the meaning. That's how I feel reading the kernel
source at the moment.

Are we up for it?

Cheers,
Wol