TPM: update char dev BKL pushdown
PATCH 1/1 TPM: update char dev BKL pushdown
This patch removes the BKL calls from the TPM driver, which were added in the overall misc-char-dev-BKL-pushdown.patch, as they are not needed. Changed num_opens from an int to atomic_t.
--- drivers/char/tpm/tpm.c | 22 +++++++--------------- drivers/char/tpm/tpm.h | 2 +- 2 files changed, 8 insertions(+), 16 deletions(-)
drivers/char/tpm/tpm.c +7 -15
@@ -960,7 +960,6 @@ int tpm_open(struct inode *inode, struct file *file)int rc =0 , minor =iminor (inode);struct tpm_chip *chip =NULL , *pos; -lock_kernel ();spin_lock (&driver_lock);list_for_each_entry (pos, &tpm_chip_list, list) {
@@ -975,34 +974,31 @@ int tpm_open(struct inode *inode, struct file *file) goto err_out; } -if (chip->num_opens) { +if (atomic_read (&chip->num_opens)) {dev_dbg (chip->dev,"Another process owns this TPM\n" ); rc = -EBUSY ; goto err_out; } - chip->num_opens++; +atomic_inc (&chip->num_opens);get_device (chip->dev);spin_unlock (&driver_lock); chip->data_buffer =kmalloc (TPM_BUFSIZE *sizeof (u8),GFP_KERNEL );if (chip->data_buffer ==NULL ) { - chip->num_opens--; +atomic_dec (&chip->num_opens);put_device (chip->dev); -unlock_kernel ();return -ENOMEM ; }atomic_set (&chip->data_pending,0 ); file->private_data = chip; -unlock_kernel ();return 0 ; err_out:spin_unlock (&driver_lock); -unlock_kernel ();return rc; }EXPORT_SYMBOL_GPL (tpm_open);
@@ -1016,7 +1012,7 @@ int tpm_release(struct inode *inode, struct file *file) file->private_data =NULL ;del_singleshot_timer_sync (&chip->user_read_timer);atomic_set (&chip->data_pending,0 ); - chip->num_opens--; +atomic_dec (chip->num_opens);put_device (chip->dev);kfree (chip->data_buffer);spin_unlock (&driver_lock);
@@ -1231,20 +1227,16 @@ struct tpm_chip *tpm_register_hardware(struct device *dev, const struct tpm_vendreturn NULL ; } -spin_lock (&driver_lock); - -list_add (&chip->list, &tpm_chip_list); - -spin_unlock (&driver_lock); -if (sysfs_create_group (&dev->kobj, chip->vendor.attr_group)) { -list_del (&chip->list);misc_deregister (&chip->vendor.miscdev);put_device (chip->dev);return NULL ; } chip->bios_dir =tpm_bios_log_setup (devname); +spin_lock (&driver_lock); +list_add (&chip->list, &tpm_chip_list); +spin_unlock (&driver_lock);return chip; }
drivers/char/tpm/tpm.h +1 -1
@@ -90,7 +90,7 @@ struct tpm_chip {struct device *dev;/* Device stuff */ int dev_num;/* /dev/tpm# */ -int num_opens;/* only one allowed */ +atomic_t num_opens;/* only one allowed */ int time_expired;/* Data passed to and from the tpm via the read/write calls */
Signature · 1 line
1.5.4.5
Quoted 6 lines · Rajiv Andrade
This patch removes the BKL calls from the TPM driver, which were added in the overall misc-char-dev-BKL-pushdown.patch, as they are not needed. Changed
num_opensfrom an int toatomic_t.Signed-off-by Mimi Zohar <[email protected]>Signed-off-by Rajiv Andrade <[email protected]>
Yes, this patch is good.
It would also be good to rename num_opens. Note that it is always either 0 or 1, and indicates whether someone has opened the chip. So 'is_open' may make more sense.
Mimi is also working on a patch (on top of this one) to use rcu to protect chip->list, and that patch will (I hope) include an explanation of all of the locking/protection involved.
Quoted 105 lines
--- drivers/char/tpm/tpm.c | 22 +++++++--------------- drivers/char/tpm/tpm.h | 2 +- 2 files changed, 8 insertions(+), 16 deletions(-)drivers/char/tpm/tpm.c +7
-15@@ -960,7 +960,6 @@ int tpm_open(struct inode *inode, struct file *file)int rc =0 , minor =iminor (inode);struct tpm_chip *chip =NULL , *pos; -lock_kernel ();spin_lock (&driver_lock);list_for_each_entry (pos, &tpm_chip_list, list) {@@ -975,34 +974,31 @@ int tpm_open(struct inode *inode, struct file *file) goto err_out; } -if (chip->num_opens) { +if (atomic_read (&chip->num_opens)) {dev_dbg (chip->dev,"Another process owns this TPM\n" ); rc = -EBUSY ; goto err_out; } - chip->num_opens++; +atomic_inc (&chip->num_opens);get_device (chip->dev);spin_unlock (&driver_lock); chip->data_buffer =kmalloc (TPM_BUFSIZE *sizeof (u8),GFP_KERNEL );if (chip->data_buffer ==NULL ) { - chip->num_opens--; +atomic_dec (&chip->num_opens);put_device (chip->dev); -unlock_kernel ();return -ENOMEM ; }atomic_set (&chip->data_pending,0 ); file->private_data = chip; -unlock_kernel ();return 0 ; err_out:spin_unlock (&driver_lock); -unlock_kernel ();return rc; }EXPORT_SYMBOL_GPL (tpm_open);@@ -1016,7 +1012,7 @@ int tpm_release(struct inode *inode, struct file *file) file->private_data =NULL ;del_singleshot_timer_sync (&chip->user_read_timer);atomic_set (&chip->data_pending,0 ); - chip->num_opens--; +atomic_dec (chip->num_opens);put_device (chip->dev);kfree (chip->data_buffer);spin_unlock (&driver_lock);@@ -1231,20 +1227,16 @@ struct tpm_chip *tpm_register_hardware(struct device *dev, const struct tpm_vendreturn NULL ; } -spin_lock (&driver_lock); - -list_add (&chip->list, &tpm_chip_list); - -spin_unlock (&driver_lock); -if (sysfs_create_group (&dev->kobj, chip->vendor.attr_group)) { -list_del (&chip->list);misc_deregister (&chip->vendor.miscdev);put_device (chip->dev);return NULL ; } chip->bios_dir =tpm_bios_log_setup (devname); +spin_lock (&driver_lock); +list_add (&chip->list, &tpm_chip_list); +spin_unlock (&driver_lock);return chip; }drivers/char/tpm/tpm.h +1
-1@@ -90,7 +90,7 @@ struct tpm_chip {struct device *dev;/* Device stuff */ int dev_num;/* /dev/tpm# */ -int num_opens;/* only one allowed */ +atomic_t num_opens;/* only one allowed */ int time_expired;/* Data passed to and from the tpm via the read/write calls */ Signature · 1 line
1.5.4.5
Quoted 5 lines · Serge E. Hallyn
Quoted 4 lines · Rajiv Andrade
This patch removes the BKL calls from the TPM driver, which were added in the overall misc-char-dev-BKL-pushdown.patch, as they are not needed. Changed
num_opensfrom an int toatomic_t.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Mimi Zohar <[email protected]>Signed-off-by Rajiv Andrade <[email protected]>Yes, this patch is good.
It would also be good to rename
num_opens. Note that it is always either 0 or 1, and indicates whether someone has opened the chip. So 'is_open' may make more sense.
And use atomic_set() instead of atomic_inc().
- James
Signature · 2 lines
James Morris <[email protected]>