Date: Mon, 14 Nov 2016 21:11:54 -0800
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: tpmdd-devel@lists.sourceforge.net,
        linux-security-module@vger.kernel.org, Peter Huewe <peterhuewe@gmx.de>,
        Marcel Selhorst <tpmdd@selhorst.net>,
        Christophe Ricard <christophe.ricard@gmail.com>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tpm: drop chip->is_open and chip->duration_adjusted
Message-ID: <20161115051106.u2xoduwf2kpcznv3@intel.com>
References: <20161114234500.24839-1-jarkko.sakkinen@linux.intel.com>
 <20161115043001.GA22482@obsidianresearch.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20161115043001.GA22482@obsidianresearch.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
User-Agent: Mutt/1.6.2-neo (2016-08-21)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1036
Lines: 27

On Mon, Nov 14, 2016 at 09:30:01PM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 14, 2016 at 03:44:58PM -0800, Jarkko Sakkinen wrote:
> > Use atomic bitops for chip->flags so that we do not need chip->is_open
> > and chip->duration_adjusted anymore.
> 
> I don't know if it s a really great idea to use atomic bit ops for
> things that do not need to be atomic.. It makes the locking scheme
> less clear. is open is genuinely different since it relies on the
> atomic for correctness.

The way I see it is one of the status flags bound to chip among the
others. I do not see this cause too much harm for clarity. It eases
debugging the driver a bit because you get more state out of 'flags'.

It also makes code little a bit more robust as flags is independent of
locks.

How strong is your opposition here? I do not see any exceptional damage
done but see some subtle but still significant benefits.

> Merging is_duration makes lots of sense though

Also timeout_adjusted should be merged (for some reason missed it).

> Jason

/Jarkko