Re: [uml-devel] [PATCH 1/1] um: ubd: Fix data corruption
On Tue, Oct 05, 2010 at 10:23:19AM +0200, Tejun Heo wrote: > H, can you please give a shot at the following one? Thank you. I applied this patch on top of stock 2.6.35.5 as usual (no other patches) and tested on my maverick image as before. I ran a fsck.ext3 on the filesystem image from the host before my test, just to make sure, and there were no errors. Unfortunately, this patch does not fix the issue either. I get errors in the guest like the following: EXT3-fs error (device ubda): htree_dirblock_to_tree: bad entry in directory #1137497: rec_len is smaller than minimal - offset=0, inode=0, rec_len=0, name_len=0 EXT3-fs (ubda): warning: empty_dir: bad directory (dir #1137497) - no `.' or `..' EXT3-fs error (device ubda): htree_dirblock_to_tree: bad entry in directory #1137587: rec_len is smaller than minimal - offset=0, inode=0, rec_len=0, name_len=0 EXT3-fs (ubda): warning: empty_dir: bad directory (dir #1137587) - no `.' or `..' EXT3-fs error (device ubda): htree_dirblock_to_tree: bad entry in directory #1137619: rec_len is smaller than minimal - offset=0, inode=0, rec_len=0, name_len=0 EXT3-fs (ubda): warning: empty_dir: bad directory (dir #1137619) - no `.' or `..' EXT3-fs error (device ubda): htree_dirblock_to_tree: bad entry in directory #1137532: rec_len is smaller than minimal - offset=0, inode=0, rec_len=0, name_len=0 EXT3-fs (ubda): warning: empty_dir: bad directory (dir #1137532) - no `.' or `..' EXT3-fs (ubda): warning: ext3_rmdir: empty directory has nlink!=2 (6) EXT3-fs (ubda): warning: ext3_rmdir: empty directory has nlink!=2 (3) And later on: EXT3-fs error (device ubda): ext3_lookup: deleted inode referenced: 867196 EXT3-fs error (device ubda): ext3_lookup: deleted inode referenced: 484932 EXT3-fs error (device ubda): ext3_lookup: deleted inode referenced: 484932 - Chris -- Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today. http://p.sf.net/sfu/beautyoftheweb ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] [PATCH 1/1] um: ubd: Fix data corruption
On Wed, Sep 29, 2010 at 7:21 AM, Jens Axboe wrote: > I think we need to find the real fix here, just disabling merging > is not a fix (it's just a nasty work-around for the real bug). > Jens, Do you have an idea which parts of the code are buggy? -- Cheers, //richard -- Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today. http://p.sf.net/sfu/beautyoftheweb ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] [PATCH 1/1] um: ubd: Fix data corruption
Hello, sorry about chiming in later. I was off last week.
On 09/29/2010 08:34 AM, Chris Frey wrote:
> On Wed, Sep 29, 2010 at 02:21:07PM +0900, Jens Axboe wrote:
>> This seems to imply that the original commit pin pointed is not
>> the only issue we have in that code atm.
>>
>> I think we need to find the real fix here, just disabling merging
>> is not a fix (it's just a nasty work-around for the real bug).
>
> You're probably right, and I'm quite willing to help test further patches
> to help get to the bottom of this.
I think we're on the right track. The problem with Jens' patch was
that it didn't consider the fact that blk_end_request() now internally
updates the current position of the request, so if restart happens
after some of part of the request is complete it will end up adding
the offsets multiple times. I think slightly modifying it to track
the current position instead of offset should do it. Chris, can you
please try the following patch and see whether the problem goes away?
Thanks.
diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 1bcd208..d8a5d8c 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -163,6 +163,7 @@ struct ubd {
struct scatterlist sg[MAX_SG];
struct request *request;
int start_sg, end_sg;
+ sector_t rq_pos;
};
#define DEFAULT_COW { \
@@ -187,6 +188,7 @@ struct ubd {
.request = NULL, \
.start_sg = 0, \
.end_sg = 0, \
+ .rq_pos = 0, \
}
/* Protected by ubd_lock */
@@ -1228,7 +1230,6 @@ static void do_ubd_request(struct request_queue *q)
{
struct io_thread_req *io_req;
struct request *req;
- sector_t sector;
int n;
while(1){
@@ -1244,7 +1245,7 @@ static void do_ubd_request(struct request_queue *q)
}
req = dev->request;
- sector = blk_rq_pos(req);
+ dev->rq_pos = blk_rq_pos(req);
while(dev->start_sg < dev->end_sg){
struct scatterlist *sg = &dev->sg[dev->start_sg];
@@ -1256,10 +1257,10 @@ static void do_ubd_request(struct request_queue *q)
return;
}
prepare_request(req, io_req,
- (unsigned long long)sector << 9,
+ (unsigned long long)dev->rq_pos << 9,
sg->offset, sg->length, sg_page(sg));
- sector += sg->length >> 9;
+ dev->rq_pos += sg->length >> 9;
n = os_write_file(thread_fd, &io_req,
sizeof(struct io_thread_req *));
if(n != sizeof(struct io_thread_req *)){
--
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3.
Spend less time writing and rewriting code and more time creating great
experiences on the web. Be a part of the beta today.
http://p.sf.net/sfu/beautyoftheweb
___
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] [PATCH 1/1] um: ubd: Fix data corruption
On Mon, Oct 04, 2010 at 06:37:36PM +0200, Tejun Heo wrote: > Hello, sorry about chiming in later. I was off last week. No problem, I'm eager to test patches to fix this. > I think we're on the right track. The problem with Jens' patch was > that it didn't consider the fact that blk_end_request() now internally > updates the current position of the request, so if restart happens > after some of part of the request is complete it will end up adding > the offsets multiple times. I think slightly modifying it to track > the current position instead of offset should do it. Chris, can you > please try the following patch and see whether the problem goes away? Unfortunately, this patch does not fix it for me. I applied your patch to kernel 2.6.35.5, and got the usual error: EXT3-fs error (device ubda): ext3_lookup: deleted inode referenced: 566188 Lots of them, actually. - Chris -- Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today. http://p.sf.net/sfu/beautyoftheweb ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] [PATCH 1/1] um: ubd: Fix data corruption
On 10/04/2010 09:51 PM, Chris Frey wrote:
> On Mon, Oct 04, 2010 at 06:37:36PM +0200, Tejun Heo wrote:
>> Hello, sorry about chiming in later. I was off last week.
>
> No problem, I'm eager to test patches to fix this.
>
>> I think we're on the right track. The problem with Jens' patch was
>> that it didn't consider the fact that blk_end_request() now internally
>> updates the current position of the request, so if restart happens
>> after some of part of the request is complete it will end up adding
>> the offsets multiple times. I think slightly modifying it to track
>> the current position instead of offset should do it. Chris, can you
>> please try the following patch and see whether the problem goes away?
>
> Unfortunately, this patch does not fix it for me. I applied your patch
> to kernel 2.6.35.5, and got the usual error:
>
> EXT3-fs error (device ubda): ext3_lookup: deleted inode referenced: 566188
>
> Lots of them, actually.
H, can you please give a shot at the following one? Thank you.
diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 1bcd208..0435d21 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -163,6 +163,7 @@ struct ubd {
struct scatterlist sg[MAX_SG];
struct request *request;
int start_sg, end_sg;
+ sector_t rq_pos;
};
#define DEFAULT_COW { \
@@ -187,6 +188,7 @@ struct ubd {
.request = NULL, \
.start_sg = 0, \
.end_sg = 0, \
+ .rq_pos = 0, \
}
/* Protected by ubd_lock */
@@ -1228,7 +1230,6 @@ static void do_ubd_request(struct request_queue *q)
{
struct io_thread_req *io_req;
struct request *req;
- sector_t sector;
int n;
while(1){
@@ -1239,12 +1240,12 @@ static void do_ubd_request(struct request_queue *q)
return;
dev->request = req;
+ dev->rq_pos = blk_rq_pos(req);
dev->start_sg = 0;
dev->end_sg = blk_rq_map_sg(q, req, dev->sg);
}
req = dev->request;
- sector = blk_rq_pos(req);
while(dev->start_sg < dev->end_sg){
struct scatterlist *sg = &dev->sg[dev->start_sg];
@@ -1256,10 +1257,10 @@ static void do_ubd_request(struct request_queue *q)
return;
}
prepare_request(req, io_req,
- (unsigned long long)sector << 9,
+ (unsigned long long)dev->rq_pos << 9,
sg->offset, sg->length, sg_page(sg));
- sector += sg->length >> 9;
+ dev->rq_pos += sg->length >> 9;
n = os_write_file(thread_fd, &io_req,
sizeof(struct io_thread_req *));
if(n != sizeof(struct io_thread_req *)){
--
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3.
Spend less time writing and rewriting code and more time creating great
experiences on the web. Be a part of the beta today.
http://p.sf.net/sfu/beautyoftheweb
___
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
