<html>
  <head>
    <meta content="text/html; charset=koi8-r" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">05.08.2014 18:08, Andrew Vagin ΠΙΫΕΤ:<br>
    </div>
    <blockquote cite="mid:20140805150843.GA20414@paralelels.com"
      type="cite">
      <pre wrap="">On Tue, Aug 05, 2014 at 05:13:46PM +0300, Ruslan Kuprieiev wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="">We need this In order to fit signals into core.img. core.img is being
written
in dump_core_task_all,which is called right in the middle of dump_one_task,
when parasite code is already injected. We can't dump signals while
under parasite, because parasite has its own signals.
</pre>
      </blockquote>
      <pre wrap="">
No, it has not. </pre>
    </blockquote>
    <br>
    What do you mean?<br>
    <br>
    <blockquote cite="mid:20140805150843.GA20414@paralelels.com"
      type="cite">
      <pre wrap="">Here is another problem.
Signals can be dumped only for stopped tasks.

Look at the folling code:
if (SI_EVENT(si.si_code) != PTRACE_EVENT_STOP) {
        /*
         * Kernel notifies us about the task being seized
         * received some
         * event other than the STOP, i.e. -- a signal. Let the
         * task
         * handle one and repeat.
         */

        if (ptrace(PTRACE_CONT, pid, NULL,
                                (void *)(unsigned long)si.si_signo)) {
                pr_perror("Can't continue signal handling, aborting");
                goto err;
        }

        goto try_again;
}

The comments says that we exepect siganls here.
</pre>
    </blockquote>
    <br>
    Yes, but seize_task is called in collect_pstree, which is called far
    before dumping.<br>
    <meta http-equiv="content-type" content="text/html; charset=koi8-r">
    cr-dump.c : cr_dump_tasks()<br>
    ššš /*<br>
    ššš š* The collect_pstree will also stop (PTRACE_SEIZE) the tasks<br>
    ššš š* thus ensuring that they don't modify anything we collect<br>
    ššš š* afterwards.<br>
    ššš š*/<br>
    <br>
    ššš if (collect_pstree(pid))<br>
    ššš ššš goto err;<br>
    <br>
    š...........................................................<br>
    <br>
    ššš for_each_pstree_item(item) {<br>
    ššš ššš if (dump_one_task(item))<br>
    ššš ššš ššš goto err;<br>
    ššš }<br>
    <br>
    The comment says that the whole tree is stopped after
    collect_pstree, so it's safe to dump signals.<br>
    <br>
    <blockquote cite="mid:20140805150843.GA20414@paralelels.com"
      type="cite">
      <blockquote type="cite">
        <pre wrap="">
This patch doesn't change anything about dumping singnals, because
in both cases task is seized and doesn't have parasite injected.

I've tested it by running sigpending from zdtm and it seems to work well.
</pre>
      </blockquote>
      <pre wrap="">
It doesn't test a race condition. Let's imagine that we have two tasks.

1. criu freezes the task 1
2. task 2 sends signal to the task 1
3. criu infects the task 1

Could you create this test?</pre>
    </blockquote>
    <pre wrap="">
Well, race conditions are hard to test =).

</pre>
    <blockquote cite="mid:20140805150843.GA20414@paralelels.com"
      type="cite">
      <blockquote type="cite">
        <pre wrap="">
05.08.2014 16:56, Andrew Vagin ΠΙΫΕΤ:
</pre>
        <blockquote type="cite">
          <pre wrap="">Why do we need this?

If a tack has a few pending signals, will it be safe?

On Tue, Aug 05, 2014 at 10:35:59AM +0200, Ruslan Kuprieiev wrote:
</pre>
          <blockquote type="cite">
            <pre wrap="">Signed-off-by: Ruslan Kuprieiev <a class="moz-txt-link-rfc2396E" href="mailto:kupruser@gmail.com">&lt;kupruser@gmail.com&gt;</a>
---
 cr-dump.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/cr-dump.c b/cr-dump.c
index d8ad0fc..c8be0bd 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -1498,6 +1498,12 @@ static int dump_one_task(struct pstree_item *item)
                 goto err;
         }
+        ret = dump_task_signals(pid, item, cr_fdset);
+        if (ret) {
+                pr_err("Dump %d signals failed %d\n", pid, ret);
+                goto err;
+        }
+
         ret = -1;
         parasite_ctl = parasite_infect_seized(pid, item, &amp;vmas, dfds, proc_args.timer_n);
         if (!parasite_ctl) {
@@ -1632,12 +1638,6 @@ static int dump_one_task(struct pstree_item *item)
                 goto err;
         }
-        ret = dump_task_signals(pid, item, cr_fdset);
-        if (ret) {
-                pr_err("Dump %d signals failed %d\n", pid, ret);
-                goto err;
-        }
-
         close_cr_fdset(&amp;cr_fdset);
 err:
         close_pid_proc();
-- 
1.9.1

_______________________________________________
CRIU mailing list
<a class="moz-txt-link-abbreviated" href="mailto:CRIU@openvz.org">CRIU@openvz.org</a>
<a class="moz-txt-link-freetext" href="https://lists.openvz.org/mailman/listinfo/criu">https://lists.openvz.org/mailman/listinfo/criu</a>
</pre>
          </blockquote>
        </blockquote>
        <pre wrap="">
</pre>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>