Re: [PATCH] bsg: bidi bio map failure fix
On Tue, Feb 12 2008, James Bottomley wrote:
On Tue, 2008-02-12 at 15:40 -0500, Pete Wyckoff wrote:
If blk_rq_map_user requires more than one bio, and fails mapping
somewhere after the first bio, it will return with rq-bio set to
non-NULL, but it will have already unmapped the partial bio. The
out: error exit section will see the non-null bio and try to unmap
it again, triggering a mapcount bug via bad_page().
Signed-off-by: Pete Wyckoff [EMAIL PROTECTED]
---
block/bsg.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 3337125..bba7154 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -295,8 +295,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4
*hdr)
dxferp = (void*)(unsigned long)hdr-din_xferp;
ret = blk_rq_map_user(q, next_rq, dxferp, hdr-din_xfer_len);
- if (ret)
+ if (ret) {
+ next_rq-bio = NULL; /* do not unmap twice */
Nice ... that's a nasty asymmetry of the blk_rq_map_user API. The map
takes a request gets a ref and fills in the bio. The unmap has to be
called on the bio leaving you to clear the now removed bio reference
manually.
It is nasty, how about fixing that instead?
diff --git a/block/blk-map.c b/block/blk-map.c
index 955d75c..bc5ce60 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -143,6 +143,7 @@ int blk_rq_map_user(struct request_queue *q, struct request
*rq,
return 0;
unmap_rq:
blk_rq_unmap_user(bio);
+ rq-bio = NULL;
return ret;
}
EXPORT_SYMBOL(blk_rq_map_user);
--
Jens Axboe
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bsg: bidi bio map failure fix
On Mon, 18 Feb 2008 13:55:08 +0100
Jens Axboe [EMAIL PROTECTED] wrote:
On Tue, Feb 12 2008, James Bottomley wrote:
On Tue, 2008-02-12 at 15:40 -0500, Pete Wyckoff wrote:
If blk_rq_map_user requires more than one bio, and fails mapping
somewhere after the first bio, it will return with rq-bio set to
non-NULL, but it will have already unmapped the partial bio. The
out: error exit section will see the non-null bio and try to unmap
it again, triggering a mapcount bug via bad_page().
Signed-off-by: Pete Wyckoff [EMAIL PROTECTED]
---
block/bsg.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 3337125..bba7154 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -295,8 +295,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4
*hdr)
dxferp = (void*)(unsigned long)hdr-din_xferp;
ret = blk_rq_map_user(q, next_rq, dxferp, hdr-din_xfer_len);
- if (ret)
+ if (ret) {
+ next_rq-bio = NULL; /* do not unmap twice */
Nice ... that's a nasty asymmetry of the blk_rq_map_user API. The map
takes a request gets a ref and fills in the bio. The unmap has to be
called on the bio leaving you to clear the now removed bio reference
manually.
It is nasty, how about fixing that instead?
Yeah, looks better for me though the blk_rq_map_user API is still a
bit hacky, as James said.
James, Pete's patch is still in scsi-fixes, so how about dropping it
and sending this patch via the block?
diff --git a/block/blk-map.c b/block/blk-map.c
index 955d75c..bc5ce60 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -143,6 +143,7 @@ int blk_rq_map_user(struct request_queue *q, struct
request *rq,
return 0;
unmap_rq:
blk_rq_unmap_user(bio);
+ rq-bio = NULL;
return ret;
}
EXPORT_SYMBOL(blk_rq_map_user);
--
Jens Axboe
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bsg: bidi bio map failure fix
On Mon, Feb 18 2008, FUJITA Tomonori wrote:
On Mon, 18 Feb 2008 13:55:08 +0100
Jens Axboe [EMAIL PROTECTED] wrote:
On Tue, Feb 12 2008, James Bottomley wrote:
On Tue, 2008-02-12 at 15:40 -0500, Pete Wyckoff wrote:
If blk_rq_map_user requires more than one bio, and fails mapping
somewhere after the first bio, it will return with rq-bio set to
non-NULL, but it will have already unmapped the partial bio. The
out: error exit section will see the non-null bio and try to unmap
it again, triggering a mapcount bug via bad_page().
Signed-off-by: Pete Wyckoff [EMAIL PROTECTED]
---
block/bsg.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 3337125..bba7154 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -295,8 +295,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4
*hdr)
dxferp = (void*)(unsigned long)hdr-din_xferp;
ret = blk_rq_map_user(q, next_rq, dxferp,
hdr-din_xfer_len);
- if (ret)
+ if (ret) {
+ next_rq-bio = NULL; /* do not unmap twice */
Nice ... that's a nasty asymmetry of the blk_rq_map_user API. The map
takes a request gets a ref and fills in the bio. The unmap has to be
called on the bio leaving you to clear the now removed bio reference
manually.
It is nasty, how about fixing that instead?
Yeah, looks better for me though the blk_rq_map_user API is still a
bit hacky, as James said.
Seems symmetric to me now, either we fail and everything is cleaned up,
or return success. What remains?
--
Jens Axboe
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bsg: bidi bio map failure fix
On Mon, 2008-02-18 at 15:46 +0100, Jens Axboe wrote: Seems symmetric to me now, either we fail and everything is cleaned up, or return success. What remains? My main symmetry complaint was the API: The map takes a request, the unmap takes a bio. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bsg: bidi bio map failure fix
On Tue, 12 Feb 2008 15:40:24 -0500
Pete Wyckoff [EMAIL PROTECTED] wrote:
If blk_rq_map_user requires more than one bio, and fails mapping
somewhere after the first bio, it will return with rq-bio set to
non-NULL, but it will have already unmapped the partial bio. The
out: error exit section will see the non-null bio and try to unmap
it again, triggering a mapcount bug via bad_page().
Signed-off-by: Pete Wyckoff [EMAIL PROTECTED]
---
block/bsg.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 3337125..bba7154 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -295,8 +295,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr)
dxferp = (void*)(unsigned long)hdr-din_xferp;
ret = blk_rq_map_user(q, next_rq, dxferp, hdr-din_xfer_len);
- if (ret)
+ if (ret) {
+ next_rq-bio = NULL; /* do not unmap twice */
goto out;
+ }
}
if (hdr-dout_xfer_len) {
Thanks!
Acked-by: FUJITA Tomonori [EMAIL PROTECTED]
James, please put this to the scsi-fixes tree.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bsg: bidi bio map failure fix
On Tue, 2008-02-12 at 15:40 -0500, Pete Wyckoff wrote:
If blk_rq_map_user requires more than one bio, and fails mapping
somewhere after the first bio, it will return with rq-bio set to
non-NULL, but it will have already unmapped the partial bio. The
out: error exit section will see the non-null bio and try to unmap
it again, triggering a mapcount bug via bad_page().
Signed-off-by: Pete Wyckoff [EMAIL PROTECTED]
---
block/bsg.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 3337125..bba7154 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -295,8 +295,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr)
dxferp = (void*)(unsigned long)hdr-din_xferp;
ret = blk_rq_map_user(q, next_rq, dxferp, hdr-din_xfer_len);
- if (ret)
+ if (ret) {
+ next_rq-bio = NULL; /* do not unmap twice */
Nice ... that's a nasty asymmetry of the blk_rq_map_user API. The map
takes a request gets a ref and fills in the bio. The unmap has to be
called on the bio leaving you to clear the now removed bio reference
manually.
James
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] bsg: bidi bio map failure fix
If blk_rq_map_user requires more than one bio, and fails mapping
somewhere after the first bio, it will return with rq-bio set to
non-NULL, but it will have already unmapped the partial bio. The
out: error exit section will see the non-null bio and try to unmap
it again, triggering a mapcount bug via bad_page().
Signed-off-by: Pete Wyckoff [EMAIL PROTECTED]
---
block/bsg.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 3337125..bba7154 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -295,8 +295,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr)
dxferp = (void*)(unsigned long)hdr-din_xferp;
ret = blk_rq_map_user(q, next_rq, dxferp, hdr-din_xfer_len);
- if (ret)
+ if (ret) {
+ next_rq-bio = NULL; /* do not unmap twice */
goto out;
+ }
}
if (hdr-dout_xfer_len) {
--
1.5.3.8
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
