Re: [PATCH 1/1] um: ubd: Fix data corruption

[Jens Axboe <jaxboe@fusionio.com>]

On 2010-09-29 07:52, Chris Frey wrote:
> On Wed, Sep 29, 2010 at 12:13:10AM +0200, Richard Weinberger wrote:
>> Am Mittwoch 29 September 2010, 00:00:00 schrieb Andrew Morton:
>>> This is a workaround, I think?  Do we know what the actual bug is?
>>> From the comment it appears to be a regression?
>>
>> Yes, it is a workaround.
>> For more details please have a look at this post:
>> http://lkml.org/lkml/2010/9/28/245
> 
> I'm going to run some tests to see how this works on my end, but in
> reading old threads, I found this response from Bram Matthys
> that casts some doubt on an older version of this patch.
> 
> 	http://marc.info/?l=user-mode-linux-user&m=126883932731656&w=2
> 
> I've added him to the CC.

It looks like that if we need to restart the requeue, then
we use the initial position and not the current index. Does
this help?

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 1bcd208..81ee063 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -162,7 +162,7 @@ struct ubd {
 	spinlock_t lock;
 	struct scatterlist sg[MAX_SG];
 	struct request *request;
-	int start_sg, end_sg;
+	int start_sg, end_sg, rq_off;
 };
 
 #define DEFAULT_COW { \
@@ -187,6 +187,7 @@ struct ubd {
 	.request =		NULL, \
 	.start_sg =		0, \
 	.end_sg =		0, \
+	.rq_off =		0, \
 }
 
 /* Protected by ubd_lock */
@@ -1241,10 +1242,11 @@ static void do_ubd_request(struct request_queue *q)
 			dev->request = req;
 			dev->start_sg = 0;
 			dev->end_sg = blk_rq_map_sg(q, req, dev->sg);
+			dev->rq_off = 0;
 		}
 
 		req = dev->request;
-		sector = blk_rq_pos(req);
+		sector = blk_rq_pos(req) + dev->rq_off;
 		while(dev->start_sg < dev->end_sg){
 			struct scatterlist *sg = &dev->sg[dev->start_sg];
 
@@ -1273,6 +1275,7 @@ static void do_ubd_request(struct request_queue *q)
 			}
 
 			dev->start_sg++;
+			dev->rq_off += sg->length >> 9;
 		}
 		dev->end_sg = 0;
 		dev->request = NULL;

-- 
Jens Axboe