- Ursprüngliche Mail -
> Von: "anton ivanov"
> An: "linux-um"
> CC: "Johannes Berg" , "richard" ,
> "anton ivanov"
>
> Gesendet: Freitag, 22. September 2023 08:52:12
> Betreff: [PATCH v5] um: Enable preemption in UML
> From: Anton Ivanov
>
> Preemption requires saving/restoring FPU s
On 22/09/2023 08:27, Richard Weinberger wrote:
- Ursprüngliche Mail -
Von: "anton ivanov"
An: "linux-um"
CC: "Johannes Berg" , "richard" , "anton
ivanov"
Gesendet: Freitag, 22. September 2023 08:52:12
Betreff: [PATCH v5] um: Enable preemption in UML
From: Anton Ivanov
Preemption r
On Fri, 2023-09-22 at 07:52 +0100, anton.iva...@cambridgegreys.com
wrote:
>
> +++ b/arch/um/include/asm/processor-generic.h
> @@ -44,6 +44,9 @@ struct thread_struct {
> } cb;
> } u;
> } request;
> +#if defined(CONFIG_PREEMPT) || defined(CONFIG_PREEMPT_VOLU
On 22/09/2023 08:30, Johannes Berg wrote:
On Fri, 2023-09-22 at 07:52 +0100, anton.iva...@cambridgegreys.com
wrote:
+++ b/arch/um/include/asm/processor-generic.h
@@ -44,6 +44,9 @@ struct thread_struct {
} cb;
} u;
} request;
+#if defined(CONFIG_P
On Fri, 2023-09-22 at 08:38 +0100, Anton Ivanov wrote:
> >
> > I had enabled CONFIG_DEBUG_ATOMIC_SLEEP because that's actually
> > something I'd really like to have in our testing.
> >
> > But with that issue I don't even know how we get there really. It
> > doesn't even happen every time we fork
On 22/09/2023 08:40, Johannes Berg wrote:
On Fri, 2023-09-22 at 08:38 +0100, Anton Ivanov wrote:
I had enabled CONFIG_DEBUG_ATOMIC_SLEEP because that's actually
something I'd really like to have in our testing.
But with that issue I don't even know how we get there really. It
doesn't even hap
On Fri, 2023-09-22 at 09:01 +0100, Anton Ivanov wrote:
>
> My favorite test which I run on all changes is:
>
> find /usr -type f -exec cat {} > /dev/null \;
>
> On Debian this forks /bin/cat for each file and does IO on it. That test
> passes here every time :(
Oh, apart from the splat, it all
On 22/09/2023 09:13, Johannes Berg wrote:
On Fri, 2023-09-22 at 09:01 +0100, Anton Ivanov wrote:
My favorite test which I run on all changes is:
find /usr -type f -exec cat {} > /dev/null \;
On Debian this forks /bin/cat for each file and does IO on it. That test passes
here every time :(
O
>
> Yes, but when does the fork actually happen?
>
Looking further at this, now I'm confused as to why it doesn't happen
_all_ the time.
I think this has pretty much always been wrong, just now we actually
notice it?
Basically, when we create a new thread (really just mm I think), we say
the f
On Fri, 2023-09-22 at 10:41 +0200, Johannes Berg wrote:
> >
> > Yes, but when does the fork actually happen?
> >
>
> Looking further at this, now I'm confused as to why it doesn't happen
> _all_ the time.
>
> I think this has pretty much always been wrong, just now we actually
> notice it?
>
>
On Fri, 2023-09-22 at 10:43 +0200, Johannes Berg wrote:
> >
> > I think this has pretty much always been wrong, just now we actually
> > notice it?
> >
> > Basically, when we create a new thread (really just mm I think), we say
> > the first thing that has to run there is fork_handler(), which
>
On 22/09/2023 09:41, Johannes Berg wrote:
Yes, but when does the fork actually happen?
Looking further at this, now I'm confused as to why it doesn't happen
_all_ the time.
I think this has pretty much always been wrong, just now we actually
notice it?
Basically, when we create a new thread
On Fri, 2023-09-22 at 10:06 +0100, Anton Ivanov wrote:
> On 22/09/2023 09:41, Johannes Berg wrote:
> > > Yes, but when does the fork actually happen?
> > >
> > Looking further at this, now I'm confused as to why it doesn't happen
> > _all_ the time.
> >
> > I think this has pretty much always bee
On 22/09/2023 10:04, Johannes Berg wrote:
On Fri, 2023-09-22 at 10:43 +0200, Johannes Berg wrote:
I think this has pretty much always been wrong, just now we actually
notice it?
Basically, when we create a new thread (really just mm I think), we say
the first thing that has to run there is for
On Fri, 2023-09-22 at 11:10 +0200, Johannes Berg wrote:
> On Fri, 2023-09-22 at 10:06 +0100, Anton Ivanov wrote:
> > On 22/09/2023 09:41, Johannes Berg wrote:
> > > > Yes, but when does the fork actually happen?
> > > >
> > > Looking further at this, now I'm confused as to why it doesn't happen
>
On 22/09/2023 10:14, Johannes Berg wrote:
On Fri, 2023-09-22 at 11:10 +0200, Johannes Berg wrote:
On Fri, 2023-09-22 at 10:06 +0100, Anton Ivanov wrote:
On 22/09/2023 09:41, Johannes Berg wrote:
Yes, but when does the fork actually happen?
Looking further at this, now I'm confused as to w
On Fri, 2023-09-22 at 10:19 +0100, Anton Ivanov wrote:
> >
> > So maybe that works - perhaps with a big comment?
>
> Ack. Will add this to the patch and run it through its paces.
>
We can also just get rid of it entirely:
diff --git a/arch/um/include/asm/mmu_context.h
b/arch/um/include/asm/mm
On Fri, 2023-09-22 at 11:51 +0200, Johannes Berg wrote:
> On Fri, 2023-09-22 at 10:19 +0100, Anton Ivanov wrote:
> > >
> > > So maybe that works - perhaps with a big comment?
> >
> > Ack. Will add this to the patch and run it through its paces.
> >
>
> We can also just get rid of it entirely:
>
On Fri, Sep 22, 2023 at 09:49:18AM +0800, Xuan Zhuo wrote:
> On Thu, 21 Sep 2023 10:02:53 -0400, "Michael S. Tsirkin"
> wrote:
> > On Mon, Aug 01, 2022 at 02:38:50PM +0800, Xuan Zhuo wrote:
> > > Introduce new helpers to implement queue reset and get queue reset
> > > status.
> > >
> > > https:/
On 22/09/2023 10:55, Johannes Berg wrote:
On Fri, 2023-09-22 at 11:51 +0200, Johannes Berg wrote:
On Fri, 2023-09-22 at 10:19 +0100, Anton Ivanov wrote:
So maybe that works - perhaps with a big comment?
Ack. Will add this to the patch and run it through its paces.
We can also just get rid
From: Anton Ivanov
1. Preemption requires saving/restoring FPU state. This patch
adds support for it using GCC intrinsics as well as appropriate
storage space in the thread structure.
2. irq critical sections need preempt_disable()/preempt_enable().
3. TLB critical sections need preempt_disable
On 22/09/2023 11:56, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
1. Preemption requires saving/restoring FPU state. This patch
adds support for it using GCC intrinsics as well as appropriate
storage space in the thread structure.
2. irq critical sections need preempt_disable()
From: Johannes Berg
While enabling PREEMPT on UML, we found that the call to
force_flush_all() cannot be done where it is, it sleeps
while atomic.
Further investigation shows that all this seems at least
a bit roundabout and possibly wrong wrong in the first
place - we copy from the 'current' pr
On Fri, 2023-09-22 at 11:55 +0100, Anton Ivanov wrote:
> On 22/09/2023 10:55, Johannes Berg wrote:
> > On Fri, 2023-09-22 at 11:51 +0200, Johannes Berg wrote:
> > > On Fri, 2023-09-22 at 10:19 +0100, Anton Ivanov wrote:
> > > > > So maybe that works - perhaps with a big comment?
> > > > Ack. Will a
>
> 4. UML TLB flush is also invoked during a fork. This happens
> with interrupts and preempt disabled which disagrees with the
> standard mm locking via rwsem. The mm lock for this code path
> had to be replaced with an rcu.
For the record, even if I figured out this gets rid of the complaints,
On 22/09/2023 12:22, Johannes Berg wrote:
4. UML TLB flush is also invoked during a fork. This happens
with interrupts and preempt disabled which disagrees with the
standard mm locking via rwsem. The mm lock for this code path
had to be replaced with an rcu.
For the record, even if I figure
On Fri, Sept 22, 2023 at 06:56, anton.iva...@cambridgegreys.com wrote:
>
> From: Anton Ivanov anton.iva...@cambridgegreys.com
>
>
> 1. Preemption requires saving/restoring FPU state. This patch
> adds support for it using GCC intrinsics as well as appropriate
> storage space in the thread struct
On 22/09/2023 12:16, Johannes Berg wrote:
From: Johannes Berg
While enabling PREEMPT on UML, we found that the call to
force_flush_all() cannot be done where it is, it sleeps
while atomic.
Further investigation shows that all this seems at least
a bit roundabout and possibly wrong wrong in t
On Fri, 2023-09-22 at 13:16 +0200, Johannes Berg wrote:
> From: Johannes Berg
>
> While enabling PREEMPT on UML, we found that the call to
> force_flush_all() cannot be done where it is, it sleeps
> while atomic.
>
> Further investigation shows that all this seems at least
> a bit roundabout and
On Fri, 2023-09-22 at 14:41 +0100, Anton Ivanov wrote:
> On 22/09/2023 12:16, Johannes Berg wrote:
> > From: Johannes Berg
> >
> > While enabling PREEMPT on UML, we found that the call to
> > force_flush_all() cannot be done where it is, it sleeps
> > while atomic.
> >
> > Further investigation
Hi,
On Fri, 2023-09-22 at 14:41 +0100, Anton Ivanov wrote:
>
> On 22/09/2023 12:16, Johannes Berg wrote:
> > From: Johannes Berg
> >
> > While enabling PREEMPT on UML, we found that the call to
> > force_flush_all() cannot be done where it is, it sleeps
> > while atomic.
> >
> > Further invest
On 22/09/2023 16:31, Benjamin Berg wrote:
Hi,
On Fri, 2023-09-22 at 14:41 +0100, Anton Ivanov wrote:
On 22/09/2023 12:16, Johannes Berg wrote:
From: Johannes Berg
While enabling PREEMPT on UML, we found that the call to
force_flush_all() cannot be done where it is, it sleeps
while atomic.
On Fri, 2023-09-22 at 13:16 +0200, Johannes Berg wrote:
>
> - /* Set parent's instruction pointer to start of clone-stub */
> - thread_regs[REGS_IP_INDEX] = STUB_CODE +
> - (unsigned long) stub_clone_handler -
> - (unsigned long) __sy
On Fri, 2023-09-22 at 13:16 +0200, Johannes Berg wrote:
> Note that this fixes what seems like an issue - we look
> at current->mm when we copy, but that doesn't seem right
> in the case of clone() without copying the MM. This is
> probably saved by the forced flush later right now.
>
After findi
On Fri, 2023-09-22 at 14:41 +0100, Anton Ivanov wrote:
>
> It is nearly twice slower than the current approach on a find /usr -type f
> -exec cat {} > /dev/null \;
>
Btw, I cannot reproduce that at all - seems about the same in my tests?
Is there anything special in your setup?
Or maybe it's
On 22/09/2023 20:41, Johannes Berg wrote:
On Fri, 2023-09-22 at 14:41 +0100, Anton Ivanov wrote:
It is nearly twice slower than the current approach on a find /usr -type f -exec
cat {} > /dev/null \;
Btw, I cannot reproduce that at all - seems about the same in my tests?
Is there anything
On Fri, 2023-09-22 at 21:08 +0100, Anton Ivanov wrote:
> On 22/09/2023 20:41, Johannes Berg wrote:
> > On Fri, 2023-09-22 at 14:41 +0100, Anton Ivanov wrote:
> > >
> > > It is nearly twice slower than the current approach on a find /usr -type
> > > f -exec cat {} > /dev/null \;
> > >
> >
> > Bt
On 22/09/2023 21:12, Johannes Berg wrote:
On Fri, 2023-09-22 at 21:08 +0100, Anton Ivanov wrote:
On 22/09/2023 20:41, Johannes Berg wrote:
On Fri, 2023-09-22 at 14:41 +0100, Anton Ivanov wrote:
It is nearly twice slower than the current approach on a find /usr -type f -exec
cat {} > /dev/null
- Ursprüngliche Mail -
> Von: "Johannes Berg"
>> -mmap_read_lock(mm);
>> +/* We use a RCU lock instead of a mm lock, because
>> + * this can be invoked out of critical/atomic sections
>> + * and that does not agree with the sleepable semantics
>> + * of the standard sem
This shouldn't pessimize since we keep copy_context_skas0() and
the setup there, but still clean up the code there to not rely on
current, and move the force_flush_all() to a more appropriate
place, which fixes the issue with that in the fork_handler().
johannes
From: Johannes Berg
This function gets a context that's already copied from
the old context if making a copy, or memset to 0 if not.
This means we don't need to play games with current,
even if those games are OK now since dup_mm() is only
ever called with current->mm as the source.
Also memset(
From: Johannes Berg
The reason to flush here is to resynchronize the mm after
a copy. For exec we don't need to since flush_thread()
will anyway clear everything.
Thus, we can just do this in arch_dup_mmap(), since when
that's invoked, the new mm has been setup sufficiently.
This isn't true if w
From: Johannes Berg
There's really not much value in having this as a
separate lock, and having it makes it harder to
clean up the init_new_context() semantics to not
rely on 'current', even if dup_mm() is really only
called with 'current->mm' as the source today.
Replace the locking accordingly
On 22/09/2023 21:12, Johannes Berg wrote:
On Fri, 2023-09-22 at 21:08 +0100, Anton Ivanov wrote:
On 22/09/2023 20:41, Johannes Berg wrote:
On Fri, 2023-09-22 at 14:41 +0100, Anton Ivanov wrote:
It is nearly twice slower than the current approach on a find /usr -type f -exec
cat {} > /dev/nul
44 matches
Mail list logo