Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Qian Cai <cai@lca.pw>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH] x86/resctrl: fix an imbalance in domain_remove_cpu
Date:   Tue, 10 Dec 2019 07:11:37 -0500
Message-Id: <A6F550BE-D7F6-4FE1-8FED-85AA69835FFB@lca.pw>
References: <CADjb_WRmyE3rRN2sLAh9ZOqAg0E3WeCkj9SwSM9dorvx4TGC2A@mail.gmail.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Reinette Chatre <reinette.chatre@intel.com>,
        Fenghua Yu <fenghua.yu@intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        John Stultz <john.stultz@linaro.org>, sboyd@kernel.org,
        Tony Luck <tony.luck@intel.com>, tj@kernel.org,
        the arch/x86 maintainers <x86@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
In-Reply-To: <CADjb_WRmyE3rRN2sLAh9ZOqAg0E3WeCkj9SwSM9dorvx4TGC2A@mail.gmail.com>
To:     Ryan Chen <yu.chen.surf@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Dec 10, 2019, at 2:44 AM, Ryan Chen <yu.chen.surf@gmail.com> wrote:
>=20
> Could you elaborate a little more on the failure symptom?
> If I understand correctly, the error you described was  due to
> r->mon_capable set to false while is_mbm_enabled() returns true?

Yes.

> Which means on this platform rdt_mon_features is non zero?

No idea. I did add some debug code that indicated resctrl_online_cpu() found=
 3 resources in for_each_capable_rdt_resource(r). Only the first one set r->=
mon_capable.

> And in get_rdt_mon_resources() it will invoke rdt_get_mon_l3_config(),
> however the only possible failure to do not set r->mon_capable is that it
> failed in dom_data_init() due to kcalloc() failure?  Then the logic in

Very likely. Should be easy to confirm.

> get_rdt_resources() is that it will ignore the return error if rdt allocat=
e
> feature is supported on this platform?  If

Yes.

> this is the case, the r->mon_capable
> is not an indicator for whether the overflow thread has been created, righ=
t?

I am not sure about that.

> Can we simply remove the check of r->mon_capable in domain_add_cpu() and
> invoke  domain_setup_mon_state() directly?

That should work too, but it is so perfect align with the r->alloc_capable c=
heck above, so I am not sure it is a good idea to break it.

> Humm, it looks like there are two places within this function
> invoked cancel_delayed_work(&d->mbm_over),
> why not adding the check for both of them?

Because I am not sure about the second one.  It was never executed due to =E2=
=80=9Ccpu !=3D d->mbm_work_cpu=E2=80=9C even after offlining all CPUs except=
 cpu 0 and never cause anything wrong yet, so I could not test it yet, but I=
 can see why it might need a similar check too if d->mbm_work_cpu is non-zer=
o and could trigger the same imbalance.=