Penchala Narasimha Reddy Chilakala, TLS-Chennai wrote:
> Hi Stefan,
>
> Thanks a lot for your suggestion and feedback.
>
> Aacraid driver has been using down_interruptible () and up () for
> quite some time. We will take your suggestion into consideration
> and keep in our mind. But it is quite difficult to implement your
> suggestion at this point of time as we need to modify the code in
> lot places and we need to test a lot.
dpcsup.c: 2x up(&fib->event_wait)
commsup.c: allocation and initialization of the semaphore
1x down_trylock(&fibptr->event_wait) +
1x down_interruptible(&fibptr->event_wait) + up()
1x up(&fib->event_wait)
So there are just two places which conditionally wait for completion (or
just one place but with a differentiation between sync and async mode),
and three places which signal "done".
You are right that this would be a non-trivial change and will require
respective testing. However, (1.) only the place which waits for
completion is the non-trivial part (because it's currently coded in a
rather obfuscated way), (2.) I expect that the result would be cleaner
code which is (3.) more obvious to be correct and (4.) can also be
checked for correctness by the lockdep facility --- this is not possible
with legacy semaphores.
Anyway; it's up to those who use or maintain this driver. I for one
converted another subsystem away from two or three different misused
semaphores to more appropriate APIs and it wasn't too hard; but I can't
do this for aacraid since I don't have hardware to test.
(In my rather irrelevant opinion, whatever you decide for now, just do
_not_ do this here:
>>> we can replace the above code with either
>>>
>>> } else if (down_interruptible (&fibptr->event_wait));
>>>
>>> Or
>>>
>>> } else {
>>> if (down_interruptible (&fibptr->event_wait))
>>> ;
>>> }
because it can really hurt in the long run if sensible warnings are hidden.)
--
Stefan Richter
-=====-==--= =-== ---==
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
