Re: [PATCH 13/13] libata: use PIO for misc ATAPI commands

[Tejun Heo <htejun@gmail.com>]

Hello, Alan.

Alan Cox wrote:
> On Wed, 28 Nov 2007 00:13:59 +0900
> Tejun Heo <htejun@gmail.com> wrote:
> 
>> ATAPI devices come with plethora of bugs and many ATA controllers have
>> trouble dealing with odd byte DMA transfers.  The problem is currently
>> somewhat masked by not allowing DMA transfers if the transfer size
>> isn't aligned to 16 bytes plus partial masking in problematic LLDs.
> 
> NAK. Same complaint I had originally and which is not addressed.

Heh, yeah, I expected NACK from you and my argument is the same.  I'll
just repeat one more time for the record as the other thread didn't cc
linux-ide.  :-)

> The whole approach being used here is fundamentally in the wrong place.
> Its also horribly damaging because of our PIO behaviour currently.
> 
> - Until our PIO works without IRQ masking you can't take this approach for
> arbitary devices without hurting users badly.

That definitely is on to do list.  I wonder Jeff is still against
turning off IRQ from the IRQ controller?

Also, even if it's being done from interrupt handler, the data transfer
is usually very short (tens of bytes).  I hardly feel any difference.
Also, let's not forget there are already drivers which do things this
way and yet others for which PIO or DMA doesn't matter at all.

> - I still see no evidence in the drives I've looked at that all of this
> is drive side (or indeed that most of it is)

Of the ten drives I tested, only two were problematic but then again I
didn't try with the 16 byte alignment test lifted.  What I'm worried
about is test coverage.  The 16 byte alignment made us avoid some
problem cases (Albert, do you recall which?) but the condition itself is
problematic because it partially masks actual problems (dumb luck (tm)).
 Plus, different drivers use different masking rules, so debugging
becomes difficult as the behavior depends on the combination of drive
and controller.

Because we can't use DMA uniformly, I went the other way and tried to
use PIO uniformly.  Another important point is that the other OS which
is probably the only platform most ATAPI device vendors test against use
PIO for these commands.

> - Control over the DMA/PIO strategy belongs with the driver. If we mess
> that up it will cost us dearly later.

And I don't get what we'll lose.  Care to deliberate?

> What we should be doing IMHO is
> 
> -	Fix the PIO IRQ masking
> -	Creating ata_std_check_atapi_dma()
> -	Let drivers select between the std_check_atapi_dma() and their
> own rules
> 
> Finally ata_std_check_atapi_dma should be blacklist based for all but the
> really generic issues (%15 etc), otherwise we punish the 99.999% of
> systems that work perfectly well for the sake of a tiny number that are
> problematic.

My argument is... Yes, we punish other systems, but the amount of
punishment is not substantial and definitely negotiable with gained
behavior uniformity between drivers and with the other OS.

Note1: If anybody can show me what we'll actually lose by forcing PIO,
I'm sold.

Note2: Let's say we fixed PIO IRQ masking.  Would that change your opinion?

Thanks.

-- 
tejun