From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759036AbbGHVLi (ORCPT ); Wed, 8 Jul 2015 17:11:38 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:43429 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758726AbbGHVLa (ORCPT ); Wed, 8 Jul 2015 17:11:30 -0400 Date: Wed, 8 Jul 2015 14:11:29 -0700 From: Greg Kroah-Hartman To: Jason Gunthorpe Cc: Jens Wiklander , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, Arnd Bergmann , Rob Herring , Herbert Xu , valentin.manea@huawei.com, jean-michel.delorme@st.com, emmanuel.michel@st.com, javier@javigon.com, Mark Rutland , Michal Simek Subject: Re: [PATCH v4 3/5] tee: generic TEE subsystem Message-ID: <20150708211129.GA29824@kroah.com> References: <1436350592-7732-1-git-send-email-jens.wiklander@linaro.org> <1436350592-7732-4-git-send-email-jens.wiklander@linaro.org> <20150708171026.GA11740@obsidianresearch.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150708171026.GA11740@obsidianresearch.com> User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 08, 2015 at 11:10:26AM -0600, Jason Gunthorpe wrote: > On Wed, Jul 08, 2015 at 12:16:30PM +0200, Jens Wiklander wrote: > > > +static void tee_device_complete_unused(struct kref *kref) > > +{ > > + struct tee_device *teedev; > > + > > + teedev = container_of(kref, struct tee_device, users); > > + /* When the mutex is released, no other tee_device_get() will succeed */ > > + teedev->desc = NULL; > > + complete(&teedev->c_no_users); > > +} > > + > > +void tee_device_put(struct tee_device *teedev) > > +{ > > + mutex_lock(&teedev->mutex); > > + /* Shouldn't put in this state */ > > + if (!WARN_ON(!teedev->desc)) > > + kref_put(&teedev->users, tee_device_complete_unused); > > + mutex_unlock(&teedev->mutex); > > +} > > + > > +bool tee_device_get(struct tee_device *teedev) > > +{ > > + mutex_lock(&teedev->mutex); > > + if (!teedev->desc) { > > + mutex_unlock(&teedev->mutex); > > + return false; > > + } > > + kref_get(&teedev->users); > > + mutex_unlock(&teedev->mutex); > > + return true; > > +} > > If you are holding the mutex then you don't really need a kref, just a > simple active count counter. > > I've been a bit learly lately about seeing krefs used for something > other than kfree, I've seen a few subtle mistakes in those schemes - > yours looks OK, only because of the lock, and the lock makes the kref > redundant.. > > > + cdev_init(&teedev->cdev, &tee_fops); > > + teedev->cdev.owner = teedesc->owner; > > This also needs to set teedev->cdev.kobj.parent. > I'm guessing: > > teedev->cdev.kobj.parent = &teedev->dev.kobj; > > TPM had the same mistake.. Really? As of a few years ago, A cdev's kobject should not be touched by anything other than the cdev core. It's not a "real" kobject in that it is never registered in sysfs, and no one sees it. I keep meaning to just use something else one of these days for that structure, as lots of people get it wrong. Or has things changed there? thanks, greg k-h