Re: [PATCH 1/2] scsi_transport_fc: FC pass through support via bsg interface - revised

Seokmann Ju
2008-10-28T14:06:11Z

[-- Attachment #1: Type: text/plain, Size: 2435 bytes --]

On Oct 28, 2008, at 12:57 AM, Boaz Harrosh wrote:

> Seokmann Ju wrote:
>> On Oct 27, 2008, at 1:20 AM, Boaz Harrosh wrote:
>>
>>> FUJITA Tomonori wrote:
>>>> On Sun, 26 Oct 2008 11:38:04 +0200
>>>> Boaz Harrosh <bharrosh@panasas.com> wrote:
>>>>
>>>>> FUJITA Tomonori wrote:
>>>>>> CC'ed Jens,
>>>>> I think that all block-queue consumers should call one of
>>>>> blk_end_request(),
>>>> This is kinda what I suggested in the previous mail but as I wrote,
>>>> some of them don't now.
>>>>
>>> I think they should, specially if they're going to use the timer.
>>> The way I see it they must. It's kind of a block layer API thing.
>>> Someone calls blk_execute_xx then eventually someone needs to call
>>> blk_end_request. You could call it from bsg but only temporary until
>>> all are fixed. (because you will need an ugly check to see if
>>> request
>>> was not already ended)
>> I made following changes but, it seems not helpful for the issue.
>> It, eventually, got failed to call blk_delete_timer() as ~/block/blk-
>> core.c:__end_that_request_first() returns non-zero.
>> Inside of the __end_that_reqeust_first(), it detected 'nbytes' is
>> bigger than 'nr_bytes' in case of bidi (where req->next_rq is not
>> NULL).
>> I'm not sure whether we need to have chains of function calls
>> initiated by the blk_end_request() or blk_end_bidi_request().
>> Would it create any problems if we directly call
>> 'blk_delete_timer()'?
>>
>
> Dear Seokmann. You miss understud me. What I'm saying is that you must
> call blk_end_bidi_request at the FC end, just after you have finished
> to consume the request, and before you return it upstream. it can be
> some thing like:
>
> + blk_end_bidi_request(rq, 0, blk_rq_bytes(rq),
> + rq->next_rq ? blk_rq_bytes(rq->next_rq) : 0);
>
> In this case __end_that_reqeust_first should never return non-zero.
Hello Boaz,
Thank you for the clarification.
I made the changes accordingly and tested it, but the problem is still
there - same result of getting non-zero returns from
__end_that_request_first().
I guess that, either, I still get confused about the location or, there
is something else going on...

Sorry, I don't have public git-web.
Here is snaptshot of the FC transport layer changes.
The fc_service_done() is the callback that the FC transport layer
provides. And that is the callback called by LLD before returning.

Please let me know for any comments.

[-- Attachment #2: 0001-scsi_transport_fc-FC-pass-through-support.patch --]
[-- Type: application/octet-stream, Size: 11573 bytes --]

From ea2dc821bb75abcec3d0861e8d7ae9c80196f759 Mon Sep 17 00:00:00 2001
From: root <root@linux-atl-00.qlogic.com>
Date: Tue, 28 Oct 2008 06:49:51 -0700
Subject: [PATCH] scsi_transport_fc: FC pass through support

Signed-off-by: Seokmann Ju <seokmann.ju@qlogic.com>
---
drivers/scsi/scsi_transport_fc.c | 220 +++++++++++++++++++++++++++++++++++++-
include/scsi/scsi_transport_fc.h | 81 ++++++++++++++-
2 files changed, 298 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 1e71abf..d7eebe6 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -43,6 +43,11 @@ static void fc_vport_sched_delete(struct work_struct *work);
static int fc_vport_setup(struct Scsi_Host *shost, int channel,
struct device *pdev, struct fc_vport_identifiers *ids,
struct fc_vport **vport);
+static enum blk_eh_timer_return fc_service_timeout(struct request *req);
+static void fc_service_done(struct fc_service *);
+static int fc_service_handler(struct Scsi_Host *, struct fc_rport *,
+ struct request *, struct request_queue *);
+static void fc_bsg_remove(struct Scsi_Host *, struct fc_rport *);

/*
* Redefine so that we can have same named attributes in the
@@ -2413,11 +2418,222 @@ fc_rport_final_delete(struct work_struct *work)

transport_remove_device(dev);
device_del(dev);
+ fc_bsg_remove(shost, rport);
transport_destroy_device(dev);
put_device(&shost->shost_gendev); /* for fc_host->rport list */
put_device(dev); /* for self-reference */
}

+static enum blk_eh_timer_return fc_service_timeout(struct request *req)
+{
+ struct fc_service *service = (void *) req->special;
+ struct Scsi_Host *shost = rport_to_shost(service->rport);
+ struct fc_internal *i = to_fc_internal(shost->transportt);
+ unsigned long flags;
+ int res = 0;
+
+ if (service->rport->port_state == FC_PORTSTATE_BLOCKED)
+ return BLK_EH_RESET_TIMER;
+
+ spin_lock_irqsave(&service->service_state_lock, flags);
+ if (!(service->service_state_flag & FC_SERVICE_STATE_DONE))
+ service->service_state_flag |= FC_SERVICE_STATE_TIMEOUT;
+ spin_unlock_irqrestore(&service->service_state_lock, flags);
+
+ if (i->f->abort_fc_service)
+ res = i->f->abort_fc_service(service);
+
+ if (res) {
+ printk(KERN_ERR "ERROR: issuing FC service to the LLD "
+ "failed with status %d\n", res);
+ fc_service_done(service);
+ }
+
+ /* the blk_end_sync_io() doesn't check the error */
+ return BLK_EH_NOT_HANDLED;
+}
+
+static void fc_service_done(struct fc_service *service)
+{
+
+ if (service->service_state_flag != FC_SERVICE_STATE_DONE) {
+ if (service->service_state_flag == FC_SERVICE_STATE_TIMEOUT)
+ printk(KERN_ERR "ERROR: FC service timed out\n");
+ else if (service->service_state_flag ==
+ FC_SERVICE_STATE_ABORTED)
+ printk(KERN_ERR "ERROR: FC service aborted\n");
+ else
+ printk(KERN_ERR "ERROR: FC service not finished\n");
+ }
+
+ if (service->srv_reply.status != FC_SERVICE_COMPLETE) {
+ printk(KERN_ERR "ERROR: FC service to rport %p failed with"
+ " status 0x%x\n", service->rport,
+ service->srv_reply.status);
+ }
+
+ if (service->srv_reply.residual) {
+ service->req->data_len = 0;
+ service->req->next_rq->data_len = service->srv_reply.residual;
+ } else {
+ service->req->data_len = 0;
+ service->req->next_rq->data_len = 0;
+ }
+
+ blk_end_bidi_request(service->req, 0, blk_rq_bytes(service->req),
+ service->req->next_rq ? blk_rq_bytes(service->req->next_rq) : 0);
+ service->req->end_io(service->req, 0);
+ kfree(service->payload_dma);
+ kfree(service->response_dma);
+ kfree(service);
+}
+
+static int
+issue_fc_service(struct fc_rport *rport, struct request *req,
+ struct request *rsp, struct request_queue *q)
+{
+ int res = -ECOMM;
+ struct fc_service *service;
+ struct Scsi_Host *shost = rport_to_shost(rport);
+ struct fc_internal *i = to_fc_internal(shost->transportt);
+ struct fc_service_request *srv_request = (struct fc_service_request *)
+ req->cmd;
+
+ service = kzalloc(sizeof (struct fc_service), GFP_KERNEL);
+ if (!service)
+ return -ENOMEM;
+
+ req->special = service;
+ service->rport = rport;
+ service->req = req;
+ service->q = q;
+ service->srv_request = srv_request;
+ service->timeout = req->timeout;
+
+ service->payload_dma = (struct scatterlist *)
+ kzalloc(sizeof (struct scatterlist) * req->nr_phys_segments,
+ GFP_KERNEL);
+ sg_init_table(service->payload_dma, req->nr_phys_segments);
+ service->payload_sg_cnt =
+ blk_rq_map_sg(q, req, service->payload_dma);
+ service->payload = (void *) bio_data(req->bio);
+ service->payload_size = req->data_len;
+
+ service->response_dma = (struct scatterlist *)
+ kzalloc(sizeof (struct scatterlist) * rsp->nr_phys_segments,
+ GFP_KERNEL);
+ sg_init_table(service->response_dma, rsp->nr_phys_segments);
+ service->response_sg_cnt =
+ blk_rq_map_sg(q, rsp, service->response_dma);
+ service->response = bio_data(rsp->bio);
+ service->response_size = rsp->data_len;
+
+ /* sense area of the request structure */
+ service->reply_seq = req->sense;
+ service->service_done = fc_service_done;
+ service->service_state_flag = FC_SERVICE_STATE_PENDING;
+
+ if (i->f->execute_fc_service)
+ res = i->f->execute_fc_service(service);
+
+ if (res) {
+ printk(KERN_ERR "ERROR: issuing FC service to the LLD "
+ "failed with status %d\n", res);
+ fc_service_done(service);
+ return res;
+ }
+
+ return 0;
+}
+
+static int
+fc_service_handler(struct Scsi_Host *shost, struct fc_rport *rport,
+ struct request *req, struct request_queue *q)
+{
+ int ret;
+ struct request *rsp = req->next_rq;
+
+ if (!rsp) {
+ printk(KERN_ERR "ERROR: space for a FC service"
+ " response is missing\n");
+ return -EINVAL;
+ }
+
+ ret = issue_fc_service(rport, req, rsp, q);
+
+ return ret;
+}
+
+static void fc_service_dispatch(struct request_queue *q)
+{
+ struct request *req;
+ struct fc_rport *rport = q->queuedata;
+ struct Scsi_Host *shost = rport_to_shost(rport);
+
+ while (!blk_queue_plugged(q)) {
+ req = elv_next_request(q);
+ if (!req)
+ break;
+
+ blkdev_dequeue_request(req);
+ spin_unlock_irq(q->queue_lock);
+
+ fc_service_handler(shost, rport, req, q);
+
+ spin_lock_irq(q->queue_lock);
+ }
+}
+
+static int
+fc_bsg_initialize(struct Scsi_Host *shost, struct fc_rport *rport)
+{
+ struct request_queue *q;
+ int error;
+ struct device *dev;
+ const char *name;
+ void (*release)(struct device *);
+
+ if (!rport) {
+ printk(KERN_ERR "ERROR: rport is NULL\n");
+ return -ENOMEM;
+ }
+
+ q = blk_init_queue(fc_service_dispatch, NULL);
+ if (!q)
+ return -ENOMEM;
+
+ dev = &rport->dev;
+ name = dev->bus_id;
+ release = NULL;
+
+ error = bsg_register_queue(q, dev, name, release);
+ if (error) {
+ blk_cleanup_queue(q);
+ return -ENOMEM;
+ }
+
+ blk_queue_max_hw_segments(q, shost->sg_tablesize);
+ blk_queue_max_phys_segments(q, SCSI_MAX_SG_CHAIN_SEGMENTS);
+ blk_queue_rq_timed_out(q, fc_service_timeout);
+ blk_queue_rq_timeout(q, BLK_DEFAULT_SG_TIMEOUT);
+
+ rport->q = q;
+ q->queuedata = rport;
+ queue_flag_set_unlocked(QUEUE_FLAG_BIDI, q);
+
+ return 0;
+}
+
+static void
+fc_bsg_remove(struct Scsi_Host *shost, struct fc_rport *rport)
+{
+ struct request_queue *q = rport->q;
+
+ if (!q)
+ return;
+
+ bsg_unregister_queue(q);
+}

/**
* fc_rport_create - allocates and creates a remote FC port.
@@ -2478,8 +2694,8 @@ fc_rport_create(struct Scsi_Host *shost, int channel,
else
rport->scsi_target_id = -1;
list_add_tail(&rport->peers, &fc_host->rports);
- get_device(&shost->shost_gendev); /* for fc_host->rport list */

+ get_device(&shost->shost_gendev); /* for fc_host->rport list */
spin_unlock_irqrestore(shost->host_lock, flags);

dev = &rport->dev;
@@ -2498,6 +2714,8 @@ fc_rport_create(struct Scsi_Host *shost, int channel,
transport_add_device(dev);
transport_configure_device(dev);

+ fc_bsg_initialize(shost, rport);
+
if (rport->roles & FC_PORT_ROLE_FCP_TARGET) {
/* initiate a scan of the target */
rport->flags |= FC_RPORT_SCAN_PENDING;
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index 49d8913..68104f5 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -33,7 +33,6 @@

struct scsi_transport_template;

-
/*
* FC Port definitions - Following FC HBAAPI guidelines
*
@@ -352,6 +351,7 @@ struct fc_rport { /* aka fc_starget_attrs */
struct delayed_work fail_io_work;
struct work_struct stgt_delete_work;
struct work_struct rport_delete_work;
+ struct request_queue *q;
} __attribute__((aligned(sizeof(unsigned long))));

/* bit field values for struct fc_rport "flags" field: */
@@ -515,6 +515,80 @@ struct fc_host_attrs {
struct workqueue_struct *devloss_work_q;
};

+#define FC_STATUS_BUF_SIZE 96
+
+enum fc_frame_type {
+ FC_FRAME_TYPE_BS,
+ FC_FRAME_TYPE_ELS,
+ FC_FRAME_TYPE_IEC = 4,
+ FC_FRAME_TYPE_IP,
+ FC_FRAME_TYPE_FCP = 8,
+ FC_FRAME_TYPE_GPP,
+ FC_FRAME_TYPE_FC_CT = 0x20,
+};
+
+enum fc_service_status {
+ FC_SERVICE_COMPLETE,
+ FC_SERVICE_TIMEOUT,
+ FC_SERVICE_ABORT,
+ FC_SERVICE_UNSUPPORTED,
+ FC_SERVICE_ERROR = -1,
+};
+
+#define FC_SERVICE_STATE_PENDING 1
+#define FC_SERVICE_STATE_DONE 2
+#define FC_SERVICE_STATE_ABORTED 4
+#define FC_SERVICE_STATE_TIMEOUT 8
+
+#define FC_SERVICE_TIMEOUT 10
+
+/* request (CDB) structure of the sg_io_v4 */
+struct fc_service_request {
+ u8 request_type;
+ u8 timeout;
+ u8 reserved0;
+ u8 reserved1;
+};
+
+/* response (request sense data) structure of the sg_io_v4 */
+struct fc_service_reply {
+ enum fc_service_status status;
+ u16 error_code;
+ u16 additional_error_code;
+ u32 residual;
+};
+
+struct fc_service {
+ struct fc_rport *rport;
+ struct list_head list;
+
+ spinlock_t service_state_lock;
+ unsigned service_state_flag;
+
+ struct request *req;
+ struct request_queue *q;
+
+ /* Used by the discovery code. */
+ struct completion completion;
+ int timeout;
+
+ struct fc_service_request *srv_request;
+ void *payload;
+ struct scatterlist *payload_dma;
+ int payload_sg_cnt;
+ int payload_size;
+ void *response;
+ struct scatterlist *response_dma;
+ int response_sg_cnt;
+ int response_size;
+
+ struct fc_service_reply srv_reply;
+ void *reply_seq; /* pointer to sense area of request */
+ void (*service_done)(struct fc_service *);
+
+ void *lld_pkt;
+};
+
#define shost_to_fc_host(x) \
((struct fc_host_attrs *)(x)->shost_data)

@@ -613,6 +687,10 @@ struct fc_function_template {
int (* tsk_mgmt_response)(struct Scsi_Host *, u64, u64, int);
int (* it_nexus_response)(struct Scsi_Host *, u64, int);

+ /* fc service handler */
+ int (*execute_fc_service)(struct fc_service *);
+ int (*abort_fc_service)(struct fc_service *);
+
/* allocation lengths for host-specific data */
u32 dd_fcrport_size;
u32 dd_fcvport_size;
@@ -736,7 +814,6 @@ fc_vport_set_state(struct fc_vport *vport, enum fc_vport_state new_state)
vport->vport_state = new_state;
}

-
struct scsi_transport_template *fc_attach_transport(
struct fc_function_template *);
void fc_release_transport(struct scsi_transport_template *);
--
1.6.0.2

[-- Attachment #3: Type: text/plain, Size: 21 bytes --]

Thank you,
Seokmann