aacraid: try to fix internal locking versus bus_dma locking

[avg]

Summary

There were many witness warnings involving bus_dma.
Not entirely sure about the correctness of the fix, but haven't seen
any regressions yet.

Test Plan

Tested with (ancient) Adaptec 5405.

[sbruno]

It looks like a lot of this code is a duplicate of D9900? Or am I mistaken?

[avg]

In D9901#204661, @sbruno wrote:

It looks like a lot of this code is a duplicate of D9900? Or am I mistaken?

Yes, most of it.
I tried to create a "stacked" review on top of D9900, but I failed and all the changes from D9900 are also in this review. Not sure how to do it properly.

[avg]

extract only the main change and drop unrelated changes

[scottl]

sys/dev/aacraid/aacraid.c

1252

I would move the lock/unlock to be outside of the for loop. That way you don't needlessly thrash the lock. If you can't do that because of the bus_dmamap_create() call, then create a second loop for calling aacraid_release_command() and do the locking around that loop.

[avg]

sys/dev/aacraid/aacraid.c

1252

Yes, the lock can not be held for the whole loop (in its current form) because of bus_dmamap_create.
Implementing your suggestion would make the code more complex, but would it really make things any better?
aac_alloc_commands should be called rather rarely, so I don't see that lock as something to worry about.
bus_dmamap_create is also relatively expensive anyway.

But if you insist I will split out the second loop.

[avg]

sys/dev/aacraid/aacraid.c

1229

@scottl I have a question about this line that my patch does not change.
Should we really advance fm->aac_commands as we iterate the loop?
Or should fm->aac_commands be set before the loop?
I think that it is supposed to point to the start of the commands being allocated, not to the end.

On the other hand, I don't see that field really being used besides allocation and deallocation.

[sbruno]

@scottl Can you respond to @avg here?

[avg]

Dear reviewers, would you prefer me to commit the change as is or to abandon it altogether?
Sorry for not offering any other option.