All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: Yair Podemsky <ypodemsk@redhat.com>
Cc: linux@armlinux.org.uk, mpe@ellerman.id.au, npiggin@gmail.com,
	christophe.leroy@csgroup.eu, hca@linux.ibm.com,
	gor@linux.ibm.com, agordeev@linux.ibm.com,
	borntraeger@linux.ibm.com, svens@linux.ibm.com,
	davem@davemloft.net, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
	hpa@zytor.com, will@kernel.org, aneesh.kumar@linux.ibm.com,
	akpm@linux-foundation.org, peterz@infradead.org, arnd@arndb.de,
	keescook@chromium.org, paulmck@kernel.org, jpoimboe@kernel.org,
	samitolvanen@google.com, ardb@kernel.org,
	juerg.haefliger@canonical.com, rmk+kernel@armlinux.org.uk,
	geert+renesas@glider.be, tony@atomide.com,
	linus.walleij@linaro.org, sebastian.reichel@collabora.com,
	nick.hawkins@hpe.com, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org,
	sparclinux@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-mm@kvack.org, mtosatti@redhat.com, vschneid@redhat.com,
	dhildenb@redhat.com, alougovs@redhat.com
Subject: Re: [PATCH 3/3] mm/mmu_gather: send tlb_remove_table_smp_sync IPI only to CPUs in kernel mode
Date: Wed, 5 Apr 2023 12:43:58 +0200	[thread overview]
Message-ID: <ZC1Q7uX4rNLg3vEg@lothringen> (raw)
In-Reply-To: <20230404134224.137038-4-ypodemsk@redhat.com>

On Tue, Apr 04, 2023 at 04:42:24PM +0300, Yair Podemsky wrote:
> @@ -191,6 +192,20 @@ static void tlb_remove_table_smp_sync(void *arg)
>  	/* Simply deliver the interrupt */
>  }
>  
> +
> +#ifdef CONFIG_CONTEXT_TRACKING
> +static bool cpu_in_kernel(int cpu, void *info)
> +{
> +	struct context_tracking *ct = per_cpu_ptr(&context_tracking, cpu);

Like Peter said, an smp_mb() is required here before the read (unless there is
already one between the page table modification and that ct->state read?).

So that you have this pairing:


           WRITE page_table                  WRITE ct->state
	   smp_mb()                          smp_mb() // implied by atomic_fetch_or
           READ ct->state                    READ page_table

> +	int state = atomic_read(&ct->state);
> +	/* will return true only for cpus in kernel space */
> +	return state & CT_STATE_MASK == CONTEXT_KERNEL;
> +}

Also note that this doesn't stricly prevent userspace from being interrupted.
You may well observe the CPU in kernel but it may receive the IPI later after
switching to userspace.

We could arrange for avoiding that with marking ct->state with a pending work bit
to flush upon user entry/exit but that's a bit more overhead so I first need to
know about your expectations here, ie: can you tolerate such an occasional
interruption or not?

Thanks.


WARNING: multiple messages have this Message-ID (diff)
From: Frederic Weisbecker <frederic@kernel.org>
To: Yair Podemsky <ypodemsk@redhat.com>
Cc: geert+renesas@glider.be, peterz@infradead.org,
	linus.walleij@linaro.org, dave.hansen@linux.intel.com,
	sebastian.reichel@collabora.com, linux-mm@kvack.org,
	hpa@zytor.com, sparclinux@vger.kernel.org,
	agordeev@linux.ibm.com, will@kernel.org, ardb@kernel.org,
	linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
	vschneid@redhat.com, arnd@arndb.de, paulmck@kernel.org,
	aneesh.kumar@linux.ibm.com, x86@kernel.org,
	linux@armlinux.org.uk, tony@atomide.com, mingo@redhat.com,
	samitolvanen@google.com, borntraeger@linux.ibm.com,
	keescook@chromium.org, gor@linux.ibm.com, hca@linux.ibm.com,
	npiggin@gmail.com, rmk+kernel@armlinux.org.uk, bp@alien8.de,
	nick.hawkins@hpe.com, tglx@linutronix.de, jpoimboe@kernel.org,
	linux-arm-kernel@lists.infradead.org, alougovs@redhat.com,
	mtosatti@redhat.com, linux-kernel@vger.kernel.org,
	juerg.haefliger@canonical.com, svens@linux.ibm.com,
	dhildenb@redhat.com, akpm@linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org, davem@davemloft.net
Subject: Re: [PATCH 3/3] mm/mmu_gather: send tlb_remove_table_smp_sync IPI only to CPUs in kernel mode
Date: Wed, 5 Apr 2023 12:43:58 +0200	[thread overview]
Message-ID: <ZC1Q7uX4rNLg3vEg@lothringen> (raw)
In-Reply-To: <20230404134224.137038-4-ypodemsk@redhat.com>

On Tue, Apr 04, 2023 at 04:42:24PM +0300, Yair Podemsky wrote:
> @@ -191,6 +192,20 @@ static void tlb_remove_table_smp_sync(void *arg)
>  	/* Simply deliver the interrupt */
>  }
>  
> +
> +#ifdef CONFIG_CONTEXT_TRACKING
> +static bool cpu_in_kernel(int cpu, void *info)
> +{
> +	struct context_tracking *ct = per_cpu_ptr(&context_tracking, cpu);

Like Peter said, an smp_mb() is required here before the read (unless there is
already one between the page table modification and that ct->state read?).

So that you have this pairing:


           WRITE page_table                  WRITE ct->state
	   smp_mb()                          smp_mb() // implied by atomic_fetch_or
           READ ct->state                    READ page_table

> +	int state = atomic_read(&ct->state);
> +	/* will return true only for cpus in kernel space */
> +	return state & CT_STATE_MASK == CONTEXT_KERNEL;
> +}

Also note that this doesn't stricly prevent userspace from being interrupted.
You may well observe the CPU in kernel but it may receive the IPI later after
switching to userspace.

We could arrange for avoiding that with marking ct->state with a pending work bit
to flush upon user entry/exit but that's a bit more overhead so I first need to
know about your expectations here, ie: can you tolerate such an occasional
interruption or not?

Thanks.


WARNING: multiple messages have this Message-ID (diff)
From: Frederic Weisbecker <frederic@kernel.org>
To: Yair Podemsky <ypodemsk@redhat.com>
Cc: linux@armlinux.org.uk, mpe@ellerman.id.au, npiggin@gmail.com,
	christophe.leroy@csgroup.eu, hca@linux.ibm.com,
	gor@linux.ibm.com, agordeev@linux.ibm.com,
	borntraeger@linux.ibm.com, svens@linux.ibm.com,
	davem@davemloft.net, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
	hpa@zytor.com, will@kernel.org, aneesh.kumar@linux.ibm.com,
	akpm@linux-foundation.org, peterz@infradead.org, arnd@arndb.de,
	keescook@chromium.org, paulmck@kernel.org, jpoimboe@kernel.org,
	samitolvanen@google.com, ardb@kernel.org,
	juerg.haefliger@canonical.com, rmk+kernel@armlinux.org.uk,
	geert+renesas@glider.be, tony@atomide.com,
	linus.walleij@linaro.org, sebastian.reichel@collabora.com,
	nick.hawkins@hpe.com, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org,
	sparclinux@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-mm@kvack.org, mtosatti@redhat.com, vschneid@redhat.com,
	dhildenb@redhat.com, alougovs@redhat.com
Subject: Re: [PATCH 3/3] mm/mmu_gather: send tlb_remove_table_smp_sync IPI only to CPUs in kernel mode
Date: Wed, 5 Apr 2023 12:43:58 +0200	[thread overview]
Message-ID: <ZC1Q7uX4rNLg3vEg@lothringen> (raw)
In-Reply-To: <20230404134224.137038-4-ypodemsk@redhat.com>

On Tue, Apr 04, 2023 at 04:42:24PM +0300, Yair Podemsky wrote:
> @@ -191,6 +192,20 @@ static void tlb_remove_table_smp_sync(void *arg)
>  	/* Simply deliver the interrupt */
>  }
>  
> +
> +#ifdef CONFIG_CONTEXT_TRACKING
> +static bool cpu_in_kernel(int cpu, void *info)
> +{
> +	struct context_tracking *ct = per_cpu_ptr(&context_tracking, cpu);

Like Peter said, an smp_mb() is required here before the read (unless there is
already one between the page table modification and that ct->state read?).

So that you have this pairing:


           WRITE page_table                  WRITE ct->state
	   smp_mb()                          smp_mb() // implied by atomic_fetch_or
           READ ct->state                    READ page_table

> +	int state = atomic_read(&ct->state);
> +	/* will return true only for cpus in kernel space */
> +	return state & CT_STATE_MASK == CONTEXT_KERNEL;
> +}

Also note that this doesn't stricly prevent userspace from being interrupted.
You may well observe the CPU in kernel but it may receive the IPI later after
switching to userspace.

We could arrange for avoiding that with marking ct->state with a pending work bit
to flush upon user entry/exit but that's a bit more overhead so I first need to
know about your expectations here, ie: can you tolerate such an occasional
interruption or not?

Thanks.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2023-04-05 10:44 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-04 13:42 [PATCH 0/3] send tlb_remove_table_smp_sync IPI only to necessary CPUs Yair Podemsky
2023-04-04 13:42 ` Yair Podemsky
2023-04-04 13:42 ` Yair Podemsky
2023-04-04 13:42 ` [PATCH 1/3] arch: Introduce ARCH_HAS_CPUMASK_BITS Yair Podemsky
2023-04-04 13:42   ` Yair Podemsky
2023-04-04 13:42   ` Yair Podemsky
2023-04-04 13:47   ` David Hildenbrand
2023-04-04 13:47     ` David Hildenbrand
2023-04-04 13:42 ` [PATCH 2/3] mm/mmu_gather: send tlb_remove_table_smp_sync IPI only to MM CPUs Yair Podemsky
2023-04-04 13:42   ` Yair Podemsky
2023-04-04 13:42   ` Yair Podemsky
2023-04-04 14:57   ` Peter Zijlstra
2023-04-04 14:57     ` Peter Zijlstra
2023-04-04 14:57     ` Peter Zijlstra
2023-04-04 13:42 ` [PATCH 3/3] mm/mmu_gather: send tlb_remove_table_smp_sync IPI only to CPUs in kernel mode Yair Podemsky
2023-04-04 13:42   ` Yair Podemsky
2023-04-04 13:42   ` Yair Podemsky
2023-04-04 14:03   ` David Hildenbrand
2023-04-04 14:03     ` David Hildenbrand
2023-04-04 15:12   ` Peter Zijlstra
2023-04-04 15:12     ` Peter Zijlstra
2023-04-04 15:12     ` Peter Zijlstra
2023-04-04 16:00     ` Peter Zijlstra
2023-04-04 16:00       ` Peter Zijlstra
2023-04-04 16:00       ` Peter Zijlstra
2023-04-05  0:53       ` Hillf Danton
2023-04-05 10:43   ` Frederic Weisbecker [this message]
2023-04-05 10:43     ` Frederic Weisbecker
2023-04-05 10:43     ` Frederic Weisbecker
2023-04-05 11:10     ` Frederic Weisbecker
2023-04-05 11:10       ` Frederic Weisbecker
2023-04-05 11:10       ` Frederic Weisbecker
2023-04-05 11:41       ` Peter Zijlstra
2023-04-05 11:41         ` Peter Zijlstra
2023-04-05 11:41         ` Peter Zijlstra
2023-04-05 12:00         ` David Hildenbrand
2023-04-05 12:00           ` David Hildenbrand
2023-04-05 12:00           ` David Hildenbrand
2023-04-05 12:05         ` Frederic Weisbecker
2023-04-05 12:05           ` Frederic Weisbecker
2023-04-05 12:05           ` Frederic Weisbecker
2023-04-05 12:31           ` Frederic Weisbecker
2023-04-05 12:31             ` Frederic Weisbecker
2023-04-05 12:31             ` Frederic Weisbecker
2023-04-05 12:45           ` Valentin Schneider
2023-04-05 12:45             ` Valentin Schneider
2023-04-05 12:45             ` Valentin Schneider
2023-04-06 13:38             ` Peter Zijlstra
2023-04-06 13:38               ` Peter Zijlstra
2023-04-06 13:38               ` Peter Zijlstra
2023-04-06 14:11               ` Valentin Schneider
2023-04-06 14:11                 ` Valentin Schneider
2023-04-06 14:11                 ` Valentin Schneider
2023-04-06 14:39                 ` Peter Zijlstra
2023-04-06 14:39                   ` Peter Zijlstra
2023-04-06 14:39                   ` Peter Zijlstra
2023-04-05 19:45       ` Marcelo Tosatti
2023-04-05 19:45         ` Marcelo Tosatti
2023-04-05 19:45         ` Marcelo Tosatti
2023-04-05 19:52         ` Peter Zijlstra
2023-04-05 19:52           ` Peter Zijlstra
2023-04-05 19:52           ` Peter Zijlstra
2023-04-06 12:38           ` Marcelo Tosatti
2023-04-06 12:38             ` Marcelo Tosatti
2023-04-06 12:38             ` Marcelo Tosatti
2023-04-06 13:29             ` Peter Zijlstra
2023-04-06 13:29               ` Peter Zijlstra
2023-04-06 13:29               ` Peter Zijlstra
2023-04-06 14:04               ` Peter Zijlstra
2023-04-06 14:04                 ` Peter Zijlstra
2023-04-06 14:04                 ` Peter Zijlstra
2023-04-06 14:42                 ` David Hildenbrand
2023-04-06 14:42                   ` David Hildenbrand
2023-04-06 14:42                   ` David Hildenbrand
2023-04-06 15:06                   ` Peter Zijlstra
2023-04-06 15:06                     ` Peter Zijlstra
2023-04-06 15:06                     ` Peter Zijlstra
2023-04-06 15:02                 ` Peter Zijlstra
2023-04-06 15:02                   ` Peter Zijlstra
2023-04-06 15:02                   ` Peter Zijlstra
2023-04-06 15:51                   ` David Hildenbrand
2023-04-06 15:51                     ` David Hildenbrand
2023-04-06 15:51                     ` David Hildenbrand
2023-04-06 18:27                     ` Peter Zijlstra
2023-04-06 18:27                       ` Peter Zijlstra
2023-04-06 18:27                       ` Peter Zijlstra
2023-04-19 11:30                       ` David Hildenbrand
2023-04-19 11:30                         ` David Hildenbrand
2023-04-19 11:30                         ` David Hildenbrand
2023-04-19 11:39                         ` Marcelo Tosatti
2023-04-19 11:39                           ` Marcelo Tosatti
2023-04-19 11:39                           ` Marcelo Tosatti
2023-04-05 19:43     ` Marcelo Tosatti
2023-04-05 19:43       ` Marcelo Tosatti
2023-04-05 19:43       ` Marcelo Tosatti
2023-04-05 19:54       ` Peter Zijlstra
2023-04-05 19:54         ` Peter Zijlstra
2023-04-05 19:54         ` Peter Zijlstra
2023-04-06 12:49         ` Marcelo Tosatti
2023-04-06 12:49           ` Marcelo Tosatti
2023-04-06 12:49           ` Marcelo Tosatti
2023-04-06 13:32           ` Peter Zijlstra
2023-04-06 13:32             ` Peter Zijlstra
2023-04-06 13:32             ` Peter Zijlstra
2023-04-19 11:01             ` Marcelo Tosatti
2023-04-19 11:01               ` Marcelo Tosatti
2023-04-19 11:01               ` Marcelo Tosatti

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZC1Q7uX4rNLg3vEg@lothringen \
    --to=frederic@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=alougovs@redhat.com \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=borntraeger@linux.ibm.com \
    --cc=bp@alien8.de \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=dhildenb@redhat.com \
    --cc=geert+renesas@glider.be \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@kernel.org \
    --cc=juerg.haefliger@canonical.com \
    --cc=keescook@chromium.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=mtosatti@redhat.com \
    --cc=nick.hawkins@hpe.com \
    --cc=npiggin@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=samitolvanen@google.com \
    --cc=sebastian.reichel@collabora.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=svens@linux.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=tony@atomide.com \
    --cc=vschneid@redhat.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --cc=ypodemsk@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.