Re: Patch to Pistachio's comportement on Xfer timeouts

[racin]

Marcus Brinkmann <address@hidden> writes:

> At Fri, 21 Jan 2005 16:19:53 +0100,
> Espen Skoglund <address@hidden> wrote:
>> Er... so what you're saying here is that a thread doing an IPC (send
>> or receive) can not decide on the timeouts for page-faults within its
>> own address space.  If a page-fault occurs within its own space, the
>> thread *must* rely on its pager to provide a valid mapping within a
>> given time period (since it can not trust the IPC partner to have set
>> a proper timeout value).  Do you seriously consider this to be a good
>> idea?
>
> The pager can also generate an exception and cancel the pending IPC,
> which is what will happen if a valid mapping can not be generated.
>
>> What Marcus says might be right for his particular usage case.  In
>> general, however, the server may very well have a great interest in
>> IPC pagefaults within its own space.
>
> This is why I suggested two options to Matthieu: The one Matthieu
> actually implemented, and a different one, which I also proposed on
> the l4ka mailing list some time ago without receiving much response:
>
> The alternative is to have four xfer timeouts: local pf send phase,
> local pf recv phase, remote pf send phase, remote pf recv phase.  This
> requires a second xfer timeout word for every IPC.
>
> The L4.X2 specs treat local and remote pagefaults during string item
> transfers identical.  I think in general, threads do have a great
> interest in making a distinction and handle local pagefaults and
> remote pagefaults differently.  This is impossible with the current L4
> design: It does not allow for this flexibility.
>
> Are you interested in a patch that adds another xfer timeout word and
> allows to make such a distinction?
>

Here is a second patch that implements the second solution.
It isn't complete nor tested yet: it lacks support for other
architectures than ia32.

For compatibility, I suggest to change libl4 interfaces like this:
*set_xfer_timeouts should set both local and remote timeouts,
*set_local_xfer_timeouts and set_remote_xfer_timeouts would set the
respective timeouts.

We chose to put the local timeout word at the reserved1 word array on
ia32, so that it isn't too far from the remote timeout word, but we
can change if it is already used for something else.

Once we have decided where to put this new word in the UTCB, I can
make the changes to our libl4 implementation (and also a few tests)

>
> BTW, retrying the IPC is not an option, because the server can not
> wait until the client is rescheduled and has investigated the result
> and actually re-entered the receive phase.  So, the server could try
> to continue the reply, but it would be a matter of luck.  If the
> client is not ready, the message would have to be dropped, and the
> whole RPC would have to be retried---if that is possible at all for
> the type of operation in question.
>

It also enables the server to map the receive string items buffers in
memory only when needed, without security problems (a server cannot
accept to wait even a little time for the client because it could be
used for DoS attacks)

Thanks,
Matthieu

diff -Naurp pistachio-bak/kernel/include/api/v4/tcb.h 
pistachio-0.4/kernel/include/api/v4/tcb.h
--- pistachio-bak/kernel/include/api/v4/tcb.h   2005-01-23 14:12:14.000000000 
+0100
+++ pistachio-0.4/kernel/include/api/v4/tcb.h   2005-01-23 14:52:15.000000000 
+0100
@@ -130,6 +130,8 @@ public:
 
     time_t get_xfer_timeout_snd ();
     time_t get_xfer_timeout_rcv ();
+    time_t get_local_xfer_timeout_snd ();
+    time_t get_local_xfer_timeout_rcv ();
     void set_actual_sender(threadid_t tid);
     threadid_t get_virtual_sender();
     threadid_t get_intended_receiver();
@@ -587,6 +589,24 @@ INLINE time_t tcb_t::get_xfer_timeout_rc
     return get_utcb()->xfer_timeout.get_rcv();
 }
 
+/**
+ * Get a thread's local send transfer timeout
+ * @return      local send transfer timeout
+ */
+INLINE time_t tcb_t::get_local_xfer_timeout_snd (void)
+{
+    return get_utcb()->local_xfer_timeout.get_snd();
+}
+
+/**
+ * Get a thread's local receive transfer timeout
+ * @return     local receive transfer timeout
+ */
+INLINE time_t tcb_t::get_local_xfer_timeout_rcv (void)
+{
+    return get_utcb()->local_xfer_timeout.get_rcv();
+}
+
 
 /**
  * enqueues the tcb into the send queue of tcb
diff -Naurp pistachio-bak/kernel/include/glue/v4-ia32/utcb.h 
pistachio-0.4/kernel/include/glue/v4-ia32/utcb.h
--- pistachio-bak/kernel/include/glue/v4-ia32/utcb.h    2005-01-23 
14:12:14.000000000 +0100
+++ pistachio-0.4/kernel/include/glue/v4-ia32/utcb.h    2005-01-23 
14:47:04.000000000 +0100
@@ -51,7 +51,8 @@ public:
     timeout_t          xfer_timeout;           /* -32          */
     threadid_t         intended_receiver;      /* -28          */
     threadid_t         virtual_sender;         /* -24          */
-    word_t             reserved1[5];           /* -20 .. -4    */
+    timeout_t           local_xfer_timeout;     /* -20          */
+    word_t             reserved1[4];           /* -16 .. -4    */
     word_t             mr[IPC_NUM_MR];         /* 0 .. 252     */
 
     /* do not delete this TCB_END_MARKER */
diff -Naurp pistachio-bak/kernel/include/glue/v4-ia32/utcb.h~ 
pistachio-0.4/kernel/include/glue/v4-ia32/utcb.h~
diff -Naurp pistachio-bak/kernel/src/api/v4/space.cc 
pistachio-0.4/kernel/src/api/v4/space.cc
--- pistachio-bak/kernel/src/api/v4/space.cc    2005-01-23 14:12:14.000000000 
+0100
+++ pistachio-0.4/kernel/src/api/v4/space.cc    2005-01-23 16:54:28.000000000 
+0100
@@ -150,7 +150,7 @@ void tunnel_pagefault (word_t addr)
  *
  * @param sender               sender thread
  */
-static void handle_xfer_timeouts (tcb_t * sender)
+static void handle_xfer_timeouts (tcb_t * sender, bool fault_in_sender_as)
 {
 #warning Handle priority inversion for xfer timeouts
     
@@ -164,23 +164,45 @@ static void handle_xfer_timeouts (tcb_t 
     ASSERT (sender->get_state() == thread_state_t::locked_running);
     ASSERT (partner->get_state() == thread_state_t::locked_waiting);
  
-    time_t snd_to = sender->get_xfer_timeout_snd ();
-    time_t rcv_to = partner->get_xfer_timeout_rcv ();
+    time_t remote_snd_to = sender->get_xfer_timeout_snd ();
+    time_t remote_rcv_to = partner->get_xfer_timeout_rcv ();
+    time_t local_snd_to = sender->get_local_xfer_timeout_snd ();
+    time_t local_rcv_to = partner->get_local_xfer_timeout_rcv ();
 
-    if (snd_to.is_zero () || rcv_to.is_zero ())
+    time_t timeout;
+
+    if (fault_in_sender_as)
     {
-       // Timeout immediately.
-       handle_ipc_timeout (thread_state_t::locked_running);
+       if (local_snd_to.is_zero () || remote_rcv_to.is_zero ())
+       {
+           // Timeout immediately.
+           handle_ipc_timeout (thread_state_t::locked_running);
+       }
+       else if (local_snd_to.is_never () && remote_rcv_to.is_never ())
+       {
+           // No timeouts.
+           return;
+       }
+
+       timeout = (local_snd_to < remote_rcv_to ? local_snd_to : remote_rcv_to);
     }
-    else if (snd_to.is_never () && rcv_to.is_never ())
+    else
     {
-       // No timeouts.
-       return;
-    }
+       if (remote_snd_to.is_zero () || local_rcv_to.is_zero ())
+       {
+           // Timeout immediately.
+           handle_ipc_timeout (thread_state_t::locked_running);
+       }
+       else if (remote_snd_to.is_never () && local_rcv_to.is_never ())
+       {
+           // No timeouts.
+           return;
+       }
 
+       timeout = (remote_snd_to < local_rcv_to ? remote_snd_to : local_rcv_to);
+    }
     // Set timeout on the sender side.
-    get_current_scheduler ()->set_timeout (sender,
-                                          snd_to < rcv_to ? snd_to : rcv_to);
+    get_current_scheduler ()->set_timeout (sender, timeout);
     sender->flags += tcb_t::has_xfer_timeout;
 }
 
@@ -225,7 +247,7 @@ void space_t::handle_pagefault(addr_t ad
                // Pagefault during IPC copy.  Initiate xfer timeout
                // counters before handling pagefault.
                current->misc.ipc_copy.copy_fault = addr;
-               handle_xfer_timeouts (current);
+               handle_xfer_timeouts (current, true);
            }
 
            // if we have a user fault we may have a stale partner
@@ -279,7 +301,7 @@ void space_t::handle_pagefault(addr_t ad
        {
            // Fault in copy area.  Tunnel pagefault through partner.
            current->misc.ipc_copy.copy_fault = addr;
-           handle_xfer_timeouts (current);
+           handle_xfer_timeouts (current, false);
 
            // On PF tunneling we temporarily set the current thread
            // into waiting for partner.