Re: new tool: blktool

Re: new tool: blktool

[Alan Cox]

On Sul, 2004-08-15 at 22:36, Jeff Garzik wrote:
> 	$ hdparm -c1 /dev/hda
> 		becomes
> 	$ blktool /dev/hda pio-data 32-bit

So you've replaced hdparm's weird but unixish command line with an
even more demented non linuxish one that doesn't handle regexps for
drive names ?

Whatever happened to

	blktool /dev/hda --pio-data=32

new tool: blktool

[Jeff Garzik]

I just posted "blktool" on my SF page,
	http://sourceforge.net/projects/gkernel/
and in BitKeeper at
	bk://gkernel.bkbits.net/blktool


blktool aims to be an easier to use, and more generic version of the 
existing utility 'hdparm'.  For example,

	$ hdparm -c1 /dev/hda
		becomes
	$ blktool /dev/hda pio-data 32-bit

	and

	$ hdparm -L0 /dev/hda
		becomes
	$ blktool /dev/hda media unlock

The utility is currently still fairly specific to IDE devices (as hdparm 
is), but that will change in the coming weeks as SCSI, I2O, and possibly 
some bits of hardware RAID control are added.

The audience for this application, like hdparm, is fairly narrow, 
specific to people who tweak their storage devices and _know what they 
are doing_.  Improper use of this tool, like hdparm, can turn your disk 
into a doorstop.

	Jeff

Re: new tool: blktool

[Jeff Garzik]

Alan Cox wrote:
> On Sul, 2004-08-15 at 22:36, Jeff Garzik wrote:
> 
>>	$ hdparm -c1 /dev/hda
>>		becomes
>>	$ blktool /dev/hda pio-data 32-bit
> 
> 
> So you've replaced hdparm's weird but unixish command line with an
> even more demented non linuxish one that doesn't handle regexps for
> drive names ?
> 
> Whatever happened to
> 
> 	blktool /dev/hda --pio-data=32


Yep, it's more like ethtool(8) or cvs(1) in its syntax.  There is big 
difference in usability (for me anyway) between "command [options]..." 
and an unordered list of --args.  Especially as the list of commands 
grows longer.  It provides more structure.

Each command can have options, --foo-bar=baz if you like, I suppose.

	Jeff

Re: new tool: blktool

[David Ford]

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

[...]

> Yep, it's more like ethtool(8) or cvs(1) in its syntax.  There is big 
> difference in usability (for me anyway) between "command [options]..." 
> and an unordered list of --args.  Especially as the list of commands 
> grows longer.  It provides more structure.
>
> Each command can have options, --foo-bar=baz if you like, I suppose.


I would rather see --option=xyz than option xyz.  End users are going to 
be using it in scripts and in the event a parameter becomes "", then it 
will become --option1= --option2=def instead of option1 option2 def.  I 
would find it easier to parse, --option= is easy to ignore, option 
option has to be recognized as an empty option instead of using option 
as the first option's argument.

Just my opinion,
-david


[-- Attachment #2: david+challenge-response.vcf --]
[-- Type: text/x-vcard, Size: 183 bytes --]

begin:vcard
fn:David Ford
n:Ford;David
email;internet:david@blue-labs.org
title:Industrial Geek
tel;home:Ask please
tel;cell:(203) 650-3611
x-mozilla-html:TRUE
version:2.1
end:vcard

Re: new tool: blktool

[Mark Lord]

hdparm works for some SCSI devices already, and support for
more is already on the way.  I imagine I can have it handle
whatever new ioctls() are being provided from libata as well.

Care to point me at them?

Thanks
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: new tool: blktool

[Mark Lord]

Supplementary or Alternatively, all of the ATA device commands issued by hdparm
can be supported in a driver by simply implementing the HDIO_DRIVE_CMD
ioctl -- This is only a few lines of code in a typical SATA/SCSI driver,
and I could easily supply a patch to implement it in libata.

Sure it's old, looks clunky, but it is simple code that works
and is used by many more tools than just hdparm today.

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Mark Lord wrote:
> hdparm works for some SCSI devices already, and support for
> more is already on the way.  I imagine I can have it handle
> whatever new ioctls() are being provided from libata as well.
> 
> Care to point me at them?
> 
> Thanks

Re: new tool: blktool

[Jeff Garzik]

Mark Lord wrote:
> hdparm works for some SCSI devices already, and support for
> more is already on the way.  I imagine I can have it handle
> whatever new ioctls() are being provided from libata as well.
> 
> Care to point me at them?


No new ioctls :)  Say no to ioctls :)

libata _may_ support HDIO_DRIVE_CMD, but more likely will support 
http://www.t10.org/ftp/t10/document.04/04-262r1.pdf

and simply use existing interfaces.

	Jeff

Re: new tool: blktool

[Jeff Garzik]

Mark Lord wrote:
> Supplementary or Alternatively, all of the ATA device commands issued by 
> hdparm
> can be supported in a driver by simply implementing the HDIO_DRIVE_CMD
> ioctl -- This is only a few lines of code in a typical SATA/SCSI driver,
> and I could easily supply a patch to implement it in libata.
> 
> Sure it's old, looks clunky, but it is simple code that works
> and is used by many more tools than just hdparm today.


True but I'm wrestling with one of its design flaws...  it doesn't 
provide the taskfile protocol.

I really really want to know before the command is submitted whether I 
am going to be receiving data, sending data, or neither.  The current 
IDE driver "guesses" by virtue of DRQ flag behavior, but such a guess is 
impossible on modern SATA controllers.

You either have to provide a lookup table (command opcode -> protocol), 
or specify it through the userland API.  HDIO_DRIVE_TASKFILE does this 
via 'data_phase'.

On a more general note, though, I certainly welcome libata patches from 
any and all sources.  Hack away!

	Jeff

Re: new tool: blktool

[Mark Lord]

 > http://www.t10.org/ftp/t10/document.04/04-262r1.pdf

Ahh.. my buddie Curtis has been busy of late, I see.

I'll implement this in hdparm and the SATA/RAID driver
that I'm working on.  Will you (Jeff) do the same in libata?

But HDIO_DRIVE_CMD is rather easy to implement as well,
and perhaps both should be there for an overlap.

Especially since the former is in rather widespread use right now.
Yup, it's missing a separate data-phase parameter,
and lots of taskfile stuff, but it's configured by default
into every kernel (the same is not true for taskfile support),
and there's really only a few limited cases of it being used
for non-data commands:  IDENTIFY, SMART, and the odd READ/WRITE
SECTOR (pio, single sector).

In drivers that I work on now, I generally just support those
limited cases, and defer anything else to either taskfile
or something more native -- but in practice,
there are no other cases.

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: new tool: blktool

[Jeff Garzik]

Mark Lord wrote:
>  > http://www.t10.org/ftp/t10/document.04/04-262r1.pdf
> 
> Ahh.. my buddie Curtis has been busy of late, I see.
> 
> I'll implement this in hdparm and the SATA/RAID driver
> that I'm working on.  Will you (Jeff) do the same in libata?

I plan on providing _some_ way of executing arbitrary taskfiles, 
possibly more than one way.  I haven't decided upon the best route. 
Since T10 took my command protocol suggestions into account in the above 
proposal, implementing that would be an efficient route to the 
arbitrary-taskfiles feature.


> But HDIO_DRIVE_CMD is rather easy to implement as well,
> and perhaps both should be there for an overlap.
> 
> Especially since the former is in rather widespread use right now.
> Yup, it's missing a separate data-phase parameter,
> and lots of taskfile stuff, but it's configured by default
> into every kernel (the same is not true for taskfile support),
> and there's really only a few limited cases of it being used
> for non-data commands:  IDENTIFY, SMART, and the odd READ/WRITE
> SECTOR (pio, single sector).

If HDIO_DRIVE_CMD was easy to do, I would have already done it.  I agree 
with you that supporting it has benefits, but you are ignoring the 
obstacles:

* without taskfile protocol, it is _impossible_ to execute some vendor 
reserved commands
* without taskfile protocol, and without a lookup table, it is 
impossible to distinguish between [non-data | data-in | data-out] on 
some controllers.  Current IDE driver does "execute and pray we can 
guess from controller behavior" which definitely won't work in a lot of 
situations.

Modern SATA controllers need to set up the DMA engine beforehand -- for 
the entire transfer -- including for PIO data xfer commands.

HDIO_DRIVE_CMD tells me nothing about data direction or taskfile 
protocol.  It forces you to guess :(

libata is designed to be flexible enough to execute the full breadth of 
the ATA command set on whatever host controller you pick -- PCI PATA, 
paride, SATA, ATA-over-ethernet, or whatever zany technology you choose. 
  But all that flexibility demands a bit more from the software side of 
things, command protocol in this case.  That's why I the T10 04-262r1 
proposal spec.

	Jeff

Re: new tool: blktool

[Mark Lord]

>> But HDIO_DRIVE_CMD is rather easy to implement as well,
>> and perhaps both should be there for an overlap.
>>
>> Especially since the former is in rather widespread use right now.
>> Yup, it's missing a separate data-phase parameter,
>> and lots of taskfile stuff, but it's configured by default
>> into every kernel (the same is not true for taskfile support),
>> and there's really only a few limited cases of it being used
>> for non-data commands:  IDENTIFY, SMART, and the odd READ/WRITE
>> SECTOR (pio, single sector).
> 
> 
> If HDIO_DRIVE_CMD was easy to do, I would have already done it.  I agree 
> with you that supporting it has benefits, but you are ignoring the 
> obstacles:

"Ignoring"?  Hardly.  I even listed a few of them above.
But in practice, HDIO_DRIVE_CMD only requires support for a very
limited set of commands.  It was never intended for arbitrary
command acceptance.  And it's not like Joe User can abuse it,
since it requires SYSADMIN and RAWIO capabilities to execute.

The command subset that accounts for just about all uses of it today is:

SET_FEATURES, SMART, IDENTIFY, READ_SECTOR, WRITE_SECTOR.
Period.

Pretty easy to support those, especially in SATA.
I know, since I've just taken a couple of hours and added it
to my SATA/RAID driver (a queuing controller with tag support).

For more generic interface, Curtis's document looks rather good.
But for backward compatibility with existing tools like the
smartmontools and hdparm, all that is needed is a limited subset
of HDIO_DRIVE_CMD (for the opcodes listed above) and also
the closely related HDIO_DRIVE_TASK ioctl for some of the SMART
commands (all non-data).

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: new tool: blktool

[Bartlomiej Zolnierkiewicz]

Hi,

On Thursday 19 August 2004 17:03, Mark Lord wrote:
> >> But HDIO_DRIVE_CMD is rather easy to implement as well,
> >> and perhaps both should be there for an overlap.
> >>
> >> Especially since the former is in rather widespread use right now.
> >> Yup, it's missing a separate data-phase parameter,
> >> and lots of taskfile stuff, but it's configured by default
> >> into every kernel (the same is not true for taskfile support),
> >> and there's really only a few limited cases of it being used
> >> for non-data commands:  IDENTIFY, SMART, and the odd READ/WRITE
> >> SECTOR (pio, single sector).
> >
> > If HDIO_DRIVE_CMD was easy to do, I would have already done it.  I agree
> > with you that supporting it has benefits, but you are ignoring the
> > obstacles:
>
> "Ignoring"?  Hardly.  I even listed a few of them above.
> But in practice, HDIO_DRIVE_CMD only requires support for a very
> limited set of commands.  It was never intended for arbitrary
> command acceptance.  And it's not like Joe User can abuse it,

Most people used it for as much arbitrary commands as they could.

> since it requires SYSADMIN and RAWIO capabilities to execute.
>
> The command subset that accounts for just about all uses of it today is:
>
> SET_FEATURES, SMART, IDENTIFY, READ_SECTOR, WRITE_SECTOR.
> Period.

WRITE_SECTOR?

IDE version of HDIO_DRIVE_CMD only implements READ_SECTOR and it is
already an abuse of this ioctl (because it does PIO-in command if buffer
is provided and no-data command otherwise).

> Pretty easy to support those, especially in SATA.
> I know, since I've just taken a couple of hours and added it
> to my SATA/RAID driver (a queuing controller with tag support).
>
> For more generic interface, Curtis's document looks rather good.
> But for backward compatibility with existing tools like the
> smartmontools and hdparm, all that is needed is a limited subset
> of HDIO_DRIVE_CMD (for the opcodes listed above) and also
> the closely related HDIO_DRIVE_TASK ioctl for some of the SMART
> commands (all non-data).

IMO it is a perfect moment to add one generic interface and start
deprecating three strange ioctls (HDIO_DRIVE_CMD / HDIO_DRIVE_TASK / 
HDIO_DRIVE_TASKFILE).

Bartlomiej

Re: new tool: blktool

[Mark Lord]

The READ/WRITE SECTOR commands are not actually used by
any software I know of with HDIO_DRIVE_CMD, so no big deal there.
The ioctl should be limited to SET_FEATURES, IDENTIFY, and SMART.

And you'll get no debate from me on wanting a better interface
moving forward -- I really like the new ATA Command Pass-Through
draft spec (updated today, by the way).

But there'll need to be some overlap between the old and new,
and currently the libata driver has no way for smartmontools
or hdparm to communicate and control drive features like
the cache/readahead settings, and the various SMART capabilities.

Simply dropping HDIO_DRIVE_CMD/HDIO_DRIVE_TASK into there would
immediately gain full compatibility with the existing toolsets,
and give some time for a newer scheme to be rolled out in the
kernel, the tools, and ultimately all of the various distros.

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: new tool: blktool

[Jeff Garzik]

Mark Lord wrote:
> But there'll need to be some overlap between the old and new,
> and currently the libata driver has no way for smartmontools
> or hdparm to communicate and control drive features like
> the cache/readahead settings, and the various SMART capabilities.


Supporting the ATA Pass-through CDB in libata will enable support for 
all of those things.  I think we all agree that there needs to be _some_ 
way of letting userspace control these attributes via direct ATA command 
submission.

As an aside, although libata _will_ support some sort of user ATA 
command execution method, some of these attributes are (or will be soon) 
available through standard SCSI methods.

For example, MODE SENSE/MODE SELECT on the caching mode page allows one 
to toggle read-ahead and writeback caching.

blktool supports both methods :)

	Jeff

Re: new tool: blktool

[Jeff Garzik]

Mark Lord wrote:
> Simply dropping HDIO_DRIVE_CMD/HDIO_DRIVE_TASK into there would
> immediately gain full compatibility with the existing toolsets,
> and give some time for a newer scheme to be rolled out in the
> kernel, the tools, and ultimately all of the various distros.

Addendum:  don't misunderstand my other emails, I do agree with what 
you're saying above.  But random thoughts (some of which conflict with 
each other):

* In Linux we want to keep ancient userland binaries working for as long 
as possible.

* I don't mind HDIO_DRIVE_TASK nearly as much as HDIO_DRIVE_CMD, since 
the command protocol is available.  But if I give in and decide that a 
command opcode->protocol lookup table is inevitable for supporting 
legacy interface, then I might as well implement both HDIO_DRIVE_TASK 
and HDIO_DRIVE_CMD.

* OTOH, this is an excellent opportunity to _not_ implement these 
ioctls, if an obviously-better interface is available.  Since libata and 
SATA are new drivers using new interfaces, it's not difficult to move 
things to new interfaces.

* And it's not a big deal to update blktool and hdparm to use <new 
method X> to send ATA taskfiles, rather than existing HDIO_DRIVE_xxx. 
(that leaves only existing applications)

Re: new tool: blktool

[Mark Lord]

Yup, full agreement here.

Except the distros.. it takes time for new tools to spill out
into all of those -- kernel updates seem to happen more frequently
there.

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: new tool: blktool

[Mark Lord]

 >* I don't mind HDIO_DRIVE_TASK nearly as much as HDIO_DRIVE_CMD,
 >* since the command protocol is available.

That's not HDIO_DRIVE_TASKFILE, by the way..  a different beast there.

HDIO_DRIVE_TASK is just a slightly different form of HDIO_DRIVE_CMD
for non-data commands (specifically, some SMART commands),
with a more complete register set being exchanged.

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: new tool: blktool

[Jeff Garzik]

Mark Lord wrote:
>  >* I don't mind HDIO_DRIVE_TASK nearly as much as HDIO_DRIVE_CMD,
>  >* since the command protocol is available.
> 
> That's not HDIO_DRIVE_TASKFILE, by the way..  a different beast there.
> 
> HDIO_DRIVE_TASK is just a slightly different form of HDIO_DRIVE_CMD
> for non-data commands (specifically, some SMART commands),
> with a more complete register set being exchanged.

Oops, indeed I meant HDIO_DRIVE_TASKFILE.

Anyway, once the infrastructure for the ATA Pass-thru CDB is implemented 
in libata, it is trivial to implement any of these three HDIO_DRIVE_xxx 
ioctls, using the ioctl data to build a scsi command internally.  A bit 
strange at first glance, but it maximizes the leverage of existing 
kernel infrastructure to send commands, time them out, etc.

	Jeff

Re: new tool: blktool

[Mark Lord]

Yup, that's how I implemented it:  they just piggy back onto
the ATA Passthrough feature.

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: new tool: blktool

[Andi Kleen]

Mark Lord <lkml@rtr.ca> writes:

> hdparm works for some SCSI devices already, and support for
> more is already on the way.  I imagine I can have it handle
> whatever new ioctls() are being provided from libata as well.

I can see Jeff's point that the one letter options are not 
very nice (I often have to look them up in the man page too ...) 
And it's probably true that it will soon run out of letters.

Any chance you could add GNU style long options using getopt_long()?

-Andi