diff mbox series

[bpf-next,3/4] bpf: Don't do preempt check when migrate is disabled

Message ID 20220629154832.56986-4-laoar.shao@gmail.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series bpf: Minor fixes | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 13 this patch: 13
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 6 this patch: 6
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 13 this patch: 13
netdev/checkpatch warning CHECK: Unnecessary parentheses around 'htab->map_locked[hash]' CHECK: Unnecessary parentheses around 'prog->active' CHECK: Unnecessary parentheses around htab->map_locked[hash] CHECK: Unnecessary parentheses around prog->active
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-2 fail Logs for Kernel LATEST on ubuntu-latest with llvm-15
bpf/vmtest-bpf-next-VM_Test-1 fail Logs for Kernel LATEST on ubuntu-latest with gcc
bpf/vmtest-bpf-next-VM_Test-3 fail Logs for Kernel LATEST on z15 with gcc

Commit Message

Yafang Shao June 29, 2022, 3:48 p.m. UTC 
It doesn't need to do the preempt check when migrate is disabled
after commit
74d862b682f5 ("sched: Make migrate_disable/enable() independent of RT").

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/bpf/bpf_task_storage.c | 8 ++++----
 kernel/bpf/hashtab.c          | 6 +++---
 kernel/bpf/trampoline.c       | 4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

Comments

Hao Luo June 30, 2022, 8:43 p.m. UTC | #1 
Hi Yafang,

On Wed, Jun 29, 2022 at 8:49 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> It doesn't need to do the preempt check when migrate is disabled
> after commit
> 74d862b682f5 ("sched: Make migrate_disable/enable() independent of RT").
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---

In my understanding, migrate_disable() doesn't imply
preempt_disable(), I think this is not safe. Am I missing something?

Hao
Yafang Shao July 2, 2022, 2:34 a.m. UTC | #2 
On Fri, Jul 1, 2022 at 4:43 AM Hao Luo <haoluo@google.com> wrote:
>
> Hi Yafang,
>
> On Wed, Jun 29, 2022 at 8:49 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > It doesn't need to do the preempt check when migrate is disabled
> > after commit
> > 74d862b682f5 ("sched: Make migrate_disable/enable() independent of RT").
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
>
> In my understanding, migrate_disable() doesn't imply
> preempt_disable(), I think this is not safe. Am I missing something?
>

It seems I have some misunderstanding of it after second thoughts.
I will think more about it.
diff mbox series

Patch

diff --git a/kernel/bpf/bpf_task_storage.c b/kernel/bpf/bpf_task_storage.c
index e9014dc62682..6f290623347e 100644
--- a/kernel/bpf/bpf_task_storage.c
+++ b/kernel/bpf/bpf_task_storage.c
@@ -26,20 +26,20 @@  static DEFINE_PER_CPU(int, bpf_task_storage_busy);
 static void bpf_task_storage_lock(void)
 {
 	migrate_disable();
-	__this_cpu_inc(bpf_task_storage_busy);
+	this_cpu_inc(bpf_task_storage_busy);
 }
 
 static void bpf_task_storage_unlock(void)
 {
-	__this_cpu_dec(bpf_task_storage_busy);
+	this_cpu_dec(bpf_task_storage_busy);
 	migrate_enable();
 }
 
 static bool bpf_task_storage_trylock(void)
 {
 	migrate_disable();
-	if (unlikely(__this_cpu_inc_return(bpf_task_storage_busy) != 1)) {
-		__this_cpu_dec(bpf_task_storage_busy);
+	if (unlikely(this_cpu_inc_return(bpf_task_storage_busy) != 1)) {
+		this_cpu_dec(bpf_task_storage_busy);
 		migrate_enable();
 		return false;
 	}
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 9d4559a1c032..6a3a95037aac 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -166,8 +166,8 @@  static inline int htab_lock_bucket(const struct bpf_htab *htab,
 	hash = hash & HASHTAB_MAP_LOCK_MASK;
 
 	migrate_disable();
-	if (unlikely(__this_cpu_inc_return(*(htab->map_locked[hash])) != 1)) {
-		__this_cpu_dec(*(htab->map_locked[hash]));
+	if (unlikely(this_cpu_inc_return(*(htab->map_locked[hash])) != 1)) {
+		this_cpu_dec(*(htab->map_locked[hash]));
 		migrate_enable();
 		return -EBUSY;
 	}
@@ -190,7 +190,7 @@  static inline void htab_unlock_bucket(const struct bpf_htab *htab,
 		raw_spin_unlock_irqrestore(&b->raw_lock, flags);
 	else
 		spin_unlock_irqrestore(&b->lock, flags);
-	__this_cpu_dec(*(htab->map_locked[hash]));
+	this_cpu_dec(*(htab->map_locked[hash]));
 	migrate_enable();
 }
 
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 93c7675f0c9e..f4486e54fdb3 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -585,7 +585,7 @@  u64 notrace __bpf_prog_enter(struct bpf_prog *prog, struct bpf_tramp_run_ctx *ru
 
 	run_ctx->saved_run_ctx = bpf_set_run_ctx(&run_ctx->run_ctx);
 
-	if (unlikely(__this_cpu_inc_return(*(prog->active)) != 1)) {
+	if (unlikely(this_cpu_inc_return(*(prog->active)) != 1)) {
 		inc_misses_counter(prog);
 		return 0;
 	}
@@ -631,7 +631,7 @@  u64 notrace __bpf_prog_enter_sleepable(struct bpf_prog *prog, struct bpf_tramp_r
 	migrate_disable();
 	might_fault();
 
-	if (unlikely(__this_cpu_inc_return(*(prog->active)) != 1)) {
+	if (unlikely(this_cpu_inc_return(*(prog->active)) != 1)) {
 		inc_misses_counter(prog);
 		return 0;
 	}