Date: Thu, 3 Jul 2014 16:00:57 +0000 (UTC) From: Don Lewis <truckman@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r268217 - stable/9/sys/kern Message-ID: <201407031600.s63G0vWW031327@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: truckman Date: Thu Jul 3 16:00:57 2014 New Revision: 268217 URL: http://svnweb.freebsd.org/changeset/base/268217 Log: MFC r266814 Initialize r_flags the same way in all cases using a sanitized copy of flags that has several bits cleared. The RF_WANTED and RF_FIRSTSHARE bits are invalid in this context, and we want to defer setting RF_ACTIVE in r_flags until later. This should make rman_get_flags() return the correct answer in all cases. Add a KASSERT() to catch callers which incorrectly pass the RF_WANTED or RF_FIRSTSHARE flags. Do a strict equality check on the share type bits of flags. In particular, do an equality check on RF_PREFETCHABLE. The previous code would allow one type of mismatch of RF_PREFETCHABLE but disallow the other type of mismatch. Also, ignore the the RF_ALIGNMENT_MASK bits since alignment validity should be handled by the amask check. This field contains an integer value, but previous code did a strange bitwise comparison on it. Leave the original value of flags unmolested as a minor debug aid. Change the start+amask overflow check to a KASSERT() since it is just meant to catch a highly unlikely programming error in the caller. Reviewed by: jhb Modified: stable/9/sys/kern/subr_rman.c Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/kern/subr_rman.c ============================================================================== --- stable/9/sys/kern/subr_rman.c Thu Jul 3 15:56:30 2014 (r268216) +++ stable/9/sys/kern/subr_rman.c Thu Jul 3 16:00:57 2014 (r268217) @@ -430,12 +430,14 @@ rman_adjust_resource(struct resource *rr return (0); } +#define SHARE_TYPE(f) (f & (RF_SHAREABLE | RF_TIMESHARE | RF_PREFETCHABLE)) + struct resource * rman_reserve_resource_bound(struct rman *rm, u_long start, u_long end, u_long count, u_long bound, u_int flags, struct device *dev) { - u_int want_activate; + u_int new_rflags; struct resource_i *r, *s, *rv; u_long rstart, rend, amask, bmask; @@ -445,8 +447,10 @@ rman_reserve_resource_bound(struct rman "length %#lx, flags %u, device %s\n", rm->rm_descr, start, end, count, flags, dev == NULL ? "<null>" : device_get_nameunit(dev))); - want_activate = (flags & RF_ACTIVE); - flags &= ~RF_ACTIVE; + KASSERT((flags & (RF_WANTED | RF_FIRSTSHARE)) == 0, + ("invalid flags %#x", flags)); + new_rflags = (flags & ~(RF_ACTIVE | RF_WANTED | RF_FIRSTSHARE)) | + RF_ALLOCATED; mtx_lock(rm->rm_mtx); @@ -461,10 +465,8 @@ rman_reserve_resource_bound(struct rman } amask = (1ul << RF_ALIGNMENT(flags)) - 1; - if (start > ULONG_MAX - amask) { - DPRINTF(("start+amask would wrap around\n")); - goto out; - } + KASSERT(start <= ULONG_MAX - amask, + ("start (%#lx) + amask (%#lx) would wrap around", start, amask)); /* If bound is 0, bmask will also be 0 */ bmask = ~(bound - 1); @@ -517,7 +519,7 @@ rman_reserve_resource_bound(struct rman if ((s->r_end - s->r_start + 1) == count) { DPRINTF(("candidate region is entire chunk\n")); rv = s; - rv->r_flags |= RF_ALLOCATED | flags; + rv->r_flags = new_rflags; rv->r_dev = dev; goto out; } @@ -537,7 +539,7 @@ rman_reserve_resource_bound(struct rman goto out; rv->r_start = rstart; rv->r_end = rstart + count - 1; - rv->r_flags = flags | RF_ALLOCATED; + rv->r_flags = new_rflags; rv->r_dev = dev; rv->r_rm = rm; @@ -598,7 +600,7 @@ rman_reserve_resource_bound(struct rman goto out; for (s = r; s && s->r_end <= end; s = TAILQ_NEXT(s, r_link)) { - if ((s->r_flags & flags) == flags && + if (SHARE_TYPE(s->r_flags) == SHARE_TYPE(flags) && s->r_start >= start && (s->r_end - s->r_start + 1) == count && (s->r_start & amask) == 0 && @@ -608,8 +610,7 @@ rman_reserve_resource_bound(struct rman goto out; rv->r_start = s->r_start; rv->r_end = s->r_end; - rv->r_flags = s->r_flags & - (RF_ALLOCATED | RF_SHAREABLE | RF_TIMESHARE); + rv->r_flags = new_rflags; rv->r_dev = dev; rv->r_rm = rm; if (s->r_sharehead == NULL) { @@ -636,13 +637,12 @@ rman_reserve_resource_bound(struct rman */ out: /* - * If the user specified RF_ACTIVE in the initial flags, - * which is reflected in `want_activate', we attempt to atomically + * 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 && want_activate) { + if (rv && (flags & RF_ACTIVE) != 0) { struct resource_i *whohas; if (int_rman_activate_resource(rm, rv, &whohas)) { int_rman_release_resource(rm, rv);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201407031600.s63G0vWW031327>