FUJITA Tomonori wrote:
> From: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
> Subject: [PATCH 4/4] bidi support: bidirectional request
> Date: Sun, 15 Apr 2007 20:33:28 +0300
>
>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>> index 645d24b..16a02ee 100644
>> --- a/include/linux/blkdev.h
>> +++ b/include/linux/blkdev.h
>> @@ -322,6 +322,7 @@ struct request {
>> void *end_io_data;
>>
>> struct request_io_part uni;
>> + struct request_io_part bidi_read;
>> };
>
> Would be more straightforward to have:
>
> struct request_io_part in;
> struct request_io_part out;
>
Yes I wish I could do that. For bidi supporting drivers this is the most logical.
But for the 99.9% of uni-directional drivers, calling rq_uni(), and being some what on
the hotish paths, this means we will need a pointer to a uni request_io_part.
This is bad because:
1st- There is no defined stage in a request life where to definitely set that pointer,
specially in the preparation stages.
2nd- hacks like scsi_error.c/scsi_send_eh_cmnd() will not work at all. Now this is a
very bad spot already, and I have a short term fix for it in the SCSI-bidi patches
(not sent yet) but a more long term solution is needed. Once such hacks are
cleaned up we can do what you say. This is exactly why I use the access functions
rq_uni/rq_io/rq_in/rq_out and not open code access.
>
>> /*
>> @@ -600,6 +601,34 @@ static inline struct request_io_part* rq_uni(struct request* req)
>> return &req->uni;
>> }
>>
>> +static inline struct request_io_part* rq_out(struct request* req)
>> +{
>> + WARN_ON_BIDI_FLAG(req);
>> + return &req->uni;
>> +}
>> +
>> +static inline struct request_io_part* rq_in(struct request* req)
>> +{
>> + WARN_ON_BIDI_FLAG(req);
>> + if (likely(rq_dma_dir(req) != DMA_BIDIRECTIONAL))
>> + return &req->uni;
>> +
>> + if (likely(req->cmd_flags & REQ_BIDI))
>> + return &req->bidi_read;
>> +
>> + return &req->uni;
>> +}
>> +
>> +static inline struct request_io_part* rq_io(struct request* req,
>> + enum dma_data_direction dir)
>> +{
>> + if (dir == DMA_FROM_DEVICE)
>> + return rq_in(req);
>> +
>> + WARN_ON( (dir != DMA_TO_DEVICE) && (dir != DMA_NONE) );
>> + return &req->uni;
>> +}
>
> static inline struct request_io_part* rq_io(struct request* req)
> {
> return (req is WRITE) ? &req->out : &req->in;
> }
Again I'm all for it. But this is a to deep of a change. Too many things changing
at once. If we keep the access functions than it does not matter, we can do it later.
Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
