Received-SPF: pass (google.com: domain of linux-kernel+bounces-48983-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Fri, 02 Feb 2024 01:18:59 +0200
Message-Id: <CYU4KPJIDWTI.1ZPBLGOCZR0BK@suppilovahvero>
Cc: "LinoSanfilippo@gmx.de" <LinoSanfilippo@gmx.de>,
 "p.rosenberger@kunbus.com" <p.rosenberger@kunbus.com>, "lukas@wunner.de"
 <lukas@wunner.de>, "jgg@ziepe.ca" <jgg@ziepe.ca>,
 "linux-integrity@vger.kernel.org" <linux-integrity@vger.kernel.org>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
 "stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH] tpm,tpm_tis: Avoid warning splat at shutdown
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Elliott, Robert (Servers)" <elliott@hpe.com>, "Lino Sanfilippo"
 <l.sanfilippo@kunbus.com>, "peterhuewe@gmx.de" <peterhuewe@gmx.de>
References: <20240201113646.31734-1-l.sanfilippo@kunbus.com>
 <MW5PR84MB184274E28D83DC337B486CEFAB432@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>
In-Reply-To: <MW5PR84MB184274E28D83DC337B486CEFAB432@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>

On Thu Feb 1, 2024 at 6:40 PM EET, Elliott, Robert (Servers) wrote:
> > From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> > Sent: Thursday, February 1, 2024 5:37 AM
> > Subject: [PATCH] tpm,tpm_tis: Avoid warning splat at shutdown
> >=20
> > If interrupts are not activated the work struct 'free_irq_work' is not
> > initialized. This results in a warning splat at module shutdown.
> >=20
> > Fix this by always initializing the work regardless of whether interrup=
ts
> > are activated or not.
>
> That's using flush_work(), which just waits for one to complete. Is there
> any case where multiple work entries could be queued, and cancel_work_syn=
c()=20
> would be necessary?

Questions are cool but please explain how this aligns with the patch
review because I already accepted the patch.

Should I drop it based on this question, and if so, why?

> tpm_tis_probe_irq() has a loop calling tpm_tis_probe_irq_single()
> for 3 to 15. Could each of those could trigger an interrupt storm and
> call tpm_tis_revert_interrupts(), which calls schedule_work()?

AFAIK no based on that TPM_CHIP_FLAG_IRQ should take care of this.

BR, Jarkko