-- 
Thanks,
Oliver
-

[ message continues ]

On Tue, 5 Feb 2008 22:48:29 +0100

Looks like you deadlocked in ub_request_fn().  I assume that you were using
ub.c in 2.6.23 and that it worked OK?  If so, we broke it, possibly via
changes to the core block layer.

I think ub.c is basically abandoned in favour of usb-storage.  If so,
perhaps we should remove or disble ub.c?
-

[ message continues ]

Actually I think it may be an argument for keeping ub, if ub exposes
a bug in the __blk_end_request. I'll look at the head of the thread
and see if Mr. Pinter has hit anything related to Mr. Ueda's work.

-- Pete
-

[ message continues ]

i reverted this commit 7d699bafe258ebd8f9b4ec182c554200b369a504 , and
now compile ...



-- 
Thanks,
Oliver
-

[ message continues ]

/usr/data/source/git/linux-2.6/drivers/block/ub.c: In function 'ub_end_rq':
/usr/data/source/git/linux-2.6/drivers/block/ub.c:819: error: implicit
declaration of function 'end_that_request_first'
/usr/data/source/git/linux-2.6/drivers/block/ub.c:820: error: implicit
declaration of function 'end_that_request_last'
make[7]: *** [drivers/block/ub.o] Error 1
make[6]: *** [drivers/block] Error 2
make[5]: *** [drivers] Error 2
make[5]: *** Waiting for unfinished jobs....




-- 
Thanks,
Oliver
-

[ message continues ]

Looks like it's just Tomo or Jens made a mistake when converting to
the new s/g API. Nothing to be too concerned about. I know I should've
reviewed their patch closer, but it seemed too simple...

-- Pete

Fix up the conversion to sg_init_table().

Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>

--- a/drivers/block/ub.c
+++ b/drivers/block/ub.c
@@ -657,7 +657,6 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)
 	if ((cmd = ub_get_cmd(lun)) == NULL)
 		return -1;
 	memset(cmd, 0, sizeof(struct ub_scsi_cmd));
-	sg_init_table(cmd->sgv, UB_MAX_REQ_SG);
 
 	blkdev_dequeue_request(rq);
 
@@ -668,6 +667,7 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)
 	/*
 	 * get scatterlist from block layer
 	 */
+	sg_init_table(&urq->sgv[0], UB_MAX_REQ_SG);
 	n_elem = blk_rq_map_sg(lun->disk->queue, rq, &urq->sgv[0]);
 	if (n_elem < 0) {
 		/* Impossible, because blk_rq_map_sg should not hit ENOMEM. */
-

[ message continues ]

On Fri, 8 Feb 2008 23:46:19 -0800

I guess I can put the blame for this on Jens' commit (45711f1a) ;)

On a serious note, it seems that two scatter lists per request leaded
to this bug. Can the scatter list in struct ub_request be removed?

-

[ message continues ]

Good question. It's an eyesore to be sure. The duplication exists
for the sake of retries combined with the separation of requests
from commands.

Please bear with me, if you're curious: commands can be launched
without requests (at probe time, for instance, or when sense is
requested). So, they need an s/g table. But then, the lifetime of
a request is greater than than of a command, in case of a retry
especially. Therefore a request needs the s/g table too.

So, one way to kill this duplication is to mandate that a
request existed for every command. It seemed like way more code
than just one memcpy() when I wrote it.

Another way would be to make commands flexible, e.g. sometimes with
just a virtual address and size, sometimes with an s/g table.
If you guys make struct scatterlist illegal to copy with memcpy
one day, this is probably what I'll do.

-- Pete
-

[ message continues ]