On 08/16/2010 03:17 PM, Grant Grundler wrote:
On Sat, Aug 14, 2010 at 9:37 PM, Wilcox, Matthew R
<matthew.r.wilcox@xxxxxxxxx> wrote:
If you will insist on sending to my Outlook address, you'll get replies in standard broken Outlook format. You've been warned.
You trumped my Gmail warning. I fold. :)
---
+/* BUG: Big endian systems need accesses to "id" wrapped with le16_to_cpu().
+ */
No, it's already been converted. See ata_dev_read_id().
Ah - good. I'll remove the comment.
I'm not sure about your use of a switch to set the sector size. Have you checked the code that GCC generates for this?
The switch probably sucks unless we could weight the order of the
tests. E.g. common cases first. But it's just an implementation detail
that is relatively easy to replace with the bitmap you had implemented
before.
All the places you dereference dev->sdev are within a callchain from ->queuecommand; sdev can't possibly go away.
It'd be clearer that this is the case if you used scmd->device->sector_size instead of dev->sdev->sector_size.
Thank you - that looks much better to me too.
--
@@ -516,7 +515,7 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
memset(scsi_cmd, 0, sizeof(scsi_cmd));
if (args[3]) {
- argsize = SECTOR_SIZE * args[3];
+ argsize = ATA_SECT_SIZE * args[3];
argbuf = kmalloc(argsize, GFP_KERNEL);
if (argbuf == NULL) {
rc = -ENOMEM;
I think this is wrong. The ioctl does PIO Data-in; as such, it should use the native sector size, not 512.
That said, there's a possibility of data corruption for programs which use this ioctl, assuming a 512-byte sector size when it's natively 4k.
This one's tricky and needs serious thought. I might error it if sector_size isn't 512 bytes :-)
It's a legacy ioctl anyway, right?
I have no idea. If it's tricky, I probably have it wrong.
Anyone else have guidance here?
The main question is whether the size of a DRQ block changes, when LBA
logical size changes? I need to review the ATA8 specs in this area, but
I would think some interfaces that return 512-byte pages for things like
SMART info would be unchanged. How do the drives behave for
PIO-Data-{In,Out} commands that are not reading/writing user data, but
rather drive metadata?
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
