* [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) @ 2010-09-28 16:16 richard -rw- weinberger 2010-09-28 17:07 ` [uml-devel] " Janjaap Bos 0 siblings, 1 reply; 7+ messages in thread From: richard -rw- weinberger @ 2010-09-28 16:16 UTC (permalink / raw) To: tj Cc: LKML, user-mode-linux-devel, axboe, jdike, Geert.Uytterhoeven, grant.likely, adrian, mike.miller, Chris Frey Hi Tejun! Chris Frey ran into some problems with ext3 on UML. See: http://marc.info/?i=20100924041410.GA18040%20()%20foursquare%20!%20net I can reproduce this issue with any file system on Linux >= 2.6.31. There seems to be a serious problem with the block layer on UML. Under high load any file system gets corrupted. The regression was most likely introduced with this three changes: 83096ebf1263b2c1ee5e653ba37d993d02e3eb7b (block: convert to pos and nr_sectors accessors) f81f2f7c9fee307e371f37424577d46f9eaf8692 (ubd: drop unnecessary rq->sector manipulation) 4d6c84d91d1a539ebc47d1a36a35e9390ba11fdc (ubd: cleanup completion path) Maybe only f81f2f7 is the bad one. They depend on each and I don't know all the internals... -- Cheers, //richard ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [uml-devel] [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) 2010-09-28 16:16 [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) richard -rw- weinberger @ 2010-09-28 17:07 ` Janjaap Bos 2010-09-28 19:52 ` Geert Uytterhoeven 0 siblings, 1 reply; 7+ messages in thread From: Janjaap Bos @ 2010-09-28 17:07 UTC (permalink / raw) To: richard -rw- weinberger Cc: tj, axboe, mike.miller, user-mode-linux-devel, adrian, jdike, LKML, grant.likely, Geert.Uytterhoeven, Chris Frey [-- Attachment #1: Type: text/plain, Size: 967 bytes --] See attached patch, and earlier message posted in March 2010 on uml user list. We are out of maintainer... Best, -Janjaap On Tue, 2010-09-28 at 18:16 +0200, richard -rw- weinberger wrote: > Hi Tejun! > > Chris Frey ran into some problems with ext3 on UML. > See: http://marc.info/?i=20100924041410.GA18040%20()%20foursquare%20!%20net > > I can reproduce this issue with any file system on Linux >= 2.6.31. > There seems to be a serious problem with the block layer on UML. > Under high load any file system gets corrupted. > > The regression was most likely introduced with this three changes: > 83096ebf1263b2c1ee5e653ba37d993d02e3eb7b (block: convert to pos and > nr_sectors accessors) > f81f2f7c9fee307e371f37424577d46f9eaf8692 (ubd: drop unnecessary > rq->sector manipulation) > 4d6c84d91d1a539ebc47d1a36a35e9390ba11fdc (ubd: cleanup completion path) > > Maybe only f81f2f7 is the bad one. They depend on each and I don't > know all the internals... > [-- Attachment #2: v2.6.33.diff --] [-- Type: text/x-patch, Size: 4560 bytes --] diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c index cf8a97f..a529c3f 100644 --- a/arch/um/drivers/line.c +++ b/arch/um/drivers/line.c @@ -305,10 +305,12 @@ int line_ioctl(struct tty_struct *tty, struct file * file, for (i = 0; i < ARRAY_SIZE(tty_ioctls); i++) if (cmd == tty_ioctls[i].cmd) break; +#ifdef DEBUG if (i == ARRAY_SIZE(tty_ioctls)) { printk(KERN_ERR "%s: %s: unknown ioctl: 0x%x\n", __func__, tty->name, cmd); } +#endif ret = -ENOIOCTLCMD; break; } diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index 5ff5546..655ed9e 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -746,8 +746,12 @@ static int ubd_open_dev(struct ubd *ubd_dev) } ubd_dev->fd = fd; + /* A setting higher than 1 sector currently (v2.6.33) generates + data loss, both for raw and cow ubd. */ + blk_queue_max_sectors(ubd_dev->queue, 1 * sizeof(long)); + blk_queue_max_phys_segments(ubd_dev->queue, 1 * sizeof(long)); + if(ubd_dev->cow.file != NULL){ - blk_queue_max_sectors(ubd_dev->queue, 8 * sizeof(long)); err = -ENOMEM; ubd_dev->cow.bitmap = vmalloc(ubd_dev->cow.bitmap_len); @@ -1223,7 +1227,7 @@ static void do_ubd_request(struct request_queue *q) struct io_thread_req *io_req; struct request *req; sector_t sector; - int n; + int n, last_sectors; while(1){ struct ubd *dev = q->queuedata; @@ -1239,9 +1243,12 @@ static void do_ubd_request(struct request_queue *q) req = dev->request; sector = blk_rq_pos(req); + last_sectors = 0; while(dev->start_sg < dev->end_sg){ struct scatterlist *sg = &dev->sg[dev->start_sg]; + sector += last_sectors; + last_sectors = 0; io_req = kmalloc(sizeof(struct io_thread_req), GFP_ATOMIC); if(io_req == NULL){ @@ -1253,7 +1260,7 @@ static void do_ubd_request(struct request_queue *q) (unsigned long long)sector << 9, sg->offset, sg->length, sg_page(sg)); - sector += sg->length >> 9; + last_sectors = sg->length >> 9; n = os_write_file(thread_fd, &io_req, sizeof(struct io_thread_req *)); if(n != sizeof(struct io_thread_req *)){ diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S index 7fcad58..0731311 100644 --- a/arch/um/kernel/dyn.lds.S +++ b/arch/um/kernel/dyn.lds.S @@ -50,8 +50,20 @@ SECTIONS .rela.got : { *(.rela.got) } .rel.bss : { *(.rel.bss .rel.bss.* .rel.gnu.linkonce.b.*) } .rela.bss : { *(.rela.bss .rela.bss.* .rela.gnu.linkonce.b.*) } - .rel.plt : { *(.rel.plt) } - .rela.plt : { *(.rela.plt) } + .rel.plt : + { + *(.rel.plt) + PROVIDE_HIDDEN (__rel_iplt_start = .); + *(.rel.iplt) + PROVIDE_HIDDEN (__rel_iplt_end = .); + } + .rela.plt : + { + *(.rela.plt) + PROVIDE_HIDDEN (__rela_iplt_start = .); + *(.rela.iplt) + PROVIDE_HIDDEN (__rela_iplt_end = .); + } .init : { KEEP (*(.init)) } =0x90909090 diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S index e7a6cca..a658604 100644 --- a/arch/um/kernel/uml.lds.S +++ b/arch/um/kernel/uml.lds.S @@ -36,6 +36,21 @@ SECTIONS *(.gnu.linkonce.t*) } + .rel.plt : + { + *(.rel.plt) + PROVIDE_HIDDEN (__rel_iplt_start = .); + *(.rel.iplt) + PROVIDE_HIDDEN (__rel_iplt_end = .); + } + .rela.plt : + { + *(.rela.plt) + PROVIDE_HIDDEN (__rela_iplt_start = .); + *(.rela.iplt) + PROVIDE_HIDDEN (__rela_iplt_end = .); + } + . = ALIGN(PAGE_SIZE); .syscall_stub : { __syscall_stub_start = .; diff --git a/arch/um/sys-x86_64/ptrace.c b/arch/um/sys-x86_64/ptrace.c index f3458d7..1c738c7 100644 --- a/arch/um/sys-x86_64/ptrace.c +++ b/arch/um/sys-x86_64/ptrace.c @@ -69,7 +69,7 @@ int poke_user(struct task_struct *child, long addr, long data) else if ((addr >= offsetof(struct user, u_debugreg[0])) && (addr <= offsetof(struct user, u_debugreg[7]))) { addr -= offsetof(struct user, u_debugreg[0]); - addr = addr >> 2; + addr = addr >> 3; // long is 8 bytes on x86_64 if ((addr == 4) || (addr == 5)) return -EIO; child->thread.arch.debugregs[addr] = data; @@ -114,7 +114,7 @@ int peek_user(struct task_struct *child, long addr, long data) else if ((addr >= offsetof(struct user, u_debugreg[0])) && (addr <= offsetof(struct user, u_debugreg[7]))) { addr -= offsetof(struct user, u_debugreg[0]); - addr = addr >> 2; + addr = addr >> 3; // long is 8 bytes on x86_64 tmp = child->thread.arch.debugregs[addr]; } return put_user(tmp, (unsigned long *) data); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [uml-devel] [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) 2010-09-28 17:07 ` [uml-devel] " Janjaap Bos @ 2010-09-28 19:52 ` Geert Uytterhoeven 2010-09-28 20:12 ` Janjaap Bos 0 siblings, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2010-09-28 19:52 UTC (permalink / raw) To: Janjaap Bos Cc: richard -rw- weinberger, tj, axboe, mike.miller, user-mode-linux-devel, adrian, jdike, LKML, grant.likely, Geert.Uytterhoeven, Chris Frey On Tue, Sep 28, 2010 at 19:07, Janjaap Bos <janjaap@bos.nl> wrote: > See attached patch, and earlier message posted in March 2010 on uml user > list. We are out of maintainer... Thanks for the patch! | --- a/arch/um/drivers/ubd_kern.c | +++ b/arch/um/drivers/ubd_kern.c | @@ -1223,7 +1227,7 @@ static void do_ubd_request(struct request_queue *q) | struct io_thread_req *io_req; | struct request *req; | sector_t sector; | - int n; | + int n, last_sectors; | | while(1){ | struct ubd *dev = q->queuedata; | @@ -1239,9 +1243,12 @@ static void do_ubd_request(struct request_queue *q) | | req = dev->request; | sector = blk_rq_pos(req); | + last_sectors = 0; | while(dev->start_sg < dev->end_sg){ | struct scatterlist *sg = &dev->sg[dev->start_sg]; | | + sector += last_sectors; | + last_sectors = 0; | io_req = kmalloc(sizeof(struct io_thread_req), | GFP_ATOMIC); | if(io_req == NULL){ | @@ -1253,7 +1260,7 @@ static void do_ubd_request(struct request_queue *q) | (unsigned long long)sector << 9, | sg->offset, sg->length, sg_page(sg)); | | - sector += sg->length >> 9; | + last_sectors = sg->length >> 9; | n = os_write_file(thread_fd, &io_req, | sizeof(struct io_thread_req *)); | if(n != sizeof(struct io_thread_req *)){ However, I'm wondering what difference this part makes? > On Tue, 2010-09-28 at 18:16 +0200, richard -rw- weinberger wrote: >> Hi Tejun! >> >> Chris Frey ran into some problems with ext3 on UML. >> See: http://marc.info/?i=20100924041410.GA18040%20()%20foursquare%20!%20net >> >> I can reproduce this issue with any file system on Linux >= 2.6.31. >> There seems to be a serious problem with the block layer on UML. >> Under high load any file system gets corrupted. >> >> The regression was most likely introduced with this three changes: >> 83096ebf1263b2c1ee5e653ba37d993d02e3eb7b (block: convert to pos and >> nr_sectors accessors) >> f81f2f7c9fee307e371f37424577d46f9eaf8692 (ubd: drop unnecessary >> rq->sector manipulation) >> 4d6c84d91d1a539ebc47d1a36a35e9390ba11fdc (ubd: cleanup completion path) >> >> Maybe only f81f2f7 is the bad one. They depend on each and I don't >> know all the internals... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [uml-devel] [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) 2010-09-28 19:52 ` Geert Uytterhoeven @ 2010-09-28 20:12 ` Janjaap Bos 2010-09-28 20:37 ` Geert Uytterhoeven 0 siblings, 1 reply; 7+ messages in thread From: Janjaap Bos @ 2010-09-28 20:12 UTC (permalink / raw) To: Geert Uytterhoeven Cc: richard -rw- weinberger, tj, axboe, mike.miller, user-mode-linux-devel, adrian, jdike, LKML, grant.likely, Geert.Uytterhoeven, Chris Frey Hi, On Tue, 2010-09-28 at 21:52 +0200, Geert Uytterhoeven wrote: > On Tue, Sep 28, 2010 at 19:07, Janjaap Bos <janjaap@bos.nl> wrote: > > See attached patch, and earlier message posted in March 2010 on uml user > > list. We are out of maintainer... > > Thanks for the patch! > > | --- a/arch/um/drivers/ubd_kern.c > | +++ b/arch/um/drivers/ubd_kern.c > | @@ -1223,7 +1227,7 @@ static void do_ubd_request(struct request_queue *q) > | struct io_thread_req *io_req; > | struct request *req; > | sector_t sector; > | - int n; > | + int n, last_sectors; > | > | while(1){ > | struct ubd *dev = q->queuedata; > | @@ -1239,9 +1243,12 @@ static void do_ubd_request(struct request_queue *q) > | > | req = dev->request; > | sector = blk_rq_pos(req); > | + last_sectors = 0; > | while(dev->start_sg < dev->end_sg){ > | struct scatterlist *sg = &dev->sg[dev->start_sg]; > | > | + sector += last_sectors; > | + last_sectors = 0; > | io_req = kmalloc(sizeof(struct io_thread_req), > | GFP_ATOMIC); > | if(io_req == NULL){ > | @@ -1253,7 +1260,7 @@ static void do_ubd_request(struct request_queue *q) > | (unsigned long long)sector << 9, > | sg->offset, sg->length, sg_page(sg)); > | > | - sector += sg->length >> 9; > | + last_sectors = sg->length >> 9; > | n = os_write_file(thread_fd, &io_req, > | sizeof(struct io_thread_req *)); > | if(n != sizeof(struct io_thread_req *)){ > > However, I'm wondering what difference this part makes? > It fixes ubd block handling integrity. With large block operations errors occurred. Probably due to lost request pointers as explained below. Need to keep a local count of sectors and delay the update. Done by reverting commit f81f2f7c9fee307e371f37424577d46f9eaf8692 using the present block api. (At least which is what I intend, but perhaps quite likely I am missing the point, also it may not be needed anymore if only single 512 byte sector blocks are used per request. Anyway... it solves the problem for me ;-) Regards, -Janjaap See: reverted: commit f81f2f7c9fee307e371f37424577d46f9eaf8692 Author: Tejun Heo <tj@kernel.org> Date: Tue Apr 28 13:06:10 2009 +0900 ubd: drop unnecessary rq->sector manipulation ubd curiously updates rq->sector while issuing the request in multiple pieces. Don't do it and simply use local copy of sector. See for original reason: commit 0a6d3a2a3813e7b25267366cfbf9a4a4698dd1c2 Author: Jeff Dike <jdike@addtoit.com> Date: Sun Jul 15 23:38:47 2007 -0700 uml: fix request->sector update It is theoretically possible for a request to finish and be freed between writing it to the I/O thread and updating the sector count. In this case, the update will dereference a freed pointer. To avoid this, I delay the update until processing the next sg segment, when the request pointer is known to be good. modified: arch/um/drivers/ubd_kern.c ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [uml-devel] [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) 2010-09-28 20:12 ` Janjaap Bos @ 2010-09-28 20:37 ` Geert Uytterhoeven 2010-09-28 21:19 ` richard -rw- weinberger 0 siblings, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2010-09-28 20:37 UTC (permalink / raw) To: Janjaap Bos Cc: richard -rw- weinberger, tj, axboe, mike.miller, user-mode-linux-devel, adrian, jdike, LKML, grant.likely, Geert.Uytterhoeven, Chris Frey On Tue, Sep 28, 2010 at 22:12, Janjaap Bos <janjaap@bos.nl> wrote: >> On Tue, 2010-09-28 at 21:52 +0200, Geert Uytterhoeven wrote: >> On Tue, Sep 28, 2010 at 19:07, Janjaap Bos <janjaap@bos.nl> wrote: >> > See attached patch, and earlier message posted in March 2010 on uml user >> > list. We are out of maintainer... >> >> Thanks for the patch! >> >> | --- a/arch/um/drivers/ubd_kern.c >> | +++ b/arch/um/drivers/ubd_kern.c >> | @@ -1223,7 +1227,7 @@ static void do_ubd_request(struct request_queue *q) >> | struct io_thread_req *io_req; >> | struct request *req; >> | sector_t sector; >> | - int n; >> | + int n, last_sectors; >> | >> | while(1){ >> | struct ubd *dev = q->queuedata; >> | @@ -1239,9 +1243,12 @@ static void do_ubd_request(struct request_queue *q) >> | >> | req = dev->request; >> | sector = blk_rq_pos(req); >> | + last_sectors = 0; >> | while(dev->start_sg < dev->end_sg){ >> | struct scatterlist *sg = &dev->sg[dev->start_sg]; >> | >> | + sector += last_sectors; >> | + last_sectors = 0; >> | io_req = kmalloc(sizeof(struct io_thread_req), >> | GFP_ATOMIC); >> | if(io_req == NULL){ >> | @@ -1253,7 +1260,7 @@ static void do_ubd_request(struct request_queue *q) >> | (unsigned long long)sector << 9, >> | sg->offset, sg->length, sg_page(sg)); >> | >> | - sector += sg->length >> 9; >> | + last_sectors = sg->length >> 9; >> | n = os_write_file(thread_fd, &io_req, >> | sizeof(struct io_thread_req *)); >> | if(n != sizeof(struct io_thread_req *)){ >> >> However, I'm wondering what difference this part makes? >> > > > It fixes ubd block handling integrity. > With large block operations errors occurred. Probably due to lost > request pointers as explained below. Need to keep a local count of > sectors and delay the update. Done by reverting commit > f81f2f7c9fee307e371f37424577d46f9eaf8692 using the present > block api. (At least which is what I intend, but perhaps quite likely I > am missing the point, also it may not be needed anymore if only single > 512 byte sector blocks are used per request. Anyway... it solves the > problem for me ;-) > > Regards, > -Janjaap > > > See: > > reverted: commit f81f2f7c9fee307e371f37424577d46f9eaf8692 > Author: Tejun Heo <tj@kernel.org> > Date: Tue Apr 28 13:06:10 2009 +0900 > ubd: drop unnecessary rq->sector manipulation > ubd curiously updates rq->sector while issuing the request > in multiple pieces. Don't do it and simply use local copy > of sector. > > See for original reason: > > commit 0a6d3a2a3813e7b25267366cfbf9a4a4698dd1c2 > Author: Jeff Dike <jdike@addtoit.com> > Date: Sun Jul 15 23:38:47 2007 -0700 > uml: fix request->sector update > > It is theoretically possible for a request to finish and be freed > between writing it to the I/O thread and updating the sector count. In > this case, the update will dereference a freed pointer. > To avoid this, I delay the update until processing the next sg > segment, when the request pointer is known to be good. > > modified: arch/um/drivers/ubd_kern.c This changeset from 2007 indeed moved the update of req->sector. However, in the new code, before or after applying your patch, there's no update of req->sector anymore. Everything is done in local variables. Is it possible that we only need the hunk below to fix the corruption? | diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c | index 5ff5546..655ed9e 100644 | --- a/arch/um/drivers/ubd_kern.c | +++ b/arch/um/drivers/ubd_kern.c | @@ -746,8 +746,12 @@ static int ubd_open_dev(struct ubd *ubd_dev) | } | ubd_dev->fd = fd; | | + /* A setting higher than 1 sector currently (v2.6.33) generates | + data loss, both for raw and cow ubd. */ | + blk_queue_max_sectors(ubd_dev->queue, 1 * sizeof(long)); | + blk_queue_max_phys_segments(ubd_dev->queue, 1 * sizeof(long)); | + | if(ubd_dev->cow.file != NULL){ | - blk_queue_max_sectors(ubd_dev->queue, 8 * sizeof(long)); | | err = -ENOMEM; | ubd_dev->cow.bitmap = vmalloc(ubd_dev->cow.bitmap_len); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [uml-devel] [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) 2010-09-28 20:37 ` Geert Uytterhoeven @ 2010-09-28 21:19 ` richard -rw- weinberger 2010-09-29 9:19 ` Geert Uytterhoeven 0 siblings, 1 reply; 7+ messages in thread From: richard -rw- weinberger @ 2010-09-28 21:19 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Janjaap Bos, tj, axboe, mike.miller, user-mode-linux-devel, adrian, jdike, LKML, grant.likely, Geert.Uytterhoeven, Chris Frey On Tue, Sep 28, 2010 at 10:37 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > This changeset from 2007 indeed moved the update of req->sector. > > However, in the new code, before or after applying your patch, there's no update > of req->sector anymore. Everything is done in local variables. > > Is it possible that we only need the hunk below to fix the corruption? Yes, it seems so. So far all my test cases work fine. I'll submit a patch in a few minutes. > | diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c > | index 5ff5546..655ed9e 100644 > | --- a/arch/um/drivers/ubd_kern.c > | +++ b/arch/um/drivers/ubd_kern.c > | @@ -746,8 +746,12 @@ static int ubd_open_dev(struct ubd *ubd_dev) > | } > | ubd_dev->fd = fd; > | > | + /* A setting higher than 1 sector currently (v2.6.33) generates > | + data loss, both for raw and cow ubd. */ > | + blk_queue_max_sectors(ubd_dev->queue, 1 * sizeof(long)); > | + blk_queue_max_phys_segments(ubd_dev->queue, 1 * sizeof(long)); > | + > | if(ubd_dev->cow.file != NULL){ > | - blk_queue_max_sectors(ubd_dev->queue, 8 * sizeof(long)); > | > | err = -ENOMEM; > | ubd_dev->cow.bitmap = vmalloc(ubd_dev->cow.bitmap_len); > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > -- Cheers, //richard ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [uml-devel] [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) 2010-09-28 21:19 ` richard -rw- weinberger @ 2010-09-29 9:19 ` Geert Uytterhoeven 0 siblings, 0 replies; 7+ messages in thread From: Geert Uytterhoeven @ 2010-09-29 9:19 UTC (permalink / raw) To: richard -rw- weinberger Cc: Janjaap Bos, tj, axboe, mike.miller, user-mode-linux-devel, adrian, jdike, LKML, grant.likely, Geert.Uytterhoeven, Chris Frey On Tue, Sep 28, 2010 at 23:19, richard -rw- weinberger <richard.weinberger@gmail.com> wrote: > On Tue, Sep 28, 2010 at 10:37 PM, Geert Uytterhoeven > <geert@linux-m68k.org> wrote: >> >> This changeset from 2007 indeed moved the update of req->sector. >> >> However, in the new code, before or after applying your patch, there's no update >> of req->sector anymore. Everything is done in local variables. >> >> Is it possible that we only need the hunk below to fix the corruption? > > Yes, it seems so. So far all my test cases work fine. > I'll submit a patch in a few minutes. Thanks, good to know! >> | diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c >> | index 5ff5546..655ed9e 100644 >> | --- a/arch/um/drivers/ubd_kern.c >> | +++ b/arch/um/drivers/ubd_kern.c >> | @@ -746,8 +746,12 @@ static int ubd_open_dev(struct ubd *ubd_dev) >> | } >> | ubd_dev->fd = fd; >> | >> | + /* A setting higher than 1 sector currently (v2.6.33) generates >> | + data loss, both for raw and cow ubd. */ >> | + blk_queue_max_sectors(ubd_dev->queue, 1 * sizeof(long)); >> | + blk_queue_max_phys_segments(ubd_dev->queue, 1 * sizeof(long)); >> | + >> | if(ubd_dev->cow.file != NULL){ >> | - blk_queue_max_sectors(ubd_dev->queue, 8 * sizeof(long)); >> | >> | err = -ENOMEM; >> | ubd_dev->cow.bitmap = vmalloc(ubd_dev->cow.bitmap_len); So we can no longer process more than one sector? Why? Could it be 4d6c84d91d1a539ebc47d1a36a35e9390ba11fdc (ubd: cleanup completion path), which is one of the three commits you pointed out? Before 4d6c84d91d1a539ebc47d1a36a35e9390ba11fdc, blk_end_request() was only called when rq->nr_sectors reached zero. Now it's called for each part. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-09-29 9:19 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-09-28 16:16 [REGRESSION] um: ubd: block layer issue (Was: ext3 filesystem corruption in user mode linux) richard -rw- weinberger 2010-09-28 17:07 ` [uml-devel] " Janjaap Bos 2010-09-28 19:52 ` Geert Uytterhoeven 2010-09-28 20:12 ` Janjaap Bos 2010-09-28 20:37 ` Geert Uytterhoeven 2010-09-28 21:19 ` richard -rw- weinberger 2010-09-29 9:19 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome