Re: [PATCH 0/4] add large command support to the block layer

[Boaz Harrosh <bharrosh@panasas.com>]

On Mon, Apr 14 2008 at 14:38 +0300, Boaz Harrosh <bharrosh@panasas.com> wrote:
> On Mon, Apr 14 2008 at 13:50 +0300, FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
>> This patchset adds large command support to the block layer.
>>
>> We rarely handle large commands. So for optimization, a struct request
>> still has a static array for a command. rq_init sets rq->cmd pointer
>> to the static array. In short, rq_init() does
>>
>> rq->cmd = rq->__cmd;
>>
>> So we can access to rq->cmd and rq->cmd_len as before.
>>
>> Boaz argues that having single cmd_len is dangerous:
>>
>>> 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
>> I don't see how it is dangerous. We don't send large commands to block
>> devices via bsg. We can send large commands to block devices via only
>> SG_IO. SG_IO (block/scsi_ioctl.c) forbids us sending commands large
>> than 16 bytes, BLK_MAX_CDB. So how can we send large commands to block
>> devices?
>>
>> The first, second, and third patches are safe cleanups (a preparation
>> for changing rq->cmd from the static array to a pointer). They don't
>> conflict with Boaz's patchset. Even if we choose Boaz's approach now,
>> we'll move to the approach to have one command length in struct
>> request in the long term. So they can be applied now.
>>
>> The forth patch adds large command support. It's a substitute for
>> Boaz's second patch. It's pretty simple except for one hack for
>> ide-io. It's necessary since the ide subsystem allocates struct
>> request without using blk_get_request. In the long term, it would
>> better to convert it to use blk_get_request properly.
>>
>>
> 
> I still think that my last patch with one cmd_len and the Union is
> nicer and more simple. And does not waist the extra pointer that is 
> never ever used but by me. (And does not need 3 preparation patches)
> 
> Boaz
> 
Tomo hi.

Please don't miss understand me.
ACK-by: Boaz Harrosh <bharrosh@panasas.com>

I really don't mind, it is fine by me and it does what I want.
So thanks yes please push this through, and I'll rebase my
two patches to yours.

Thanks
Boaz