Re: [PATCH v7 07/10] tmp, tmp_tis: Implement usage counter for locality

From: Jarkko Sakkinen <jarkko@kernel.org>
To: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Cc: peterhuewe@gmx.de, jgg@ziepe.ca, stefanb@linux.vnet.ibm.com,
	linux@mniewoehner.de, linux-integrity@vger.kernel.org,
	linux-kernel@vger.kernel.org, l.sanfilippo@kunbus.com,
	lukas@wunner.de, p.rosenberger@kunbus.com
Subject: Re: [PATCH v7 07/10] tmp, tmp_tis: Implement usage counter for locality
Date: Mon, 11 Jul 2022 05:50:58 +0300	[thread overview]
Message-ID: <YsuQEoVuVa00gIdE@kernel.org> (raw)
In-Reply-To: <f0e33bc4-335c-322a-9295-18d6bc0b8286@gmx.de>

On Mon, Jul 04, 2022 at 07:45:12PM +0200, Lino Sanfilippo wrote:
> 
> 
> On 01.07.22 01:29, Jarkko Sakkinen wrote:
> 
> >
> > I'm kind of thinking that should tpm_tis_data have a lock for its
> > contents?
> 
> Most of the tpm_tis_data structure elements are set once during init and
> then never changed but only read. So no need for locking for these. The
> exceptions I see are
> 
> - flags
> - locality_count
> - locality

I'd still go for single data struct lock, since this lock would
be taken in every transmit flow. It makes the whole thing easier
to maintain over time, and does not really affect scalability.

This brings me to another question: what does this lock protect
against given that tpm_try_get_ops() already takes tpm_mutex?
It's not clear and that should be somehow reasoned in the commit
message.

Anyway, *if* a lock is needed the granularity should be the whole
struct.

BR, Jarkko