| Message ID | 20091201070833.CC084B7BD9@ozlabs.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | f0353813afe784330622596ff141e7525ccd99de |
| Headers | show |
On 12/01/2009 04:08 PM, Benjamin Herrenschmidt wrote: > In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop() > instead of ap->ops->bmdma_stop(). This can be a problem for controllers > that use their own bmdma_stop for which the generic sff one isn't suitable > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Oh... that's a scary bug lurking around. Thanks for catching it. Acked-by: Tejun Heo <tj@kernel.org>
On Tue, 2009-12-01 at 16:25 +0900, Tejun Heo wrote: > On 12/01/2009 04:08 PM, Benjamin Herrenschmidt wrote: > > In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop() > > instead of ap->ops->bmdma_stop(). This can be a problem for controllers > > that use their own bmdma_stop for which the generic sff one isn't suitable > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > Oh... that's a scary bug lurking around. Thanks for catching it. > > Acked-by: Tejun Heo <tj@kernel.org> Feel free to pick that one up earlier if you want (ie for 2.6.32), as long as we manage to get it in in 2.6.33 -before- I push powerpc-next to Linus, I'm happy :-) (or we can have it in both trees). Cheers, Ben.
On 12/01/2009 04:29 PM, Benjamin Herrenschmidt wrote: > On Tue, 2009-12-01 at 16:25 +0900, Tejun Heo wrote: >> On 12/01/2009 04:08 PM, Benjamin Herrenschmidt wrote: >>> In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop() >>> instead of ap->ops->bmdma_stop(). This can be a problem for controllers >>> that use their own bmdma_stop for which the generic sff one isn't suitable >>> >>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> >> Oh... that's a scary bug lurking around. Thanks for catching it. >> >> Acked-by: Tejun Heo <tj@kernel.org> > > Feel free to pick that one up earlier if you want (ie for 2.6.32), as > long as we manage to get it in in 2.6.33 -before- I push powerpc-next to > Linus, I'm happy :-) (or we can have it in both trees). As nasty as the bug might be, given that we haven't had too much problem with that, I think it would be best to give it some time to be tested before releasing it to users. It causes behavior differences for all bmdma drivers which implement custom bmdma_stop. It's an apparent bug fix but well the nasty ones are always apparent bug fixes, right? :-) Thanks.
On Tue, 2009-12-01 at 16:33 +0900, Tejun Heo wrote: > > Feel free to pick that one up earlier if you want (ie for 2.6.32), > as > > long as we manage to get it in in 2.6.33 -before- I push > powerpc-next to > > Linus, I'm happy :-) (or we can have it in both trees). > > As nasty as the bug might be, given that we haven't had too much > problem with that, I think it would be best to give it some time to be > tested before releasing it to users. It causes behavior differences > for all bmdma drivers which implement custom bmdma_stop. It's an > apparent bug fix but well the nasty ones are always apparent bug > fixes, right? :-) Yeah, I agree. I wouldn't be comfortable pushing it into .32 to be honest after a quick grep of how many drivers actually use that hook and considering that no problem was reported so far... I'll merge it with the rest of the powerpc stuff during the .33 merge window if you are ok with that. Cheers, Ben.
--- linux-work.orig/drivers/ata/libata-sff.c 2009-12-01 17:48:27.000000000 +1100 +++ linux-work/drivers/ata/libata-sff.c 2009-12-01 17:48:48.000000000 +1100 @@ -2384,7 +2384,7 @@ void ata_sff_post_internal_cmd(struct at ap->hsm_task_state = HSM_ST_IDLE; if (ap->ioaddr.bmdma_addr) - ata_bmdma_stop(qc); + ap->ops->bmdma_stop(qc); spin_unlock_irqrestore(ap->lock, flags); }
In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop() instead of ap->ops->bmdma_stop(). This can be a problem for controllers that use their own bmdma_stop for which the generic sff one isn't suitable Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- drivers/ata/libata-sff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)