Re: [PATCH 14/15] block/bsg: move queue creation into bsg_setup_queue

2017-01-11 Thread Christoph Hellwig 
On Wed, Jan 11, 2017 at 05:01:22PM -0500, Mike Snitzer wrote:
> But I've seen you reference the need to stop multipath from allocating
> its own requests.  Are you referring to old request_fn request-based
> multipath's clone_old_rq:alloc_old_clone_request?

Yes, that one is the issue.  It allocates a struct request "blind",
that is without known what queue it goes to.  With this queue (or blk-mq
for that matter) we need to know the queue, because the request structures
might have additional data behind it and require additional initialization
for drivers that require per-request data.  We make use of the per-request
data for SCSI passthrough in this patch.

> Or how blk-mq request-based multipath gets a request from the blk-mq tag
> space (via blk_mq_alloc_request)?

That's fine because it works on the queue of the device that I/O is
submitted to.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/15] block/bsg: move queue creation into bsg_setup_queue

2017-01-11 Thread Mike Snitzer 
On Wed, Jan 11 2017 at  3:45am -0500,
Christoph Hellwig  wrote:

> On Wed, Jan 11, 2017 at 09:42:44AM +0100, Johannes Thumshirn wrote:
> > On Tue, Jan 10, 2017 at 04:06:19PM +0100, Christoph Hellwig wrote:
> > > Simply the boilerplate code needed for bsg nodes a bit.
> > > 
> > > Signed-off-by: Christoph Hellwig 
> > > ---
> > 
> > that reminds me of posting my SAS bsg-lib patch...
> 
> Yes.  Having SAS use bsg-lib, and bsg-lib switched away from abusing
> struct request_queue would make this series a lot cleaner.
> 
> So maybe we should get that into the scsi tree for 4.10 together
> with the prep patches in this series as a priority and defer the actual
> struct request changes once again.  That should also give us some more
> time to sort out the dm-mpath story..

I'm not aware of the story you're referring to.  I'm missing the actual
challenge you're facing.

But I've seen you reference the need to stop multipath from allocating
its own requests.  Are you referring to old request_fn request-based
multipath's clone_old_rq:alloc_old_clone_request?

Or how blk-mq request-based multipath gets a request from the blk-mq tag
space (via blk_mq_alloc_request)?

Or both?  How is that holding you back?
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/15] block/bsg: move queue creation into bsg_setup_queue

2017-01-11 Thread Hannes Reinecke 
On 01/11/2017 10:01 AM, Christoph Hellwig wrote:
> On Wed, Jan 11, 2017 at 09:59:17AM +0100, Hannes Reinecke wrote:
>> I'd advocate to discuss this at LSF.
>> Now that Mike moved the bio-based mpath stuff back in things got even
>> more complex.
> 
> Yeah.  If we'd _only_ have bio based support it would simplify things
> a lot, but as a third parallel path it's not exactly making things easier.
> 
>> I'll be posting a patchset for reimplementing multipath as a stand-alone
>> driver shortly; that'll give us a good starting point on how we want
>> multipath to evolve.
>>
>> Who knows; we might even manage to move multipath out of device-mapper
>> altogether.
>> That would make Mike very happy, and I wouldn't mind, either :-)
> 
> Heh.  I'm curious how you want to do that while keeping existing setups
> working, though.

which will become challenging, indeed.

ATM it's just a testbed on how things could work; we've got most of the
required infrastucture in the kernel nowadays, so that we can just drop
most of the complexity from the present multipath-tools mess.

In the end it might even boil down to update the existing device-mapper
multipath implementation. We'll see.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/15] block/bsg: move queue creation into bsg_setup_queue

2017-01-11 Thread Christoph Hellwig 
On Wed, Jan 11, 2017 at 09:59:17AM +0100, Hannes Reinecke wrote:
> I'd advocate to discuss this at LSF.
> Now that Mike moved the bio-based mpath stuff back in things got even
> more complex.

Yeah.  If we'd _only_ have bio based support it would simplify things
a lot, but as a third parallel path it's not exactly making things easier.

> I'll be posting a patchset for reimplementing multipath as a stand-alone
> driver shortly; that'll give us a good starting point on how we want
> multipath to evolve.
> 
> Who knows; we might even manage to move multipath out of device-mapper
> altogether.
> That would make Mike very happy, and I wouldn't mind, either :-)

Heh.  I'm curious how you want to do that while keeping existing setups
working, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/15] block/bsg: move queue creation into bsg_setup_queue

2017-01-11 Thread Hannes Reinecke 
On 01/11/2017 09:45 AM, Christoph Hellwig wrote:
> On Wed, Jan 11, 2017 at 09:42:44AM +0100, Johannes Thumshirn wrote:
>> On Tue, Jan 10, 2017 at 04:06:19PM +0100, Christoph Hellwig wrote:
>>> Simply the boilerplate code needed for bsg nodes a bit.
>>>
>>> Signed-off-by: Christoph Hellwig 
>>> ---
>>
>> that reminds me of posting my SAS bsg-lib patch...
> 
> Yes.  Having SAS use bsg-lib, and bsg-lib switched away from abusing
> struct request_queue would make this series a lot cleaner.
> 
> So maybe we should get that into the scsi tree for 4.10 together
> with the prep patches in this series as a priority and defer the actual
> struct request changes once again.  That should also give us some more
> time to sort out the dm-mpath story..
I'd advocate to discuss this at LSF.
Now that Mike moved the bio-based mpath stuff back in things got even
more complex.

I'll be posting a patchset for reimplementing multipath as a stand-alone
driver shortly; that'll give us a good starting point on how we want
multipath to evolve.

Who knows; we might even manage to move multipath out of device-mapper
altogether.
That would make Mike very happy, and I wouldn't mind, either :-)

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/15] block/bsg: move queue creation into bsg_setup_queue

2017-01-11 Thread Christoph Hellwig 
On Wed, Jan 11, 2017 at 09:42:44AM +0100, Johannes Thumshirn wrote:
> On Tue, Jan 10, 2017 at 04:06:19PM +0100, Christoph Hellwig wrote:
> > Simply the boilerplate code needed for bsg nodes a bit.
> > 
> > Signed-off-by: Christoph Hellwig 
> > ---
> 
> that reminds me of posting my SAS bsg-lib patch...

Yes.  Having SAS use bsg-lib, and bsg-lib switched away from abusing
struct request_queue would make this series a lot cleaner.

So maybe we should get that into the scsi tree for 4.10 together
with the prep patches in this series as a priority and defer the actual
struct request changes once again.  That should also give us some more
time to sort out the dm-mpath story..
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/15] block/bsg: move queue creation into bsg_setup_queue

2017-01-11 Thread Johannes Thumshirn 
On Tue, Jan 10, 2017 at 04:06:19PM +0100, Christoph Hellwig wrote:
> Simply the boilerplate code needed for bsg nodes a bit.
> 
> Signed-off-by: Christoph Hellwig 
> ---

that reminds me of posting my SAS bsg-lib patch...

Anyways looks good,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 14/15] block/bsg: move queue creation into bsg_setup_queue

2017-01-10 Thread Christoph Hellwig 
Simply the boilerplate code needed for bsg nodes a bit.

Signed-off-by: Christoph Hellwig 
---
 block/bsg-lib.c | 21 +++--
 drivers/scsi/scsi_transport_fc.c| 36 
 drivers/scsi/scsi_transport_iscsi.c | 15 ---
 include/linux/bsg-lib.h |  5 ++---
 4 files changed, 25 insertions(+), 52 deletions(-)

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 9d652a9..c74acf4 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -177,7 +177,7 @@ static int bsg_create_job(struct device *dev, struct 
request *req)
  *
  * Drivers/subsys should pass this to the queue init function.
  */
-void bsg_request_fn(struct request_queue *q)
+static void bsg_request_fn(struct request_queue *q)
__releases(q->queue_lock)
__acquires(q->queue_lock)
 {
@@ -214,24 +214,24 @@ void bsg_request_fn(struct request_queue *q)
put_device(dev);
spin_lock_irq(q->queue_lock);
 }
-EXPORT_SYMBOL_GPL(bsg_request_fn);
 
 /**
  * bsg_setup_queue - Create and add the bsg hooks so we can receive requests
  * @dev: device to attach bsg device to
- * @q: request queue setup by caller
  * @name: device to give bsg device
  * @job_fn: bsg job handler
  * @dd_job_size: size of LLD data needed for each job
- *
- * The caller should have setup the reuqest queue with bsg_request_fn
- * as the request_fn.
  */
-int bsg_setup_queue(struct device *dev, struct request_queue *q,
-   char *name, bsg_job_fn *job_fn, int dd_job_size)
+struct request_queue *bsg_setup_queue(struct device *dev, char *name,
+   bsg_job_fn *job_fn, int dd_job_size)
 {
+   struct request_queue *q;
int ret;
 
+   q = blk_init_queue(bsg_request_fn, NULL);
+   if (!q)
+   return ERR_PTR(-ENOMEM);
+
q->queuedata = dev;
q->bsg_job_size = dd_job_size;
q->bsg_job_fn = job_fn;
@@ -243,9 +243,10 @@ int bsg_setup_queue(struct device *dev, struct 
request_queue *q,
if (ret) {
printk(KERN_ERR "%s: bsg interface failed to "
   "initialize - register queue\n", dev->kobj.name);
-   return ret;
+   blk_cleanup_queue(q);
+   return ERR_PTR(ret);
}
 
-   return 0;
+   return q;
 }
 EXPORT_SYMBOL_GPL(bsg_setup_queue);
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index afcedec..13dcb9b 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3765,7 +3765,6 @@ fc_bsg_hostadd(struct Scsi_Host *shost, struct 
fc_host_attrs *fc_host)
struct device *dev = >shost_gendev;
struct fc_internal *i = to_fc_internal(shost->transportt);
struct request_queue *q;
-   int err;
char bsg_name[20];
 
fc_host->rqst_q = NULL;
@@ -3776,24 +3775,14 @@ fc_bsg_hostadd(struct Scsi_Host *shost, struct 
fc_host_attrs *fc_host)
snprintf(bsg_name, sizeof(bsg_name),
 "fc_host%d", shost->host_no);
 
-   q = blk_init_queue(bsg_request_fn, NULL);
-   if (!q) {
-   dev_err(dev,
-   "fc_host%d: bsg interface failed to initialize - no 
request queue\n",
-   shost->host_no);
-   return -ENOMEM;
-   }
-
-   __scsi_init_queue(shost, q);
-   err = bsg_setup_queue(dev, q, bsg_name, fc_bsg_dispatch,
-i->f->dd_bsg_size);
-   if (err) {
+   q = bsg_setup_queue(dev, bsg_name, fc_bsg_dispatch, i->f->dd_bsg_size);
+   if (IS_ERR(q)) {
dev_err(dev,
"fc_host%d: bsg interface failed to initialize - setup 
queue\n",
shost->host_no);
-   blk_cleanup_queue(q);
-   return err;
+   return PTR_ERR(q);
}
+   __scsi_init_queue(shost, q);
blk_queue_rq_timed_out(q, fc_bsg_job_timeout);
blk_queue_rq_timeout(q, FC_DEFAULT_BSG_TIMEOUT);
fc_host->rqst_q = q;
@@ -3825,27 +3814,18 @@ fc_bsg_rportadd(struct Scsi_Host *shost, struct 
fc_rport *rport)
struct device *dev = >dev;
struct fc_internal *i = to_fc_internal(shost->transportt);
struct request_queue *q;
-   int err;
 
rport->rqst_q = NULL;
 
if (!i->f->bsg_request)
return -ENOTSUPP;
 
-   q = blk_init_queue(bsg_request_fn, NULL);
-   if (!q) {
-   dev_err(dev, "bsg interface failed to initialize - no request 
queue\n");
-   return -ENOMEM;
-   }
-
-   __scsi_init_queue(shost, q);
-   err = bsg_setup_queue(dev, q, NULL, fc_bsg_dispatch, i->f->dd_bsg_size);
-   if (err) {
+   q = bsg_setup_queue(dev, NULL, fc_bsg_dispatch, i->f->dd_bsg_size);
+   if (IS_ERR(q)) {
dev_err(dev, "failed to setup bsg queue\n");
-   blk_cleanup_queue(q);
-   return err;
+