On Thu, Feb 25, 2016 at 10:48:45AM -0800, Dan Williams wrote: > On Thu, Feb 25, 2016 at 10:42 AM, Shaohua Li <shli@xxxxxxxxxx> wrote: > > On Thu, Feb 25, 2016 at 05:05:17PM +0100, Artur Paszkiewicz wrote: > >> On 02/25/2016 02:17 AM, Shaohua Li wrote: > >> > On Thu, Feb 25, 2016 at 11:31:04AM +1100, Neil Brown wrote: > >> >> On Thu, Feb 25 2016, Shaohua Li wrote: > >> >> > >> >>> > >> >>> As for the bug, write requests run in raid5d, mddev_suspend() waits for all IO, > >> >>> which waits for the write requests. So this is a clear deadlock. I think we > >> >>> should delete the check_reshape() in md_check_recovery(). If we change > >> >>> layout/disks/chunk_size, check_reshape() is already called. If we start an > >> >>> array, the .run() already handles new layout. There is no point > >> >>> md_check_recovery() check_reshape() again. > >> >> > >> >> Are you sure? > >> >> Did you look at the commit which added that code? > >> >> commit b4c4c7b8095298ff4ce20b40bf180ada070812d0 > >> >> > >> >> When there is an IO error, reshape (or resync or recovery) will abort > >> >> and then possibly be automatically restarted. > >> > > >> > thanks pointing out this. > >> >> Without the check here a reshape might be attempted on an array which > >> >> has failed. Not sure if that would be harmful, but it would certainly > >> >> be pointless. > >> >> > >> >> But you are right that this is causing the problem. > >> >> Maybe we should keep track of the size of the 'scribble' arrays and only > >> >> call resize_chunks if the size needs to change? Similar to what > >> >> resize_stripes does. > >> > > >> > yep, this is my first solution, but think check_reshape() is useless here > >> > later, apparently miss the restart case. I'll go this way. > >> > >> My idea was to replace mddev_suspend()/mddev_resume() in resize_chunks() > >> with a rw lock that would prevent collisions with raid_run_ops(), since > >> scribble is used only there. But if the parity operations are executed > >> asynchronously this would also need to wait until all the submitted > >> operations have completed. Seems a bit overkill, but I came up with > >> this: > > > > Looks it should work, but it's overkill indead, especially the extra lock, we > > can replace it with srcu though. The 'track scribble array size' is much > > simpler, so I'd prefer that way. In the future, we probably should move > > resize_stripes()/resize_chunks() to .start_reshape(). > > resize_stripes()/resize_chunks() sounds not qualified as .check_reshape(). > > > > Any time any linux-raid mail mentions the raid5_run_ops infrastructure > I am prompted to remind that async_tx needs to die and be up leveled > to md directly. The "help wanted" request is still pending. A quick search shows async_tx has another user: exofs -- 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
