From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomas Henzl Subject: Re: PATCH 1/5] scsi: megaraid_sas - Add Online Controller Reset to MegaRAID SAS drive Date: Fri, 08 Oct 2010 17:51:32 +0200 Message-ID: <4CAF3E04.1050304@redhat.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: bo yang Cc: linux-scsi@vger.kernel.org, akpm@osdl.org, linux-kernel@vger.kernel.org, James.Bottomley@suse.de, bo.yang@lsi.com List-Id: linux-scsi@vger.kernel.org On 09/23/2010 04:36 AM, bo yang wrote: > This patch is too big. I am using attachment to submit. Please > use attached file to apply. Also let me know if it can't be accepted. > > To add the Online controller reset support, driver need to do: > a). reset the controller chips -- Xscale and Gen2 which will change > the function calls and add the reset function related to this two > chips. > b). during the reset, driver will store the pending cmds which not > returned by FW to driver's pending queue. Driver will re-issue those > pending cmds again to FW after the OCR finished. > c). In driver's timeout routine, driver will report to OS as reset. > Also driver's queue routine will block the cmds until the OCR > finished. > d). in Driver's ISR routine, if driver get the FW state as state > change, FW in Failure status and FW support online controller > reset (OCR), driver will start to do the controller reset. > e). In driver's IOCTL routine, the application cmds will wait for the > OCR to finish, then issue the cmds to FW. > > Signed-off-by Bo Yang > > --- > drivers/scsi/megaraid/megaraid_sas.c | 756 ++++++++++++++++++++++++++++++++--- > drivers/scsi/megaraid/megaraid_sas.h | 88 +++- > 2 files changed, 787 insertions(+), 57 deletions(-) Hi Bo, in the workqueue function you sleep for 30s, it's scheduled here - schedule_work(&instance->work_init); +process_fw_state_change_wq(struct work_struct *work) +{ ... + /*waitting for about 20 second before start the second init*/ + for (wait = 0; wait < 30; wait++) { + msleep(1000); + } - this is not a good practice to sleep for a so long time I think - you should use in your exit function some synchronization for example 'cancel_work_sync', without that if someone rmmods your module, it could then lead to a memory corruption Tomas