Re: [PATCH 4/7] um: time-travel/signals: fix ndelay() in interrupt

2021-02-25 Thread Johannes Berg
On Tue, 2021-02-23 at 16:27 +0100, Johannes Berg wrote:
> 
> +void unblock_signals_hard(void)
> +{
> + if (!signals_blocked)
> + return;
> +
> + if (signals_pending && signals_enabled) {
> + /* this is a bit inefficient, but that's not really important */
> + block_signals();
> + unblock_signals();
> + } else if (signals_pending & SIGIO_MASK) {
> + /* we need to run time-travel handlers even if not enabled */
> + sigio_run_timetravel_handlers();
> + }
> +
> + signals_blocked = 0;
> +}

This is, of course, racy & wrong - we could set signals_pending just
after checking it.

Need to make this

{
if (!signals_blocked)
return;
signals_blocked = 0;
barrier();

if (signals_pending && signals_enabled) {
...


Anyway, I need to repost this series, but I'll wait a bit longer for
further feedback before that, since I know Arnd and others wanted to
take a look at the IOMEM and PCI bits.

johannes



[PATCH 4/7] um: time-travel/signals: fix ndelay() in interrupt

2021-02-23 Thread Johannes Berg
From: Johannes Berg 

We should be able to ndelay() from any context, even from an
interrupt context! However, this is broken (not functionally,
but locking-wise) in time-travel because we'll get into the
time-travel code and enable interrupts to handle messages on
other time-travel aware subsystems (only virtio for now).

Luckily, I've already reworked the time-travel aware signal
(interrupt) delivery for suspend/resume to have a time travel
handler, which runs directly in the context of the signal and
not from the Linux interrupt.

In order to fix this time-travel issue then, we need to do a
few things:

 1) rework the signal handling code to not block SIGIO (if
time-travel is enabled, just to simplify the other case)
but rather let it bubble through the system, all the way
*past* the IRQ's timetravel_handler, stopping it after
that and before real interrupt delivery if IRQs are not
actually enabled;
 2) rework time-travel to not enable interrupts while it's
waiting for a message;
 3) rework time-travel to not (just) disable interrupts but
rather block signals at a lower level while it needs them
disabled for communicating with the controller.

Finally, since now we can actually spend even virtual time
in interrupts-disabled sections, the delay warning when we
deliver a time-travel delayed interrupt is no longer valid,
things can (and should) now get delayed.

Signed-off-by: Johannes Berg 
---
 arch/um/include/shared/irq_user.h |  1 +
 arch/um/include/shared/os.h   |  3 +++
 arch/um/kernel/irq.c  | 38 +-
 arch/um/kernel/time.c | 35 +---
 arch/um/os-Linux/signal.c | 44 ---
 5 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/arch/um/include/shared/irq_user.h 
b/arch/um/include/shared/irq_user.h
index 07239e801a5b..065829f443ae 100644
--- a/arch/um/include/shared/irq_user.h
+++ b/arch/um/include/shared/irq_user.h
@@ -17,6 +17,7 @@ enum um_irq_type {
 
 struct siginfo;
 extern void sigio_handler(int sig, struct siginfo *unused_si, struct 
uml_pt_regs *regs);
+void sigio_run_timetravel_handlers(void);
 extern void free_irq_by_fd(int fd);
 extern void deactivate_fd(int fd, int irqnum);
 extern int deactivate_all_fds(void);
diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index 13d86f94cf0f..afd5fbc1c867 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -243,6 +243,9 @@ extern int set_signals_trace(int enable);
 extern int os_is_signal_stack(void);
 extern void deliver_alarm(void);
 extern void register_pm_wake_signal(void);
+extern void block_signals_hard(void);
+extern void unblock_signals_hard(void);
+extern void mark_sigio_pending(void);
 
 /* util.c */
 extern void stack_protections(unsigned long address);
diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
index 82af5191e73d..76448b85292f 100644
--- a/arch/um/kernel/irq.c
+++ b/arch/um/kernel/irq.c
@@ -123,7 +123,8 @@ static bool irq_do_timetravel_handler(struct irq_entry 
*entry,
 #endif
 
 static void sigio_reg_handler(int idx, struct irq_entry *entry, enum 
um_irq_type t,
- struct uml_pt_regs *regs)
+ struct uml_pt_regs *regs,
+ bool timetravel_handlers_only)
 {
struct irq_reg *reg = &entry->reg[t];
 
@@ -136,18 +137,32 @@ static void sigio_reg_handler(int idx, struct irq_entry 
*entry, enum um_irq_type
if (irq_do_timetravel_handler(entry, t))
return;
 
-   if (irqs_suspended)
+#ifdef CONFIG_UML_TIME_TRAVEL_SUPPORT
+   /*
+* We can only get here with signals disabled if we have time-travel
+* support, otherwise we cannot have the hard handlers that may need
+* to run even in interrupts-disabled sections and therefore sigio is
+* blocked as well when interrupts are disabled.
+*/
+   if (!get_signals()) {
+   mark_sigio_pending();
+   return;
+   }
+#endif
+
+   if (timetravel_handlers_only)
return;
 
irq_io_loop(reg, regs);
 }
 
-void sigio_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs 
*regs)
+static void _sigio_handler(struct uml_pt_regs *regs,
+  bool timetravel_handlers_only)
 {
struct irq_entry *irq_entry;
int n, i;
 
-   if (irqs_suspended && !um_irq_timetravel_handler_used())
+   if (timetravel_handlers_only && !um_irq_timetravel_handler_used())
return;
 
while (1) {
@@ -172,14 +187,20 @@ void sigio_handler(int sig, struct siginfo *unused_si, 
struct uml_pt_regs *regs)
irq_entry = os_epoll_get_data_pointer(i);
 
for (t = 0; t < NUM_IRQ_TYPES; t++)
-   sigio_reg_handler(i, irq_entry, t, regs);
+   sigio_reg_handler(i, irq_entry, t, r