| Message ID | 1364558095.14860.59.camel@i7.infradead.org |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hi David, > Fix this by using ap->sector_buf instead of a stack buffer. > > --- > I'm somewhat ignorant, but it looks like this is kind of thing is what > ap->sector_buf is *for*, right? And in ata_dev_configure() nobody else > is likely to be using it? Your fix is fine with me, thanks. I didn't notice this warning since CONFIG_DMA_API_DEBUG was not set here. Regards, Shane
On Fri, 2013-03-29 at 11:54 +0000, David Woodhouse wrote: > Commit 803739d25c2343da6d2f95eebdcbc08bf67097d4 ("[libata] replace > sata_settings with devslp_timing"), which was also Cc: stable, used a > stack buffer to receive data from ata_read_log_page()... That commit also changes from using ata_id_has_ncq() to ata_id_has_devslp() as the condition for reading the SATA settings log page (log 0x30 page 0x08), because of compatibility issues with some drives which *do* implement NCQ but not DEVSLP. It's not entirely clear to me that this change was correct. Even if the drive does support DEVSLP, the timings in the SATA settings log page are optional, and we're supposed to use default values if they are not provided. I can imagine some DEVSLP-capable drives not having that page either. Shouldn't we be reading page zero of the 0x30 log, which *is* mandatory, and looking there to see which pages are supported?
On 03/29/2013 07:54 AM, David Woodhouse wrote: > Commit 803739d25c2343da6d2f95eebdcbc08bf67097d4 ("[libata] replace > sata_settings with devslp_timing"), which was also Cc: stable, used a > stack buffer to receive data from ata_read_log_page(), which triggers > the following warning: > ahci 0000:00:1f.2: DMA-API: device driver maps memory fromstack [addr=ffff880140469948] > > Fix this by using ap->sector_buf instead of a stack buffer. > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> > Cc: stable@kernel.org applied > I'm somewhat ignorant, but it looks like this is kind of thing is what > ap->sector_buf is *for*, right? And in ata_dev_configure() nobody else > is likely to be using it? yep -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 497adea..b1160ee 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2329,7 +2329,7 @@ int ata_dev_configure(struct ata_device *dev) * from SATA Settings page of Identify Device Data Log. */ if (ata_id_has_devslp(dev->id)) { - u8 sata_setting[ATA_SECT_SIZE]; + u8 *sata_setting = ap->sector_buf; int i, j; dev->flags |= ATA_DFLAG_DEVSLP;
Commit 803739d25c2343da6d2f95eebdcbc08bf67097d4 ("[libata] replace sata_settings with devslp_timing"), which was also Cc: stable, used a stack buffer to receive data from ata_read_log_page(), which triggers the following warning: ahci 0000:00:1f.2: DMA-API: device driver maps memory fromstack [addr=ffff880140469948] Fix this by using ap->sector_buf instead of a stack buffer. Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> Cc: stable@kernel.org --- I'm somewhat ignorant, but it looks like this is kind of thing is what ap->sector_buf is *for*, right? And in ata_dev_configure() nobody else is likely to be using it?