From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: gdth new set of patches for 2.6.24 stable Date: Sun, 17 Feb 2008 11:24:51 -0600 Message-ID: <1203269091.3082.47.camel@localhost.localdomain> References: <47B48CAE.2020705@panasas.com> <47B864CB.4040604@panasas.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: Received: from accolon.hansenpartnership.com ([76.243.235.52]:48703 "EHLO accolon.hansenpartnership.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752653AbYBQRZD (ORCPT ); Sun, 17 Feb 2008 12:25:03 -0500 In-Reply-To: <47B864CB.4040604@panasas.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Boaz Harrosh Cc: Tobias Oetiker , allied internet ag- Stefan Priebe , Jon Chelton , Andrew Morton , Dave Milter , Jeff Garzik , Christoph Hellwig , linux-scsi , David Brownell , Greg Kroah-Hartman On Sun, 2008-02-17 at 18:46 +0200, Boaz Harrosh wrote: > On Thu, Feb 14 2008 at 20:47 +0200, Boaz Harrosh wrote: > > Submitted are a new set of patches, that fix lots of problems > > with the gdth driver. > > > > It fixes the following problems: > > - scan for drives on hosts. (Already in mainline) > > - truly fixes the exit/reboot problems but does call flush() before > > reboot. > > - fix crash when accessing array with icpcon management application. > > - fix crash when doing $ cat /proc/sys/gdth/0. > > This one still has the below WARN_ON in messages (see below) > > So there is one more thing hiding in there. > > - use pci_get_device > > One of the testers requested if we can also put the move to pci_get_device > > patch with removal of dependency on PCI_LEGACY, to the stable release. > > > > The patches are for and based on Linux-2.6.24. here is the list of patches: > > [PATCH 1/5] gdth: update deprecated pci_find_device > > [PATCH 2/5] gdth: scan for scsi devices > > [PATCH 3/5] gdth: bugfix for the at-exit problems > > [PATCH 4/5] gdth: fix to internal commands execution > > [PATCH 5/5] gdth: remove gdth cooked up command accessors > > > > Please all test and report your findings. > > > > Thanks in advance > > Boaz > > > > --- > > > > WARNING: at arch/x86/kernel/pci-dma_32.c:66 dma_free_coherent() > > Pid: 5501, comm: cat Not tainted 2.6.24 #43 > > [] dma_free_coherent+0x93/0x95 > > [] gdth_ioctl_free+0x4c/0x69 > > [] gdth_proc_info+0x165f/0x182c > > [] update_curr+0xeb/0xf2 > > [] task_rq_lock+0x29/0x50 > > [] try_to_wake_up+0x42/0x342 > > [] try_to_wake_up+0x42/0x342 > > [] __wake_up_common+0x46/0x6d > > [] __wake_up+0x32/0x42 > > [] n_tty_receive_buf+0x2e8/0xe97 > > [] n_tty_receive_buf+0x2e8/0xe97 > > [] update_curr+0x7b/0xf2 > > [] enqueue_task_fair+0x27/0x30 > > [] enqueue_task+0xa/0x14 > > [] proc_scsi_read+0x29/0x3d > > [] proc_scsi_read+0x0/0x3d > > [] proc_file_read+0x1c6/0x279 > > [] proc_file_read+0x0/0x279 > > [] proc_reg_read+0x53/0x71 > > [] proc_reg_read+0x0/0x71 > > [] vfs_read+0x85/0x11b > > [] sys_read+0x41/0x6a > > [] sysenter_past_esp+0x5f/0x85 > > > > - > James hi. > > All my testers have reported back that with these 5 patches applied they can > now run with a 2.6.24 kernel the same way they ran before. However there is > that reported issue, with the dma_free_coherent WARN_ON (above). The code was > like that from day one and it is a very old issue, however it is a regression > because 2.6.24 introduced that new WARN_ON. > (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba) > >From posts on lkml and even recent one in linux-scsi about the arcmsr driver > it looks that all a driver can do is work around it with different kernel mechanisms > and driver rewrites. I'm afraid I need your help here. I'm not sure I understand > why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and what > is needed to replace it. Could you please have a look in gdth_proc.c and also in > gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and advise > what can I do in it's place. Please bear in mind that we need it for 2.6.24, as > a bugfix. > > Apart from the above issue, please accept patches 3,4,5 above they have now > been tested and are reported to bring broken system back to production. > (Given that you approve off course). And mark them for inclusion to the > 2.6.24 stable releases. (Or is there some thing that I should do) > > --- > Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not > pose any harm. Some people have reported stability with temporarily disabling > it. For testers that want to try, here it is below. At your own risk. Isn't this the correct fix? pscratch is a permanent address (it's allocated at boot time and never changes). All you need the smp lock for is mediating the scratch in use flag. James diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c index de57734..ce0228e 100644 --- a/drivers/scsi/gdth_proc.c +++ b/drivers/scsi/gdth_proc.c @@ -694,15 +694,13 @@ static void gdth_ioctl_free(gdth_ha_str *ha, int size, char *buf, ulong64 paddr) { ulong flags; - spin_lock_irqsave(&ha->smp_lock, flags); - if (buf == ha->pscratch) { + spin_lock_irqsave(&ha->smp_lock, flags); ha->scratch_busy = FALSE; + spin_unlock_irqrestore(&ha->smp_lock, flags); } else { pci_free_consistent(ha->pdev, size, buf, paddr); } - - spin_unlock_irqrestore(&ha->smp_lock, flags); } #ifdef GDTH_IOCTL_PROC