public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Get rid of check_resource() before it becomes a problem
@ 2002-10-30  1:40 Rusty Russell
  2002-10-30  2:33 ` Jeff Garzik
  0 siblings, 1 reply; 3+ messages in thread
From: Rusty Russell @ 2002-10-30  1:40 UTC (permalink / raw)
  To: linux-kernel; +Cc: torvalds, dhinds, alan

AFAICT, it has the same race issues as the old check_region() which is
being slowly and painfully eliminated.

Remove the temptation, and open-code the one user (PCMCIA).

Rusty.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.44/drivers/pcmcia/rsrc_mgr.c working-2.5.44-proc-register/drivers/pcmcia/rsrc_mgr.c
--- linux-2.5.44/drivers/pcmcia/rsrc_mgr.c	2002-10-15 15:29:59.000000000 +1000
+++ working-2.5.44-proc-register/drivers/pcmcia/rsrc_mgr.c	2002-10-30 12:35:20.000000000 +1100
@@ -124,16 +124,36 @@ static struct resource *resource_parent(
 	return &ioport_resource;
 }
 
+/* FIXME: Fundamentally racy. */
 static inline int check_io_resource(unsigned long b, unsigned long n,
 				    struct pci_dev *dev)
 {
-	return check_resource(resource_parent(b, n, IORESOURCE_IO, dev), b, n);
+	struct resource *region;
+
+	region = __request_region(resource_parent(b, n, IORESOURCE_IO, dev),
+				  b, n, "check_io_resource");
+	if (!region)
+		return -EBUSY;
+
+	release_resource(region);
+	kfree(region);
+	return 0;
 }
 
+/* FIXME: Fundamentally racy. */
 static inline int check_mem_resource(unsigned long b, unsigned long n,
 				     struct pci_dev *dev)
 {
-	return check_resource(resource_parent(b, n, IORESOURCE_MEM, dev), b, n);
+	struct resource *region;
+
+	region = __request_region(resource_parent(b, n, IORESOURCE_MEM, dev),
+				  b, n, "check_mem_resource");
+	if (!region)
+		return -EBUSY;
+
+	release_resource(region);
+	kfree(region);
+	return 0;
 }
 
 static struct resource *make_resource(unsigned long b, unsigned long n,
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.44/include/linux/ioport.h working-2.5.44-proc-register/include/linux/ioport.h
--- linux-2.5.44/include/linux/ioport.h	2002-05-09 12:40:19.000000000 +1000
+++ working-2.5.44-proc-register/include/linux/ioport.h	2002-10-30 12:25:54.000000000 +1100
@@ -85,7 +85,6 @@ extern struct resource iomem_resource;
 
 extern int get_resource_list(struct resource *, char *buf, int size);
 
-extern int check_resource(struct resource *root, unsigned long, unsigned long);
 extern int request_resource(struct resource *root, struct resource *new);
 extern int release_resource(struct resource *new);
 extern int allocate_resource(struct resource *root, struct resource *new,
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.44/kernel/resource.c working-2.5.44-proc-register/kernel/resource.c
--- linux-2.5.44/kernel/resource.c	2002-08-11 15:31:43.000000000 +1000
+++ working-2.5.44-proc-register/kernel/resource.c	2002-10-30 12:26:12.000000000 +1100
@@ -131,20 +131,6 @@ int release_resource(struct resource *ol
 	return retval;
 }
 
-int check_resource(struct resource *root, unsigned long start, unsigned long len)
-{
-	struct resource *conflict, tmp;
-
-	tmp.start = start;
-	tmp.end = start + len - 1;
-	write_lock(&resource_lock);
-	conflict = __request_resource(root, &tmp);
-	if (!conflict)
-		__release_resource(&tmp);
-	write_unlock(&resource_lock);
-	return conflict ? -EBUSY : 0;
-}
-
 /*
  * Find empty slot in the resource tree given range and alignment.
  */

--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Get rid of check_resource() before it becomes a problem
  2002-10-30  1:40 [PATCH] Get rid of check_resource() before it becomes a problem Rusty Russell
@ 2002-10-30  2:33 ` Jeff Garzik
  2002-10-30  3:15   ` Rusty Russell
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Garzik @ 2002-10-30  2:33 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, torvalds, dhinds, alan

Need to zap the export from kernel/ksyms.c too



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Get rid of check_resource() before it becomes a problem
  2002-10-30  2:33 ` Jeff Garzik
@ 2002-10-30  3:15   ` Rusty Russell
  0 siblings, 0 replies; 3+ messages in thread
From: Rusty Russell @ 2002-10-30  3:15 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-kernel, torvalds, dhinds, alan

In message <3DBF44F8.8010307@pobox.com> you write:
> Need to zap the export from kernel/ksyms.c too

Ack.

Here it is, fresh against 2.5.44.
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: Remove inherently racy check_resource
Author: Rusty Russell
Status: Trivial

D: The new resource interface foolishly replicated the (obsolete,
D: racy) spirit of the check_region call as check_resource.  You
D: should use request_resource/release_resource instead.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .8919-linux-2.5.45/drivers/pcmcia/rsrc_mgr.c .8919-linux-2.5.45.updated/drivers/pcmcia/rsrc_mgr.c
--- .8919-linux-2.5.45/drivers/pcmcia/rsrc_mgr.c	2002-10-15 15:29:59.000000000 +1000
+++ .8919-linux-2.5.45.updated/drivers/pcmcia/rsrc_mgr.c	2002-10-30 14:03:09.000000000 +1100
@@ -124,16 +124,36 @@ static struct resource *resource_parent(
 	return &ioport_resource;
 }
 
+/* FIXME: Fundamentally racy. */
 static inline int check_io_resource(unsigned long b, unsigned long n,
 				    struct pci_dev *dev)
 {
-	return check_resource(resource_parent(b, n, IORESOURCE_IO, dev), b, n);
+	struct resource *region;
+
+	region = __request_region(resource_parent(b, n, IORESOURCE_IO, dev),
+				  b, n, "check_io_resource");
+	if (!region)
+		return -EBUSY;
+
+	release_resource(region);
+	kfree(region);
+	return 0;
 }
 
+/* FIXME: Fundamentally racy. */
 static inline int check_mem_resource(unsigned long b, unsigned long n,
 				     struct pci_dev *dev)
 {
-	return check_resource(resource_parent(b, n, IORESOURCE_MEM, dev), b, n);
+	struct resource *region;
+
+	region = __request_region(resource_parent(b, n, IORESOURCE_MEM, dev),
+				  b, n, "check_mem_resource");
+	if (!region)
+		return -EBUSY;
+
+	release_resource(region);
+	kfree(region);
+	return 0;
 }
 
 static struct resource *make_resource(unsigned long b, unsigned long n,
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .8919-linux-2.5.45/include/linux/ioport.h .8919-linux-2.5.45.updated/include/linux/ioport.h
--- .8919-linux-2.5.45/include/linux/ioport.h	2002-05-09 12:40:19.000000000 +1000
+++ .8919-linux-2.5.45.updated/include/linux/ioport.h	2002-10-30 14:03:09.000000000 +1100
@@ -85,7 +85,6 @@ extern struct resource iomem_resource;
 
 extern int get_resource_list(struct resource *, char *buf, int size);
 
-extern int check_resource(struct resource *root, unsigned long, unsigned long);
 extern int request_resource(struct resource *root, struct resource *new);
 extern int release_resource(struct resource *new);
 extern int allocate_resource(struct resource *root, struct resource *new,
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .8919-linux-2.5.45/kernel/ksyms.c .8919-linux-2.5.45.updated/kernel/ksyms.c
--- .8919-linux-2.5.45/kernel/ksyms.c	2002-10-30 12:53:10.000000000 +1100
+++ .8919-linux-2.5.45.updated/kernel/ksyms.c	2002-10-30 14:06:18.000000000 +1100
@@ -445,7 +445,6 @@ EXPORT_SYMBOL(enable_hlt);
 EXPORT_SYMBOL(request_resource);
 EXPORT_SYMBOL(release_resource);
 EXPORT_SYMBOL(allocate_resource);
-EXPORT_SYMBOL(check_resource);
 EXPORT_SYMBOL(__request_region);
 EXPORT_SYMBOL(__check_region);
 EXPORT_SYMBOL(__release_region);
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .8919-linux-2.5.45/kernel/resource.c .8919-linux-2.5.45.updated/kernel/resource.c
--- .8919-linux-2.5.45/kernel/resource.c	2002-08-11 15:31:43.000000000 +1000
+++ .8919-linux-2.5.45.updated/kernel/resource.c	2002-10-30 14:03:09.000000000 +1100
@@ -131,20 +131,6 @@ int release_resource(struct resource *ol
 	return retval;
 }
 
-int check_resource(struct resource *root, unsigned long start, unsigned long len)
-{
-	struct resource *conflict, tmp;
-
-	tmp.start = start;
-	tmp.end = start + len - 1;
-	write_lock(&resource_lock);
-	conflict = __request_resource(root, &tmp);
-	if (!conflict)
-		__release_resource(&tmp);
-	write_unlock(&resource_lock);
-	return conflict ? -EBUSY : 0;
-}
-
 /*
  * Find empty slot in the resource tree given range and alignment.
  */

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2002-10-30  3:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-10-30  1:40 [PATCH] Get rid of check_resource() before it becomes a problem Rusty Russell
2002-10-30  2:33 ` Jeff Garzik
2002-10-30  3:15   ` Rusty Russell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox