2007-05-10 12:13:25

by Geert Uytterhoeven

[permalink] [raw]
Subject: HDIO_DRIVE_CMD and hdparm

Hi,

`hdparm -t' uses HDIO_DRIVE_CMD(null) to flush the disk's buffer.
When using it on my own block device (the new PS3 disk storage driver), hdparm
gives the following error message:

| HDIO_DRIVE_CMD(null) (wait for flush complete) failed: Inappropriate ioctl for device

When using it on an ATA or SCSI device, no such error message is printed.

According to the hdparm sources, hdparm expects the HDIO_DRIVE_CMD(null) ioctl
to either succeed, or to fail with errno EINVAL.

Apparently handling of ioctls is different for the different device types:
- ATA/SATA handle HDIO_DRIVE_CMD(null) (and a few other variants)
=> fine for hdparm
- SCSI doesn't handle HDIO_DRIVE_CMD(null), and returns EINVAL
=> fine for hdparm
- If a block device doesn't support the ioctl, blkdev_driver_ioctl() returns
ENOTTY
=> hdparm error message

Which one is correct?
- blkdev_ioctl()/blkdev_driver_ioctl() return -ENOTTY
- scsi_cmd_ioctl() returns -ENOTTY
- scsi_ioctl() returns -EINVAL
- cdrom_ioctl() returns -ENOSYS to mean not handled, continue
- some block layer routines return -ENOIOCTLCMD to mean not handled, continue

This causes constructs with different tests like e.g.:

sr_block_ioctl()
{
...
ret = cdrom_ioctl(...);
if (ret != -ENOSYS)
return ret;
...
return scsi_ioctl(...);
}

cdrom_ioctl()
{
...
ret = scsi_cmd_ioctl(...);
if (ret != -ENOTTY)
return ret;
...
return -ENOSYS;
}

blkdev_ioctl()
{
...
ret = blkdev_locked_ioctl(...);
...
if (ret != -ENOIOCTLCMD)
return ret;

return blkdev_driver_ioctl(...);
}

My questions:
1. Does any of these have to be fixed?
2. Shall I add a dummy HDIO_DRIVE_CMD(null) handler to my block device to
return -EINVAL?
3. Shall I just ignore the hdparm error message?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- Sony Network and Software Technology Center Europe (NSCE)
[email protected] ------- The Corporate Village, Da Vincilaan 7-D1
Voice +32-2-7008453 Fax +32-2-7008622 ---------------- B-1935 Zaventem, Belgium


2007-05-10 12:41:58

by Christoph Hellwig

[permalink] [raw]
Subject: Re: HDIO_DRIVE_CMD and hdparm

On Thu, May 10, 2007 at 02:12:59PM +0200, Geert Uytterhoeven wrote:
> Hi,
>
> `hdparm -t' uses HDIO_DRIVE_CMD(null) to flush the disk's buffer.
> When using it on my own block device (the new PS3 disk storage driver), hdparm
> gives the following error message:
>
> | HDIO_DRIVE_CMD(null) (wait for flush complete) failed: Inappropriate ioctl for device
>
> When using it on an ATA or SCSI device, no such error message is printed.
>
> According to the hdparm sources, hdparm expects the HDIO_DRIVE_CMD(null) ioctl
> to either succeed, or to fail with errno EINVAL.
>
> Apparently handling of ioctls is different for the different device types:
> - ATA/SATA handle HDIO_DRIVE_CMD(null) (and a few other variants)
> => fine for hdparm
> - SCSI doesn't handle HDIO_DRIVE_CMD(null), and returns EINVAL
> => fine for hdparm
> - If a block device doesn't support the ioctl, blkdev_driver_ioctl() returns
> ENOTTY
> => hdparm error message
>
> Which one is correct?
> - blkdev_ioctl()/blkdev_driver_ioctl() return -ENOTTY
> - scsi_cmd_ioctl() returns -ENOTTY
> - scsi_ioctl() returns -EINVAL
> - cdrom_ioctl() returns -ENOSYS to mean not handled, continue
> - some block layer routines return -ENOIOCTLCMD to mean not handled, continue

ENOTTY is the traditional unix errno value for this ioctl is not implemented.
ENOIOCTLCMD is a new fashioned code meaning about the same. In the block
layer the latter should be used as generic code should handlde this.

>
> My questions:
> 1. Does any of these have to be fixed?
> 2. Shall I add a dummy HDIO_DRIVE_CMD(null) handler to my block device to
> return -EINVAL?
> 3. Shall I just ignore the hdparm error message?

I suspect you can just ignore this. Even better send a patch to the hdparm
maintainer to deal with ENOTTY aswell.

2007-05-10 12:48:24

by Alan

[permalink] [raw]
Subject: Re: HDIO_DRIVE_CMD and hdparm

> - SCSI doesn't handle HDIO_DRIVE_CMD(null), and returns EINVAL
> => fine for hdparm
> - If a block device doesn't support the ioctl, blkdev_driver_ioctl() returns
> ENOTTY
> => hdparm error message

Those are both correct
-ENOTTY I don't know this ioctl
-EINVAL I know this ioctl, usage wrong
0 Hey it worked

ENOIOCTLCMD Internal (not user exposed)
for fallthrough

ENOSYS CD-ROM being weird.

2007-05-10 13:14:14

by Mark Lord

[permalink] [raw]
Subject: Re: HDIO_DRIVE_CMD and hdparm

Alan Cox wrote:
>> - SCSI doesn't handle HDIO_DRIVE_CMD(null), and returns EINVAL
>> => fine for hdparm
>> - If a block device doesn't support the ioctl, blkdev_driver_ioctl() returns
>> ENOTTY
>> => hdparm error message
>
> Those are both correct
> -ENOTTY I don't know this ioctl
> -EINVAL I know this ioctl, usage wrong

Exactly.
I'll have hdparm-7.4 not complain on ENOTTY as well.

-ml

2007-05-10 13:20:42

by Mark Lord

[permalink] [raw]
Subject: Re: HDIO_DRIVE_CMD and hdparm

Geert Uytterhoeven wrote:
> Hi,
>
> `hdparm -t' uses HDIO_DRIVE_CMD(null) to flush the disk's buffer.

More correctly, that command is supposed to act like an I/O queue "barrier"
operation, not returning from the syscall until everything queued in front
of it has been issued/completed.

I believe that only the original IDE driver actually implements it, though.
And hdparm-7.4 (not released yet) will no longer complain about ENOTTY.

Note that current versions of hdparm use SG_IO/ATA_16 (SAT) for nearly everything
now, only falling back to the older ioctl's for drivers which reject the SAT attempt.

I'd love to find a USB drive enclosure that supports SAT.
Anyone know of one?

And does the USB storage layer actually pass the ATA_16 packets to the device?

Cheers