Re: [PATCH] Block layer: separate out queue-oriented ioctls
From: Alan Stern
Date: Sun Feb 18 2007 - 11:44:58 EST
On Sat, 17 Feb 2007, Douglas Gilbert wrote:
> Jens Axboe wrote:
> > On Fri, Feb 16 2007, Alan Stern wrote:
> >> From: James Bottomley <James.Bottomley@xxxxxxxxxxxx>
> >> This patch (as854) separates out the two queue-oriented ioctls from
> >> the rest of the block-layer ioctls. The idea is that they should
> >> apply to any driver using a request_queue, even if the driver doesn't
> >> implement a block-device interface. The prototypical example is the
> >> sg driver, to which the patch adds the new interface.
> >> This will make it possible for cdrecord and related programs to
> >> retrieve reliably the max_sectors value, regardless of whether the
> >> user points it to an sr or an sg device. In particular, this will
> >> resolve Bugzilla entry #7026.
> > The block bits are fine with me, the sg calling point is a bit of a sore
> > thumb (a char driver calling into block layer ioctls) though. So the
> > block layer bits are certainly ok with me, if Doug acks the sg bit I'll
> > merge everything up.
> > (patch left below)
> Does this need to be in the sg driver?
Something like it does. Otherwise cdrecord (or any other sg user) has no
way to retrieve the max_sectors value for sg devices.
> What is the hardware sector size of a SES or OSD device?
I have no idea, and I don't see why it is relevant. (I don't even know
what "SES" and "OSD" refer to.)
> As for the max_sector variable, wouldn't it be better
> to generate a new ioctl that yielded the limit in bytes?
Maybe. I wouldn't mind doing it that way. But we would have to leave in
the old ioctl, and we probably would still want it to be usable for sg
devices. Not to mention that it would be silly to have two ioctls which
always return exactly the same values except for a factor of 512.
> Making a driver variable that implicitly assumes sectors
> are 512 bytes in length more visible to the user space
> seems like a step in the wrong direction.
> Alternatively the SG_GET_RESERVED_SIZE ioctl could be
> modified to yield no more than max_sectors*512 .
There should be one single ioctl which can be applied uniformly to all
CD-type devices (in fact, to all devices using a request_queue) to learn
max_sectors. This rules out using SG_GET_RESERVED_SIZE.
Furthermore, if you changed SG_GET_RESERVED_SIZE in this way you would
only increase the confusion. The reserved size isn't directly related to
the maximum allowed DMA length, and there's no point pretending it is.
What if it turns out that the reserved size is smaller than max_sectors?
Then you'd force user programs to do I/O in chunks that were smaller than
> >> + /* block ioctls first */
> Why first??
I don't know -- James will have to answer. I don't see that it makes any
real difference; the new code can easily be moved to the end.
> That would allow the block layer to overtake any currently
> defined and working sg ioctl.
> Surely, if it was put in, it should be in the default case.
Would the patch be okay with you if I rework it to put the new ioctls
under the default case in sg?
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/