[KJ] linux-2.6.15-rc7/drivers/pci/syscall.c

[KJ] linux-2.6.15-rc7/drivers/pci/syscall.c

[carlos neira]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

Greetings :

patch :linux-2.6.15-rc7/drivers/pci/syscall.c

replaced a  lock_kernel with spinlocks  , compile tested ok






-- 
"Employ your time in improving yourself by other men's writings, so that you 
shall gain easily what others have labored hard for."
Socrates

[-- Attachment #2: syscall.patch --]
[-- Type: text/x-diff, Size: 1367 bytes --]

diff -urN /usr/src/linux-2.6.15-rc7/drivers/pci/syscall.c /home/cneira/Linuxjanitoring/linux-janitorpatching/drivers/pci/syscall.c
--- /usr/src/linux-2.6.15-rc7/drivers/pci/syscall.c	2005-12-24 20:47:48.000000000 -0300
+++ /home/cneira/Linuxjanitoring/linux-janitorpatching/drivers/pci/syscall.c	2006-01-07 22:53:23.000000000 -0300
@@ -11,6 +11,7 @@
 #include <linux/errno.h>
 #include <linux/pci.h>
 #include <linux/smp_lock.h>
+#include <linux/spinlock.h>
 #include <linux/syscalls.h>
 #include <asm/uaccess.h>
 #include "pci.h"
@@ -21,6 +22,7 @@
 		   void __user *buf)
 {
 	struct pci_dev *dev;
+
 	u8 byte;
 	u16 word;
 	u32 dword;
@@ -35,7 +37,7 @@
 	if (!dev)
 		goto error;
 
-	lock_kernel();
+	spin_lock(&pciconfig_read_lock)
 	switch (len) {
 	case 1:
 		cfg_ret = pci_user_read_config_byte(dev, off, &byte);
@@ -48,10 +50,10 @@
 		break;
 	default:
 		err = -EINVAL;
-		unlock_kernel();
+		spin_unlock(&pciconfig_read_lock);
 		goto error;
 	};
-	unlock_kernel();
+	spin_unlock(&pciconfig_read_lock);
 
 	err = -EIO;
 	if (cfg_ret != PCIBIOS_SUCCESSFUL)
@@ -106,7 +108,7 @@
 	if (!dev)
 		return -ENODEV;
 
-	lock_kernel();
+	spin_lock(&pciconfig_write_lock)
 	switch(len) {
 	case 1:
 		err = get_user(byte, (u8 __user *)buf);
@@ -139,7 +141,7 @@
 		err = -EINVAL;
 		break;
 	};
-	unlock_kernel();
+	spin_unlock(&pciconfig_write_lock)
 
 	return err;
 }

[-- Attachment #3: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] linux-2.6.15-rc7/drivers/pci/syscall.c

[Matthew Wilcox]

[-- Attachment #1: Type: text/plain, Size: 237 bytes --]

On Mon, Jan 09, 2006 at 11:38:03AM +0000, carlos neira wrote:
> Greetings :
> 
> patch :linux-2.6.15-rc7/drivers/pci/syscall.c
> 
> replaced a  lock_kernel with spinlocks  , compile tested ok

That's not good enough.  Why is this safe?


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] linux-2.6.15-rc7/drivers/pci/syscall.c

[Greg KH]

[-- Attachment #1: Type: text/plain, Size: 319 bytes --]

On Mon, Jan 09, 2006 at 11:38:03AM +0000, carlos neira wrote:
> Greetings :
> 
> patch :linux-2.6.15-rc7/drivers/pci/syscall.c
> 
> replaced a  lock_kernel with spinlocks  , compile tested ok

What arch did you compile this for?

And why remove the lock_kernel calls here, are they hurting anything?

thanks,

greg k-h

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] linux-2.6.15-rc7/drivers/pci/syscall.c

[Jesper Juhl]

On 1/9/06, Greg KH <greg@kroah.com> wrote:
> On Mon, Jan 09, 2006 at 11:38:03AM +0000, carlos neira wrote:
> > Greetings :
> >
> > patch :linux-2.6.15-rc7/drivers/pci/syscall.c
> >
> > replaced a  lock_kernel with spinlocks  , compile tested ok
>
> What arch did you compile this for?
>
> And why remove the lock_kernel calls here, are they hurting anything?
>
I can't comment on the specific patch, but in general I thought we
wanted to get rid of lock_kernel() / unlock_kernel() over time?  If
that is indeed the case, then patches like this one are a step in the
right direction - no?

--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] linux-2.6.15-rc7/drivers/pci/syscall.c

[Greg KH]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

On Mon, Jan 09, 2006 at 08:02:19PM +0100, Jesper Juhl wrote:
> On 1/9/06, Greg KH <greg@kroah.com> wrote:
> > On Mon, Jan 09, 2006 at 11:38:03AM +0000, carlos neira wrote:
> > > Greetings :
> > >
> > > patch :linux-2.6.15-rc7/drivers/pci/syscall.c
> > >
> > > replaced a  lock_kernel with spinlocks  , compile tested ok
> >
> > What arch did you compile this for?
> >
> > And why remove the lock_kernel calls here, are they hurting anything?
> >
> I can't comment on the specific patch, but in general I thought we
> wanted to get rid of lock_kernel() / unlock_kernel() over time?  If
> that is indeed the case, then patches like this one are a step in the
> right direction - no?

"over time, if it makes sense to do so."  

For this specific example, the code should be converted to the kthread
api, which would remove these instances.

thanks,

greg k-h

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] linux-2.6.15-rc7/drivers/pci/syscall.c

[Matthew Wilcox]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On Mon, Jan 09, 2006 at 11:12:01AM -0800, Greg KH wrote:
> On Mon, Jan 09, 2006 at 08:02:19PM +0100, Jesper Juhl wrote:
> > On 1/9/06, Greg KH <greg@kroah.com> wrote:
> > > On Mon, Jan 09, 2006 at 11:38:03AM +0000, carlos neira wrote:
> > > > Greetings :
> > > >
> > > > patch :linux-2.6.15-rc7/drivers/pci/syscall.c
> > > >
> > > > replaced a  lock_kernel with spinlocks  , compile tested ok
> > >
> > > What arch did you compile this for?
> > >
> > > And why remove the lock_kernel calls here, are they hurting anything?
> 
> "over time, if it makes sense to do so."  
> 
> For this specific example, the code should be converted to the kthread
> api, which would remove these instances.

I think you're confused with which patch is being commented on.  The locks
around daemonize() should be converted to kthread(), but this patch is
for the PCI syscalls.  As I said in a recent post to linux-pci, I think
they can be deleted without bad effect.


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] linux-2.6.15-rc7/drivers/pci/syscall.c

[Greg KH]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

On Mon, Jan 09, 2006 at 12:29:32PM -0700, Matthew Wilcox wrote:
> On Mon, Jan 09, 2006 at 11:12:01AM -0800, Greg KH wrote:
> > On Mon, Jan 09, 2006 at 08:02:19PM +0100, Jesper Juhl wrote:
> > > On 1/9/06, Greg KH <greg@kroah.com> wrote:
> > > > On Mon, Jan 09, 2006 at 11:38:03AM +0000, carlos neira wrote:
> > > > > Greetings :
> > > > >
> > > > > patch :linux-2.6.15-rc7/drivers/pci/syscall.c
> > > > >
> > > > > replaced a  lock_kernel with spinlocks  , compile tested ok
> > > >
> > > > What arch did you compile this for?
> > > >
> > > > And why remove the lock_kernel calls here, are they hurting anything?
> > 
> > "over time, if it makes sense to do so."  
> > 
> > For this specific example, the code should be converted to the kthread
> > api, which would remove these instances.
> 
> I think you're confused with which patch is being commented on.  The locks
> around daemonize() should be converted to kthread(), but this patch is
> for the PCI syscalls.  As I said in a recent post to linux-pci, I think
> they can be deleted without bad effect.

Ugh, yeah, sorry, -ETOOMANYPATCHES

And yes, I think they can probably be deleted, but not replaced with a
spinlock like was done here.

thanks,

greg k-h

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors