linux-hyperv.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Kelley <mikelley@microsoft.com>
To: "תומר אבוטבול" <tomer432100@gmail.com>,
	"David Mozes" <david.mozes@silk.us>
Cc: David Moses <mosesster@gmail.com>,
	"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] x86/hyper-v: guard against cpu mask changes in hyperv_flush_tlb_others()
Date: Fri, 6 Aug 2021 21:51:35 +0000	[thread overview]
Message-ID: <MWHPR21MB15935468547C25294A253E0AD7F39@MWHPR21MB1593.namprd21.prod.outlook.com> (raw)
In-Reply-To: <CAHkVu0-ZCXDRZL92d_G3oKpPuKvmY=YEbu9nbx9vkZHnhHFD8Q@mail.gmail.com>

From: תומר אבוטבול <tomer432100@gmail.com>  Sent: Friday, August 6, 2021 11:03 AM

> Attaching the patches Michael asked for debugging 
> 1) Print the cpumask when < num_possible_cpus():
> diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
> index e666f7eaf32d..620f656d6195 100644
> --- a/arch/x86/hyperv/mmu.c
> +++ b/arch/x86/hyperv/mmu.c
> @@ -60,6 +60,7 @@ static void hyperv_flush_tlb_others(const struct cpumask *cpus,
>         struct hv_tlb_flush *flush;
>         u64 status = U64_MAX;
>         unsigned long flags;
> +       unsigned int cpu_last;
>
>        trace_hyperv_mmu_flush_tlb_others(cpus, info);
>
> @@ -68,6 +69,11 @@ static void hyperv_flush_tlb_others(const struct cpumask *cpus,
>
>        local_irq_save(flags);
>
> +       cpu_last = cpumask_last(cpus);
> +       if (cpu_last > num_possible_cpus()) {

I think this should be ">=" since cpus are numbered starting at zero.
In your VM with 64 CPUs, having CPU #64 in the list would be error.

> +               pr_emerg("ERROR_HYPERV: cpu_last=%*pbl", cpumask_pr_args(cpus));
> +       }
> +
>        /*
>         * Only check the mask _after_ interrupt has been disabled to avoid the
>         * mask changing under our feet.
>
> 2) disable the Hyper-V specific flush routines:
> diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
> index e666f7eaf32d..8e77cc84775a 100644
> --- a/arch/x86/hyperv/mmu.c
> +++ b/arch/x86/hyperv/mmu.c
> @@ -235,6 +235,7 @@ static u64 hyperv_flush_tlb_others_ex(const struct cpumask *cpus,
>
> void hyperv_setup_mmu_ops(void)
> {
> +  return;
>        if (!(ms_hyperv.hints & HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED))
>                return;

Otherwise, this code looks good to me and matches what I had in mind.

Note that the function native_flush_tlb_others() is used when the Hyper-V specific
flush function is disabled per patch #2 above, or when hv_cpu_to_vp_index() returns
VP_INVALID.  In a quick glance through the code, it appears that native_flush_tlb_others()
will work even if there's a non-existent CPU in the cpumask that is passed as an argument.
So perhaps an immediate workaround is Patch #2 above.  

Perhaps hyperv_flush_tlb_others() should be made equally tolerant of a non-existent
CPU being in the list. But if you are willing, I'm still interested in the results of an
experiment with just Patch #1.  I'm curious about what the CPU list looks like when
it has a non-existent CPU.  Is it complete garbage, or is there just one non-existent
CPU?

The other curiosity is that I haven't seen this Linux panic reported by other users,
and I think it would have come to our attention if it were happening with any frequency.
You see the problem fairly regularly.  So I'm wondering what the difference is.

Michael

  parent reply	other threads:[~2021-08-06 21:51 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CA+qYZY3a-FHfWNL2=na6O8TRJYu9kaeyp80VNDxaDTi2EBGoog@mail.gmail.com>
2021-08-06 10:43 ` Michael Kelley
2021-08-06 17:35   ` David Mozes
     [not found]     ` <CAHkVu0-ZCXDRZL92d_G3oKpPuKvmY=YEbu9nbx9vkZHnhHFD8Q@mail.gmail.com>
2021-08-06 21:51       ` Michael Kelley [this message]
2021-08-07  5:00         ` David Moses
2021-08-17  9:16           ` David Mozes
2021-08-17 11:29             ` Wei Liu
2021-08-19 11:05               ` David Mozes
     [not found]               ` <CA+qYZY1U04SkyHo7X+rDeE=nUy_X5nxLfShyuLJFzXnFp2A6uw@mail.gmail.com>
     [not found]                 ` <VI1PR0401MB24153DEC767B0126B1030E07F1C09@VI1PR0401MB2415.eurprd04.prod.outlook.com>
2021-08-22 15:24                   ` Wei Liu
2021-08-22 16:25                     ` David Mozes
2021-08-22 17:32                       ` Wei Liu
     [not found] <VI1PR0401MB24150B31A1D63176BBB788D2F1F19@VI1PR0401MB2415.eurprd04.prod.outlook.com>
2021-08-05 18:08 ` Michael Kelley
2020-10-01  1:38 Sasha Levin
2020-10-01  9:40 ` Vitaly Kuznetsov
2020-10-01 11:53   ` Wei Liu
2020-10-01 13:04     ` Sasha Levin
2020-10-03 17:40       ` Michael Kelley
2020-10-05 14:58         ` Wei Liu
2021-01-05 16:59           ` Michael Kelley
2021-01-05 17:10             ` Wei Liu
2021-01-08 15:22             ` Sasha Levin
2020-10-01 13:10     ` Vitaly Kuznetsov

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=MWHPR21MB15935468547C25294A253E0AD7F39@MWHPR21MB1593.namprd21.prod.outlook.com \
    --to=mikelley@microsoft.com \
    --cc=david.mozes@silk.us \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mosesster@gmail.com \
    --cc=tomer432100@gmail.com \
    --subject='RE: [PATCH] x86/hyper-v: guard against cpu mask changes in hyperv_flush_tlb_others()' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).