Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754767AbcCOF6e (ORCPT ); Tue, 15 Mar 2016 01:58:34 -0400 Received: from mga03.intel.com ([134.134.136.65]:2438 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753544AbcCOF6Y (ORCPT ); Tue, 15 Mar 2016 01:58:24 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,337,1455004800"; d="scan'208";a="937258828" Date: Tue, 15 Mar 2016 07:58:14 +0200 From: Jarkko Sakkinen To: Stefan Berger Cc: tpmdd-devel@lists.sourceforge.net, jgunthorpe@obsidianresearch.com, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v8 06/10] tpm: Replace device number bitmap with IDR Message-ID: <20160315055814.GA4947@intel.com> References: <1457909680-14085-1-git-send-email-stefanb@linux.vnet.ibm.com> <1457909680-14085-7-git-send-email-stefanb@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1457909680-14085-7-git-send-email-stefanb@linux.vnet.ibm.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6651 Lines: 226 On Sun, Mar 13, 2016 at 06:54:36PM -0400, Stefan Berger wrote: > Replace the device number bitmap with IDR. Extend the number of devices we > can create to 64k. > Since an IDR allows us to associate a pointer with an ID, we use this now > to rewrite tpm_chip_find_get() to simply look up the chip pointer by the > given device ID. > > Protect the IDR calls with a mutex. I've merged patches up to this to my next branch i.e. you don't have to carry them in the seres anymore. /Jarkko > Signed-off-by: Stefan Berger > Reviewed-by: Jason Gunthorpe > Reviewed-by: Jarkko Sakkinen > Tested-by: Jarkko Sakkinen > Signed-off-by: Jarkko Sakkinen > --- > drivers/char/tpm/tpm-chip.c | 84 +++++++++++++++++++++------------------- > drivers/char/tpm/tpm-interface.c | 1 + > drivers/char/tpm/tpm.h | 5 +-- > 3 files changed, 48 insertions(+), 42 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index 5880377..f62c851 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -29,9 +29,8 @@ > #include "tpm.h" > #include "tpm_eventlog.h" > > -static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES); > -static LIST_HEAD(tpm_chip_list); > -static DEFINE_SPINLOCK(driver_lock); > +DEFINE_IDR(dev_nums_idr); > +static DEFINE_MUTEX(idr_lock); > > struct class *tpm_class; > dev_t tpm_devt; > @@ -88,20 +87,30 @@ EXPORT_SYMBOL_GPL(tpm_put_ops); > */ > struct tpm_chip *tpm_chip_find_get(int chip_num) > { > - struct tpm_chip *pos, *chip = NULL; > + struct tpm_chip *chip, *res = NULL; > + int chip_prev; > + > + mutex_lock(&idr_lock); > + > + if (chip_num == TPM_ANY_NUM) { > + chip_num = 0; > + do { > + chip_prev = chip_num; > + chip = idr_get_next(&dev_nums_idr, &chip_num); > + if (chip && !tpm_try_get_ops(chip)) { > + res = chip; > + break; > + } > + } while (chip_prev != chip_num); > + } else { > + chip = idr_find_slowpath(&dev_nums_idr, chip_num); > + if (chip && !tpm_try_get_ops(chip)) > + res = chip; > + } > > - rcu_read_lock(); > - list_for_each_entry_rcu(pos, &tpm_chip_list, list) { > - if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num) > - continue; > + mutex_unlock(&idr_lock); > > - /* rcu prevents chip from being free'd */ > - if (!tpm_try_get_ops(pos)) > - chip = pos; > - break; > - } > - rcu_read_unlock(); > - return chip; > + return res; > } > > /** > @@ -114,9 +123,10 @@ static void tpm_dev_release(struct device *dev) > { > struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev); > > - spin_lock(&driver_lock); > - clear_bit(chip->dev_num, dev_mask); > - spin_unlock(&driver_lock); > + mutex_lock(&idr_lock); > + idr_remove(&dev_nums_idr, chip->dev_num); > + mutex_unlock(&idr_lock); > + > kfree(chip); > } > > @@ -142,21 +152,18 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev, > > mutex_init(&chip->tpm_mutex); > init_rwsem(&chip->ops_sem); > - INIT_LIST_HEAD(&chip->list); > > chip->ops = ops; > > - spin_lock(&driver_lock); > - chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES); > - spin_unlock(&driver_lock); > - > - if (chip->dev_num >= TPM_NUM_DEVICES) { > + mutex_lock(&idr_lock); > + rc = idr_alloc(&dev_nums_idr, NULL, 0, TPM_NUM_DEVICES, GFP_KERNEL); > + mutex_unlock(&idr_lock); > + if (rc < 0) { > dev_err(dev, "No available tpm device numbers\n"); > kfree(chip); > - return ERR_PTR(-ENOMEM); > + return ERR_PTR(rc); > } > - > - set_bit(chip->dev_num, dev_mask); > + chip->dev_num = rc; > > device_initialize(&chip->dev); > > @@ -242,19 +249,28 @@ static int tpm_add_char_device(struct tpm_chip *chip) > return rc; > } > > + /* Make the chip available. */ > + mutex_lock(&idr_lock); > + idr_replace(&dev_nums_idr, chip, chip->dev_num); > + mutex_unlock(&idr_lock); > + > return rc; > } > > static void tpm_del_char_device(struct tpm_chip *chip) > { > cdev_del(&chip->cdev); > + device_del(&chip->dev); > + > + /* Make the chip unavailable. */ > + mutex_lock(&idr_lock); > + idr_replace(&dev_nums_idr, NULL, chip->dev_num); > + mutex_unlock(&idr_lock); > > /* Make the driver uncallable. */ > down_write(&chip->ops_sem); > chip->ops = NULL; > up_write(&chip->ops_sem); > - > - device_del(&chip->dev); > } > > static int tpm1_chip_register(struct tpm_chip *chip) > @@ -309,11 +325,6 @@ int tpm_chip_register(struct tpm_chip *chip) > if (rc) > goto out_err; > > - /* Make the chip available. */ > - spin_lock(&driver_lock); > - list_add_tail_rcu(&chip->list, &tpm_chip_list); > - spin_unlock(&driver_lock); > - > chip->flags |= TPM_CHIP_FLAG_REGISTERED; > > if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { > @@ -350,11 +361,6 @@ void tpm_chip_unregister(struct tpm_chip *chip) > if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED)) > return; > > - spin_lock(&driver_lock); > - list_del_rcu(&chip->list); > - spin_unlock(&driver_lock); > - synchronize_rcu(); > - > if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) > sysfs_remove_link(&chip->dev.parent->kobj, "ppi"); > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index 5caf154..5397b64 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -1139,6 +1139,7 @@ static int __init tpm_init(void) > > static void __exit tpm_exit(void) > { > + idr_destroy(&dev_nums_idr); > class_destroy(tpm_class); > unregister_chrdev_region(tpm_devt, TPM_NUM_DEVICES); > } > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index 5fcf788..928b47f 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -34,7 +34,7 @@ > enum tpm_const { > TPM_MINOR = 224, /* officially assigned */ > TPM_BUFSIZE = 4096, > - TPM_NUM_DEVICES = 256, > + TPM_NUM_DEVICES = 65536, > TPM_RETRY = 50, /* 5 seconds */ > }; > > @@ -195,8 +195,6 @@ struct tpm_chip { > acpi_handle acpi_dev_handle; > char ppi_version[TPM_PPI_VERSION_LEN + 1]; > #endif /* CONFIG_ACPI */ > - > - struct list_head list; > }; > > #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) > @@ -492,6 +490,7 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value) > extern struct class *tpm_class; > extern dev_t tpm_devt; > extern const struct file_operations tpm_fops; > +extern struct idr dev_nums_idr; > > ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *); > ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf, > -- > 2.4.3 >