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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 17, 2016 at 09:44:39AM +0000, Wols Lists wrote:
> 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?

Yep, that makes sense. the journal (current write-through mode and upcoming
write-back mode) does deserve a description. I'll add something into
Documentation dir in kernel source.

Thanks,
Shaohua
--
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



[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux