Skip to contentSkip to conversation

Correct: fix staging/spectra: don't use locked_ioctl

PATCH In review Javier Martinez Canillas 2 participants 3 messages Jul 10, 2010 – Jul 22, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 3/3 Correct: fix staging/spectra: don't use locked_ioctl

Last patch has a style problem. Sending the correct one. Sorry for the noise

Since BKL was removed from block ioctl handling code, locked_ioctl doesn't exist anymore.

Using ioctl instead and doing the locking manually.

From b2bfc3da2c83066ad773df81787fc9416f5382c2 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <[email protected]>
Date: Sat, 10 Jul 2010 00:00:16 -0400
Subject: [PATCH 3/3] fix staging/spectra: don't use locked_ioctl

---
 drivers/staging/spectra/ffsport.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)
drivers/staging/spectra/ffsport.c +14 -1
index 00a0ac0..7b34635 100644
@@ -27,6 +27,7 @@
 #include <linux/kthread.h>
 #include <linux/log2.h>
 #include <linux/init.h>
+#include <linux/smp_lock.h>
 
 /**** Helper functions used for Div, Remainder operation on u64 ****/
 
@@ -589,11 +590,23 @@ int GLOB_SBD_ioctl(struct block_device *bdev, fmode_t mode,
 	return -ENOTTY;
 }
 
+int GLOB_SBD_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+		unsigned int cmd, unsigned long arg)
+{
+	int ret;
+
+	lock_kernel();
+	ret = GLOB_SBD_ioctl(bdev, mode, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 static struct block_device_operations GLOB_SBD_ops = {
 	.owner = THIS_MODULE,
 	.open = GLOB_SBD_open,
 	.release = GLOB_SBD_release,
-	.locked_ioctl = GLOB_SBD_ioctl,
+	.ioctl = GLOB_SBD_unlocked_ioctl,
 	.getgeo = GLOB_SBD_getgeo,
 };
 
Signature · 1 line
1.7.0.4
#1 lore raw

Re: [PATCH 3/3] Correct: fix staging/spectra: don't use

Quoted 31 lines · Javier Martinez Canillas

Last patch has a style problem. Sending the correct one. Sorry for the noise

Since BKL was removed from block ioctl handling code, locked_ioctl doesn't exist anymore.

Using ioctl instead and doing the locking manually.

Quoted 1 line

From b2bfc3da2c83066ad773df81787fc9416f5382c2 Mon Sep 17 00:00:00 2001

From: Javier Martinez Canillas <[email protected]>
Date: Sat, 10 Jul 2010 00:00:16 -0400
Subject: [PATCH 3/3] fix staging/spectra: don't use locked_ioctl

---
 drivers/staging/spectra/ffsport.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)
drivers/staging/spectra/ffsport.c +1 -0
index 00a0ac0..7b34635 100644
@@ -27,6 +27,7 @@
 #include <linux/kthread.h>
 #include <linux/log2.h>
 #include <linux/init.h>
+#include <linux/smp_lock.h>
 
 /**** Helper functions used for Div, Remainder operation on u64 ****/
 

drivers/staging/spectra/ffsport.c int GLOB_SBD_ioctl(struct block_device *bdev, fmode_t mode, · line 598

Quoted 9 lines
drivers/staging/spectra/ffsport.c +6 -0
@@ -589,11 +590,23 @@ int GLOB_SBD_ioctl(struct block_device *bdev, fmode_t mode,
 	return -ENOTTY;
 }
 
+int GLOB_SBD_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+		unsigned int cmd, unsigned long arg)
+{
+	int ret;
+
+	lock_kernel();

No, we are trying to get rid of the lock_kernel() calls, care to just make this a local lock instead?

thanks,

greg k-h

#2 lore raw
In reply to Javier Martinez Canillas
Quoted 5 lines · Greg KH
Quoted 1 line

+     lock_kernel();

No, we are trying to get rid of the lock_kernel() calls, care to just make this a local lock instead?

Since I'm not the driver author I thought that the idea of moving the BKL from block ioctl to the driver code would be a first step to get rid of the lock_kernel() call.

This patch at least could make the driver compile again and someone who better understands the driver code could make a finner (local) lock.

But I will figure out what needs to be protect and send a new patch that uses some sincronization primitive to make a local lock.

Thanks a lot.

-----------------------------------------
Javier Martínez Canillas
+595 981 88 66 58