On Wed, Aug 17 2016, Shaohua Li wrote: >> > >> > We will have the same deadlock issue with just stopping/restarting the reclaim >> > thread. As stopping the thread will wait for the thread, which probably is >> > doing r5l_do_reclaim and writting the superblock. Since we are writting the >> > superblock, we must hold the reconfig_mutex. >> >> When you say "writing the superblock" you presumably mean "blocked in >> r5l_write_super_and_discard_space(), waiting for MD_CHANGE_PENDING to >> be cleared" ?? > right >> With a bit of care you could wait for MD_CHANGE_PENDING to clear, or for >> ->quiesce to be set, and then exit gracefully. > > Can you give details about this please? .quiesce is called with reconfig_mutex > hold, so the MD_CHANGE_PENDING will never get cleared. raid5_quiesce(mddev, 1) sets conf->quiesce and then calls r5l_quiesce(). r5l_quiesce() tells the reclaim_thread to exit and waits for it to do so. But the reclaim thread might be in r5l_do_reclaim() -> r5l_write_super_and_discard_space() waiting for MD_CHANGE_PENDING to clear. That will only get cleared when the main thread can get the reconfig_mutex, which the thread calling raid5_quiesce() might hold. So we get a deadlock. My suggestion is to change r5l_write_super_and_discard_space() so that it waits for *either* MD_CHANGE_PENDING to be clear, or conf->quiesce to be set. That will avoid the deadlock. Whatever thread called raid5_quiesce() will now be in control of the array without any async IO going on. If it needs the metadata to be sync, it can do that itself. If not, then it doesn't really matter that r5l_write_super_and_discard_space() didn't wait. r5l_write_super_and_discard_space() shouldn't call discard if the superblock write didn't complete, and probably r5l_do_reclaim() shouldn't update last_checkpoint and last_cp_seq in that case. This is what I mean by "with a bit of care" and "exit gracefully". Maybe I should have said "abort cleanly". The goal is to get the thread to exit. It doesn't need to complete what it was doing, it just needs to make sure that it leaves things in a tidy state so that when it starts up again, it can pick up where it left off. NeilBrown
Attachment:
signature.asc
Description: PGP signature
