RE: [PATCH 1/2] I/OAT: Add watchdog/reset functionality to ioatdma driver

[Sosnowski, Maciej]

Dan Williams wrote:
> On Fri, Jul 11, 2008 at 4:36 AM, Maciej Sosnowski
> <maciej.sosnowski@xxxxxxxxx> wrote:
>> Due to occasional DMA channel hangs observed for I/OAT versions 1.2
>> and 2.0 a watchdog has been introduced to check every 2 seconds
>> if all channels progress normally.
>> If stuck channel is detected, driver resets it.
>> The reset is done in two parts. The second part is scheduled
>> by the first one to reinitialize the channel after the restart.
>> 
> 
> checkpatch had some valid suggestions, please give it a run.

Hi Dan,
Thanks a lot for your review.
I have just sent v2 of this patch series updated according to your
suggestions.
This includes resolving checkpatch issues.

> A general comment about this change it seems to add a bit of
> complexity.  The driver now has two workqueue routines and a tasklet.
> Might some of these be combined?  In other words, do we need a free
> running watchdog when no descriptors are in flight?  It seems that
> using a timer to kick the cleanup tasklet could be used in place of
> the ioat_dma_chan_watchdog workqueue.
 
The idea was to run watchdog not too frequently 
in order to minimize its impact on the performance.
That's why it is called every 2 secs.
Cleanup is a differnt story and it is called more frequently,
so it would be difficult to combine these two things.
BTW - cleanup tasklet is not used at the moment
(TCP does not use interrupts but polling for cleanups).

>> Sleep time in self_test has been increased to 10ms
>> since in some cases 1ms appeared not to be enough.
> 
> This should be broken out into a separate patch.  Perhaps with an
> explanation of why it needs extra time for this particular
> transaction.

It has been separated in v2.

> 
> Regards,
> Dan

Thanks,
Maciej
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/