FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 02-01-2008, 07:00 PM
Mike Christie
 
Default scsi_dh: scsi handling of REQ_LB_OP_TRANSITION

Chandra Seetharaman wrote:

@@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
static void scsi_softirq_done(struct request *rq)
{
struct scsi_cmnd *cmd = rq->completion_data;
- unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
int disposition;
+ struct request_queue *q;
+ unsigned long wait_for, flags;

+ if (blk_linux_request(rq)) {

+ q = rq->q;
+ spin_lock_irqsave(q->queue_lock, flags);
+ /*
+ * we always return 1 and the caller should
+ * check rq->errors for the complete status
+ */
+ end_that_request_last(rq, 1);
+ spin_unlock_irqrestore(q->queue_lock, flags);
+ return;
+ }
+
+
+ wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
INIT_LIST_HEAD(&cmd->eh_entry);


.....


+
/*
* Function: scsi_request_fn()
*
@@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
* accept it.
*/
req = elv_next_request(q);
- if (!req || !scsi_dev_queue_ready(q, sdev))
+ if (!req)
+ break;
+
+ /*
+ * We do not account for linux blk req in the device
+ * or host busy accounting because it is not necessarily
+ * a scsi command that is sent to some object. The lower
+ * level can translate it into a request/scsi_cmnd, if
+ * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
+ */
+ if (blk_linux_request(req)) {
+ blkdev_dequeue_request(req);
+ scsi_execute_blk_linux_cmd(req);
+ continue;
+ }
+
+ if (!scsi_dev_queue_ready(q, sdev))
break;


I think these two pieces are one of the reasons I have not pushed the
patches. I thought the completion and execution pieces here are a little
ugly and seem to just wedge themselves in where they want to be.


Is there any way to make the insertion of non-scsi commands more common?
Do we have the code for being able to send requests directly to
something like a fc rport done? Could we maybe inject these special
commands to the hw handler using something similar to how bsg would send
non scsi commands to weird objects (objects like rport, sessions, and
not devices we traditionally associated with queues like scsi_devices).
Just a thought with no code that is why the ugly code existed still


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 02-04-2008, 05:59 PM
Chandra Seetharaman
 
Default scsi_dh: scsi handling of REQ_LB_OP_TRANSITION

On Fri, 2008-02-01 at 14:00 -0600, Mike Christie wrote:
> Chandra Seetharaman wrote:
> > @@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
> > static void scsi_softirq_done(struct request *rq)
> > {
> > struct scsi_cmnd *cmd = rq->completion_data;
> > - unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > int disposition;
> > + struct request_queue *q;
> > + unsigned long wait_for, flags;
> >
> > + if (blk_linux_request(rq)) {
> > + q = rq->q;
> > + spin_lock_irqsave(q->queue_lock, flags);
> > + /*
> > + * we always return 1 and the caller should
> > + * check rq->errors for the complete status
> > + */
> > + end_that_request_last(rq, 1);
> > + spin_unlock_irqrestore(q->queue_lock, flags);
> > + return;
> > + }
> > +
> > +
> > + wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > INIT_LIST_HEAD(&cmd->eh_entry);
> >
> .....
>
> > +
> > /*
> > * Function: scsi_request_fn()
> > *
> > @@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
> > * accept it.
> > */
> > req = elv_next_request(q);
> > - if (!req || !scsi_dev_queue_ready(q, sdev))
> > + if (!req)
> > + break;
> > +
> > + /*
> > + * We do not account for linux blk req in the device
> > + * or host busy accounting because it is not necessarily
> > + * a scsi command that is sent to some object. The lower
> > + * level can translate it into a request/scsi_cmnd, if
> > + * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
> > + */
> > + if (blk_linux_request(req)) {
> > + blkdev_dequeue_request(req);
> > + scsi_execute_blk_linux_cmd(req);
> > + continue;
> > + }
> > +
> > + if (!scsi_dev_queue_ready(q, sdev))
> > break;
>
> I think these two pieces are one of the reasons I have not pushed the
> patches. I thought the completion and execution pieces here are a little
> ugly and seem to just wedge themselves in where they want to be.
>
> Is there any way to make the insertion of non-scsi commands more common?
> Do we have the code for being able to send requests directly to
> something like a fc rport done? Could we maybe inject these special
> commands to the hw handler using something similar to how bsg would send
> non scsi commands to weird objects (objects like rport, sessions, and
> not devices we traditionally associated with queues like scsi_devices).
> Just a thought with no code that is why the ugly code existed still

Can't it be done with this code itself ?

If the underlying functionality is going to be provided by the hardware
handler, then can't we add additional commands (like transition) when we
need them ?

Or am I missing something ?

--

----------------------------------------------------------------------
Chandra Seetharaman | Be careful what you choose....
- sekharan@us.ibm.com | .......you may get it.
----------------------------------------------------------------------


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 02-04-2008, 06:02 PM
James Bottomley
 
Default scsi_dh: scsi handling of REQ_LB_OP_TRANSITION

On Fri, 2008-02-01 at 14:00 -0600, Mike Christie wrote:
> Chandra Seetharaman wrote:
> > @@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
> > static void scsi_softirq_done(struct request *rq)
> > {
> > struct scsi_cmnd *cmd = rq->completion_data;
> > - unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > int disposition;
> > + struct request_queue *q;
> > + unsigned long wait_for, flags;
> >
> > + if (blk_linux_request(rq)) {
> > + q = rq->q;
> > + spin_lock_irqsave(q->queue_lock, flags);
> > + /*
> > + * we always return 1 and the caller should
> > + * check rq->errors for the complete status
> > + */
> > + end_that_request_last(rq, 1);
> > + spin_unlock_irqrestore(q->queue_lock, flags);
> > + return;
> > + }
> > +
> > +
> > + wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > INIT_LIST_HEAD(&cmd->eh_entry);
> >
> .....
>
> > +
> > /*
> > * Function: scsi_request_fn()
> > *
> > @@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
> > * accept it.
> > */
> > req = elv_next_request(q);
> > - if (!req || !scsi_dev_queue_ready(q, sdev))
> > + if (!req)
> > + break;
> > +
> > + /*
> > + * We do not account for linux blk req in the device
> > + * or host busy accounting because it is not necessarily
> > + * a scsi command that is sent to some object. The lower
> > + * level can translate it into a request/scsi_cmnd, if
> > + * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
> > + */
> > + if (blk_linux_request(req)) {
> > + blkdev_dequeue_request(req);
> > + scsi_execute_blk_linux_cmd(req);
> > + continue;
> > + }
> > +
> > + if (!scsi_dev_queue_ready(q, sdev))
> > break;
>
> I think these two pieces are one of the reasons I have not pushed the
> patches. I thought the completion and execution pieces here are a little
> ugly and seem to just wedge themselves in where they want to be.
>
> Is there any way to make the insertion of non-scsi commands more common?
> Do we have the code for being able to send requests directly to
> something like a fc rport done? Could we maybe inject these special
> commands to the hw handler using something similar to how bsg would send
> non scsi commands to weird objects (objects like rport, sessions, and
> not devices we traditionally associated with queues like scsi_devices).
> Just a thought with no code that is why the ugly code existed still

We sort of do. The bsg code in scsi_transport_sas to send SMP frames to
expander devices would be an example of non-scsi commands going via a
mechanism other than being encapsulated in SCSI. I don't know if that's
the complete solution in this case, but you could investigate it.

James


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 02-06-2008, 06:00 PM
Mike Anderson
 
Default scsi_dh: scsi handling of REQ_LB_OP_TRANSITION

James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
>
> On Fri, 2008-02-01 at 14:00 -0600, Mike Christie wrote:
> > Chandra Seetharaman wrote:
> > > @@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
> > > static void scsi_softirq_done(struct request *rq)
> > > {
> > > struct scsi_cmnd *cmd = rq->completion_data;
> > > - unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > > int disposition;
> > > + struct request_queue *q;
> > > + unsigned long wait_for, flags;
> > >
> > > + if (blk_linux_request(rq)) {
> > > + q = rq->q;
> > > + spin_lock_irqsave(q->queue_lock, flags);
> > > + /*
> > > + * we always return 1 and the caller should
> > > + * check rq->errors for the complete status
> > > + */
> > > + end_that_request_last(rq, 1);
> > > + spin_unlock_irqrestore(q->queue_lock, flags);
> > > + return;
> > > + }
> > > +
> > > +
> > > + wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > > INIT_LIST_HEAD(&cmd->eh_entry);
> > >
> > .....
> >
> > > +
> > > /*
> > > * Function: scsi_request_fn()
> > > *
> > > @@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
> > > * accept it.
> > > */
> > > req = elv_next_request(q);
> > > - if (!req || !scsi_dev_queue_ready(q, sdev))
> > > + if (!req)
> > > + break;
> > > +
> > > + /*
> > > + * We do not account for linux blk req in the device
> > > + * or host busy accounting because it is not necessarily
> > > + * a scsi command that is sent to some object. The lower
> > > + * level can translate it into a request/scsi_cmnd, if
> > > + * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
> > > + */
> > > + if (blk_linux_request(req)) {
> > > + blkdev_dequeue_request(req);
> > > + scsi_execute_blk_linux_cmd(req);
> > > + continue;
> > > + }
> > > +
> > > + if (!scsi_dev_queue_ready(q, sdev))
> > > break;
> >
> > I think these two pieces are one of the reasons I have not pushed the
> > patches. I thought the completion and execution pieces here are a little
> > ugly and seem to just wedge themselves in where they want to be.
> >
> > Is there any way to make the insertion of non-scsi commands more common?
> > Do we have the code for being able to send requests directly to
> > something like a fc rport done? Could we maybe inject these special
> > commands to the hw handler using something similar to how bsg would send
> > non scsi commands to weird objects (objects like rport, sessions, and
> > not devices we traditionally associated with queues like scsi_devices).
> > Just a thought with no code that is why the ugly code existed still
>
> We sort of do. The bsg code in scsi_transport_sas to send SMP frames to
> expander devices would be an example of non-scsi commands going via a
> mechanism other than being encapsulated in SCSI. I don't know if that's
> the complete solution in this case, but you could investigate it.

I looked at the bsg code in scsi_transport_sas and all I see it doing is
calling blk_init_queue to set the request_fn. The request_fn
(*smp_request) just processes one cmd_type. Is there code is another tree
that has more processing?

A idea to allow for more control / flexibility cmd_type handlers could be
added inside request_fn, prep_rq_fn, softirq_done_fn.

I thought about this being at a higher level in the block layer, but it
would be hard to handle the request_fn cleanly at the high level. The
localized change would reduce impact on users who do not want or need per
cmd_type handlers.

A SCSI example might be something like:

static void scsi_softirq_done(struct request *rq)
{
...
sdev->cmd_type_handler[rq->cmd_type]->softirq_done(rq)
...
}

int scsi_prep_fn(struct request_queue *q, struct request *req)
{
...
sdev->cmd_type_handler[rq->cmd_type]->prep_fn(req)
...
}

static void scsi_request_fn(struct request_queue *q)
{
...
sdev->cmd_type_handler[rq->cmd_type]->request_fn(req)
...
}

This is just moving the code inside the cmd_type "if" checks, but it may
reduce the number of cmd_type "if" checks in some paths (if we make
multiple decisions based on cmd_type). On init of the sdev default
handlers would be installed.

-andmike
--
Michael Anderson
andmike@linux.vnet.ibm.com

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 02-06-2008, 07:52 PM
James Bottomley
 
Default scsi_dh: scsi handling of REQ_LB_OP_TRANSITION

On Wed, 2008-02-06 at 11:00 -0800, Mike Anderson wrote:
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> >
> > On Fri, 2008-02-01 at 14:00 -0600, Mike Christie wrote:
> > > Chandra Seetharaman wrote:
> > > > @@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
> > > > static void scsi_softirq_done(struct request *rq)
> > > > {
> > > > struct scsi_cmnd *cmd = rq->completion_data;
> > > > - unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > > > int disposition;
> > > > + struct request_queue *q;
> > > > + unsigned long wait_for, flags;
> > > >
> > > > + if (blk_linux_request(rq)) {
> > > > + q = rq->q;
> > > > + spin_lock_irqsave(q->queue_lock, flags);
> > > > + /*
> > > > + * we always return 1 and the caller should
> > > > + * check rq->errors for the complete status
> > > > + */
> > > > + end_that_request_last(rq, 1);
> > > > + spin_unlock_irqrestore(q->queue_lock, flags);
> > > > + return;
> > > > + }
> > > > +
> > > > +
> > > > + wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > > > INIT_LIST_HEAD(&cmd->eh_entry);
> > > >
> > > .....
> > >
> > > > +
> > > > /*
> > > > * Function: scsi_request_fn()
> > > > *
> > > > @@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
> > > > * accept it.
> > > > */
> > > > req = elv_next_request(q);
> > > > - if (!req || !scsi_dev_queue_ready(q, sdev))
> > > > + if (!req)
> > > > + break;
> > > > +
> > > > + /*
> > > > + * We do not account for linux blk req in the device
> > > > + * or host busy accounting because it is not necessarily
> > > > + * a scsi command that is sent to some object. The lower
> > > > + * level can translate it into a request/scsi_cmnd, if
> > > > + * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
> > > > + */
> > > > + if (blk_linux_request(req)) {
> > > > + blkdev_dequeue_request(req);
> > > > + scsi_execute_blk_linux_cmd(req);
> > > > + continue;
> > > > + }
> > > > +
> > > > + if (!scsi_dev_queue_ready(q, sdev))
> > > > break;
> > >
> > > I think these two pieces are one of the reasons I have not pushed the
> > > patches. I thought the completion and execution pieces here are a little
> > > ugly and seem to just wedge themselves in where they want to be.
> > >
> > > Is there any way to make the insertion of non-scsi commands more common?
> > > Do we have the code for being able to send requests directly to
> > > something like a fc rport done? Could we maybe inject these special
> > > commands to the hw handler using something similar to how bsg would send
> > > non scsi commands to weird objects (objects like rport, sessions, and
> > > not devices we traditionally associated with queues like scsi_devices).
> > > Just a thought with no code that is why the ugly code existed still
> >
> > We sort of do. The bsg code in scsi_transport_sas to send SMP frames to
> > expander devices would be an example of non-scsi commands going via a
> > mechanism other than being encapsulated in SCSI. I don't know if that's
> > the complete solution in this case, but you could investigate it.
>
> I looked at the bsg code in scsi_transport_sas and all I see it doing is
> calling blk_init_queue to set the request_fn. The request_fn
> (*smp_request) just processes one cmd_type. Is there code is another tree
> that has more processing?

No ... that's it. It's designed to expose a frame driven SMP
communication channel to expanders via a block tap.

Part of the problem seems to be that your current code is very much
trying to do this in-band. A block tap like the SMP handlers are
effectively out of band

> A idea to allow for more control / flexibility cmd_type handlers could be
> added inside request_fn, prep_rq_fn, softirq_done_fn.
>
> I thought about this being at a higher level in the block layer, but it
> would be hard to handle the request_fn cleanly at the high level. The
> localized change would reduce impact on users who do not want or need per
> cmd_type handlers.

But this type of thinking does lead to a lot of apparent nastiness
inside your actual handlers. Trying to do all of this in-band has you
doing a lot of callback driven async I/O stuff using
blk_execute_rq_nowait(). It might be a lot cleaner to do it out of band
on a thread using the standard waiting interfaces.

James


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 12:20 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org