On Sun, Apr 06 2008 at 12:35 +0300, Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
> On Fri, Apr 04 2008 at 14:46 +0300, Jens Axboe <jens.axboe@xxxxxxxxxx> wrote:
>> On Thu, Apr 03 2008, Boaz Harrosh wrote:
>>> static void req_bio_endio(struct request *rq, struct bio *bio,
>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>> index 6f79d40..2f87c9d 100644
>>> --- a/include/linux/blkdev.h
>>> +++ b/include/linux/blkdev.h
>>> @@ -213,8 +213,15 @@ struct request {
>>> /*
>>> * when request is used as a packet command carrier
>>> */
>>> - unsigned int cmd_len;
>>> - unsigned char cmd[BLK_MAX_CDB];
>>> + unsigned short cmd_len;
>>> + unsigned short ext_cdb_len; /* length of ext_cdb buffer */
>>> + union {
>>> + unsigned char cmd[BLK_MAX_CDB];
>>> + unsigned char *ext_cdb;/* an optional extended cdb.
>>> + * points to a user buffer that must
>>> + * be valid until end of request
>>> + */
>>> + };
>> Why not just something ala
>>
>> unsigned short cmd_len;
>> unsigned char __cmd[BLK_MAX_CDB];
>> unsigned char *cmd;
>>
>> and then have rq_init() do
>>
>> rq->cmd = rq->__cmd;
>>
>> and just have a function for setting up a larger ->cmd and adjusting
>> ->cmd_len in the process?
>>
>> Then rq_set_cdb() would be
>>
>> static inline void rq_set_cdb(struct request *rq, u8 *cdb, short cdb_len)
>> {
>> rq->cmd = cdb;
>> rq->cmd_len = cdb_len;
>> }
>>
>> and rq_get_cdb() plus rq_get_cdb_len() could just go away.
>>
>
> Because this way it is dangerous if large commands are issued to legacy
> drivers. In scsi-land we have .cmd_len at host template that will govern if
> we are allow to issue larger commands to the driver. In block devices we do
> not have such a facility, and the danger is if such commands are issued through
> bsg or other means, even by malicious code. What you say is the ideal and it
> is what I've done for scsi, but for block devices we can not do that yet.
> With the way I did it here, Legacy drivers will see zero length command and
> will do the right thing, from what I've seen.
>
> Boaz
>
I forgot to say.
With the proposed way, we are saving the space of the pointer. And the final outcome
of eventually eliminating the buffer, is the same.
Let me summarize.
- support extended, arbitrary large commands by introducing the notion that cdb space
can be pointed to not carried.
- Eventually transition all block drivers and users to the new system, for all commands
not just large ones.
- Do so without introducing any extra cost or instability, but also let the transition
be done gradually, and not at once, (hence more stability).
I have thought about that long and hard, my first patch of the matter was 18 month ago.
I still think this is the smoothest way to go, and not that ugly, really.
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
