Hi Stefan, Yes, you are right that the up() is used in 3 places and down_trylock, down_interruptible in one place each, but we need to do other changes like when we return a value ERESTATSYS from that particular function, the value will be checked in many places to take appropriate action and fibptr->done also checked in some other places. Considering all these, I mentioned that we need to modify the code in lot places. I hope you understand. Thanks, Narasimha Reddy -----Original Message----- From: Stefan Richter [mailto:stefanr@xxxxxxxxxxxxxxxxx] Sent: Wednesday, November 04, 2009 12:33 AM To: Penchala Narasimha Reddy Chilakala, TLS-Chennai Cc: James Bottomley; 'linux-scsi@xxxxxxxxxxxxxxx'; ServeRAID Driver Subject: Re: [PATCH ] scsi-misc-2.6: File System going into read-only mode 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/ DISCLAIMER: ----------------------------------------------------------------------------------------------------------------------- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or HCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any mail and attachments please check them for viruses and defect. ----------------------------------------------------------------------------------------------------------------------- -- 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
