Re: [PATCH] MD: Add del_timer_sync to mddev_suspend (fix nasty panic)

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



On May 16, 2012, at 7:50 PM, NeilBrown wrote:

On Tue, 15 May 2012 23:06:14 -0500 Jonathan Brassow <jbrassow@xxxxxxxxxx>
wrote:

Neil,

I've been seeing some really bad panics take place on dm-raid.c.  I've
found that it is because the mddev->safemode_timer is firing after the
mddev structure has been freed.  I've attached a patch to fix the
problem below, but I have some questions (outlined in the patch header).
I also have a debugging patch that prints something during each of the
suspend stages and when md_write_end resets the timer so that you can
see the problem in action - let me know if you want that patch also.

brassow

Hi Jon,
thanks for the patch.  It looks simple and can clearly fix a problem
so at this point in the cycle I propose to submit it to Linus as-is, even
though I'm not convinced it is perfect, and you didn't give me a s-o-b line.
See more blow.

Thanks Neil.

<pet peeve>.
No Signed-off-by: line?  why is that.

"Signed-off-by" *only* means "I certify that I have any necessary right to
submit this patch, and I agree to it being used in the way that all other
code in this project can be used" - only with more words.  See the Developer's
Certificate of Origin 1.1

Refusing to add a Signed-off-by: because you don't think the code is "ready"
yet in some sense is just plain wrong.  I never ever want to even see a patch
that doesn't have Signed-off-by, because I don't know if I've been given
permission to use it.

Certainly add an 'RFC-by' if you want to say something about the quality of
the patch, but don't for that reason exclude the signed-off-by
</pet peeve>

"RFC-by" must be some overly clever thing I thought up or saw at one point.  i can certainly break that habit.  I had seen that possibly setting 'mddev->safemode = 2' might be a possible solution.  I was also a bit confused if I should pull the 'del_timer_sync' from 'md_stop_writes'.  This is why I put the RFC in there, but I see your meaning.

Thanks,
 brassow
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel

[DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]     [Fedora Docs]

Add to Google Powered by Linux