Hi,
On 12/14/2022 6:38 PM, xiangxia.m.yue@xxxxxxxxx wrote:
> From: Tonghao Zhang <xiangxia.m.yue@xxxxxxxxx>
>
> The deadlock still may occur while accessed in NMI and non-NMI
> context. Because in NMI, we still may access the same bucket but with
> different map_locked index.
>
> For example, on the same CPU, .max_entries = 2, we update the hash map,
> with key = 4, while running bpf prog in NMI nmi_handle(), to update
> hash map with key = 20, so it will have the same bucket index but have
> different map_locked index.
>
> To fix this issue, using min mask to hash again.
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@xxxxxxxxx>
> Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
> Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> Cc: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Cc: Martin KaFai Lau <martin.lau@xxxxxxxxx>
> Cc: Song Liu <song@xxxxxxxxxx>
> Cc: Yonghong Song <yhs@xxxxxx>
> Cc: John Fastabend <john.fastabend@xxxxxxxxx>
> Cc: KP Singh <kpsingh@xxxxxxxxxx>
> Cc: Stanislav Fomichev <sdf@xxxxxxxxxx>
> Cc: Hao Luo <haoluo@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Hou Tao <houtao1@xxxxxxxxxx>
> ---
> kernel/bpf/hashtab.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 5aa2b5525f79..8b25036a8690 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -152,7 +152,7 @@ static inline int htab_lock_bucket(const struct bpf_htab *htab,
> {
> unsigned long flags;
>
> - hash = hash & HASHTAB_MAP_LOCK_MASK;
> + hash = hash & min(HASHTAB_MAP_LOCK_MASK, htab->n_buckets -1);
There is warning for kernel test robot and it seems that min_t(...) is required
here.
Otherwise, this patch looks good to me, so:
Acked-by: Hou Tao <houtao1@xxxxxxxxxx>
>
> preempt_disable();
> if (unlikely(__this_cpu_inc_return(*(htab->map_locked[hash])) != 1)) {
> @@ -171,7 +171,7 @@ static inline void htab_unlock_bucket(const struct bpf_htab *htab,
> struct bucket *b, u32 hash,
> unsigned long flags)
> {
> - hash = hash & HASHTAB_MAP_LOCK_MASK;
> + hash = hash & min(HASHTAB_MAP_LOCK_MASK, htab->n_buckets -1);
> raw_spin_unlock_irqrestore(&b->raw_lock, flags);
> __this_cpu_dec(*(htab->map_locked[hash]));
> preempt_enable();
