On Wed, Aug 03 2016, NeilBrown wrote: > [ Unknown signature status ] > On Sun, Jul 31 2016, shli@xxxxxxxxxx wrote: > >> From: Shaohua Li <shli@xxxxxx> >> >> .quiesce is called with mddev lock hold at most places. There are few >> exceptions. Calling .quesce without the lock hold could create races. For >> example, the .quesce of raid1 can't be recursively. The purpose of the patches >> is to fix a race in raid5-cache. The raid5-cache .quesce will write md >> superblock and should be called with mddev lock hold. >> >> Cc: NeilBrown <neilb@xxxxxxxx> >> Signed-off-by: Shaohua Li <shli@xxxxxx> > > Acked-by: NeilBrown <neilb@xxxxxxxx> > > This should be safe but I'm not sure I really like it. > The raid1 quiesce could be changed so that it can be called recursively. > The raid5-cache situation would be harder to get right and maybe this is > the best solution... It's just that 'quiesce' should be a fairly > light-weight operation, just waiting for pending requests to flush. It > shouldn't really *need* a lock. Actually, the more I think about this, the less I like it. I would much rather make .quiesce lighter weight so that no locking was needed. For r5l_quiesce, that probable means removed the "r5l_do_reclaim()". Stopping and restarting the reclaim thread seems reasonable, but calling r5l_do_reclaim() should not be needed. It should be done periodically by the thread, and at 'stop' time, but otherwise isn't needed. You would need to hold some mutex while calling md_register_thread, but that could be probably be log->io_mutex, or maybe even some other new mutex Could you explore following that path instead? Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
