From owner-svn-src-all@FreeBSD.ORG Wed May 28 16:57:17 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D9E29544; Wed, 28 May 2014 16:57:17 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C79652C2E; Wed, 28 May 2014 16:57:17 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.8/8.14.8) with ESMTP id s4SGvHfb059642; Wed, 28 May 2014 16:57:17 GMT (envelope-from truckman@svn.freebsd.org) Received: (from truckman@localhost) by svn.freebsd.org (8.14.8/8.14.8/Submit) id s4SGvHDo059641; Wed, 28 May 2014 16:57:17 GMT (envelope-from truckman@svn.freebsd.org) Message-Id: <201405281657.s4SGvHDo059641@svn.freebsd.org> From: Don Lewis Date: Wed, 28 May 2014 16:57:17 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r266814 - head/sys/kern X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 May 2014 16:57:17 -0000 Author: truckman Date: Wed May 28 16:57:17 2014 New Revision: 266814 URL: http://svnweb.freebsd.org/changeset/base/266814 Log: 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 MFC after: 1 month Modified: head/sys/kern/subr_rman.c Modified: head/sys/kern/subr_rman.c ============================================================================== --- head/sys/kern/subr_rman.c Wed May 28 16:50:18 2014 (r266813) +++ head/sys/kern/subr_rman.c Wed May 28 16:57:17 2014 (r266814) @@ -435,12 +435,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; @@ -450,8 +452,10 @@ rman_reserve_resource_bound(struct rman "length %#lx, flags %u, device %s\n", rm->rm_descr, start, end, count, flags, dev == 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); @@ -466,10 +470,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); @@ -522,7 +524,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; } @@ -542,7 +544,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; @@ -603,7 +605,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 && @@ -613,8 +615,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) { @@ -641,13 +642,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);