[patchset 0/4] osd: Stop usage of blk_rq_append_bio

[patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Boaz Harrosh]

Osd library needs to submit pre-allocated bios, form several sources.
osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
On top of that the osd library needs to append additional segments to the
IO memory, for get/set attributes and more.

All these are done today by use of a temporary hack - blk_rq_append_bio.
This is bad on few accounts.
1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
2. blk_rq_append_bio does not support chained bios.
3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
   has a bug.

The proposed solution adds two new fixtures to the block layer, and a corresponding
fixing patch to osd. These are:

[PATCH 1/4] allow blk_rq_map_kern to append to requests
[PATCH 2/4] libosd: Use new blk_rq_map_kern

  This is originally a James patch and it's used, to let blk_rq_map_kern append it's buffer
  to existing bio, and there for is able to be called multiple times in a loop, to append
  multiple segments. This API can also be useful for scsi/block targets that have segment
  information in some other memory structure (like scatterlist) and wants to set it into
  a request. Until such time that they have a proper support for mapping scatterlists directly.
  (Since above called on long lists might not be good for performance)

  Here in osd it makes tons of sense, and should be considered for inclusion.
  (The patches are based on linus-tip but should patch on block tree)

[RFC 3/4] New blk_make_request(), takes bio, returns a request
[RFC 4/4] libosd: Use of new blk_make_request
  
  Here I propose a new block API, that will support proper delegation of a bio
  to a full request. Please read inside the patch descriptions for details.
  After this patch both osd and block layer will have the proper support for osdblk
  driver as well as future needs.
  These patches also eliminate the last use of blk_rq_append_bio which can be now un-exported.

  These two patches conflic with Tejun's branch and are based on linus-tip. Upon positive review
  I will serialize them with Tejun and submit them properly. But first they must be agreed upon.
  Jens, I specially need your opinion on this?

Cheers
Boaz

[PATCH 1/4] allow blk_rq_map_kern to append to requests

[Boaz Harrosh]

From: James Bottomley <James.Bottomley@HansenPartnership.com>

Use blk_rq_append_bio() internally instead of blk_rq_bio_prep()
so blk_rq_map_kern can be called multiple times, to map multiple
buffers.

This is in the effort to un-export blk_rq_append_bio()

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 block/blk-map.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/block/blk-map.c b/block/blk-map.c
index f103729..ada399e 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -282,7 +282,8 @@ EXPORT_SYMBOL(blk_rq_unmap_user);
  *
  * Description:
  *    Data will be mapped directly if possible. Otherwise a bounce
- *    buffer is used.
+ *    buffer is used. Can be called multple times to append multple
+ *    buffers.
  */
 int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf,
 		    unsigned int len, gfp_t gfp_mask)
@@ -290,6 +291,7 @@ int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf,
 	int reading = rq_data_dir(rq) == READ;
 	int do_copy = 0;
 	struct bio *bio;
+	int ret;
 
 	if (len > (q->max_hw_sectors << 9))
 		return -EINVAL;
@@ -311,7 +313,13 @@ int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf,
 	if (do_copy)
 		rq->cmd_flags |= REQ_COPY_USER;
 
-	blk_rq_bio_prep(q, rq, bio);
+	ret = blk_rq_append_bio(q, rq, bio);
+	if (unlikely(ret)) {
+		/* request is too big */
+		bio_put(bio);
+		return ret;
+	}
+
 	blk_queue_bounce(q, &rq->bio);
 	rq->buffer = rq->data = NULL;
 	return 0;
-- 
1.6.2.1

[PATCH 2/4] libosd: Use new blk_rq_map_kern

[Boaz Harrosh]

Now that blk_rq_map_kern will append the buffer onto the
request we can use it easily for adding extra segments
(eg. attributes)

This patch is dependent on a block layer patch titled:
   [BLOCK] allow blk_rq_map_kern to append to requests

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |   24 ++----------------------
 1 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 3f5ec57..a36c406 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -921,26 +921,6 @@ int osd_req_add_set_attr_list(struct osd_request *or,
 }
 EXPORT_SYMBOL(osd_req_add_set_attr_list);
 
-static int _append_map_kern(struct request *req,
-	void *buff, unsigned len, gfp_t flags)
-{
-	struct bio *bio;
-	int ret;
-
-	bio = bio_map_kern(req->q, buff, len, flags);
-	if (IS_ERR(bio)) {
-		OSD_ERR("Failed bio_map_kern(%p, %d) => %ld\n", buff, len,
-			PTR_ERR(bio));
-		return PTR_ERR(bio);
-	}
-	ret = blk_rq_append_bio(req->q, req, bio);
-	if (ret) {
-		OSD_ERR("Failed blk_rq_append_bio(%p) => %d\n", bio, ret);
-		bio_put(bio);
-	}
-	return ret;
-}
-
 static int _req_append_segment(struct osd_request *or,
 	unsigned padding, struct _osd_req_data_segment *seg,
 	struct _osd_req_data_segment *last_seg, struct _osd_io_info *io)
@@ -956,14 +936,14 @@ static int _req_append_segment(struct osd_request *or,
 		else
 			pad_buff = io->pad_buff;
 
-		ret = _append_map_kern(io->req, pad_buff, padding,
+		ret = blk_rq_map_kern(io->req->q, io->req, pad_buff, padding,
 				       or->alloc_flags);
 		if (ret)
 			return ret;
 		io->total_bytes += padding;
 	}
 
-	ret = _append_map_kern(io->req, seg->buff, seg->total_bytes,
+	ret = blk_rq_map_kern(io->req->q, io->req, seg->buff, seg->total_bytes,
 			       or->alloc_flags);
 	if (ret)
 		return ret;
-- 
1.6.2.1

[RFC 3/4] New blk_make_request(), takes bio, returns a request

[Boaz Harrosh]

New block API:
given a struct bio allocates a new request. This is the parallel of
generic_make_request for BLOCK_PC commands users.

The passed bio may be a chained-bio. The bio is bounced if needed
inside the call to this member.

This is in the effort of un-exporting blk_rq_append_bio().

Requested-comments-by: Boaz Harrosh <bharrosh@panasas.com>
TO: Jens Axboe <Jens.Axboe@oracle.com>
CC: Tejun Heo <tj@kernel.org>
CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
CC: James Bottomley <James.Bottomley@hansenpartnership.com>
CC: Jeff Garzik <jeff@garzik.org>
---
 block/blk-core.c       |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h |    2 ++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 2998fe3..d79c6e8 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -902,6 +902,52 @@ struct request *blk_get_request(struct request_queue *q, int rw, gfp_t gfp_mask)
 EXPORT_SYMBOL(blk_get_request);
 
 /**
+ * blk_make_request - given a bio, allocate a corresponding struct request.
+ *
+ * @bio:  The bio describing the memory mappings that will be submitted for IO.
+ *        It may be a chained-bio properly constructed by block/bio layer.
+ *
+ * blk_make_request is the parallel of generic_make_request for BLOCK_PC
+ * type commands. Where the struct request needs to be farther initialized by
+ * the caller. It is passed a &struct bio, which describes the memory info to
+ * be read/written to/from.
+ *
+ * The caller of blk_make_request must make sure that bi_io_vec
+ * are set to describe the memory buffers. That bio_data_dir() will return
+ * the needed direction of the request. (And all bio's in the passed bio-chain
+ * are properly set accordingly), the passed total_bytes may be shorter then
+ * the passed total_bytes(bio-chain).
+ *
+ * If called under none-sleepable conditions, mapped bio buffers must not
+ * need bouncing, by calling the appropriate masked or flagged allocator,
+ * suitable for the target device. Otherwise the call to blk_queue_bounce will
+ * BUG.
+ */
+struct request *blk_make_request(struct request_queue *q, struct bio *bio,
+				 gfp_t gfp_mask)
+{
+	struct request *rq = blk_get_request(q, bio_data_dir(bio), gfp_mask);
+
+	if (unlikely(!rq))
+		return ERR_PTR(-ENOMEM);
+
+	for_each_bio(bio) {
+		struct bio *bounce_bio = bio;
+		int ret;
+
+		blk_queue_bounce(q, &bounce_bio);
+		ret = blk_rq_append_bio(q, rq, bounce_bio);
+		if (unlikely(ret)) {
+			blk_put_request(rq);
+			return ERR_PTR(ret);
+		}
+	}
+
+	return rq;
+}
+EXPORT_SYMBOL(blk_make_request);
+
+/**
  * blk_start_queueing - initiate dispatch of requests to device
  * @q:		request queue to kick into gear
  *
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index b4f71f1..19728b0 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -752,6 +752,8 @@ extern void blk_rq_init(struct request_queue *q, struct request *rq);
 extern void blk_put_request(struct request *);
 extern void __blk_put_request(struct request_queue *, struct request *);
 extern struct request *blk_get_request(struct request_queue *, int, gfp_t);
+extern struct request *blk_make_request(struct request_queue *, struct bio *,
+					gfp_t);
 extern void blk_insert_request(struct request_queue *, struct request *, int, void *);
 extern void blk_requeue_request(struct request_queue *, struct request *);
 extern int blk_rq_check_limits(struct request_queue *q, struct request *rq);
-- 
1.6.2.1

[RFC 4/4] libosd: Use of new blk_make_request

[Boaz Harrosh]

Use new blk_make_request() to allocate a request from bio
and avoid using deprecated blk_rq_append_bio().

This patch is dependent on a block layer patch titled:
    [BLOCK] New blk_make_request() takes bio returns request

This is the last usage of blk_rq_append_bio in osd, it can now
be un-exported.

Also at this point Jeff's osdblk driver will properly
work, as osd_initiator now supports chained-bios.

Request-comments-by: Boaz Harrosh <bharrosh@panasas.com>
CC: Jeff Garzik <jeff@garzik.org>
CC: Tejun Heo <tj@kernel.org>
CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
CC: James Bottomley <James.Bottomley@hansenpartnership.com>
---
 drivers/scsi/osd/osd_initiator.c |   48 ++++++++++++++++++-------------------
 1 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index a36c406..3d460f8 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -1304,6 +1304,21 @@ static int _osd_req_finalize_data_integrity(struct osd_request *or,
 /*
  * osd_finalize_request and helpers
  */
+static struct request *_make_request(struct request_queue *q, bool has_write,
+			      struct _osd_io_info *oii, gfp_t flags)
+{
+	if (oii->bio)
+		return blk_make_request(q, oii->bio, flags);
+	else {
+		struct request *req;
+
+		req = blk_get_request(q, has_write ? WRITE : READ, flags);
+		if (unlikely(!req))
+			return ERR_PTR(-ENOMEM);
+
+		return req;
+	}
+}
 
 static int _init_blk_request(struct osd_request *or,
 	bool has_in, bool has_out)
@@ -1312,11 +1327,13 @@ static int _init_blk_request(struct osd_request *or,
 	struct scsi_device *scsi_device = or->osd_dev->scsi_device;
 	struct request_queue *q = scsi_device->request_queue;
 	struct request *req;
-	int ret = -ENOMEM;
+	int ret;
 
-	req = blk_get_request(q, has_out, flags);
-	if (!req)
+	req = _make_request(q, has_out, has_out ? &or->out : &or->in, flags);
+	if (IS_ERR(req)) {
+		ret = PTR_ERR(req);
 		goto out;
+	}
 
 	or->request = req;
 	req->cmd_type = REQ_TYPE_BLOCK_PC;
@@ -1329,9 +1346,10 @@ static int _init_blk_request(struct osd_request *or,
 		or->out.req = req;
 		if (has_in) {
 			/* allocate bidi request */
-			req = blk_get_request(q, READ, flags);
-			if (!req) {
+			req = _make_request(q, false, &or->in, flags);
+			if (IS_ERR(req)) {
 				OSD_DEBUG("blk_get_request for bidi failed\n");
+				ret = PTR_ERR(req);
 				goto out;
 			}
 			req->cmd_type = REQ_TYPE_BLOCK_PC;
@@ -1376,26 +1394,6 @@ int osd_finalize_request(struct osd_request *or,
 		return ret;
 	}
 
-	if (or->out.bio) {
-		ret = blk_rq_append_bio(or->request->q, or->out.req,
-					or->out.bio);
-		if (ret) {
-			OSD_DEBUG("blk_rq_append_bio out failed\n");
-			return ret;
-		}
-		OSD_DEBUG("out bytes=%llu (bytes_req=%u)\n",
-			_LLU(or->out.total_bytes), or->out.req->data_len);
-	}
-	if (or->in.bio) {
-		ret = blk_rq_append_bio(or->request->q, or->in.req, or->in.bio);
-		if (ret) {
-			OSD_DEBUG("blk_rq_append_bio in failed\n");
-			return ret;
-		}
-		OSD_DEBUG("in bytes=%llu (bytes_req=%u)\n",
-			_LLU(or->in.total_bytes), or->in.req->data_len);
-	}
-
 	or->out.pad_buff = sg_out_pad_buffer;
 	or->in.pad_buff = sg_in_pad_buffer;
 
-- 
1.6.2.1

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Jeff Garzik]

Boaz Harrosh wrote:
> Osd library needs to submit pre-allocated bios, form several sources.
> osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
> On top of that the osd library needs to append additional segments to the
> IO memory, for get/set attributes and more.
> 
> All these are done today by use of a temporary hack - blk_rq_append_bio.
> This is bad on few accounts.
> 1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
> 2. blk_rq_append_bio does not support chained bios.
> 3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
>    has a bug.
> 
> The proposed solution adds two new fixtures to the block layer, and a corresponding
> fixing patch to osd. These are:

Just wanted to say... thanks for working on this.  I'll test osdblk anew 
once this stuff goes in, and then send you another osdblk patch.

	Jeff

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Tejun Heo]

Boaz Harrosh wrote:
> Osd library needs to submit pre-allocated bios, form several sources.
> osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
> On top of that the osd library needs to append additional segments to the
> IO memory, for get/set attributes and more.
> 
> All these are done today by use of a temporary hack - blk_rq_append_bio.
> This is bad on few accounts.
> 1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
> 2. blk_rq_append_bio does not support chained bios.
> 3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
>    has a bug.
> 
> The proposed solution adds two new fixtures to the block layer, and a corresponding
> fixing patch to osd. These are:
> 
> [PATCH 1/4] allow blk_rq_map_kern to append to requests

blk_rq_map_kern() part of modification looks good to me.

Thanks.

-- 
tejun

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Jens Axboe]

On Thu, May 07 2009, Boaz Harrosh wrote:
> 
> Osd library needs to submit pre-allocated bios, form several sources.
> osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
> On top of that the osd library needs to append additional segments to the
> IO memory, for get/set attributes and more.
> 
> All these are done today by use of a temporary hack - blk_rq_append_bio.
> This is bad on few accounts.
> 1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
> 2. blk_rq_append_bio does not support chained bios.
> 3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
>    has a bug.
> 
> The proposed solution adds two new fixtures to the block layer, and a corresponding
> fixing patch to osd. These are:
> 
> [PATCH 1/4] allow blk_rq_map_kern to append to requests
> [PATCH 2/4] libosd: Use new blk_rq_map_kern
> 
>   This is originally a James patch and it's used, to let blk_rq_map_kern append it's buffer
>   to existing bio, and there for is able to be called multiple times in a loop, to append
>   multiple segments. This API can also be useful for scsi/block targets that have segment
>   information in some other memory structure (like scatterlist) and wants to set it into
>   a request. Until such time that they have a proper support for mapping scatterlists directly.
>   (Since above called on long lists might not be good for performance)
> 
>   Here in osd it makes tons of sense, and should be considered for inclusion.
>   (The patches are based on linus-tip but should patch on block tree)
> 
> [RFC 3/4] New blk_make_request(), takes bio, returns a request
> [RFC 4/4] libosd: Use of new blk_make_request
>   
>   Here I propose a new block API, that will support proper delegation of a bio
>   to a full request. Please read inside the patch descriptions for details.
>   After this patch both osd and block layer will have the proper support for osdblk
>   driver as well as future needs.
>   These patches also eliminate the last use of blk_rq_append_bio which can be now un-exported.
> 
>   These two patches conflic with Tejun's branch and are based on linus-tip. Upon positive review
>   I will serialize them with Tejun and submit them properly. But first they must be agreed upon.
>   Jens, I specially need your opinion on this?

Looks sane to me. Can you resubmit against 'for-2.6.31' of the block git
repo?

-- 
Jens Axboe

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Boaz Harrosh]

On 05/12/2009 02:25 PM, Jens Axboe wrote:
> On Thu, May 07 2009, Boaz Harrosh wrote:
>> Osd library needs to submit pre-allocated bios, form several sources.
>> osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
>> On top of that the osd library needs to append additional segments to the
>> IO memory, for get/set attributes and more.
>>
>> All these are done today by use of a temporary hack - blk_rq_append_bio.
>> This is bad on few accounts.
>> 1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
>> 2. blk_rq_append_bio does not support chained bios.
>> 3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
>>    has a bug.
>>
>> The proposed solution adds two new fixtures to the block layer, and a corresponding
>> fixing patch to osd. These are:
>>
>> [PATCH 1/4] allow blk_rq_map_kern to append to requests
>> [PATCH 2/4] libosd: Use new blk_rq_map_kern
>>
>>   This is originally a James patch and it's used, to let blk_rq_map_kern append it's buffer
>>   to existing bio, and there for is able to be called multiple times in a loop, to append
>>   multiple segments. This API can also be useful for scsi/block targets that have segment
>>   information in some other memory structure (like scatterlist) and wants to set it into
>>   a request. Until such time that they have a proper support for mapping scatterlists directly.
>>   (Since above called on long lists might not be good for performance)
>>
>>   Here in osd it makes tons of sense, and should be considered for inclusion.
>>   (The patches are based on linus-tip but should patch on block tree)
>>
>> [RFC 3/4] New blk_make_request(), takes bio, returns a request
>> [RFC 4/4] libosd: Use of new blk_make_request
>>   
>>   Here I propose a new block API, that will support proper delegation of a bio
>>   to a full request. Please read inside the patch descriptions for details.
>>   After this patch both osd and block layer will have the proper support for osdblk
>>   driver as well as future needs.
>>   These patches also eliminate the last use of blk_rq_append_bio which can be now un-exported.
>>
>>   These two patches conflic with Tejun's branch and are based on linus-tip. Upon positive review
>>   I will serialize them with Tejun and submit them properly. But first they must be agreed upon.
>>   Jens, I specially need your opinion on this?
> 
> Looks sane to me. Can you resubmit against 'for-2.6.31' of the block git
> repo?
> 

Thanks Jens.

I have done the rebase and ran some tests, however I was unable to test these patches
as is, because there are some inter tree fallouts.

Jens, James, Stephan, I please need your help

The situation is like that.
- Both block/for-next and scsi/master are based on an old osd upstream-point (v2.6.30-rc3--ce8a7424)
- Linus tip has important OSD patches that went in via scsi-rc-fixes which changed Wire format
- If I try and merge block/for-next ontop of plain linus/master I get a merge conflict
- If I try merge scsi/master block/for-next I get build errors / conflicts

So there is no sane tree point that I can test on.

It would be nice if both Jens block/for-next and scsi-misc/master could be rebased on Linus rc5++
and resolve these conflicts. (And scsi-misc conflicts with Tejun's cleanups be put in a second stage
tree)

Should I send the patches as is half tested? Or wait for things to settle after I tested them
with all changes included?

I have cut a new osd/linux-next branch which is based, not on linus, but on v2.6.30-rc3--ce8a7424
the base point for block/for-next and scsi-misc/master. So in next it should all come together
well, and I will try to clone tomorrow's next and test on top of that.

Please Advise on what I should do?

Thanks in advance
Boaz

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Boaz Harrosh]

On 05/13/2009 05:28 PM, Boaz Harrosh wrote:
> On 05/12/2009 02:25 PM, Jens Axboe wrote:
>> On Thu, May 07 2009, Boaz Harrosh wrote:
>>> Osd library needs to submit pre-allocated bios, form several sources.
>>> osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
>>> On top of that the osd library needs to append additional segments to the
>>> IO memory, for get/set attributes and more.
>>>
>>> All these are done today by use of a temporary hack - blk_rq_append_bio.
>>> This is bad on few accounts.
>>> 1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
>>> 2. blk_rq_append_bio does not support chained bios.
>>> 3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
>>>    has a bug.
>>>
>>> The proposed solution adds two new fixtures to the block layer, and a corresponding
>>> fixing patch to osd. These are:
>>>
>>> [PATCH 1/4] allow blk_rq_map_kern to append to requests
>>> [PATCH 2/4] libosd: Use new blk_rq_map_kern
>>>
>>>   This is originally a James patch and it's used, to let blk_rq_map_kern append it's buffer
>>>   to existing bio, and there for is able to be called multiple times in a loop, to append
>>>   multiple segments. This API can also be useful for scsi/block targets that have segment
>>>   information in some other memory structure (like scatterlist) and wants to set it into
>>>   a request. Until such time that they have a proper support for mapping scatterlists directly.
>>>   (Since above called on long lists might not be good for performance)
>>>
>>>   Here in osd it makes tons of sense, and should be considered for inclusion.
>>>   (The patches are based on linus-tip but should patch on block tree)
>>>
>>> [RFC 3/4] New blk_make_request(), takes bio, returns a request
>>> [RFC 4/4] libosd: Use of new blk_make_request
>>>   
>>>   Here I propose a new block API, that will support proper delegation of a bio
>>>   to a full request. Please read inside the patch descriptions for details.
>>>   After this patch both osd and block layer will have the proper support for osdblk
>>>   driver as well as future needs.
>>>   These patches also eliminate the last use of blk_rq_append_bio which can be now un-exported.
>>>
>>>   These two patches conflic with Tejun's branch and are based on linus-tip. Upon positive review
>>>   I will serialize them with Tejun and submit them properly. But first they must be agreed upon.
>>>   Jens, I specially need your opinion on this?
>> Looks sane to me. Can you resubmit against 'for-2.6.31' of the block git
>> repo?
>>
> 
> Thanks Jens.
> 
> I have done the rebase and ran some tests, however I was unable to test these patches
> as is, because there are some inter tree fallouts.
> 
> Jens, James, Stephan, I please need your help
> 
> The situation is like that.
> - Both block/for-next and scsi/master are based on an old osd upstream-point (v2.6.30-rc3--ce8a7424)
> - Linus tip has important OSD patches that went in via scsi-rc-fixes which changed Wire format
> - If I try and merge block/for-next ontop of plain linus/master I get a merge conflict
> - If I try merge scsi/master block/for-next I get build errors / conflicts
> 
> So there is no sane tree point that I can test on.
> 
> It would be nice if both Jens block/for-next and scsi-misc/master could be rebased on Linus rc5++
> and resolve these conflicts. (And scsi-misc conflicts with Tejun's cleanups be put in a second stage
> tree)
> 
> Should I send the patches as is half tested? Or wait for things to settle after I tested them
> with all changes included?
> 
> I have cut a new osd/linux-next branch which is based, not on linus, but on v2.6.30-rc3--ce8a7424
> the base point for block/for-next and scsi-misc/master. So in next it should all come together
> well, and I will try to clone tomorrow's next and test on top of that.
> 

This will not work I have one patch [3/4] New blk_make_request(), takes bio, returns a request
which will conflict with block/for-next if I rebase it on v2.6.30-rc3--ce8a7424.

Should I cut osd/linux-next on top of block/for-next ?

> Please Advise on what I should do?
> 
> Thanks in advance
> Boaz
> --
> 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

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[James Bottomley]

On Wed, 2009-05-13 at 17:36 +0300, Boaz Harrosh wrote:
> On 05/13/2009 05:28 PM, Boaz Harrosh wrote:
> > On 05/12/2009 02:25 PM, Jens Axboe wrote:
> >> On Thu, May 07 2009, Boaz Harrosh wrote:
> >>> Osd library needs to submit pre-allocated bios, form several sources.
> >>> osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
> >>> On top of that the osd library needs to append additional segments to the
> >>> IO memory, for get/set attributes and more.
> >>>
> >>> All these are done today by use of a temporary hack - blk_rq_append_bio.
> >>> This is bad on few accounts.
> >>> 1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
> >>> 2. blk_rq_append_bio does not support chained bios.
> >>> 3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
> >>>    has a bug.
> >>>
> >>> The proposed solution adds two new fixtures to the block layer, and a corresponding
> >>> fixing patch to osd. These are:
> >>>
> >>> [PATCH 1/4] allow blk_rq_map_kern to append to requests
> >>> [PATCH 2/4] libosd: Use new blk_rq_map_kern
> >>>
> >>>   This is originally a James patch and it's used, to let blk_rq_map_kern append it's buffer
> >>>   to existing bio, and there for is able to be called multiple times in a loop, to append
> >>>   multiple segments. This API can also be useful for scsi/block targets that have segment
> >>>   information in some other memory structure (like scatterlist) and wants to set it into
> >>>   a request. Until such time that they have a proper support for mapping scatterlists directly.
> >>>   (Since above called on long lists might not be good for performance)
> >>>
> >>>   Here in osd it makes tons of sense, and should be considered for inclusion.
> >>>   (The patches are based on linus-tip but should patch on block tree)
> >>>
> >>> [RFC 3/4] New blk_make_request(), takes bio, returns a request
> >>> [RFC 4/4] libosd: Use of new blk_make_request
> >>>   
> >>>   Here I propose a new block API, that will support proper delegation of a bio
> >>>   to a full request. Please read inside the patch descriptions for details.
> >>>   After this patch both osd and block layer will have the proper support for osdblk
> >>>   driver as well as future needs.
> >>>   These patches also eliminate the last use of blk_rq_append_bio which can be now un-exported.
> >>>
> >>>   These two patches conflic with Tejun's branch and are based on linus-tip. Upon positive review
> >>>   I will serialize them with Tejun and submit them properly. But first they must be agreed upon.
> >>>   Jens, I specially need your opinion on this?
> >> Looks sane to me. Can you resubmit against 'for-2.6.31' of the block git
> >> repo?
> >>
> > 
> > Thanks Jens.
> > 
> > I have done the rebase and ran some tests, however I was unable to test these patches
> > as is, because there are some inter tree fallouts.
> > 
> > Jens, James, Stephan, I please need your help
> > 
> > The situation is like that.
> > - Both block/for-next and scsi/master are based on an old osd upstream-point (v2.6.30-rc3--ce8a7424)
> > - Linus tip has important OSD patches that went in via scsi-rc-fixes which changed Wire format

So just pull them into Linus head and build on that ... as long as you
explain what the base was, I can rebase scsi-misc (or run a post merge
tree) to cope.  It needs rebasing anyway to redo the mvsas patches.

> > - If I try and merge block/for-next ontop of plain linus/master I get a merge conflict
> > - If I try merge scsi/master block/for-next I get build errors / conflicts

This is the problem of the renames ... I think we need a block postmerge
tree to fix this up, but that probably needs sorting out first.

> > So there is no sane tree point that I can test on.
> > 
> > It would be nice if both Jens block/for-next and scsi-misc/master could be rebased on Linus rc5++
> > and resolve these conflicts. (And scsi-misc conflicts with Tejun's cleanups be put in a second stage
> > tree)
> > 
> > Should I send the patches as is half tested? Or wait for things to settle after I tested them
> > with all changes included?
> > 
> > I have cut a new osd/linux-next branch which is based, not on linus, but on v2.6.30-rc3--ce8a7424
> > the base point for block/for-next and scsi-misc/master. So in next it should all come together
> > well, and I will try to clone tomorrow's next and test on top of that.
> > 
> 
> This will not work I have one patch [3/4] New blk_make_request(), takes bio, returns a request
> which will conflict with block/for-next if I rebase it on v2.6.30-rc3--ce8a7424.
> 
> Should I cut osd/linux-next on top of block/for-next ?

What you really want is on the combination of the necessary trees.  If
it's only block, then sure ... if it's block and SCSI, that's postmerge
territory.

James

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Boaz Harrosh]

On 05/13/2009 05:47 PM, James Bottomley wrote:
> On Wed, 2009-05-13 at 17:36 +0300, Boaz Harrosh wrote:
>> On 05/13/2009 05:28 PM, Boaz Harrosh wrote:
>>> On 05/12/2009 02:25 PM, Jens Axboe wrote:
>>>> On Thu, May 07 2009, Boaz Harrosh wrote:
>>>>> Osd library needs to submit pre-allocated bios, form several sources.
>>>>> osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
>>>>> On top of that the osd library needs to append additional segments to the
>>>>> IO memory, for get/set attributes and more.
>>>>>
>>>>> All these are done today by use of a temporary hack - blk_rq_append_bio.
>>>>> This is bad on few accounts.
>>>>> 1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
>>>>> 2. blk_rq_append_bio does not support chained bios.
>>>>> 3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
>>>>>    has a bug.
>>>>>
>>>>> The proposed solution adds two new fixtures to the block layer, and a corresponding
>>>>> fixing patch to osd. These are:
>>>>>
>>>>> [PATCH 1/4] allow blk_rq_map_kern to append to requests
>>>>> [PATCH 2/4] libosd: Use new blk_rq_map_kern
>>>>>
>>>>>   This is originally a James patch and it's used, to let blk_rq_map_kern append it's buffer
>>>>>   to existing bio, and there for is able to be called multiple times in a loop, to append
>>>>>   multiple segments. This API can also be useful for scsi/block targets that have segment
>>>>>   information in some other memory structure (like scatterlist) and wants to set it into
>>>>>   a request. Until such time that they have a proper support for mapping scatterlists directly.
>>>>>   (Since above called on long lists might not be good for performance)
>>>>>
>>>>>   Here in osd it makes tons of sense, and should be considered for inclusion.
>>>>>   (The patches are based on linus-tip but should patch on block tree)
>>>>>
>>>>> [RFC 3/4] New blk_make_request(), takes bio, returns a request
>>>>> [RFC 4/4] libosd: Use of new blk_make_request
>>>>>   
>>>>>   Here I propose a new block API, that will support proper delegation of a bio
>>>>>   to a full request. Please read inside the patch descriptions for details.
>>>>>   After this patch both osd and block layer will have the proper support for osdblk
>>>>>   driver as well as future needs.
>>>>>   These patches also eliminate the last use of blk_rq_append_bio which can be now un-exported.
>>>>>
>>>>>   These two patches conflic with Tejun's branch and are based on linus-tip. Upon positive review
>>>>>   I will serialize them with Tejun and submit them properly. But first they must be agreed upon.
>>>>>   Jens, I specially need your opinion on this?
>>>> Looks sane to me. Can you resubmit against 'for-2.6.31' of the block git
>>>> repo?
>>>>
>>> Thanks Jens.
>>>
>>> I have done the rebase and ran some tests, however I was unable to test these patches
>>> as is, because there are some inter tree fallouts.
>>>
>>> Jens, James, Stephan, I please need your help
>>>
>>> The situation is like that.
>>> - Both block/for-next and scsi/master are based on an old osd upstream-point (v2.6.30-rc3--ce8a7424)
>>> - Linus tip has important OSD patches that went in via scsi-rc-fixes which changed Wire format
> 
> So just pull them into Linus head and build on that ... as long as you
> explain what the base was, I can rebase scsi-misc (or run a post merge
> tree) to cope.  It needs rebasing anyway to redo the mvsas patches.
> 
>>> - If I try and merge block/for-next ontop of plain linus/master I get a merge conflict
>>> - If I try merge scsi/master block/for-next I get build errors / conflicts
> 
> This is the problem of the renames ... I think we need a block postmerge
> tree to fix this up, but that probably needs sorting out first.
> 
>>> So there is no sane tree point that I can test on.
>>>
>>> It would be nice if both Jens block/for-next and scsi-misc/master could be rebased on Linus rc5++
>>> and resolve these conflicts. (And scsi-misc conflicts with Tejun's cleanups be put in a second stage
>>> tree)
>>>
>>> Should I send the patches as is half tested? Or wait for things to settle after I tested them
>>> with all changes included?
>>>
>>> I have cut a new osd/linux-next branch which is based, not on linus, but on v2.6.30-rc3--ce8a7424
>>> the base point for block/for-next and scsi-misc/master. So in next it should all come together
>>> well, and I will try to clone tomorrow's next and test on top of that.
>>>
>> This will not work I have one patch [3/4] New blk_make_request(), takes bio, returns a request
>> which will conflict with block/for-next if I rebase it on v2.6.30-rc3--ce8a7424.
>>
>> Should I cut osd/linux-next on top of block/for-next ?
> 
> What you really want is on the combination of the necessary trees.  If
> it's only block, then sure ... if it's block and SCSI, that's postmerge
> territory.
> 
> James
> 
> 

Thank you for your reply.

I did more-less what you said rebased block/for-next on linus-tip and
fixed the merge with scsi-misc as per Stephan advise. Plus my patches
last. Test ran well.

I see that you rebased by now, though I suspect the fc's blk_end_request
call will fail to build if merged with block tree.

I'm also seeing some recent changes to block git so I suspect that Jens is
in the middle of rebasing too. (I hope)

As far as OSD, I managed to separate the two block-based changes to osd
from the rest of the changes scheduled for 2.6.31 in such a way that they do not
conflict and can merge either way (block first or scsi-misc first).

I'll repost all these patches. Sunday hopfuly after Jens rebases.

Thanks
Boaz

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[James Bottomley]

On Thu, 2009-05-14 at 17:53 +0300, Boaz Harrosh wrote:
> On 05/13/2009 05:47 PM, James Bottomley wrote:
> > On Wed, 2009-05-13 at 17:36 +0300, Boaz Harrosh wrote:
> >> On 05/13/2009 05:28 PM, Boaz Harrosh wrote:
> >>> On 05/12/2009 02:25 PM, Jens Axboe wrote:
> >>>> On Thu, May 07 2009, Boaz Harrosh wrote:
> >>>>> Osd library needs to submit pre-allocated bios, form several sources.
> >>>>> osdblk exofs and pNFS-layout driver all have prepared bios for IO submission.
> >>>>> On top of that the osd library needs to append additional segments to the
> >>>>> IO memory, for get/set attributes and more.
> >>>>>
> >>>>> All these are done today by use of a temporary hack - blk_rq_append_bio.
> >>>>> This is bad on few accounts.
> >>>>> 1. blk_rq_append_bio was not meant to be exported and is very specific to its users.
> >>>>> 2. blk_rq_append_bio does not support chained bios.
> >>>>> 3. blk_rq_append_bio does not bounce the bio and therefore current osd implementation
> >>>>>    has a bug.
> >>>>>
> >>>>> The proposed solution adds two new fixtures to the block layer, and a corresponding
> >>>>> fixing patch to osd. These are:
> >>>>>
> >>>>> [PATCH 1/4] allow blk_rq_map_kern to append to requests
> >>>>> [PATCH 2/4] libosd: Use new blk_rq_map_kern
> >>>>>
> >>>>>   This is originally a James patch and it's used, to let blk_rq_map_kern append it's buffer
> >>>>>   to existing bio, and there for is able to be called multiple times in a loop, to append
> >>>>>   multiple segments. This API can also be useful for scsi/block targets that have segment
> >>>>>   information in some other memory structure (like scatterlist) and wants to set it into
> >>>>>   a request. Until such time that they have a proper support for mapping scatterlists directly.
> >>>>>   (Since above called on long lists might not be good for performance)
> >>>>>
> >>>>>   Here in osd it makes tons of sense, and should be considered for inclusion.
> >>>>>   (The patches are based on linus-tip but should patch on block tree)
> >>>>>
> >>>>> [RFC 3/4] New blk_make_request(), takes bio, returns a request
> >>>>> [RFC 4/4] libosd: Use of new blk_make_request
> >>>>>   
> >>>>>   Here I propose a new block API, that will support proper delegation of a bio
> >>>>>   to a full request. Please read inside the patch descriptions for details.
> >>>>>   After this patch both osd and block layer will have the proper support for osdblk
> >>>>>   driver as well as future needs.
> >>>>>   These patches also eliminate the last use of blk_rq_append_bio which can be now un-exported.
> >>>>>
> >>>>>   These two patches conflic with Tejun's branch and are based on linus-tip. Upon positive review
> >>>>>   I will serialize them with Tejun and submit them properly. But first they must be agreed upon.
> >>>>>   Jens, I specially need your opinion on this?
> >>>> Looks sane to me. Can you resubmit against 'for-2.6.31' of the block git
> >>>> repo?
> >>>>
> >>> Thanks Jens.
> >>>
> >>> I have done the rebase and ran some tests, however I was unable to test these patches
> >>> as is, because there are some inter tree fallouts.
> >>>
> >>> Jens, James, Stephan, I please need your help
> >>>
> >>> The situation is like that.
> >>> - Both block/for-next and scsi/master are based on an old osd upstream-point (v2.6.30-rc3--ce8a7424)
> >>> - Linus tip has important OSD patches that went in via scsi-rc-fixes which changed Wire format
> > 
> > So just pull them into Linus head and build on that ... as long as you
> > explain what the base was, I can rebase scsi-misc (or run a post merge
> > tree) to cope.  It needs rebasing anyway to redo the mvsas patches.
> > 
> >>> - If I try and merge block/for-next ontop of plain linus/master I get a merge conflict
> >>> - If I try merge scsi/master block/for-next I get build errors / conflicts
> > 
> > This is the problem of the renames ... I think we need a block postmerge
> > tree to fix this up, but that probably needs sorting out first.
> > 
> >>> So there is no sane tree point that I can test on.
> >>>
> >>> It would be nice if both Jens block/for-next and scsi-misc/master could be rebased on Linus rc5++
> >>> and resolve these conflicts. (And scsi-misc conflicts with Tejun's cleanups be put in a second stage
> >>> tree)
> >>>
> >>> Should I send the patches as is half tested? Or wait for things to settle after I tested them
> >>> with all changes included?
> >>>
> >>> I have cut a new osd/linux-next branch which is based, not on linus, but on v2.6.30-rc3--ce8a7424
> >>> the base point for block/for-next and scsi-misc/master. So in next it should all come together
> >>> well, and I will try to clone tomorrow's next and test on top of that.
> >>>
> >> This will not work I have one patch [3/4] New blk_make_request(), takes bio, returns a request
> >> which will conflict with block/for-next if I rebase it on v2.6.30-rc3--ce8a7424.
> >>
> >> Should I cut osd/linux-next on top of block/for-next ?
> > 
> > What you really want is on the combination of the necessary trees.  If
> > it's only block, then sure ... if it's block and SCSI, that's postmerge
> > territory.
> > 
> > James
> > 
> > 
> 
> Thank you for your reply.
> 
> I did more-less what you said rebased block/for-next on linus-tip and
> fixed the merge with scsi-misc as per Stephan advise. Plus my patches
> last. Test ran well.
> 
> I see that you rebased by now, though I suspect the fc's blk_end_request
> call will fail to build if merged with block tree.

Yes, that's the bit we need a postmerge tree for.  It has to build on
it's own in scsi-misc, but it's making use of an API Tejun is altering,
so the block postmerge has to do the API alteration based on the SCSI
tree.

> I'm also seeing some recent changes to block git so I suspect that Jens is
> in the middle of rebasing too. (I hope)
> 
> As far as OSD, I managed to separate the two block-based changes to osd
> from the rest of the changes scheduled for 2.6.31 in such a way that they do not
> conflict and can merge either way (block first or scsi-misc first).
> 
> I'll repost all these patches. Sunday hopfuly after Jens rebases.

OK ... thanks.

James

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Boaz Harrosh]

On 05/14/2009 06:35 PM, James Bottomley wrote:
<snip>
>>
>> I see that you rebased by now, though I suspect the fc's blk_end_request
>> call will fail to build if merged with block tree.
> 
> Yes, that's the bit we need a postmerge tree for.  It has to build on
> it's own in scsi-misc, but it's making use of an API Tejun is altering,
> so the block postmerge has to do the API alteration based on the SCSI
> tree.
> 

Tejun's cleanup is a bit tricky and cross trees wide. It will need to be
completely postponed to post merge which will be a pity since there are
so many patches ontop of it in block-next. Unless we want to sacrifice
the build ability of the tree between block-merge and the fixup to this driver.

Perhaps you could do a scsi-post-merge tree that has only this driver, already
with the new needed code?

>> I'm also seeing some recent changes to block git so I suspect that Jens is
>> in the middle of rebasing too. (I hope)
>>
<snip>

Boaz

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Boaz Harrosh]

On 05/14/2009 07:11 PM, Boaz Harrosh wrote:
> 
> Perhaps you could do a scsi-post-merge tree that has only this driver, already
> with the new needed code?
> 

Below is a patch that should be squashed into scsi-misc's:
 [04576e3] [SCSI] FC Pass Thru support

In order to compile and work ontop of Tejun's block layer revamps,
in block-next

Tejun please review, specially the blk_fetch_request bit. Thanks

Boaz
---
From: Boaz Harrosh <bharrosh@panasas.com>
Date: Thu, 14 May 2009 19:32:46 +0300
Subject: [SQUASHME] into: FC Pass Thru support

This patch should be squashed into
  [SCSI] FC Pass Thru support

If it needs to compile after Tejun's block-layer revamps

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/scsi_transport_fc.c |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 4df8c3c..41c90fe 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3409,7 +3409,6 @@ fc_bsg_jobdone(struct fc_bsg_job *job)
 	struct request *req = job->req;
 	struct request *rsp = req->next_rq;
 	unsigned long flags;
-	unsigned rsp_len = 0, req_len = blk_rq_bytes(req);
 	int err;
 
 	spin_lock_irqsave(&job->job_lock, flags);
@@ -3425,16 +3424,15 @@ fc_bsg_jobdone(struct fc_bsg_job *job)
 		job->req->sense_len = job->reply_len;
 
 	/* we assume all request payload was transferred, residual == 0 */
-	req->data_len = 0;
+	req->resid_len = 0;
 
 	if (rsp) {
-		rsp_len = blk_rq_bytes(rsp);
-		BUG_ON(job->reply->reply_payload_rcv_len > rsp_len);
 		/* set reply (bidi) residual */
-		rsp->data_len = (rsp_len - job->reply->reply_payload_rcv_len);
+		rsp->resid_len = blk_rq_bytes(rsp) -
+					job->reply->reply_payload_rcv_len;
 	}
 
-	blk_end_bidi_request(req, err, req_len, rsp_len);
+	blk_end_request_all(req, err);
 
 	fc_destroy_bsgjob(job);
 }
@@ -3496,7 +3494,7 @@ fc_bsg_map_buffer(struct fc_bsg_buffer *buf, struct request *req)
 		return -ENOMEM;
 	sg_init_table(buf->sg_list, req->nr_phys_segments);
 	buf->sg_cnt = blk_rq_map_sg(req->q, req, buf->sg_list);
-	buf->payload_len = req->data_len;
+	buf->payload_len = blk_rq_bytes(req);
 	return 0;
 }
 
@@ -3762,14 +3760,12 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost,
 		return;
 
 	while (!blk_queue_plugged(q)) {
-		req = elv_next_request(q);
-		if (!req)
-			break;
-
 		if (rport && (rport->port_state == FC_PORTSTATE_BLOCKED))
 				break;
 
-		blkdev_dequeue_request(req);
+		req = blk_fetch_request(q);
+		if (!req)
+			break;
 
 		if (rport && (rport->port_state != FC_PORTSTATE_ONLINE)) {
 			req->errors = -ENXIO;
-- 
1.6.2.1

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Boaz Harrosh]

On 05/14/2009 07:46 PM, James Bottomley wrote:
> On Thu, 2009-05-14 at 19:11 +0300, Boaz Harrosh wrote:
>> On 05/14/2009 06:35 PM, James Bottomley wrote:
>> <snip>
>>>> I see that you rebased by now, though I suspect the fc's blk_end_request
>>>> call will fail to build if merged with block tree.
>>> Yes, that's the bit we need a postmerge tree for.  It has to build on
>>> it's own in scsi-misc, but it's making use of an API Tejun is altering,
>>> so the block postmerge has to do the API alteration based on the SCSI
>>> tree.
>>>
>> Tejun's cleanup is a bit tricky and cross trees wide. It will need to be
>> completely postponed to post merge which will be a pity since there are
>> so many patches ontop of it in block-next. Unless we want to sacrifice
>> the build ability of the tree between block-merge and the fixup to this driver.
>>
>> Perhaps you could do a scsi-post-merge tree that has only this driver, already
>> with the new needed code?
> 
> If necessary.  However, the disadvantage of doing it this way around is
> that I have to squash together the two patches (FC transport patch and
> the update to tejun's API) otherwise we get a bisection break.
> 
> James

Either, almost the complete block tree together with below patch needs to
be post-merge, rebased over a single scsi-misc patch: "FC Pass Thru"

or a single scsi-misc patch is rebased post-merge over block-tree, yes
with below squashed in.

But merging the trees as they are will cause bisection break from the
second merge to below patch.

So since the block tree is much more code, I'd say squash below and post-
merge the single "FC Pass Thru support" patch.

Tejun, I need your review on this patch have you had a chance to look at it?

Boaz

> 
> Below is a patch that should be squashed into scsi-misc's:
>  [04576e3] [SCSI] FC Pass Thru support
> 
> In order to compile and work ontop of Tejun's block layer revamps,
> in block-next
> 
> Tejun please review, specially the blk_fetch_request bit. Thanks
> 
> Boaz
> ---
> From: Boaz Harrosh <bharrosh@panasas.com>
> Date: Thu, 14 May 2009 19:32:46 +0300
> Subject: [SQUASHME] into: FC Pass Thru support
> 
> This patch should be squashed into
>   [SCSI] FC Pass Thru support
> 
> If it needs to compile after Tejun's block-layer revamps
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
>  drivers/scsi/scsi_transport_fc.c |   20 ++++++++------------
>  1 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 4df8c3c..41c90fe 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -3409,7 +3409,6 @@ fc_bsg_jobdone(struct fc_bsg_job *job)
>  	struct request *req = job->req;
>  	struct request *rsp = req->next_rq;
>  	unsigned long flags;
> -	unsigned rsp_len = 0, req_len = blk_rq_bytes(req);
>  	int err;
>  
>  	spin_lock_irqsave(&job->job_lock, flags);
> @@ -3425,16 +3424,15 @@ fc_bsg_jobdone(struct fc_bsg_job *job)
>  		job->req->sense_len = job->reply_len;
>  
>  	/* we assume all request payload was transferred, residual == 0 */
> -	req->data_len = 0;
> +	req->resid_len = 0;
>  
>  	if (rsp) {
> -		rsp_len = blk_rq_bytes(rsp);
> -		BUG_ON(job->reply->reply_payload_rcv_len > rsp_len);
>  		/* set reply (bidi) residual */
> -		rsp->data_len = (rsp_len - job->reply->reply_payload_rcv_len);
> +		rsp->resid_len = blk_rq_bytes(rsp) -
> +					job->reply->reply_payload_rcv_len;
>  	}
>  
> -	blk_end_bidi_request(req, err, req_len, rsp_len);
> +	blk_end_request_all(req, err);
>  
>  	fc_destroy_bsgjob(job);
>  }
> @@ -3496,7 +3494,7 @@ fc_bsg_map_buffer(struct fc_bsg_buffer *buf, struct request *req)
>  		return -ENOMEM;
>  	sg_init_table(buf->sg_list, req->nr_phys_segments);
>  	buf->sg_cnt = blk_rq_map_sg(req->q, req, buf->sg_list);
> -	buf->payload_len = req->data_len;
> +	buf->payload_len = blk_rq_bytes(req);
>  	return 0;
>  }
>  
> @@ -3762,14 +3760,12 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost,
>  		return;
>  
>  	while (!blk_queue_plugged(q)) {
> -		req = elv_next_request(q);
> -		if (!req)
> -			break;
> -
>  		if (rport && (rport->port_state == FC_PORTSTATE_BLOCKED))
>  				break;
>  
> -		blkdev_dequeue_request(req);
> +		req = blk_fetch_request(q);
> +		if (!req)
> +			break;
>  
>  		if (rport && (rport->port_state != FC_PORTSTATE_ONLINE)) {
>  			req->errors = -ENXIO;

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[James Bottomley]

On Thu, 2009-05-14 at 19:11 +0300, Boaz Harrosh wrote:
> On 05/14/2009 06:35 PM, James Bottomley wrote:
> <snip>
> >>
> >> I see that you rebased by now, though I suspect the fc's blk_end_request
> >> call will fail to build if merged with block tree.
> > 
> > Yes, that's the bit we need a postmerge tree for.  It has to build on
> > it's own in scsi-misc, but it's making use of an API Tejun is altering,
> > so the block postmerge has to do the API alteration based on the SCSI
> > tree.
> > 
> 
> Tejun's cleanup is a bit tricky and cross trees wide. It will need to be
> completely postponed to post merge which will be a pity since there are
> so many patches ontop of it in block-next. Unless we want to sacrifice
> the build ability of the tree between block-merge and the fixup to this driver.
> 
> Perhaps you could do a scsi-post-merge tree that has only this driver, already
> with the new needed code?

If necessary.  However, the disadvantage of doing it this way around is
that I have to squash together the two patches (FC transport patch and
the update to tejun's API) otherwise we get a bisection break.

James

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Stephen Rothwell]

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

Hi Boaz,

On Wed, 13 May 2009 17:28:10 +0300 Boaz Harrosh <bharrosh@panasas.com> wrote:
>
> I have done the rebase and ran some tests, however I was unable to test these patches
> as is, because there are some inter tree fallouts.
> 
> Jens, James, Stephan, I please need your help
> 
> The situation is like that.
> - Both block/for-next and scsi/master are based on an old osd upstream-point (v2.6.30-rc3--ce8a7424)
> - Linus tip has important OSD patches that went in via scsi-rc-fixes which changed Wire format
> - If I try and merge block/for-next ontop of plain linus/master I get a merge conflict
> - If I try merge scsi/master block/for-next I get build errors / conflicts
> 
> So there is no sane tree point that I can test on.

We could ask Jens and James to both merge either Linus' tree or just the
scsi-rc-fixes tree into their trees.  Currently when I merge the scsi
tree I get only one very minor conflict.  The block tree, on the other
hand, had quite big problems today - but that was due to interactions
with the scsi tree.

If the above merges were done, then I suspect that the scsi tree with
commit 1bfe9caaff367601134c14fc428017419f628f7d ("[SCSI] FC Pass Thru
support") reverted would merge ok with the block tree and give you a base
for testing.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Boaz Harrosh]

On 05/13/2009 05:52 PM, Stephen Rothwell wrote:
> Hi Boaz,
> 
> On Wed, 13 May 2009 17:28:10 +0300 Boaz Harrosh <bharrosh@panasas.com> wrote:
>> I have done the rebase and ran some tests, however I was unable to test these patches
>> as is, because there are some inter tree fallouts.
>>
>> Jens, James, Stephan, I please need your help
>>
>> The situation is like that.
>> - Both block/for-next and scsi/master are based on an old osd upstream-point (v2.6.30-rc3--ce8a7424)
>> - Linus tip has important OSD patches that went in via scsi-rc-fixes which changed Wire format
>> - If I try and merge block/for-next ontop of plain linus/master I get a merge conflict
>> - If I try merge scsi/master block/for-next I get build errors / conflicts
>>
>> So there is no sane tree point that I can test on.
> 
> We could ask Jens and James to both merge either Linus' tree or just the
> scsi-rc-fixes tree into their trees.  Currently when I merge the scsi
> tree I get only one very minor conflict.  The block tree, on the other
> hand, had quite big problems today - but that was due to interactions
> with the scsi tree.
> 
> If the above merges were done, then I suspect that the scsi tree with
> commit 1bfe9caaff367601134c14fc428017419f628f7d ("[SCSI] FC Pass Thru
> support") reverted would merge ok with the block tree and give you a base
> for testing.
> 

That sounds good, I'll wait a couple of days and see what they decide to do.

Mean while I've collected all relevant patches from Linus and cherry-picked them
above block, and below my patches. And am testing this way. That should give me
99% coverage as scsi-misc does not have anything interacting with me, only
Linus and block do.

Once I figure it out I will push a osd/linux-next also

Thanks
Boaz

Re: [patchset 0/4] osd: Stop usage of blk_rq_append_bio

[Stephen Rothwell]

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

On Thu, 14 May 2009 00:52:31 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Wed, 13 May 2009 17:28:10 +0300 Boaz Harrosh <bharrosh@panasas.com> wrote:
> >
> > I have done the rebase and ran some tests, however I was unable to test these patches
> > as is, because there are some inter tree fallouts.
> > 
> > Jens, James, Stephan, I please need your help
> > 
> > The situation is like that.
> > - Both block/for-next and scsi/master are based on an old osd upstream-point (v2.6.30-rc3--ce8a7424)
> > - Linus tip has important OSD patches that went in via scsi-rc-fixes which changed Wire format
> > - If I try and merge block/for-next ontop of plain linus/master I get a merge conflict
> > - If I try merge scsi/master block/for-next I get build errors / conflicts
> > 
> > So there is no sane tree point that I can test on.
> 
> We could ask Jens and James to both merge either Linus' tree or just the
> scsi-rc-fixes tree into their trees.  Currently when I merge the scsi

Actually even against v2.6.30-rc3-393-ge9da4d7 (as that was the top of the
scsi-rc-fixes tree when it was merged into Linus' tree).

> tree I get only one very minor conflict.  The block tree, on the other
> hand, had quite big problems today - but that was due to interactions
> with the scsi tree.
> 
> If the above merges were done, then I suspect that the scsi tree with
> commit 1bfe9caaff367601134c14fc428017419f628f7d ("[SCSI] FC Pass Thru
> support") reverted would merge ok with the block tree and give you a base
> for testing.

This assumes, of course, that neither the scsi or block trees in
linux-next get rebased ...

How about:
	base the osd tree on Linus' tree.
	test the osd merged with the block tree (rather than rebased on
top of it).
	test the osd tree merged with the scsi tree.
	test all three merged (with the commit above reverted).
	submit the osd tree to linux-next.

The merge conflicts between the block tree and Linus' tree can be
resolved by doing "git checkout block/for-next drivers/block/hd.c
drivers/block/mg_disk.c".  (They could also be fixed by the block tree
merging Linus' tree, of course).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]