Date: Sun, 6 Jul 2014 17:38:12 -0700 (PDT) From: Don Lewis <truckman@FreeBSD.org> To: arch@FreeBSD.org, jhb@FreeBSD.org Subject: [patch] axe RF_TIMESHARE? Message-ID: <201407070038.s670cCkb022030@gw.catspoiler.org>
next in thread | raw e-mail | index | archive | help
When he was reviewing my previous batch of subr_rman.c patches, John
Baldwin suggested getting rid of the RF_TIMESHARE feature. There is
nothing in the tree that uses it, and he didn't think that anything ever
used it.
Getting rid of RF_TIMESHARE gets rid of quite a bit of complexity and
unbloats the the size of the code on i386 by more than 500 bytes.
Before:
text data bss dec hex filename
9663 376 28 10067 2753 subr_rman.o
After:
text data bss dec hex filename
9128 376 28 9532 253c subr_rman.o
The int_rman_activate_resource() and int_rman_deactivate_resource()
functions become trivial and I elected to manually inline both.
It looks to me like the special handling of RF_ACTIVE isn't needed in
reserve_resource_bound() isn't needed and doesn't have to be deferred to
the end of the function. The comments seemed to indicate that this code
was iffy for RF_TIMESHARE in any case.
Should we nuke RF_TIMESHARE?
Should this change be MFC'ed, and if so, how far back?
Comments on the attached patch?
Index: sys/kern/subr_rman.c
===================================================================
--- sys/kern/subr_rman.c (revision 268273)
+++ sys/kern/subr_rman.c (working copy)
@@ -109,9 +109,6 @@
struct rman_head rman_head;
static struct mtx rman_mtx; /* mutex to protect rman_head */
-static int int_rman_activate_resource(struct rman *rm, struct resource_i *r,
- struct resource_i **whohas);
-static int int_rman_deactivate_resource(struct resource_i *r);
static int int_rman_release_resource(struct rman *rm, struct resource_i *r);
static __inline struct resource_i *
@@ -321,7 +318,7 @@
/* Not supported for shared resources. */
r = rr->__r_i;
- if (r->r_flags & (RF_TIMESHARE | RF_SHAREABLE))
+ if (r->r_flags & RF_SHAREABLE)
return (EINVAL);
/*
@@ -434,7 +431,7 @@
return (0);
}
-#define SHARE_TYPE(f) (f & (RF_SHAREABLE | RF_TIMESHARE | RF_PREFETCHABLE))
+#define SHARE_TYPE(f) (f & (RF_SHAREABLE | RF_PREFETCHABLE))
struct resource *
rman_reserve_resource_bound(struct rman *rm, u_long start, u_long end,
@@ -451,10 +448,9 @@
"length %#lx, flags %u, device %s\n", rm->rm_descr, start, end,
count, flags,
dev == NULL ? "<null>" : device_get_nameunit(dev)));
- KASSERT((flags & (RF_WANTED | RF_FIRSTSHARE)) == 0,
+ KASSERT((flags & RF_FIRSTSHARE) == 0,
("invalid flags %#x", flags));
- new_rflags = (flags & ~(RF_ACTIVE | RF_WANTED | RF_FIRSTSHARE)) |
- RF_ALLOCATED;
+ new_rflags = (flags & ~RF_FIRSTSHARE) | RF_ALLOCATED;
mtx_lock(rm->rm_mtx);
@@ -600,7 +596,7 @@
* additional work, but this does not seem warranted.)
*/
DPRINTF(("no unshared regions found\n"));
- if ((flags & (RF_SHAREABLE | RF_TIMESHARE)) == 0)
+ if ((flags & RF_SHAREABLE) == 0)
goto out;
for (s = r; s && s->r_end <= end; s = TAILQ_NEXT(s, r_link)) {
@@ -635,25 +631,11 @@
goto out;
}
}
-
/*
* We couldn't find anything.
*/
+
out:
- /*
- * If the user specified RF_ACTIVE in flags, we attempt to atomically
- * activate the resource. If this fails, we release the resource
- * and indicate overall failure. (This behavior probably doesn't
- * make sense for RF_TIMESHARE-type resources.)
- */
- if (rv && (flags & RF_ACTIVE) != 0) {
- struct resource_i *whohas;
- if (int_rman_activate_resource(rm, rv, &whohas)) {
- int_rman_release_resource(rm, rv);
- rv = NULL;
- }
- }
-
mtx_unlock(rm->rm_mtx);
return (rv == NULL ? NULL : &rv->r_r);
}
@@ -667,91 +649,17 @@
dev));
}
-static int
-int_rman_activate_resource(struct rman *rm, struct resource_i *r,
- struct resource_i **whohas)
-{
- struct resource_i *s;
- int ok;
-
- /*
- * If we are not timesharing, then there is nothing much to do.
- * If we already have the resource, then there is nothing at all to do.
- * If we are not on a sharing list with anybody else, then there is
- * little to do.
- */
- if ((r->r_flags & RF_TIMESHARE) == 0
- || (r->r_flags & RF_ACTIVE) != 0
- || r->r_sharehead == NULL) {
- r->r_flags |= RF_ACTIVE;
- return 0;
- }
-
- ok = 1;
- for (s = LIST_FIRST(r->r_sharehead); s && ok;
- s = LIST_NEXT(s, r_sharelink)) {
- if ((s->r_flags & RF_ACTIVE) != 0) {
- ok = 0;
- *whohas = s;
- }
- }
- if (ok) {
- r->r_flags |= RF_ACTIVE;
- return 0;
- }
- return EBUSY;
-}
-
int
rman_activate_resource(struct resource *re)
{
- int rv;
- struct resource_i *r, *whohas;
+ struct resource_i *r;
struct rman *rm;
r = re->__r_i;
rm = r->r_rm;
mtx_lock(rm->rm_mtx);
- rv = int_rman_activate_resource(rm, r, &whohas);
+ r->r_flags |= RF_ACTIVE;
mtx_unlock(rm->rm_mtx);
- return rv;
-}
-
-int
-rman_await_resource(struct resource *re, int pri, int timo)
-{
- int rv;
- struct resource_i *r, *whohas;
- struct rman *rm;
-
- r = re->__r_i;
- rm = r->r_rm;
- mtx_lock(rm->rm_mtx);
- for (;;) {
- rv = int_rman_activate_resource(rm, r, &whohas);
- if (rv != EBUSY)
- return (rv); /* returns with mutex held */
-
- if (r->r_sharehead == NULL)
- panic("rman_await_resource");
- whohas->r_flags |= RF_WANTED;
- rv = msleep(r->r_sharehead, rm->rm_mtx, pri, "rmwait", timo);
- if (rv) {
- mtx_unlock(rm->rm_mtx);
- return (rv);
- }
- }
-}
-
-static int
-int_rman_deactivate_resource(struct resource_i *r)
-{
-
- r->r_flags &= ~RF_ACTIVE;
- if (r->r_flags & RF_WANTED) {
- r->r_flags &= ~RF_WANTED;
- wakeup(r->r_sharehead);
- }
return 0;
}
@@ -762,7 +670,7 @@
rm = r->__r_i->r_rm;
mtx_lock(rm->rm_mtx);
- int_rman_deactivate_resource(r->__r_i);
+ r->__r_i->r_flags &= ~RF_ACTIVE;
mtx_unlock(rm->rm_mtx);
return 0;
}
@@ -773,7 +681,7 @@
struct resource_i *s, *t;
if (r->r_flags & RF_ACTIVE)
- int_rman_deactivate_resource(r);
+ r->r_flags &= ~RF_ACTIVE;
/*
* Check for a sharing list first. If there is one, then we don't
Index: sys/sys/rman.h
===================================================================
--- sys/sys/rman.h (revision 268273)
+++ sys/sys/rman.h (working copy)
@@ -42,8 +42,8 @@
#define RF_ALLOCATED 0x0001 /* resource has been reserved */
#define RF_ACTIVE 0x0002 /* resource allocation has been activated */
#define RF_SHAREABLE 0x0004 /* resource permits contemporaneous sharing */
-#define RF_TIMESHARE 0x0008 /* resource permits time-division sharing */
-#define RF_WANTED 0x0010 /* somebody is waiting for this resource */
+#define RF_SPARE1 0x0008
+#define RF_SPARE2 0x0010
#define RF_FIRSTSHARE 0x0020 /* first in sharing list */
#define RF_PREFETCHABLE 0x0040 /* resource is prefetchable */
#define RF_OPTIONAL 0x0080 /* for bus_alloc_resources() */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201407070038.s670cCkb022030>