Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: ibe194615:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-441-ga3ab13cd6d-fm-20230517.001-ga3ab13cd
Mime-Version: 1.0
Message-Id: <f69305de-b3f1-46ee-b8b5-a20729c8c1bb@app.fastmail.com>
In-Reply-To: <27c7824e-ec90-c68f-3e76-92525ed7e393@linux.intel.com>
References: <mpearson-lenovo@squebb.ca>
 <20230526171658.3886-1-mpearson-lenovo@squebb.ca>
 <20230526171658.3886-5-mpearson-lenovo@squebb.ca>
 <27c7824e-ec90-c68f-3e76-92525ed7e393@linux.intel.com>
Date:   Mon, 29 May 2023 10:03:02 -0400
From:   "Mark Pearson" <mpearson-lenovo@squebb.ca>
To:     =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc:     "Hans de Goede" <hdegoede@redhat.com>,
        "markgross@kernel.org" <markgross@kernel.org>,
        "platform-driver-x86@vger.kernel.org" 
        <platform-driver-x86@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 5/5] platform/x86: think-lmi: mutex protection around multiple
 WMI calls
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Ilpo,

On Mon, May 29, 2023, at 8:23 AM, Ilpo J=C3=A4rvinen wrote:
> On Fri, 26 May 2023, Mark Pearson wrote:
>
>> Add mutex protection around cases where an operation needs multiple
>> WMI calls - e.g. setting password.
>
> So you need this feature already for Patch 1/5? If that's the case, yo=
u=20
> should reorder the patches and put it before 1/5.

You're right....I was being lazy and just adding it to the end of the se=
ries as it was easier. I can re-order.

As a side note, the chances of two people changing things on a system at=
 the same time is rather unlikely - it just doesn't make sense as it's s=
omething done by an administrator. But a fix is a fix.

>
> That "e.g. setting password" sounds vague enough that I'm left to wond=
er=20
> if there are other cases in the driver which need locking too. It woul=
d be=20
> useful to be precise with wording here. It will help immensely when=20
> somebody looks this changelog 5 years from now if you explain all case=
s=20
> that need locking up front.

OK. There are two cases and I can list both cases explicitly.

>
> So, is this needed also for some existing code, then Fixes tag might b=
e in=20
> order? (I'm looking e.g. that cert auth block in current_value_store()=20
> which also does more than one call).

True - I can add that.=20

Thanks for the review. I'll hold off a couple of days before making thos=
e changes in case there is any feedback

Mark