* [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler
@ 2012-04-10 17:06 Moger, Babu
2012-04-18 5:32 ` Mike Christie
2012-04-20 16:27 ` Chandra Seetharaman
0 siblings, 2 replies; 6+ messages in thread
From: Moger, Babu @ 2012-04-10 17:06 UTC (permalink / raw)
To: linux-scsi
Cc: device-mapper development, Krishnasamy, Somasundaram, Stankey, Robert
This patch adds the dynamic match functionality to scsi_dh_rdac driver.
New vendor and product strings are normally not there in the kernel. Whenever
the new product is tested with this driver we see failures during the disco-
very. Without the handler attached, the default scsi mid-level retries would
take effect for certain check-conditions. Default retries(5) are not enough in
some cases(especially with large configuration or huge lun cases).
This patch will send an inquiry if the vendor/product strings are not in the
device list and match with vendor signature.
Signed-off-by: Babu Moger <babu.moger@netapp.com>
Reviewed-by: Somasundaram Krishnasamy <Somasundaram.Krishnasamy@netapp.com>
---
--- linux-3.4-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c.orig 2012-04-07 20:30:41.000000000 -0500
+++ linux-3.4-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c 2012-04-10 10:48:40.000000000 -0500
@@ -818,7 +818,10 @@ static const struct scsi_dh_devlist rdac
static bool rdac_match(struct scsi_device *sdev)
{
- int i;
+ int i, err;
+ struct c8_inquiry inqp;
+ struct request *rq;
+ struct request_queue *q = sdev->request_queue;
if (scsi_device_tpgs(sdev))
return false;
@@ -831,6 +834,27 @@ static bool rdac_match(struct scsi_devic
return true;
}
}
+
+ /* Now lets try to match the signature on 0xC8 page */
+ memset(&inqp, 0, sizeof(struct c8_inquiry));
+ rq = get_rdac_req(sdev, &inqp, sizeof(struct c8_inquiry), READ);
+ if (!rq)
+ return false;
+
+ rq->cmd[0] = INQUIRY;
+ rq->cmd[1] = 1;
+ rq->cmd[2] = 0xC8;
+ rq->cmd[4] = sizeof(struct c8_inquiry);
+ rq->cmd_len = COMMAND_SIZE(INQUIRY);
+
+ err = blk_execute_rq(q, NULL, rq, 1);
+ blk_put_request(rq);
+ if (err != -EIO) {
+ if (inqp.page_id[0] == 'e' && inqp.page_id[1] == 'd' &&
+ inqp.page_id[2] == 'i' && inqp.page_id[3] == 'd')
+ return true;
+ }
+
return false;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler
2012-04-10 17:06 [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler Moger, Babu
@ 2012-04-18 5:32 ` Mike Christie
2012-04-20 16:27 ` Chandra Seetharaman
1 sibling, 0 replies; 6+ messages in thread
From: Mike Christie @ 2012-04-18 5:32 UTC (permalink / raw)
To: Moger, Babu
Cc: linux-scsi, device-mapper development, Krishnasamy, Somasundaram,
Stankey, Robert
On 04/10/2012 12:06 PM, Moger, Babu wrote:
> This patch adds the dynamic match functionality to scsi_dh_rdac driver.
> New vendor and product strings are normally not there in the kernel. Whenever
> the new product is tested with this driver we see failures during the disco-
> very. Without the handler attached, the default scsi mid-level retries would
> take effect for certain check-conditions. Default retries(5) are not enough in
> some cases(especially with large configuration or huge lun cases).
>
> This patch will send an inquiry if the vendor/product strings are not in the
> device list and match with vendor signature.
>
> Signed-off-by: Babu Moger <babu.moger@netapp.com>
> Reviewed-by: Somasundaram Krishnasamy <Somasundaram.Krishnasamy@netapp.com>
> ---
>
> --- linux-3.4-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c.orig 2012-04-07 20:30:41.000000000 -0500
> +++ linux-3.4-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c 2012-04-10 10:48:40.000000000 -0500
> @@ -818,7 +818,10 @@ static const struct scsi_dh_devlist rdac
>
> static bool rdac_match(struct scsi_device *sdev)
> {
> - int i;
> + int i, err;
> + struct c8_inquiry inqp;
The per function stack use limit is 512 bytes right? If so then the
patch seems ok to me (thought maybe the c8_inquiry was too large but it
is under that 512 limit), so:
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler
2012-04-10 17:06 [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler Moger, Babu
2012-04-18 5:32 ` Mike Christie
@ 2012-04-20 16:27 ` Chandra Seetharaman
2012-04-20 16:48 ` Mike Christie
1 sibling, 1 reply; 6+ messages in thread
From: Chandra Seetharaman @ 2012-04-20 16:27 UTC (permalink / raw)
To: Moger, Babu
Cc: linux-scsi, device-mapper development, Krishnasamy, Somasundaram
Babu,
So, in effect, any SCSI devices with INQ C8 page with "edid" would be
treated as a rdac device, right ?
Is that enough to identify _any_ SCSI device as a rdac device (even in
future) ?
Also, instead of repeating the code, can you use submit_inquiry() ?
Chandra
On Tue, 2012-04-10 at 17:06 +0000, Moger, Babu wrote:
> This patch adds the dynamic match functionality to scsi_dh_rdac driver.
> New vendor and product strings are normally not there in the kernel. Whenever
> the new product is tested with this driver we see failures during the disco-
> very. Without the handler attached, the default scsi mid-level retries would
> take effect for certain check-conditions. Default retries(5) are not enough in
> some cases(especially with large configuration or huge lun cases).
>
> This patch will send an inquiry if the vendor/product strings are not in the
> device list and match with vendor signature.
>
> Signed-off-by: Babu Moger <babu.moger@netapp.com>
> Reviewed-by: Somasundaram Krishnasamy <Somasundaram.Krishnasamy@netapp.com>
> ---
>
> --- linux-3.4-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c.orig 2012-04-07 20:30:41.000000000 -0500
> +++ linux-3.4-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c 2012-04-10 10:48:40.000000000 -0500
> @@ -818,7 +818,10 @@ static const struct scsi_dh_devlist rdac
>
> static bool rdac_match(struct scsi_device *sdev)
> {
> - int i;
> + int i, err;
> + struct c8_inquiry inqp;
> + struct request *rq;
> + struct request_queue *q = sdev->request_queue;
>
> if (scsi_device_tpgs(sdev))
> return false;
> @@ -831,6 +834,27 @@ static bool rdac_match(struct scsi_devic
> return true;
> }
> }
> +
> + /* Now lets try to match the signature on 0xC8 page */
> + memset(&inqp, 0, sizeof(struct c8_inquiry));
> + rq = get_rdac_req(sdev, &inqp, sizeof(struct c8_inquiry), READ);
> + if (!rq)
> + return false;
> +
> + rq->cmd[0] = INQUIRY;
> + rq->cmd[1] = 1;
> + rq->cmd[2] = 0xC8;
> + rq->cmd[4] = sizeof(struct c8_inquiry);
> + rq->cmd_len = COMMAND_SIZE(INQUIRY);
> +
> + err = blk_execute_rq(q, NULL, rq, 1);
> + blk_put_request(rq);
> + if (err != -EIO) {
> + if (inqp.page_id[0] == 'e' && inqp.page_id[1] == 'd' &&
> + inqp.page_id[2] == 'i' && inqp.page_id[3] == 'd')
> + return true;
> + }
> +
> return false;
> }
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler
2012-04-20 16:27 ` Chandra Seetharaman
@ 2012-04-20 16:48 ` Mike Christie
2012-04-20 18:05 ` Chandra Seetharaman
0 siblings, 1 reply; 6+ messages in thread
From: Mike Christie @ 2012-04-20 16:48 UTC (permalink / raw)
To: sekharan
Cc: Moger, Babu, linux-scsi, device-mapper development, Krishnasamy,
Somasundaram
On 04/20/2012 11:27 AM, Chandra Seetharaman wrote:
> Babu,
>
> So, in effect, any SCSI devices with INQ C8 page with "edid" would be
> treated as a rdac device, right ?
>
> Is that enough to identify _any_ SCSI device as a rdac device (even in
> future) ?
>
> Also, instead of repeating the code, can you use submit_inquiry() ?
>
Yeah you are right, I think we should modify submit_inquiry to take in
the sense and data buffers instead of taking in the rdac_dh_data struct.
At the time the match function is run rdac_dh_data might not be allocated.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler
2012-04-20 16:48 ` Mike Christie
@ 2012-04-20 18:05 ` Chandra Seetharaman
2012-04-20 20:45 ` Moger, Babu
0 siblings, 1 reply; 6+ messages in thread
From: Chandra Seetharaman @ 2012-04-20 18:05 UTC (permalink / raw)
To: Mike Christie
Cc: Moger, Babu, linux-scsi, device-mapper development, Krishnasamy,
Somasundaram
On Fri, 2012-04-20 at 11:48 -0500, Mike Christie wrote:
> On 04/20/2012 11:27 AM, Chandra Seetharaman wrote:
> > Babu,
> >
> > So, in effect, any SCSI devices with INQ C8 page with "edid" would be
> > treated as a rdac device, right ?
> >
> > Is that enough to identify _any_ SCSI device as a rdac device (even in
> > future) ?
> >
> > Also, instead of repeating the code, can you use submit_inquiry() ?
> >
>
> Yeah you are right, I think we should modify submit_inquiry to take in
> the sense and data buffers instead of taking in the rdac_dh_data struct.
> At the time the match function is run rdac_dh_data might not be allocated.
oh yeah,
I did not not realize that...
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler
2012-04-20 18:05 ` Chandra Seetharaman
@ 2012-04-20 20:45 ` Moger, Babu
0 siblings, 0 replies; 6+ messages in thread
From: Moger, Babu @ 2012-04-20 20:45 UTC (permalink / raw)
To: sekharan, Mike Christie
Cc: linux-scsi, device-mapper development, Krishnasamy, Somasundaram, hare
Chandra/Mike, Thanks for your comments.
> -----Original Message-----
> From: Chandra Seetharaman [mailto:sekharan@us.ibm.com]
> Sent: Friday, April 20, 2012 1:06 PM
> To: Mike Christie
> Cc: Moger, Babu; linux-scsi@vger.kernel.org; device-mapper development;
> Krishnasamy, Somasundaram
> Subject: Re: [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac
> handler
>
> On Fri, 2012-04-20 at 11:48 -0500, Mike Christie wrote:
> > On 04/20/2012 11:27 AM, Chandra Seetharaman wrote:
> > > Babu,
> > >
> > > So, in effect, any SCSI devices with INQ C8 page with "edid" would be
> > > treated as a rdac device, right
> > > Is that enough to identify _any_ SCSI device as a rdac device (even in
> > > future) ?
These checks are only to pass the match function. Actual checks in attach function.
So, it should not cause any issues.
> > >
> > > Also, instead of repeating the code, can you use submit_inquiry() ?
> > >
> >
> > Yeah you are right, I think we should modify submit_inquiry to take in
> > the sense and data buffers instead of taking in the rdac_dh_data struct.
> > At the time the match function is run rdac_dh_data might not be allocated.
>
> oh yeah,
>
> I did not not realize that...
> >
Changing the submit_inquiry involves some more changes. Will try to see if
I can make those changes in my next set of patches.
Actually the bigger problem here is match is called with spin_lock held.
We are making the blocking calls with spin lock held. Thanks to Hannes for
pointing that out. Will replace the spin_lock with mutex lock.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-20 20:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-10 17:06 [PATCH] scsi_dh_rdac: Add dynamic match functionality to rdac handler Moger, Babu
2012-04-18 5:32 ` Mike Christie
2012-04-20 16:27 ` Chandra Seetharaman
2012-04-20 16:48 ` Mike Christie
2012-04-20 18:05 ` Chandra Seetharaman
2012-04-20 20:45 ` Moger, Babu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.