| Message ID | 577B9B99.90101@virtuozzo.com |
|---|---|
| State | Rejected |
| Series | "x86 Compatible C/R, part 2" |
| Headers | show |
diff --git a/criu/page-xfer.c b/criu/page-xfer.c index 8c2c799..967a587 100644 --- a/criu/page-xfer.c +++ b/criu/page-xfer.c @@ -370,7 +370,7 @@ int page_xfer_dump_pages(struct page_xfer *xfer, struct page_pipe *pp, struct iovec hole = get_iov(pp->holes, cur_hole, pp->compat_iov); - if (hole.iov_base < iov.iov_base) + if (hole.iov_base >= iov.iov_base) break; ret = page_xfer_dump_hole(xfer, &hole, off); if (ret)
On 07/05/2016 02:35 PM, Pavel Emelyanov wrote: > >>>> ========================== Run zdtm/static/env00 in h ========================== >>>> Start test >>>> ./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST >>>> Run criu pre-dump >>>> Run criu pre-dump >>>> Run criu dump >>>> Run criu restore >>>> =[log]=> dump/zdtm/static/env00/24/3/restore.log >>>> ------------------------ grep Error ------------------------ >>>> (00.035223) 24: Error (mem.c:826): Page entry address 400000 outside of VMA 7fffddbcf000-7fffddbd1000 >>>> (00.035323) Error (cr-restore.c:1885): Restoring FAILED. >>>> ------------------------ ERROR OVER ------------------------ >>>> ################# Test zdtm/static/env00 FAIL at CRIU restore ################## >>>> ##################################### FAIL ##################################### >>>> >>>> https://github.com/xemul/criu/issues/185 >>> >>> The pagemap image for test becomes unsorted. >> >> Yes, thanks, I have reproduced and also found image unsorted. >> Working on fix. > > > diff --git a/criu/page-xfer.c b/criu/page-xfer.c > index 8c2c799..967a587 100644 > --- a/criu/page-xfer.c > +++ b/criu/page-xfer.c > @@ -370,7 +370,7 @@ int page_xfer_dump_pages(struct page_xfer *xfer, struct page_pipe *pp, > struct iovec hole = get_iov(pp->holes, cur_hole, > pp->compat_iov); > > - if (hole.iov_base < iov.iov_base) > + if (hole.iov_base >= iov.iov_base) > break; > ret = page_xfer_dump_hole(xfer, &hole, off); > if (ret) > > Is it just like this? > Oh, right, I was starring at this and wondering, why it's skipped. Big thanks, sorry for this typo.
On 07/05/2016 02:35 PM, Pavel Emelyanov wrote: > >>>> ========================== Run zdtm/static/env00 in h ========================== >>>> Start test >>>> ./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST >>>> Run criu pre-dump >>>> Run criu pre-dump >>>> Run criu dump >>>> Run criu restore >>>> =[log]=> dump/zdtm/static/env00/24/3/restore.log >>>> ------------------------ grep Error ------------------------ >>>> (00.035223) 24: Error (mem.c:826): Page entry address 400000 outside of VMA 7fffddbcf000-7fffddbd1000 >>>> (00.035323) Error (cr-restore.c:1885): Restoring FAILED. >>>> ------------------------ ERROR OVER ------------------------ >>>> ################# Test zdtm/static/env00 FAIL at CRIU restore ################## >>>> ##################################### FAIL ##################################### >>>> >>>> https://github.com/xemul/criu/issues/185 >>> >>> The pagemap image for test becomes unsorted. >> >> Yes, thanks, I have reproduced and also found image unsorted. >> Working on fix. > > > diff --git a/criu/page-xfer.c b/criu/page-xfer.c > index 8c2c799..967a587 100644 > --- a/criu/page-xfer.c > +++ b/criu/page-xfer.c > @@ -370,7 +370,7 @@ int page_xfer_dump_pages(struct page_xfer *xfer, struct page_pipe *pp, > struct iovec hole = get_iov(pp->holes, cur_hole, > pp->compat_iov); > > - if (hole.iov_base < iov.iov_base) > + if (hole.iov_base >= iov.iov_base) > break; > ret = page_xfer_dump_hole(xfer, &hole, off); > if (ret) > > Is it just like this? And while we on it, don't you mind, if I change: static struct iovec get_iov(...) to this: static struct iovec *get_iov(...) And introduce put_iov(struct iovec *, bool compat) to eleminate needless copy here. On the moment of patch I wanted to return pointer to static iovec_compat, but found the need in reentrancy, so left coping here, but it's pretty annoing.
On 07/05/2016 02:47 PM, Dmitry Safonov wrote: > On 07/05/2016 02:35 PM, Pavel Emelyanov wrote: >> >>>>> ========================== Run zdtm/static/env00 in h >>>>> ========================== >>>>> Start test >>>>> ./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST >>>>> Run criu pre-dump >>>>> Run criu pre-dump >>>>> Run criu dump >>>>> Run criu restore >>>>> =[log]=> dump/zdtm/static/env00/24/3/restore.log >>>>> ------------------------ grep Error ------------------------ >>>>> (00.035223) 24: Error (mem.c:826): Page entry address 400000 >>>>> outside of VMA 7fffddbcf000-7fffddbd1000 >>>>> (00.035323) Error (cr-restore.c:1885): Restoring FAILED. >>>>> ------------------------ ERROR OVER ------------------------ >>>>> ################# Test zdtm/static/env00 FAIL at CRIU restore >>>>> ################## >>>>> ##################################### FAIL >>>>> ##################################### >>>>> >>>>> https://github.com/xemul/criu/issues/185 >>>> >>>> The pagemap image for test becomes unsorted. >>> >>> Yes, thanks, I have reproduced and also found image unsorted. >>> Working on fix. >> >> >> diff --git a/criu/page-xfer.c b/criu/page-xfer.c >> index 8c2c799..967a587 100644 >> --- a/criu/page-xfer.c >> +++ b/criu/page-xfer.c >> @@ -370,7 +370,7 @@ int page_xfer_dump_pages(struct page_xfer *xfer, >> struct page_pipe *pp, >> struct iovec hole = get_iov(pp->holes, >> cur_hole, >> pp->compat_iov); >> >> - if (hole.iov_base < iov.iov_base) >> + if (hole.iov_base >= iov.iov_base) >> break; >> ret = page_xfer_dump_hole(xfer, &hole, >> off); >> if (ret) >> >> Is it just like this? > > > And while we on it, don't you mind, if I change: > static struct iovec get_iov(...) > to this: > static struct iovec *get_iov(...) > > And introduce put_iov(struct iovec *, bool compat) > to eleminate needless copy here. > On the moment of patch I wanted to return pointer to static > iovec_compat, but found the need in reentrancy, so left > coping here, but it's pretty annoing. Or maybe it's not worth? Just coping two pointers/numbers and additional function call is quite the same.
On 07/05/2016 02:47 PM, Dmitry Safonov wrote: > On 07/05/2016 02:35 PM, Pavel Emelyanov wrote: >> >>>>> ========================== Run zdtm/static/env00 in h ========================== >>>>> Start test >>>>> ./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST >>>>> Run criu pre-dump >>>>> Run criu pre-dump >>>>> Run criu dump >>>>> Run criu restore >>>>> =[log]=> dump/zdtm/static/env00/24/3/restore.log >>>>> ------------------------ grep Error ------------------------ >>>>> (00.035223) 24: Error (mem.c:826): Page entry address 400000 outside of VMA 7fffddbcf000-7fffddbd1000 >>>>> (00.035323) Error (cr-restore.c:1885): Restoring FAILED. >>>>> ------------------------ ERROR OVER ------------------------ >>>>> ################# Test zdtm/static/env00 FAIL at CRIU restore ################## >>>>> ##################################### FAIL ##################################### >>>>> >>>>> https://github.com/xemul/criu/issues/185 >>>> >>>> The pagemap image for test becomes unsorted. >>> >>> Yes, thanks, I have reproduced and also found image unsorted. >>> Working on fix. >> >> >> diff --git a/criu/page-xfer.c b/criu/page-xfer.c >> index 8c2c799..967a587 100644 >> --- a/criu/page-xfer.c >> +++ b/criu/page-xfer.c >> @@ -370,7 +370,7 @@ int page_xfer_dump_pages(struct page_xfer *xfer, struct page_pipe *pp, >> struct iovec hole = get_iov(pp->holes, cur_hole, >> pp->compat_iov); >> >> - if (hole.iov_base < iov.iov_base) >> + if (hole.iov_base >= iov.iov_base) >> break; >> ret = page_xfer_dump_hole(xfer, &hole, off); >> if (ret) >> >> Is it just like this? > > > And while we on it, don't you mind, if I change: > static struct iovec get_iov(...) > to this: > static struct iovec *get_iov(...) From my perspective if we want optimization like this we'd better do static void get_iov(struct iov *into, ...); ;) -- Pavel > And introduce put_iov(struct iovec *, bool compat) > to eleminate needless copy here. > On the moment of patch I wanted to return pointer to static > iovec_compat, but found the need in reentrancy, so left > coping here, but it's pretty annoing. > . >
On 07/05/2016 03:11 PM, Pavel Emelyanov wrote: > On 07/05/2016 02:47 PM, Dmitry Safonov wrote: >> And while we on it, don't you mind, if I change: >> static struct iovec get_iov(...) >> to this: >> static struct iovec *get_iov(...) > > From my perspective if we want optimization like this we'd better do > > static void get_iov(struct iov *into, ...); > > ;) > Yep, that's better. I'll make a patch, thanks again :)