From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: Drives missing at boot Date: Mon, 19 Jul 2010 23:01:54 +0200 Message-ID: <4C44BD42.3030904@kernel.org> References: <4C2F5ECB.1040505@kernel.org> <4C2F61AB.9030806@kernel.org> <4C317C04.20500@kernel.org> <4C32CE50.6010501@kernel.org> <4C341598.6040004@kernel.org> <4C34A1D5.1090202@kernel.org> <4C34B8BE.5080504@kernel.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------090206080000010303060208" Return-path: Received: from hera.kernel.org ([140.211.167.34]:51939 "EHLO hera.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966763Ab0GSVB4 (ORCPT ); Mon, 19 Jul 2010 17:01:56 -0400 In-Reply-To: Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Mark Knecht Cc: Linux Kernel List , "linux-ide@vger.kernel.org" This is a multi-part message in MIME format. --------------090206080000010303060208 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hello, On 07/19/2010 09:31 PM, Mark Knecht wrote: > With about 10-12 day of testing, 1-2 boots/day, I've not had a > single boot failure since adding the patch. Only twice has it said > tries=2. Every other time it's tries=1. The machine seems to work fine > either way. Hmmm... can you please test the attached patch instead? It seems likely that the root cause is not flakiness of SIDPR but incorrect locking in libata EH code. Thanks. -- tejun --------------090206080000010303060208 Content-Type: text/x-patch; name="ata_piix-sidpr-lock.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="ata_piix-sidpr-lock.patch" diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c index 7409f98..3971bc0 100644 --- a/drivers/ata/ata_piix.c +++ b/drivers/ata/ata_piix.c @@ -158,6 +158,7 @@ struct piix_map_db { struct piix_host_priv { const int *map; u32 saved_iocfg; + spinlock_t sidpr_lock; /* FIXME: remove once locking in EH is fixed */ void __iomem *sidpr; }; @@ -951,12 +952,15 @@ static int piix_sidpr_scr_read(struct ata_link *link, unsigned int reg, u32 *val) { struct piix_host_priv *hpriv = link->ap->host->private_data; + unsigned long flags; if (reg >= ARRAY_SIZE(piix_sidx_map)) return -EINVAL; + spin_lock_irqsave(&hpriv->sidpr_lock, flags); piix_sidpr_sel(link, reg); *val = ioread32(hpriv->sidpr + PIIX_SIDPR_DATA); + spin_unlock_irqrestore(&hpriv->sidpr_lock, flags); return 0; } @@ -964,12 +968,15 @@ static int piix_sidpr_scr_write(struct ata_link *link, unsigned int reg, u32 val) { struct piix_host_priv *hpriv = link->ap->host->private_data; + unsigned long flags; if (reg >= ARRAY_SIZE(piix_sidx_map)) return -EINVAL; + spin_lock_irqsave(&hpriv->sidpr_lock, flags); piix_sidpr_sel(link, reg); iowrite32(val, hpriv->sidpr + PIIX_SIDPR_DATA); + spin_unlock_irqrestore(&hpriv->sidpr_lock, flags); return 0; } @@ -1566,6 +1573,7 @@ static int __devinit piix_init_one(struct pci_dev *pdev, hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL); if (!hpriv) return -ENOMEM; + spin_lock_init(&hpriv->sidpr_lock); /* Save IOCFG, this will be used for cable detection, quirk * detection and restoration on detach. This is necessary --------------090206080000010303060208--