[BUGFIX 0/2] BUG Fixes for OSD in mainline (2.6.30)

[BUGFIX 0/2] BUG Fixes for OSD in mainline (2.6.30)

[Boaz Harrosh]

A very serious and stupid bug was found by users in current osd code,
which caused it not to be usable in it's current form.

It would hard-lock immediately on any asynchronous execution call
on an SMP Kernel. Just to show that it was only run on uni-machines
until today.

Since found I've setup a test rig with SMP machines banging on each
other with multi threaded/devices/mounts and they are still up.
(Same setup I used to run with UMLs before)

Thanks to Xu Yang <onlyxuyang@qq.com> for reporting this and his devotion
to finding a fix.

These are the patches

[PATCH 1/2] libosd: BUGFIX: blk_put_request can't be called from within request_end_io

    A fix for a very serious and stupid bug in osd_initiator. It used to
    call blk_put_request() regardless of if it was from the end_io callback
    or if called after a sync execution. It should call the unlocked version
    __blk_put_request() instead.

    Also fixed is the remove of _abort_unexecuted_bios hack, and use of
    blk_end_request(,-ERROR,) to deallocate half baked requests. I've
    audited the code and it should be safe.

    Reported and
    Tested-by: Xu Yang <onlyxuyang@qq.com>
    Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

[PATCH 2/2] osd_uld: Remove creation of osd_scsi class symlink

    Remove the creation of the symlink from the device to
    it's class. On modern systems this is already created by
    a udev rule and would WARN on load. On old systems it is
    not needed, none of the current osd user-mode tools use
    this link.

    Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

Please apply for inclusion in the next round of scsi fixes for
2.6.30

Thank you
Boaz

[PATCH 1/2] libosd: BUG: blk_put_request called from within request_end_io

[Boaz Harrosh]

A fix for a very serious and stupid bug in osd_initiator. It
used to call blk_put_request() regardless of if it was from
the end_io callback or if called after a sync execution.
It should call the unlocked version __blk_put_request() instead.

Also fixed is the remove of _abort_unexecuted_bios hack, and use of
blk_end_request(,-ERROR,) to deallocate half baked requests. I've
audited the code and it should be safe.

Reported and
Tested-by: Xu Yang <onlyxuyang@qq.com>
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |   42 +++++++++++++++++++++----------------
 1 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 552f58b..2a5f077 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -338,20 +338,6 @@ struct osd_request *osd_start_request(struct osd_dev *dev, gfp_t gfp)
 }
 EXPORT_SYMBOL(osd_start_request);
 
-/*
- * If osd_finalize_request() was called but the request was not executed through
- * the block layer, then we must release BIOs.
- */
-static void _abort_unexecuted_bios(struct request *rq)
-{
-	struct bio *bio;
-
-	while ((bio = rq->bio) != NULL) {
-		rq->bio = bio->bi_next;
-		bio_endio(bio, 0);
-	}
-}
-
 static void _osd_free_seg(struct osd_request *or __unused,
 	struct _osd_req_data_segment *seg)
 {
@@ -363,9 +349,30 @@ static void _osd_free_seg(struct osd_request *or __unused,
 	seg->alloc_size = 0;
 }
 
+static void _put_request(struct request *rq , bool is_async)
+{
+	if (is_async) {
+		WARN_ON(rq->bio);
+		__blk_put_request(rq->q, rq);
+	} else {
+		/*
+		 * If osd_finalize_request() was called but the request was not
+		 * executed through the block layer, then we must release BIOs.
+		 * TODO: Keep error code in or->async_error. Need to audit all
+		 *       code paths.
+		 */
+		if (unlikely(rq->bio))
+			blk_end_request(rq, -ENOMEM, blk_rq_bytes(rq));
+		else
+			blk_put_request(rq);
+	}
+}
+
 void osd_end_request(struct osd_request *or)
 {
 	struct request *rq = or->request;
+	/* IMPORTANT: make sure this agrees with osd_execute_request_async */
+	bool is_async = (or->request->end_io_data == or);
 
 	_osd_free_seg(or, &or->set_attr);
 	_osd_free_seg(or, &or->enc_get_attr);
@@ -373,12 +380,11 @@ void osd_end_request(struct osd_request *or)
 
 	if (rq) {
 		if (rq->next_rq) {
-			_abort_unexecuted_bios(rq->next_rq);
-			blk_put_request(rq->next_rq);
+			_put_request(rq->next_rq, is_async);
+			rq->next_rq = NULL;
 		}
 
-		_abort_unexecuted_bios(rq);
-		blk_put_request(rq);
+		_put_request(rq, is_async);
 	}
 	_osd_request_free(or);
 }
-- 
1.6.2.1

[PATCH 2/2] osd_uld: Remove creation of osd_scsi class symlink

[Boaz Harrosh]

Remove the creation of the symlink from the device to
it's class. On modern systems this is already created by
a udev rule and would WARN on load. On old systems it is
not needed, none of the current osd user-mode tools use
this link.

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

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index f8b1a74..f644c95 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -345,10 +345,6 @@ static int osd_probe(struct device *dev)
 	}
 
 	dev_set_drvdata(oud->class_member, oud);
-	error = sysfs_create_link(&scsi_device->sdev_gendev.kobj,
-				  &oud->class_member->kobj, osd_symlink);
-	if (error)
-		OSD_ERR("warning: unable to make symlink\n");
 
 	OSD_INFO("osd_probe %s\n", disk->disk_name);
 	return 0;
@@ -377,8 +373,6 @@ static int osd_remove(struct device *dev)
 			scsi_device);
 	}
 
-	sysfs_remove_link(&oud->od.scsi_device->sdev_gendev.kobj, osd_symlink);
-
 	if (oud->class_member)
 		device_destroy(osd_sysfs_class,
 			       MKDEV(SCSI_OSD_MAJOR, oud->minor));
-- 
1.6.2.1