Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-23 Thread Benny Halevy
Muli Ben-Yehuda wrote:
> On Tue, Jan 23, 2007 at 03:45:00PM +0200, Benny Halevy wrote:
> 
 +static inline int dma_uni_dir(enum dma_data_direction dir)
 +{
 +  return (dir == DMA_TO_DEVICE) || (dir == DMA_FROM_DEVICE) ||
 + (dir == DMA_NONE);
 +}
>>> While this doesn't look very useful. Why is "DMA_NONE" a uni-dir? I
>>> suggest replacing this with an open coded (dir != DMA_BIDIRECTIONAL).
>> The idea was to be resilient to invalid values. (dir != DMA_BIDIRECTIONAL)
>> is fine of course, but I'd add a BUG_ON such as (dir < 0 || dir >
>> DMA_BIDIRECTIONAL)
> 
> If DMA_NONE isn't actually allowed here, you can use valid_dma_direction().

DMA_NONE should be allowed as it is used by commands that do no I/O and these
are handled on uni-directional path.

BTW, the BUG_ON I suggested has a bug of course since (countering my intuition)
DMA_BIDIRECTIONAL==0, so it should be BUG_ON(dir < 0 || dir > DMA_NONE)
instead.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-23 Thread Muli Ben-Yehuda
On Tue, Jan 23, 2007 at 03:45:00PM +0200, Benny Halevy wrote:

> >> +static inline int dma_uni_dir(enum dma_data_direction dir)
> >> +{
> >> +  return (dir == DMA_TO_DEVICE) || (dir == DMA_FROM_DEVICE) ||
> >> + (dir == DMA_NONE);
> >> +}
> > 
> > While this doesn't look very useful. Why is "DMA_NONE" a uni-dir? I
> > suggest replacing this with an open coded (dir != DMA_BIDIRECTIONAL).
> 
> The idea was to be resilient to invalid values. (dir != DMA_BIDIRECTIONAL)
> is fine of course, but I'd add a BUG_ON such as (dir < 0 || dir >
> DMA_BIDIRECTIONAL)

If DMA_NONE isn't actually allowed here, you can use valid_dma_direction().

Cheers,
Muli
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-23 Thread Benny Halevy
Douglas Gilbert wrote:
> Benny Halevy wrote:
>> Douglas Gilbert wrote:
> 
> Perhaps the right use of DMA_BIRECTIONAL needs to be
> defined.
> 
> Could it be used with a XDWRITE(10) SCSI command
> defined in sbc3r07.pdf at http://www.t10.org ? I suspect
> using two scatter gather lists would be a better approach.

Exactly. This is a classic example of a bidirectional command
and indeed two scatter-gather lists (that are mapped into two
bio lists) are used.

> 
 - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
   requests were used and bzero'ed.
>>> With a bi-directional transfer is it always unambiguous
>>> which transfer occurs first (or could they occur at
>>> the same time)?
>> The bidi transfers can occur in any order and in parallel.
> 
> Then it is not sufficient for modern SCSI transports in which
> certain bidirectional commands (probably most) have a well
> defined order.
> 
> So DMA_BIDIRECTIONAL looks PCI specific and it may have
> been a mistake to replace other subsystem's direction flags
> with it. RDMA might be an interesting case.
> 

I would say that it might make sense to define an equivalent
for dma_data_direction at the block layer, for example:

enum req_io_direction {
REQ_IO_NONE = 0,
REQ_IN_FROM_DEVICE = 1,
REQ_OUT_TO_DEVICE = 2,
REQ_BIDIRECTIONAL = 3,
};

can be used in struct request and upper layers.

Besides the fact that having separate I/O buffers for bidirectional
transfers makes block I/O different from pci bidi I/O,
this enum makes more sense "arithmetically" and has
a much better meaning for the zero value.
Today DMA_BIDIRECTIONAL is used in several places as the default and "invalid"
value since no-one ever used it before. I'd rather have the value 0 mean
REQ_IO_NONE (or REQ_IO_INVALID if we want such thing).

Benny
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-23 Thread Benny Halevy
Muli Ben-Yehuda wrote:
> On Mon, Jan 22, 2007 at 01:21:28AM +0200, Boaz Harrosh wrote:
> 
>> - Introduce a new enum dma_data_direction data_dir member in struct request.
>>   and remove the RW bit from request->cmd_flag
> 
> Some architecture use 'enum dma_data_direction' and some 'int
> dma_data_direction'. The consensus was to move to int over
> time. Please use 'int dma_data_direction'.

That should be fine (although I'm not sure what made you go this way :)
Please see my reply to Douglas, proposing an enum req_io_direction
at the block layer and up which will provide a better enumeration
for our use.

>>
>> +static inline int dma_write_dir(enum dma_data_direction dir)
>> +{
>> +return (dir == DMA_TO_DEVICE) || (dir == DMA_BIDIRECTIONAL);
>> +}
> 
> "write" can mean "write to device" or "write to memory", depending on
> context. Not exactly something which should be a generic
> helper. Rename to 'dma_to_device(int dir)'?

much better. thanks!

> 
>> +static inline int dma_uni_dir(enum dma_data_direction dir)
>> +{
>> +return (dir == DMA_TO_DEVICE) || (dir == DMA_FROM_DEVICE) ||
>> +   (dir == DMA_NONE);
>> +}
> 
> While this doesn't look very useful. Why is "DMA_NONE" a uni-dir? I
> suggest replacing this with an open coded (dir != DMA_BIDIRECTIONAL).

The idea was to be resilient to invalid values. (dir != DMA_BIDIRECTIONAL)
is fine of course, but I'd add a BUG_ON such as (dir < 0 || dir > 
DMA_BIDIRECTIONAL)

Benny
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-23 Thread Benny Halevy
Muli Ben-Yehuda wrote:
 On Mon, Jan 22, 2007 at 01:21:28AM +0200, Boaz Harrosh wrote:
 
 - Introduce a new enum dma_data_direction data_dir member in struct request.
   and remove the RW bit from request-cmd_flag
 
 Some architecture use 'enum dma_data_direction' and some 'int
 dma_data_direction'. The consensus was to move to int over
 time. Please use 'int dma_data_direction'.

That should be fine (although I'm not sure what made you go this way :)
Please see my reply to Douglas, proposing an enum req_io_direction
at the block layer and up which will provide a better enumeration
for our use.


 +static inline int dma_write_dir(enum dma_data_direction dir)
 +{
 +return (dir == DMA_TO_DEVICE) || (dir == DMA_BIDIRECTIONAL);
 +}
 
 write can mean write to device or write to memory, depending on
 context. Not exactly something which should be a generic
 helper. Rename to 'dma_to_device(int dir)'?

much better. thanks!

 
 +static inline int dma_uni_dir(enum dma_data_direction dir)
 +{
 +return (dir == DMA_TO_DEVICE) || (dir == DMA_FROM_DEVICE) ||
 +   (dir == DMA_NONE);
 +}
 
 While this doesn't look very useful. Why is DMA_NONE a uni-dir? I
 suggest replacing this with an open coded (dir != DMA_BIDIRECTIONAL).

The idea was to be resilient to invalid values. (dir != DMA_BIDIRECTIONAL)
is fine of course, but I'd add a BUG_ON such as (dir  0 || dir  
DMA_BIDIRECTIONAL)

Benny
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-23 Thread Benny Halevy
Douglas Gilbert wrote:
 Benny Halevy wrote:
 Douglas Gilbert wrote:
 
 Perhaps the right use of DMA_BIRECTIONAL needs to be
 defined.
 
 Could it be used with a XDWRITE(10) SCSI command
 defined in sbc3r07.pdf at http://www.t10.org ? I suspect
 using two scatter gather lists would be a better approach.

Exactly. This is a classic example of a bidirectional command
and indeed two scatter-gather lists (that are mapped into two
bio lists) are used.

 
 - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
   requests were used and bzero'ed.
 With a bi-directional transfer is it always unambiguous
 which transfer occurs first (or could they occur at
 the same time)?
 The bidi transfers can occur in any order and in parallel.
 
 Then it is not sufficient for modern SCSI transports in which
 certain bidirectional commands (probably most) have a well
 defined order.
 
 So DMA_BIDIRECTIONAL looks PCI specific and it may have
 been a mistake to replace other subsystem's direction flags
 with it. RDMA might be an interesting case.
 

I would say that it might make sense to define an equivalent
for dma_data_direction at the block layer, for example:

enum req_io_direction {
REQ_IO_NONE = 0,
REQ_IN_FROM_DEVICE = 1,
REQ_OUT_TO_DEVICE = 2,
REQ_BIDIRECTIONAL = 3,
};

can be used in struct request and upper layers.

Besides the fact that having separate I/O buffers for bidirectional
transfers makes block I/O different from pci bidi I/O,
this enum makes more sense arithmetically and has
a much better meaning for the zero value.
Today DMA_BIDIRECTIONAL is used in several places as the default and invalid
value since no-one ever used it before. I'd rather have the value 0 mean
REQ_IO_NONE (or REQ_IO_INVALID if we want such thing).

Benny
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-23 Thread Muli Ben-Yehuda
On Tue, Jan 23, 2007 at 03:45:00PM +0200, Benny Halevy wrote:

  +static inline int dma_uni_dir(enum dma_data_direction dir)
  +{
  +  return (dir == DMA_TO_DEVICE) || (dir == DMA_FROM_DEVICE) ||
  + (dir == DMA_NONE);
  +}
  
  While this doesn't look very useful. Why is DMA_NONE a uni-dir? I
  suggest replacing this with an open coded (dir != DMA_BIDIRECTIONAL).
 
 The idea was to be resilient to invalid values. (dir != DMA_BIDIRECTIONAL)
 is fine of course, but I'd add a BUG_ON such as (dir  0 || dir 
 DMA_BIDIRECTIONAL)

If DMA_NONE isn't actually allowed here, you can use valid_dma_direction().

Cheers,
Muli
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-23 Thread Benny Halevy
Muli Ben-Yehuda wrote:
 On Tue, Jan 23, 2007 at 03:45:00PM +0200, Benny Halevy wrote:
 
 +static inline int dma_uni_dir(enum dma_data_direction dir)
 +{
 +  return (dir == DMA_TO_DEVICE) || (dir == DMA_FROM_DEVICE) ||
 + (dir == DMA_NONE);
 +}
 While this doesn't look very useful. Why is DMA_NONE a uni-dir? I
 suggest replacing this with an open coded (dir != DMA_BIDIRECTIONAL).
 The idea was to be resilient to invalid values. (dir != DMA_BIDIRECTIONAL)
 is fine of course, but I'd add a BUG_ON such as (dir  0 || dir 
 DMA_BIDIRECTIONAL)
 
 If DMA_NONE isn't actually allowed here, you can use valid_dma_direction().

DMA_NONE should be allowed as it is used by commands that do no I/O and these
are handled on uni-directional path.

BTW, the BUG_ON I suggested has a bug of course since (countering my intuition)
DMA_BIDIRECTIONAL==0, so it should be BUG_ON(dir  0 || dir  DMA_NONE)
instead.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-22 Thread William Studenmund

On Jan 21, 2007, at 10:06 PM, Benny Halevy wrote:


Douglas Gilbert wrote:

Boaz Harrosh wrote:
- Introduce a new enum dma_data_direction data_dir member in  
struct request.

  and remove the RW bit from request->cmd_flag
- Add new API to query request direction.
- Adjust existing API and implementation.
- Cleanup wrong use of DMA_BIDIRECTIONAL
- Introduce new blk_rq_init_unqueued_req() and use it in places  
ad-hoc

  requests were used and bzero'ed.


With a bi-directional transfer is it always unambiguous
which transfer occurs first (or could they occur at
the same time)?


The bidi transfers can occur in any order and in parallel.


Yes.

However it's not that hard. If you're a target (which open-iscsi  
isn't), you know what the command needs. If you're an initiator, you  
just send the unsolicited data (if any), then let the target tell you  
what it needs next.


While it is possible for commands to have multiple phases, all the  
BiDi commands I'm familiar with use the Data-Out phase to send extra  
command data, then use the Data-In phase to get either an extended  
response to the query (such as a search command) or extended status  
(OSD commands). So it's not that bad in reality.


Take care,

Bill
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-22 Thread James Bottomley
On Mon, 2007-01-22 at 10:05 -0500, Douglas Gilbert wrote:
> Perhaps the right use of DMA_BIRECTIONAL needs to be
> defined.
> 
> Could it be used with a XDWRITE(10) SCSI command
> defined in sbc3r07.pdf at http://www.t10.org ? I suspect
> using two scatter gather lists would be a better approach.
> 
> >>> - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
> >>>   requests were used and bzero'ed.
> >> With a bi-directional transfer is it always unambiguous
> >> which transfer occurs first (or could they occur at
> >> the same time)?
> > 
> > The bidi transfers can occur in any order and in parallel.

> Then it is not sufficient for modern SCSI transports in which
> certain bidirectional commands (probably most) have a well
> defined order.

Right, that's why I think bi-directional needs to be one way op followed
by one way op ... even if it is to the same buffer.  That should be a
general enough paradigm for everything.

> So DMA_BIDIRECTIONAL looks PCI specific and it may have
> been a mistake to replace other subsystem's direction flags
> with it. RDMA might be an interesting case.

It's bus specific ... it means that the bus must be programmed to expect
the device to transfer both to and from the memory buffer.  There are a
very few drivers which do this when they don't know the actual transfer
direction, so it might be reasonably tested on architectures ... but
we'd probably have to check.

James


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-22 Thread Douglas Gilbert
Benny Halevy wrote:
> Douglas Gilbert wrote:
>> Boaz Harrosh wrote:
>>> - Introduce a new enum dma_data_direction data_dir member in struct request.
>>>   and remove the RW bit from request->cmd_flag
>>> - Add new API to query request direction.
>>> - Adjust existing API and implementation.
>>> - Cleanup wrong use of DMA_BIDIRECTIONAL

Perhaps the right use of DMA_BIRECTIONAL needs to be
defined.

Could it be used with a XDWRITE(10) SCSI command
defined in sbc3r07.pdf at http://www.t10.org ? I suspect
using two scatter gather lists would be a better approach.

>>> - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
>>>   requests were used and bzero'ed.
>> With a bi-directional transfer is it always unambiguous
>> which transfer occurs first (or could they occur at
>> the same time)?
> 
> The bidi transfers can occur in any order and in parallel.

Then it is not sufficient for modern SCSI transports in which
certain bidirectional commands (probably most) have a well
defined order.

So DMA_BIDIRECTIONAL looks PCI specific and it may have
been a mistake to replace other subsystem's direction flags
with it. RDMA might be an interesting case.

Doug Gilbert


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-22 Thread Douglas Gilbert
Benny Halevy wrote:
 Douglas Gilbert wrote:
 Boaz Harrosh wrote:
 - Introduce a new enum dma_data_direction data_dir member in struct request.
   and remove the RW bit from request-cmd_flag
 - Add new API to query request direction.
 - Adjust existing API and implementation.
 - Cleanup wrong use of DMA_BIDIRECTIONAL

Perhaps the right use of DMA_BIRECTIONAL needs to be
defined.

Could it be used with a XDWRITE(10) SCSI command
defined in sbc3r07.pdf at http://www.t10.org ? I suspect
using two scatter gather lists would be a better approach.

 - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
   requests were used and bzero'ed.
 With a bi-directional transfer is it always unambiguous
 which transfer occurs first (or could they occur at
 the same time)?
 
 The bidi transfers can occur in any order and in parallel.

Then it is not sufficient for modern SCSI transports in which
certain bidirectional commands (probably most) have a well
defined order.

So DMA_BIDIRECTIONAL looks PCI specific and it may have
been a mistake to replace other subsystem's direction flags
with it. RDMA might be an interesting case.

Doug Gilbert


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-22 Thread James Bottomley
On Mon, 2007-01-22 at 10:05 -0500, Douglas Gilbert wrote:
 Perhaps the right use of DMA_BIRECTIONAL needs to be
 defined.
 
 Could it be used with a XDWRITE(10) SCSI command
 defined in sbc3r07.pdf at http://www.t10.org ? I suspect
 using two scatter gather lists would be a better approach.
 
  - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
requests were used and bzero'ed.
  With a bi-directional transfer is it always unambiguous
  which transfer occurs first (or could they occur at
  the same time)?
  
  The bidi transfers can occur in any order and in parallel.

 Then it is not sufficient for modern SCSI transports in which
 certain bidirectional commands (probably most) have a well
 defined order.

Right, that's why I think bi-directional needs to be one way op followed
by one way op ... even if it is to the same buffer.  That should be a
general enough paradigm for everything.

 So DMA_BIDIRECTIONAL looks PCI specific and it may have
 been a mistake to replace other subsystem's direction flags
 with it. RDMA might be an interesting case.

It's bus specific ... it means that the bus must be programmed to expect
the device to transfer both to and from the memory buffer.  There are a
very few drivers which do this when they don't know the actual transfer
direction, so it might be reasonably tested on architectures ... but
we'd probably have to check.

James


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-22 Thread William Studenmund

On Jan 21, 2007, at 10:06 PM, Benny Halevy wrote:


Douglas Gilbert wrote:

Boaz Harrosh wrote:
- Introduce a new enum dma_data_direction data_dir member in  
struct request.

  and remove the RW bit from request-cmd_flag
- Add new API to query request direction.
- Adjust existing API and implementation.
- Cleanup wrong use of DMA_BIDIRECTIONAL
- Introduce new blk_rq_init_unqueued_req() and use it in places  
ad-hoc

  requests were used and bzero'ed.


With a bi-directional transfer is it always unambiguous
which transfer occurs first (or could they occur at
the same time)?


The bidi transfers can occur in any order and in parallel.


Yes.

However it's not that hard. If you're a target (which open-iscsi  
isn't), you know what the command needs. If you're an initiator, you  
just send the unsolicited data (if any), then let the target tell you  
what it needs next.


While it is possible for commands to have multiple phases, all the  
BiDi commands I'm familiar with use the Data-Out phase to send extra  
command data, then use the Data-In phase to get either an extended  
response to the query (such as a search command) or extended status  
(OSD commands). So it's not that bad in reality.


Take care,

Bill
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-21 Thread Benny Halevy
Douglas Gilbert wrote:
> Boaz Harrosh wrote:
>> - Introduce a new enum dma_data_direction data_dir member in struct request.
>>   and remove the RW bit from request->cmd_flag
>> - Add new API to query request direction.
>> - Adjust existing API and implementation.
>> - Cleanup wrong use of DMA_BIDIRECTIONAL
>> - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
>>   requests were used and bzero'ed.
> 
> With a bi-directional transfer is it always unambiguous
> which transfer occurs first (or could they occur at
> the same time)?

The bidi transfers can occur in any order and in parallel.

> 
> Doug Gilbert
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-21 Thread Muli Ben-Yehuda
On Mon, Jan 22, 2007 at 01:21:28AM +0200, Boaz Harrosh wrote:

> - Introduce a new enum dma_data_direction data_dir member in struct request.
>   and remove the RW bit from request->cmd_flag

Some architecture use 'enum dma_data_direction' and some 'int
dma_data_direction'. The consensus was to move to int over
time. Please use 'int dma_data_direction'.

> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index ff203c4..abbca7b 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -13,6 +13,28 @@ enum dma_data_direction {
>   DMA_NONE = 3,
>  };
> 
> +static inline int dma_write_dir(enum dma_data_direction dir)
> +{
> + return (dir == DMA_TO_DEVICE) || (dir == DMA_BIDIRECTIONAL);
> +}

"write" can mean "write to device" or "write to memory", depending on
context. Not exactly something which should be a generic
helper. Rename to 'dma_to_device(int dir)'?

> +static inline int dma_uni_dir(enum dma_data_direction dir)
> +{
> + return (dir == DMA_TO_DEVICE) || (dir == DMA_FROM_DEVICE) ||
> +(dir == DMA_NONE);
> +}

While this doesn't look very useful. Why is "DMA_NONE" a uni-dir? I
suggest replacing this with an open coded (dir != DMA_BIDIRECTIONAL).

Cheers,
Muli
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-21 Thread Douglas Gilbert
Boaz Harrosh wrote:
> - Introduce a new enum dma_data_direction data_dir member in struct request.
>   and remove the RW bit from request->cmd_flag
> - Add new API to query request direction.
> - Adjust existing API and implementation.
> - Cleanup wrong use of DMA_BIDIRECTIONAL
> - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
>   requests were used and bzero'ed.

With a bi-directional transfer is it always unambiguous
which transfer occurs first (or could they occur at
the same time)?

Doug Gilbert
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-21 Thread Douglas Gilbert
Boaz Harrosh wrote:
 - Introduce a new enum dma_data_direction data_dir member in struct request.
   and remove the RW bit from request-cmd_flag
 - Add new API to query request direction.
 - Adjust existing API and implementation.
 - Cleanup wrong use of DMA_BIDIRECTIONAL
 - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
   requests were used and bzero'ed.

With a bi-directional transfer is it always unambiguous
which transfer occurs first (or could they occur at
the same time)?

Doug Gilbert
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-21 Thread Muli Ben-Yehuda
On Mon, Jan 22, 2007 at 01:21:28AM +0200, Boaz Harrosh wrote:

 - Introduce a new enum dma_data_direction data_dir member in struct request.
   and remove the RW bit from request-cmd_flag

Some architecture use 'enum dma_data_direction' and some 'int
dma_data_direction'. The consensus was to move to int over
time. Please use 'int dma_data_direction'.

 diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
 index ff203c4..abbca7b 100644
 --- a/include/linux/dma-mapping.h
 +++ b/include/linux/dma-mapping.h
 @@ -13,6 +13,28 @@ enum dma_data_direction {
   DMA_NONE = 3,
  };
 
 +static inline int dma_write_dir(enum dma_data_direction dir)
 +{
 + return (dir == DMA_TO_DEVICE) || (dir == DMA_BIDIRECTIONAL);
 +}

write can mean write to device or write to memory, depending on
context. Not exactly something which should be a generic
helper. Rename to 'dma_to_device(int dir)'?

 +static inline int dma_uni_dir(enum dma_data_direction dir)
 +{
 + return (dir == DMA_TO_DEVICE) || (dir == DMA_FROM_DEVICE) ||
 +(dir == DMA_NONE);
 +}

While this doesn't look very useful. Why is DMA_NONE a uni-dir? I
suggest replacing this with an open coded (dir != DMA_BIDIRECTIONAL).

Cheers,
Muli
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC 1/6] bidi support: request dma_data_direction

2007-01-21 Thread Benny Halevy
Douglas Gilbert wrote:
 Boaz Harrosh wrote:
 - Introduce a new enum dma_data_direction data_dir member in struct request.
   and remove the RW bit from request-cmd_flag
 - Add new API to query request direction.
 - Adjust existing API and implementation.
 - Cleanup wrong use of DMA_BIDIRECTIONAL
 - Introduce new blk_rq_init_unqueued_req() and use it in places ad-hoc
   requests were used and bzero'ed.
 
 With a bi-directional transfer is it always unambiguous
 which transfer occurs first (or could they occur at
 the same time)?

The bidi transfers can occur in any order and in parallel.

 
 Doug Gilbert
 -
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/