OpenVZ Forum - RDF feed
type: channel
items: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
items: Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes

[RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: >From fd13986de32af31621b1badbcf7bfb5626648e0e Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> Date: Mon, 16 Jun 2008 18:41:05 -0700 Subject: [PATCH] Save/restore state of unnamed pipes Design: Current Linux kernels provide ability to read/write contents of FIFOs using /proc. i.e 'cat /proc/pid/fd/read-side-fd' prints the unread data in the FIFO. Similarly, 'cat foo > /proc/pid/fd/read-sid-fd' appends the contents of 'foo' to the unread contents of the FIFO. So to save/restore the state of the pipe, a simple implementation is to read the from the unnamed pipe's fd and save to the checkpoint-file. When restoring, create a pipe (using PT_PIPE()) in the child process, read the contents of the pipe from the checkpoint file and write it to the newly created pipe. Its fairly straightforward, except for couple of notes: - when we read contents of '/proc/pid/fd/read-side-fd' we drain the pipe such that when the checkpointed application resumes, it will not find any data. To fix this, we read from the 'read-side-fd' and write it back to the 'read-side-fd' in addition to writing to the checkpoint file. - there does not seem to be a mechanism to determine the count of unread bytes in the file. Current implmentation assumes a maximum of 64K bytes (PIPE_BUFS * PAGE_SIZE on i386) and fails if the pipe is not fully drained. Basic unit-testing done at this point (using tests/pipe.c). TODO: - Additional testing (with multiple-processes and multiple-pipes) - Named-pipes Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- cr.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 203 insertions(+), 12 deletions(-) diff --git a/cr.c b/cr.c index 5163a3d..0cb9774 100644 --- a/cr.c +++ b/cr.c @@ -84,6 +84,11 @@ typedef struct fdinfo_t { char name[128]; /* file name. NULL if anonymous (pipe, socketpair) */ } fdinfo_t; +typedef struct fifoinfo_t { + int fi_fd; /* fifo's read-side fd */ + int fi_length; /* number of…
creator: Sukadev Bhattiprolu
date: 2008-06-17T21:29:50-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): > > >From fd13986de32af31621b1badbcf7bfb5626648e0e Mon Sep 17 00:00:00 2001 > From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > Date: Mon, 16 Jun 2008 18:41:05 -0700 > Subject: [PATCH] Save/restore state of unnamed pipes > > Design: > > Current Linux kernels provide ability to read/write contents of FIFOs > using /proc. i.e 'cat /proc/pid/fd/read-side-fd' prints the unread data > in the FIFO. Similarly, 'cat foo > /proc/pid/fd/read-sid-fd' appends > the contents of 'foo' to the unread contents of the FIFO. > > So to save/restore the state of the pipe, a simple implementation is > to read the from the unnamed pipe's fd and save to the checkpoint-file. > When restoring, create a pipe (using PT_PIPE()) in the child process, > read the contents of the pipe from the checkpoint file and write it to > the newly created pipe. > > Its fairly straightforward, except for couple of notes: > > - when we read contents of '/proc/pid/fd/read-side-fd' we drain > the pipe such that when the checkpointed application resumes, > it will not find any data. To fix this, we read from the > 'read-side-fd' and write it back to the 'read-side-fd' in > addition to writing to the checkpoint file. > > - there does not seem to be a mechanism to determine the count > of unread bytes in the file. Current implmentation assumes a > maximum of 64K bytes (PIPE_BUFS * PAGE_SIZE on i386) and fails > if the pipe is not fully drained. > > Basic unit-testing done at this point (using tests/pipe.c). > > TODO: > - Additional testing (with multiple-processes and multiple-pipes) > - Named-pipes > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > cr.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 files changed, 203 insertions(+), 12 deletions(-) > > diff --git a/cr.c b/cr.c > index 5163a3d..0cb9774 100644 > --- a/cr.c > +++ b/cr.c > @@ -84,6 +84,11 @@ typedef struct fdinfo_t { > char name[128]; /* file name. NU…
creator: serue
date: 2008-06-17T22:30:39-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: Serge E. Hallyn [serue@us.ibm.com] wrote: <snip> | > + | > + rc = read(fifo_fd, &c, 1); | > + if (rc != -1 && errno != EAGAIN) { | | Won't errno only be set if rc == -1? Did you mean || here? Yes I meant ||. I also had 'errno = 0' before the read, but seem to have deleted it when I moved code around. <snip> | > + } else if (S_ISFIFO(fdinfo->mode)) { | > + int pipefds[2] = { 0, 0 }; | > + | > + /* | > + * We create the pipe when we see the pipe's read-fd. | > + * Just ignore the pipe's write-fd. | > + */ | > + if (fdinfo->flag == O_WRONLY) | > + continue; | > + | > + DEBUG("Creating pipe for fd %d\n", fdinfo->fdnum); | > + | > + t_d(PT_PIPE(pid, pipefds)); | > + t_d(pipefds[0]); | > + t_d(pipefds[1]); | > + | > + if (pipefds[0] != fdinfo->fdnum) { | > + DEBUG("Hmm, new pipe has fds %d, %d " | > + "Old pipe had fd %d\n", pipefds[0], | > + pipefds[1], fdinfo->fdnum); getchar(); | | Can you explain what you're doing here? I would have expected you to | dup2() to get back the correct fd, so maybe I'm missing something... You are right, I should use dup2() here. Will send an updated patch. Thanks, Suka _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Sukadev Bhattiprolu
date: 2008-06-17T22:55:57-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: On Tue, 2008-06-17 at 17:30 -0500, Serge E. Hallyn wrote: > Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): > > > > >From fd13986de32af31621b1badbcf7bfb5626648e0e Mon Sep 17 00:00:00 2001 > > From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > > Date: Mon, 16 Jun 2008 18:41:05 -0700 > > Subject: [PATCH] Save/restore state of unnamed pipes > > > > Design: > > > > Current Linux kernels provide ability to read/write contents of FIFOs > > using /proc. i.e 'cat /proc/pid/fd/read-side-fd' prints the unread data > > in the FIFO. Similarly, 'cat foo > /proc/pid/fd/read-sid-fd' appends > > the contents of 'foo' to the unread contents of the FIFO. > > > > So to save/restore the state of the pipe, a simple implementation is > > to read the from the unnamed pipe's fd and save to the checkpoint-file. > > When restoring, create a pipe (using PT_PIPE()) in the child process, > > read the contents of the pipe from the checkpoint file and write it to > > the newly created pipe. > > > > Its fairly straightforward, except for couple of notes: > > > > - when we read contents of '/proc/pid/fd/read-side-fd' we drain > > the pipe such that when the checkpointed application resumes, > > it will not find any data. To fix this, we read from the > > 'read-side-fd' and write it back to the 'read-side-fd' in > > addition to writing to the checkpoint file. > > > > - there does not seem to be a mechanism to determine the count > > of unread bytes in the file. Current implmentation assumes a > > maximum of 64K bytes (PIPE_BUFS * PAGE_SIZE on i386) and fails > > if the pipe is not fully drained. > > > > Basic unit-testing done at this point (using tests/pipe.c). > > > > TODO: > > - Additional testing (with multiple-processes and multiple-pipes) > > - Named-pipes > > > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > > --- > > cr.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- > > 1 files changed, 203 insertions(+), 12 deletions(-) > > > > diff --git a/cr.…
creator: Matt Helsley
date: 2008-06-17T23:31:12-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: Matt Helsley [matthltc@us.ibm.com] wrote: | | On Tue, 2008-06-17 at 17:30 -0500, Serge E. Hallyn wrote: | > Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): | > > | > > >From fd13986de32af31621b1badbcf7bfb5626648e0e Mon Sep 17 00:00:00 2001 | > > From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> | > > Date: Mon, 16 Jun 2008 18:41:05 -0700 | > > Subject: [PATCH] Save/restore state of unnamed pipes | > > | > > Design: | > > | > > Current Linux kernels provide ability to read/write contents of FIFOs | > > using /proc. i.e 'cat /proc/pid/fd/read-side-fd' prints the unread data | > > in the FIFO. Similarly, 'cat foo > /proc/pid/fd/read-sid-fd' appends | > > the contents of 'foo' to the unread contents of the FIFO. | > > | > > So to save/restore the state of the pipe, a simple implementation is | > > to read the from the unnamed pipe's fd and save to the checkpoint-file. | > > When restoring, create a pipe (using PT_PIPE()) in the child process, | > > read the contents of the pipe from the checkpoint file and write it to | > > the newly created pipe. | > > | > > Its fairly straightforward, except for couple of notes: | > > | > > - when we read contents of '/proc/pid/fd/read-side-fd' we drain | > > the pipe such that when the checkpointed application resumes, | > > it will not find any data. To fix this, we read from the | > > 'read-side-fd' and write it back to the 'read-side-fd' in | > > addition to writing to the checkpoint file. | > > | > > - there does not seem to be a mechanism to determine the count | > > of unread bytes in the file. Current implmentation assumes a | > > maximum of 64K bytes (PIPE_BUFS * PAGE_SIZE on i386) and fails | > > if the pipe is not fully drained. | > > | > > Basic unit-testing done at this point (using tests/pipe.c). | > > | > > TODO: | > > - Additional testing (with multiple-processes and multiple-pipes) | > > - Named-pipes | > > | > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> | > > --- | > > cr.c | 215 +++++++++++++++++…
creator: Sukadev Bhattiprolu
date: 2008-06-18T00:32:14-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: On Tue, 2008-06-17 at 17:32 -0700, sukadev@us.ibm.com wrote: > Matt Helsley [matthltc@us.ibm.com] wrote: > | > | On Tue, 2008-06-17 at 17:30 -0500, Serge E. Hallyn wrote: > | > Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): > | > > > | > > >From fd13986de32af31621b1badbcf7bfb5626648e0e Mon Sep 17 00:00:00 2001 > | > > From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > | > > Date: Mon, 16 Jun 2008 18:41:05 -0700 > | > > Subject: [PATCH] Save/restore state of unnamed pipes > | > > > | > > Design: > | > > > | > > Current Linux kernels provide ability to read/write contents of FIFOs > | > > using /proc. i.e 'cat /proc/pid/fd/read-side-fd' prints the unread data > | > > in the FIFO. Similarly, 'cat foo > /proc/pid/fd/read-sid-fd' appends > | > > the contents of 'foo' to the unread contents of the FIFO. > | > > > | > > So to save/restore the state of the pipe, a simple implementation is > | > > to read the from the unnamed pipe's fd and save to the checkpoint-file. > | > > When restoring, create a pipe (using PT_PIPE()) in the child process, > | > > read the contents of the pipe from the checkpoint file and write it to > | > > the newly created pipe. > | > > > | > > Its fairly straightforward, except for couple of notes: > | > > > | > > - when we read contents of '/proc/pid/fd/read-side-fd' we drain > | > > the pipe such that when the checkpointed application resumes, > | > > it will not find any data. To fix this, we read from the > | > > 'read-side-fd' and write it back to the 'read-side-fd' in > | > > addition to writing to the checkpoint file. > | > > > | > > - there does not seem to be a mechanism to determine the count > | > > of unread bytes in the file. Current implmentation assumes a > | > > maximum of 64K bytes (PIPE_BUFS * PAGE_SIZE on i386) and fails > | > > if the pipe is not fully drained. > | > > > | > > Basic unit-testing done at this point (using tests/pipe.c). > | > > > | > > TODO: > | > > - Additional testing (with multiple-processes and multipl…
creator: Matt Helsley
date: 2008-06-18T02:04:06-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: Matt Helsley [matthltc@us.ibm.com] wrote: | > | | > | | > | pipe(pipefds); /* returns 5 and 4 in elements 0 and 1 */ | > | /* use fds after last_fd as trampolines for fds we want to create */ | > | dup2(pipefds[0], last_fd + 1); | > | dup2(pipefds[1], last_fd + 2); | > | close(pipefds[0]); | > | close(pipefds[1]); | > | dup2(last_fd + 1, <orig pipefd[0]>); | > | dup2(last_fd + 2, <orig pipefd[1]>); | > | close(last_fd + 1); | > | close(last_fd + 2); | > | | > | | > | Which is alot more code but should work no matter which fds we get back | > | from pipe(). Of course this assumes the checkpointed application hasn't | > | used all of its fds. :( | > | | > | > This sounds like a good idea too, but we could use any fd that has not | > yet been used in the restart-process right ? It would break if all fds | | Yes, but we don't know which fd is available unless we allocate it | without dup2(). Here's how it could be done without last_fd (again, | dropping PT_FUNC notation): | | /* | * Move fds from src to dest. Useful for correctly "moving" pipe fds and | * other cases where we have a small number of fds to move to their | * original fd. | * | * Assumes dest_fds and src_fds are of the same, small length since | * this is O(num_fds^2). | * | * If num_fds == 1 then use plain dup2(). | * | * Use this in place of multiple dup2() calls (num_fds > 1) unless you are | * absolutely certain the set of dest fds do not intersect the set of src fds. | * Does NOT magically prevent you from accidentally clobbering fds outside the | * src_fds array. | */ | void move_fds(int *dest_fds, int *src_fds, const unsigned int num_fds) | { | int i; | unsigned int num_moved = 0; | | for (i = 0; i < num_fds; i++) { | int j; | | if (src_fds[i] == dest_fds[i]) | continue; /* nothing to be done */ | | /* src fd != dest fd so we need to perform: | dup2(src fd, dest fd); | but dup2() closes dest fd if it already exists. | This means we could accidentally close one of | the src fds. Avoid this by searchi…
creator: Sukadev Bhattiprolu
date: 2008-06-18T18:00:25-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: On Wed, 2008-06-18 at 11:00 -0700, sukadev@us.ibm.com wrote: > Matt Helsley [matthltc@us.ibm.com] wrote: > > | > | > | > | > | > | pipe(pipefds); /* returns 5 and 4 in elements 0 and 1 */ > | > | /* use fds after last_fd as trampolines for fds we want to create */ > | > | dup2(pipefds[0], last_fd + 1); > | > | dup2(pipefds[1], last_fd + 2); > | > | close(pipefds[0]); > | > | close(pipefds[1]); > | > | dup2(last_fd + 1, <orig pipefd[0]>); > | > | dup2(last_fd + 2, <orig pipefd[1]>); > | > | close(last_fd + 1); > | > | close(last_fd + 2); > | > | > | > | > | > | Which is alot more code but should work no matter which fds we get back > | > | from pipe(). Of course this assumes the checkpointed application hasn't > | > | used all of its fds. :( > | > | > | > > | > This sounds like a good idea too, but we could use any fd that has not > | > yet been used in the restart-process right ? It would break if all fds > | > | Yes, but we don't know which fd is available unless we allocate it > | without dup2(). Here's how it could be done without last_fd (again, > | dropping PT_FUNC notation): > | > | /* > | * Move fds from src to dest. Useful for correctly "moving" pipe fds and > | * other cases where we have a small number of fds to move to their > | * original fd. > | * > | * Assumes dest_fds and src_fds are of the same, small length since > | * this is O(num_fds^2). > | * > | * If num_fds == 1 then use plain dup2(). > | * > | * Use this in place of multiple dup2() calls (num_fds > 1) unless you are > | * absolutely certain the set of dest fds do not intersect the set of src fds. > | * Does NOT magically prevent you from accidentally clobbering fds outside the > | * src_fds array. > | */ > | void move_fds(int *dest_fds, int *src_fds, const unsigned int num_fds) > | { > | int i; > | unsigned int num_moved = 0; > | > | for (i = 0; i < num_fds; i++) { > | int j; > | > | if (src_fds[i] == dest_fds[i]) > | continue; /* nothing to be done */ > | > | /* src fd != dest fd so we need …
creator: Matt Helsley
date: 2008-06-18T19:57:35-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: Matt Helsley [matthltc@us.ibm.com] wrote: | | > So let me rephrase the problem. | > | > Suppose the checkpointed application was using fds in following | > "orig-fd-set" | > | > { [0..10], 18, 27 } | > | > where 18 and 27 are part of a pipe. For simplicity lets assume that | > 18 is the read-side-fd. | | so orig_pipefd[0] == 18 | and orig_pipefd[1] == 27 | | > We checkpointed this application and are now trying to restart it. | > | > In the restarted application, we would call | > | > dup2(fd1, fd2), | > | > where 'fd1' is some new, random fd and 'fd2' is an fd in 'orig-fd-set' | ^^^^^^ Even if they were truly random, this | does not preclude fd1 from having the same value as an fd in the | remaining orig-fd-set -- such as one of the two we're about to try and | restart with pipe(). I agree. fd1 could be an hither-to-unseen fd from the 'orig-fd-set'. | | > (say fd2 = 18). | | fd1 = restarted_pipefd[0] | fd2 = restarted_pipefd[1] | | In my example fd1 == 27 and fd2 == 18 | | > IIUC, there is a risk here of 'fd2' being closed accidentally while | > it is in use. | | Yes, that's the risk. | | > But, the only way I can see 'fd2' being in use in the restarted process | > is if _cryo_ opened some file _during_ restart and did not close. I ran | | Both file descriptors returned from pipe() are in use during restart | and closing one of them would not be proper. Cryo hasn't "forgotten" to | close one of them -- cryo needs to dup2() both of them to their | "destination" fds. But if they have been swapped or if just one is the | "destination" of the other then you could end up with a broken pipe. Ok I see what you are saying. The assumption I have is that we would process the fds from 'orig-fd-set' in ascending order. Its good to confirm that assumption now :-) proc_readfd_common() seems to return the fds in ascending order (so readdir() of "/proc/pid/fd/" would get them in ascending order - no ?) If we process 'orig-fd-set' in order and suppose we create the pipe for the sma…
creator: Sukadev Bhattiprolu
date: 2008-06-18T21:56:12-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: On Wed, 2008-06-18 at 14:56 -0700, sukadev@us.ibm.com wrote: > Matt Helsley [matthltc@us.ibm.com] wrote: > | > | > So let me rephrase the problem. > | > > | > Suppose the checkpointed application was using fds in following > | > "orig-fd-set" > | > > | > { [0..10], 18, 27 } > | > > | > where 18 and 27 are part of a pipe. For simplicity lets assume that > | > 18 is the read-side-fd. > | > | so orig_pipefd[0] == 18 > | and orig_pipefd[1] == 27 > | > | > We checkpointed this application and are now trying to restart it. > | > > | > In the restarted application, we would call > | > > | > dup2(fd1, fd2), > | > > | > where 'fd1' is some new, random fd and 'fd2' is an fd in 'orig-fd-set' > | ^^^^^^ Even if they were truly random, this > | does not preclude fd1 from having the same value as an fd in the > | remaining orig-fd-set -- such as one of the two we're about to try and > | restart with pipe(). > > I agree. fd1 could be an hither-to-unseen fd from the 'orig-fd-set'. > > | > | > (say fd2 = 18). > | > | fd1 = restarted_pipefd[0] > | fd2 = restarted_pipefd[1] > | > | In my example fd1 == 27 and fd2 == 18 > | > | > IIUC, there is a risk here of 'fd2' being closed accidentally while > | > it is in use. > | > | Yes, that's the risk. > | > | > But, the only way I can see 'fd2' being in use in the restarted process > | > is if _cryo_ opened some file _during_ restart and did not close. I ran > | > | Both file descriptors returned from pipe() are in use during restart > | and closing one of them would not be proper. Cryo hasn't "forgotten" to > | close one of them -- cryo needs to dup2() both of them to their > | "destination" fds. But if they have been swapped or if just one is the > | "destination" of the other then you could end up with a broken pipe. > > Ok I see what you are saying. > > The assumption I have is that we would process the fds from 'orig-fd-set' > in ascending order. Its good to confirm that assumption now :-) OK, but wouldn't that violate the "pipes first"…
creator: Matt Helsley
date: 2008-06-18T22:38:09-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: | | Now restart does : | | int pipefds[2]; | | pipe(pipefds); /* | * kernel is allowed to return pipefds[0] == 12 and | * pipefds[1] == 11 | */ | | dup2(pipefds[0], 11); /* closes pipefds[1]! */ | dup2(pipefds[1], 12); Aah. I see it now (finally). Thanks, Suka _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Sukadev Bhattiprolu
date: 2008-06-18T22:55:55-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: Matt Helsley [matthltc@us.ibm.com] wrote: <snip> | | > | I don't see anything in the pipe man page, at least, that suggests we | > | can safely assume pipefds[0] < pipefds[1]. | > | | > | The solution could be to use "trampoline" fds. Suppose last_fd is the | > | largest fd that exists in the final checkpointed/restarting application. | > | We could do (Skipping the PT_FUNC "notation" for clarity): | > | > | | > | | > | pipe(pipefds); /* returns 5 and 4 in elements 0 and 1 */ | > | /* use fds after last_fd as trampolines for fds we want to create */ | > | dup2(pipefds[0], last_fd + 1); | > | dup2(pipefds[1], last_fd + 2); | > | close(pipefds[0]); | > | close(pipefds[1]); | > | dup2(last_fd + 1, <orig pipefd[0]>); | > | dup2(last_fd + 2, <orig pipefd[1]>); | > | close(last_fd + 1); | > | close(last_fd + 2); | > | | > | | > | Which is alot more code but should work no matter which fds we get back | > | from pipe(). Of course this assumes the checkpointed application hasn't | > | used all of its fds. :( | > | It appears that this last_fd approach will fit in easier with current design of cryo (where we process one or two fds at a time and don't have the src_fds and dest_fds handy). BTW, we should be able to accomplish the above with a single-unused fd right (i.e no need for last_fd+2) ? | > | > This sounds like a good idea too, but we could use any fd that has not | > yet been used in the restart-process right ? It would break if all fds | | Yes, but we don't know which fd is available unless we allocate it | without dup2(). Right. I was thinking we could find that out at the time of checkpoint (a brute-force fstat(i, &statbuf) for i = 0..n or something more efficient). Well just thought of another approach. Basically, we have a temporary need for an unused fd for use as a trampoline. So, why not 'set-aside' an fd for that purpose and after all other fds have been created, go back and create this fd ? i.e lets say the first regular file we open is associated with 'fd =…
creator: Sukadev Bhattiprolu
date: 2008-06-19T07:59:53-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: On Thu, 2008-06-19 at 00:59 -0700, sukadev@us.ibm.com wrote: > Matt Helsley [matthltc@us.ibm.com] wrote: > > <snip> > | > | > | I don't see anything in the pipe man page, at least, that suggests we > | > | can safely assume pipefds[0] < pipefds[1]. > | > | > | > | The solution could be to use "trampoline" fds. Suppose last_fd is the > | > | largest fd that exists in the final checkpointed/restarting application. > | > | We could do (Skipping the PT_FUNC "notation" for clarity): > | > > | > | > | > | > | > | pipe(pipefds); /* returns 5 and 4 in elements 0 and 1 */ > | > | /* use fds after last_fd as trampolines for fds we want to create */ > | > | dup2(pipefds[0], last_fd + 1); > | > | dup2(pipefds[1], last_fd + 2); > | > | close(pipefds[0]); > | > | close(pipefds[1]); > | > | dup2(last_fd + 1, <orig pipefd[0]>); > | > | dup2(last_fd + 2, <orig pipefd[1]>); > | > | close(last_fd + 1); > | > | close(last_fd + 2); > | > | > | > | > | > | Which is alot more code but should work no matter which fds we get back > | > | from pipe(). Of course this assumes the checkpointed application hasn't > | > | used all of its fds. :( > | > | > > It appears that this last_fd approach will fit in easier with current > design of cryo (where we process one or two fds at a time and don't have > the src_fds and dest_fds handy). > > BTW, we should be able to accomplish the above with a single-unused fd > right (i.e no need for last_fd+2) ? Yes, I think that's sufficient: int pipefds[2]; ... restarted_read_fd = 11; restarted_write_fd = 12; ... pipe(pipefds); /* * pipe() may have returned one (or both) of the restarted fds * at the wrong end of the pipe. This could cause dup2() to * accidentaly close the pipe. Avoid that with an extra dup(). */ if (pipefds[1] == restarted_read_fd) { dup2(pipefds[1], last_fd + 1); pipefds[1] = last_fd + 1; } if (pipefds[0] != restarted_read_fd) { dup2(pipefds[0], restarted_read_fd); close(pipefds[0]); } if (pipefds[0] != restarted_read_fd) { dup2(pipefds[1], re…
creator: Matt Helsley
date: 2008-06-19T23:46:00-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: Matt Helsley [matthltc@us.ibm.com] wrote: | | Yes, I think that's sufficient: | | int pipefds[2]; | | ... | | restarted_read_fd = 11; | restarted_write_fd = 12; | | ... | | pipe(pipefds); | | /* | * pipe() may have returned one (or both) of the restarted fds | * at the wrong end of the pipe. This could cause dup2() to | * accidentaly close the pipe. Avoid that with an extra dup(). | */ | if (pipefds[1] == restarted_read_fd) { | dup2(pipefds[1], last_fd + 1); | pipefds[1] = last_fd + 1; | } | | if (pipefds[0] != restarted_read_fd) { | dup2(pipefds[0], restarted_read_fd); | close(pipefds[0]); | } | | if (pipefds[0] != restarted_read_fd) { | dup2(pipefds[1], restarted_write_fd); | close(pipefds[1]); | } Shouldn't the last if be if (pipefds[1] != restarted_wrte_fd) ? (otherwise it would break if pipefds[0] = 11 and pipefds[1] = 200) I came up with something similar, but with an extra close(). And in my code, I had restarted_* names referring to pipefds[] making it a bit confusing initially. How about using actual_fds[] (instead of pipefds) and expected_fds[] instead of (restart_*) ? Thanks, Suka | | I think this code does the minimal number of operations needed in the | restarted application too -- it counts on the second dup2() closing one | of the fds if pipefds[1] == restarted_read_fd. | | Cheers, | -Matt _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Sukadev Bhattiprolu
date: 2008-06-20T01:54:10-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: On Thu, 2008-06-19 at 18:54 -0700, sukadev@us.ibm.com wrote: > Matt Helsley [matthltc@us.ibm.com] wrote: <snip> > | if (pipefds[0] != restarted_read_fd) { > | dup2(pipefds[1], restarted_write_fd); > | close(pipefds[1]); > | } > > Shouldn't the last if be > > if (pipefds[1] != restarted_wrte_fd) ? > > (otherwise it would break if pipefds[0] = 11 and pipefds[1] = 200) Argh, copy-paste error. You are correct. > I came up with something similar, but with an extra close(). And > in my code, I had restarted_* names referring to pipefds[] making > it a bit confusing initially. > > How about using actual_fds[] (instead of pipefds) and expected_fds[] > instead of (restart_*) ? I like actual_fds[] instead of pipefds[] but still prefer the restart_* names over expected_fds[]. Cheers, -Matt _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Matt Helsley
date: 2008-06-20T02:48:19-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: sukadev@us.ibm.com wrote: > From fd13986de32af31621b1badbcf7bfb5626648e0e Mon Sep 17 00:00:00 2001 > From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > Date: Mon, 16 Jun 2008 18:41:05 -0700 > Subject: [PATCH] Save/restore state of unnamed pipes > > Design: > > Current Linux kernels provide ability to read/write contents of FIFOs > using /proc. i.e 'cat /proc/pid/fd/read-side-fd' prints the unread data > in the FIFO. Similarly, 'cat foo > /proc/pid/fd/read-sid-fd' appends > the contents of 'foo' to the unread contents of the FIFO. > > So to save/restore the state of the pipe, a simple implementation is > to read the from the unnamed pipe's fd and save to the checkpoint-file. > When restoring, create a pipe (using PT_PIPE()) in the child process, > read the contents of the pipe from the checkpoint file and write it to > the newly created pipe. > > Its fairly straightforward, except for couple of notes: > > - when we read contents of '/proc/pid/fd/read-side-fd' we drain > the pipe such that when the checkpointed application resumes, > it will not find any data. To fix this, we read from the > 'read-side-fd' and write it back to the 'read-side-fd' in > addition to writing to the checkpoint file. One issue with this approach is that the operation (checkpoint) becomes non-transparent to the "checkpointee"s: re-writing the data back into the pipe/fifo may generate a SIGIO at the read-side of the pipe/fifo, or an inotify event on the inode, without a real reason. This may confuse the application if its logic relies on "normal" behavior. > > - there does not seem to be a mechanism to determine the count > of unread bytes in the file. Current implmentation assumes a > maximum of 64K bytes (PIPE_BUFS * PAGE_SIZE on i386) and fails > if the pipe is not fully drained. > > Basic unit-testing done at this point (using tests/pipe.c). > > TODO: > - Additional testing (with multiple-processes and multiple-pipes) > - Named-pipes > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us…
creator: Oren Laadan
date: 2008-06-22T21:40:52-00:00

Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes
type: item
description: On Sun, 2008-06-22 at 17:40 -0400, Oren Laadan wrote: > One issue with this approach is that the operation (checkpoint) becomes > non-transparent to the "checkpointee"s: re-writing the data back into > the pipe/fifo may generate a SIGIO at the read-side of the pipe/fifo, or > an inotify event on the inode, without a real reason. This may confuse > the application if its logic relies on "normal" behavior. Yeah, I guess this is true, somewhat. But, for something expecting to read from a pipe, they'll get a SIGIO when the data is first written. If the signal has not yet been delivered then we're fine because we won't generate *another* one. If they've already received the signal and left the data in the pipe, what harm does another SIGIO do? The kernel is free to give the app a SIGIO for every single byte stuck in the pipe, right? So, what difference does any number of SIGIOs make when there's data sitting in the pipe unread? As for inotify, yeah that might be an actual issue. It's probably good to note this potential issue, and to watch for anyone doing inotifies on a pipe fd when we get to checkpointing inotify. Overall, seems like an awfully minor issue to me. -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2008-06-23T23:30:28-00:00