svn commit: r354703 - head/sys/dev/ioat

[Scott Long]

> On Nov 15, 2019, at 2:18 AM, Konstantin Belousov <kostikbel at gmail.com> wrote:
> 
> On Thu, Nov 14, 2019 at 09:41:36AM -0500, Alexander Motin wrote:
>> On 14.11.2019 05:11, Konstantin Belousov wrote:
>>> On Thu, Nov 14, 2019 at 04:39:49AM +0000, Alexander Motin wrote:
>>>> Author: mav
>>>> Date: Thu Nov 14 04:39:48 2019
>>>> New Revision: 354703
>>>> URL: https://svnweb.freebsd.org/changeset/base/354703
>>>> 
>>>> Log:
>>>>  Pass more reasonable WAIT flags to bus_dma(9) calls.
>>>> 
>>>>  MFC after:	2 weeks
>>>> 
>>>> Modified:
>>>>  head/sys/dev/ioat/ioat.c
>>>> 
>>>> Modified: head/sys/dev/ioat/ioat.c
>>>> ==============================================================================
>>>> --- head/sys/dev/ioat/ioat.c	Thu Nov 14 04:34:58 2019	(r354702)
>>>> +++ head/sys/dev/ioat/ioat.c	Thu Nov 14 04:39:48 2019	(r354703)
>>>> @@ -555,13 +555,14 @@ ioat3_attach(device_t device)
>>>> 	    &ioat->comp_update_tag);
>>>> 
>>>> 	error = bus_dmamem_alloc(ioat->comp_update_tag,
>>>> -	    (void **)&ioat->comp_update, BUS_DMA_ZERO, &ioat->comp_update_map);
>>>> +	    (void **)&ioat->comp_update, BUS_DMA_ZERO | BUS_DMA_WAITOK,
>>>> +	    &ioat->comp_update_map);
>>> For waitok, you need to provide locking function in the tag.
>> 
>> I'm sorry, but why?  It is alloc(), not load().  For load() it makes
>> sense since it calls back, but this is just a form of malloc(), isn't
>> it?  I've looked through the both x86 implementations and found nothing
>> suspicious.
> 
> I see.  Still, if you (or somebody else) ever decided to use WAITOK loads, 
> it would be much safer to provide the lock function now.
> 

Loads are always non-blocking, and the WAITOK flag is not part of that
API.  A load may defer its callback, and the asynchronous execution of that
callback is why we have the loaned lock.  If a blocking alloc is performed in
a context where the caller holds a lock, then it’s the caller’s responsibility
to indicate NOWAIT and deal with the possible failure.  Just like with
malloc and contigmalloc, the API does not, nor will it ever, drop and
require locks on behalf of the caller.  The API tried to enforce the practice
that static resource allocation should happen at initialization time when
locks don’t need to be held.

It’s unfortunate that the NOWAIT flag is overloaded for different meanings
in the load functions vs the alloc functions.  Maybe this is what is causing
confusion?

TL;DR: ALexander’s change is correct.

Scott