Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1643480 > unrolled thread

Virtually Mapped Stacks: Do not disable interrupts

Back to article view | Back to linux.kernel

Contents

  Virtually Mapped Stacks: Do not disable interrupts Christoph Lameter <cl@linux.com> - 2017-05-17 18:00 +0200
    Re: Virtually Mapped Stacks: Do not disable interrupts Andy Lutomirski <luto@amacapital.net> - 2017-05-19 07:40 +0200
      Re: Virtually Mapped Stacks: Do not disable interrupts Christoph Lameter <cl@linux.com> - 2017-05-19 16:10 +0200

#1643480 — Virtually Mapped Stacks: Do not disable interrupts

The reason to disable interrupts seems to be to avoid switching
to a different processor while handling per cpu data using
individual loads and stores. If we use per cpu RMV primitives
we will not have to disable interrupts.

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/kernel/fork.c
===================================================================
--- linux.orig/kernel/fork.c
+++ linux/kernel/fork.c
@@ -205,19 +205,17 @@ static unsigned long *alloc_thread_stack
 	void *stack;
 	int i;

-	local_irq_disable();
 	for (i = 0; i < NR_CACHED_STACKS; i++) {
-		struct vm_struct *s = this_cpu_read(cached_stacks[i]);
+		struct vm_struct *s;
+
+		s = this_cpu_xchg(cached_stacks[i], NULL);

 		if (!s)
 			continue;
-		this_cpu_write(cached_stacks[i], NULL);

 		tsk->stack_vm_area = s;
-		local_irq_enable();
 		return s->addr;
 	}
-	local_irq_enable();

 	stack = __vmalloc_node_range(THREAD_SIZE, THREAD_SIZE,
 				     VMALLOC_START, VMALLOC_END,
@@ -245,19 +243,15 @@ static inline void free_thread_stack(str
 {
 #ifdef CONFIG_VMAP_STACK
 	if (task_stack_vm_area(tsk)) {
-		unsigned long flags;
 		int i;

-		local_irq_save(flags);
 		for (i = 0; i < NR_CACHED_STACKS; i++) {
-			if (this_cpu_read(cached_stacks[i]))
+			if (this_cpu_cmpxchg(cached_stacks[i],
+					NULL, tsk->stack_vm_area) != NULL)
 				continue;

-			this_cpu_write(cached_stacks[i], tsk->stack_vm_area);
-			local_irq_restore(flags);
 			return;
 		}
-		local_irq_restore(flags);

 		vfree_atomic(tsk->stack);
 		return;

[toc] | [next] | [standalone]

#1645209

On Wed, May 17, 2017 at 8:58 AM, Christoph Lameter <cl@linux.com> wrote:
> The reason to disable interrupts seems to be to avoid switching
> to a different processor while handling per cpu data using
> individual loads and stores. If we use per cpu RMV primitives
> we will not have to disable interrupts.

I like this, except that those primitives can be quite expensive, I
think, and they're being called in a loop.  What if you first did a
this_cpu_read() to see if the value in the cache slot might be useful
before doing the heavyweight exchange?

--Andy

[toc] | [prev] | [next] | [standalone]

#1645625

On Thu, 18 May 2017, Andy Lutomirski wrote:

> On Wed, May 17, 2017 at 8:58 AM, Christoph Lameter <cl@linux.com> wrote:
> > The reason to disable interrupts seems to be to avoid switching
> > to a different processor while handling per cpu data using
> > individual loads and stores. If we use per cpu RMV primitives
> > we will not have to disable interrupts.
>
> I like this, except that those primitives can be quite expensive, I
> think, and they're being called in a loop.  What if you first did a
> this_cpu_read() to see if the value in the cache slot might be useful
> before doing the heavyweight exchange?

These operations are not expensive because they are unlocked operations
(in constrast to the usuual "lock cmpxchg") and do not
require coherency to be guaranteed between processors. That is why they
were made available because they are so much cheaper.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web