Skip to contentSkip to conversation

libata: BLKFLSBUF ioctl broken

DISCUSSION Mark Lord 5 participants 12 messages Oct 21, 2004 – Oct 25, 2004

Subsystems (2)
SCSI Block layer
lore
Share & tools
lore

Mark Lord

author
#0 lore raw

Like the subject line says.. BLKFLSBUF (and friends) are broken for SATA. I'm getting tons of emails about this now that 2.6.9 is out.

Jeff: any chance of you fixing this soon?

Cheers

Signature · 2 lines
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")
#1 lore raw
Quoted 4 lines · Mark Lord

Like the subject line says.. BLKFLSBUF (and friends) are broken for SATA. I'm getting tons of emails about this now that 2.6.9 is out.

Jeff: any chance of you fixing this soon?

More details... define broken.

libata doesn't handle BLKFLSBUF directly, and never has, so nothing changed in that regard for 2.6.9.

Jeff

author
#2 lore raw

Try issuing a BLKFLSBUF to a SATA drive.
As of 2.6.9 this apparently returns EOPNOTSUPP to userspace.

I don't have any libata controllers here to try it with.

Signature · 2 lines
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")
#3 lore raw
Quoted 2 lines · Mark Lord

Try issuing a BLKFLSBUF to a SATA drive.
As of 2.6.9 this apparently returns EOPNOTSUPP to userspace.

Ok, I think I see what's going on. Can you get your users to try the attached patch?

Maybe libata should be returning ENOTTY, but that's _quite_ nonsensical considering that EOPNOTSUPP describes the error much better (and is a standard I-don't-support-that-operation return code in tons of other Linux ioctl-handling code).

Quoted 1 line

I don't have any libata controllers here to try it with.

You mean you don't have an Intel PATA controller lying around anywhere??? :) To enable Intel PATA controllers, set the ATA_ENABLE_PATA define in include/linux/libata.h, and recompile the ata_piix driver:

#undef ATA_DEBUG                /* debugging output */
#undef ATA_VERBOSE_DEBUG        /* yet more debugging output */
#undef ATA_IRQ_TRAP             /* define to ack screaming irqs */
#undef ATA_NDEBUG               /* define to disable quick runtime checks */
#undef ATA_ENABLE_ATAPI         /* define to enable ATAPI support */
#undef ATA_ENABLE_PATA          /* define to enable PATA support in some

* low-level drivers */
#undef ATAPI_ENABLE_DMADIR /* enables ATAPI DMADIR bridge support */

===== drivers/block/ioctl.c 1.62 vs edited =====

drivers/block/ioctl.c +2 -1
@@ -195,7 +195,8 @@
 		if (disk->fops->ioctl) {
 			ret = disk->fops->ioctl(inode, file, cmd, arg);
 			/* -EINVAL to handle old uncorrected drivers */
-			if (ret != -EINVAL && ret != -ENOTTY)
+			if (ret != -EINVAL && ret != -ENOTTY &&
+			    ret != -EOPNOTSUPP)
 				return ret;
 		}
 		fsync_bdev(bdev);
#4 lore raw
Quoted 2 lines · Jeff Garzik

Ok, I think I see what's going on. Can you get your users to try the attached patch?

This patch is definitely wrong.

Quoted 4 lines

Maybe libata should be returning ENOTTY, but that's _quite_ nonsensical considering that EOPNOTSUPP describes the error much better (and is a standard I-don't-support-that-operation return code in tons of other Linux ioctl-handling code).

libata should be returning -ENOTTY "I don't know what you are talking about", and the core block layer will then fallback and do generic handling

-EOPNOTSUPP is saying "I do know what is going on but its broken so fail it totally".

#5 lore raw
Quoted 6 lines · Alan Cox

libata should be returning -ENOTTY "I don't know what you are talking about", and the core block layer will then fallback and do generic handling

-EOPNOTSUPP is saying "I do know what is going on but its broken so fail it totally".

Are you describing behavior mandated in some standard?

Because that differs with _tons_ of existing ioctl code, whose default return code is EOPNOTSUPP when no known operation is found in a C "switch(cmd)" statement.

grep for it...

Jeff

#7 lore raw
In reply to Jeff Garzik
Quoted 4 lines · Jeff Garzik
Quoted 2 lines

-EOPNOTSUPP is saying "I do know what is going on but its broken so fail it totally".

Are you describing behavior mandated in some standard?

Yes. ENOTTY means "the ioctl you threw at me isn't one I understand", always has been in Unix proper.

-EINVAL: "I recognize this ioctl but the arguments are bogus" -ENOTTY: "I don't recognize this ioctl at all"
-ENOIOCTLCMD: Internal (never user visible) used by some driver layers to pass down "please do the defaults". This one might be a sane addition to the block layer list.

Quoted 5 lines

Because that differs with _tons_ of existing ioctl code, whose default return code is EOPNOTSUPP when no known operation is found in a C "switch(cmd)" statement.

grep for it...

We are slowly fixing the abusers of -EINVAL/-EOPNOTSUPP. Note btw that networking stuff is in many places a little different in its returns.

Alan

#8 lore raw
Quoted 15 lines · Alan Cox
Quoted 4 lines · Jeff Garzik
Quoted 2 lines

-EOPNOTSUPP is saying "I do know what is going on but its broken so fail it totally".

Are you describing behavior mandated in some standard?

Yes. ENOTTY means "the ioctl you threw at me isn't one I understand", always has been in Unix proper.

(quoted diff) +0 -3
-EINVAL: "I recognize this ioctl but the arguments are bogus"
-ENOTTY: "I don't recognize this ioctl at all"
-ENOIOCTLCMD: Internal (never user visible) used by some driver layers

to pass down "please do the defaults". This one might be a sane addition to the block layer list.

Quoted 8 lines
Quoted 5 lines

Because that differs with _tons_ of existing ioctl code, whose default return code is EOPNOTSUPP when no known operation is found in a C "switch(cmd)" statement.

grep for it...

We are slowly fixing the abusers of -EINVAL/-EOPNOTSUPP. Note btw that networking stuff is in many places a little different in its returns.

What about the case of "This is normally a valid command, but this particulat device is incapable of this particular subcommand"?

Hypothetical instance: a disk drive that indicates it is not capable of changing it's write cache setting?

Quoted 8 lines

Alan

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#11 lore raw
In reply to Alan Cox

Review

Quoted 2 lines
(quoted diff) +0 -2
-EINVAL: "I recognize this ioctl but the arguments are bogus"
-ENOTTY: "I don't recognize this ioctl at all"

You mean of course "this ioctl command code."

--
Bryan Henderson IBM Almaden Research Center
San Jose CA Filesystems

#9 lore raw
In reply to Doug Maxey
Quoted 8 lines · Doug Maxey
Quoted 2 lines

We are slowly fixing the abusers of -EINVAL/-EOPNOTSUPP. Note btw that networking stuff is in many places a little different in its returns.

What about the case of "This is normally a valid command, but this particulat device is incapable of this particular subcommand"?

Hypothetical instance: a disk drive that indicates it is not capable of changing it's write cache setting?

Thats less clear. Traditional Unix has tended to use EINVAL for that too, while Linux re-uses things like EOPNOTSUPP and that may well be a better approach - its at least fairly consistent in Linux.

#10 lore raw
In reply to Doug Maxey
Quoted 5 lines

What about the case of "This is normally a valid command, but this particulat device is incapable of this particular subcommand"?

Hypothetical instance: a disk drive that indicates it is not capable of changing it's write cache setting?

I believe there is a fundamental difference between "A device of this kind can't do that" and "that doesn't make any sense for a device of this kind." It's the difference between a request to boil the ocean and a request to boil Christmas. ENOTTY is for the request to boil Christmas. If you look at the history (from when it just meant "hey, this isn't a terminal (tty)"), it's clear: An ioctl to set the backspace character on a disk drive isn't impossible -- it's nonsensical.

--
Bryan Henderson IBM Almaden Research Center
San Jose CA Filesystems