From owner-svn-src-all@FreeBSD.ORG Fri Aug 21 02:59:08 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E233E1065698; Fri, 21 Aug 2009 02:59:07 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id D0EE58FC64; Fri, 21 Aug 2009 02:59:07 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n7L2x7RJ008958; Fri, 21 Aug 2009 02:59:07 GMT (envelope-from jhb@svn.freebsd.org) Received: (from jhb@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n7L2x7oc008957; Fri, 21 Aug 2009 02:59:07 GMT (envelope-from jhb@svn.freebsd.org) Message-Id: <200908210259.n7L2x7oc008957@svn.freebsd.org> From: John Baldwin Date: Fri, 21 Aug 2009 02:59:07 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r196417 - in head: share/man/man9 sys/kern X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 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: Fri, 21 Aug 2009 02:59:08 -0000 Author: jhb Date: Fri Aug 21 02:59:07 2009 New Revision: 196417 URL: http://svn.freebsd.org/changeset/base/196417 Log: This patch fixes two bugs in sglist(9) and improves robustness of the API via better semantics if a request to append an address range to an existing list fails. - When cloning an sglist, properly set the length in the new sglist instead of leaving the new list empty. - Properly compute the amount of data added to an sglist via _sglist_append_buf(). This allows sglist_consume_uio() to properly update uio_resid. - When a request to append an address range to a scatter/gather list fails, restore the sglist to the state it had at the start of the function call instead of resetting it to an empty list. Requested by: np (3) Approved by: re (kib) Modified: head/share/man/man9/sglist.9 head/sys/kern/subr_sglist.c Modified: head/share/man/man9/sglist.9 ============================================================================== --- head/share/man/man9/sglist.9 Fri Aug 21 01:12:06 2009 (r196416) +++ head/share/man/man9/sglist.9 Fri Aug 21 02:59:07 2009 (r196417) @@ -191,6 +191,8 @@ Specifically, the family of routines can be used to append the physical address ranges described by an object to the end of a scatter/gather list. All of these routines return 0 on success or an error on failure. +If a request to append an address range to a scatter/gather list fails, +the scatter/gather list will remain unchanged. .Pp The .Nm sglist_append @@ -445,6 +447,7 @@ There are not enough available segments to append the physical address ranges from .Fa second . .El +.Pp The .Nm sglist_slice function returns the following errors on failure: @@ -470,6 +473,7 @@ list in .Fa *slice to describe the requested physical address ranges. .El +.Pp The .Nm sglist_split function returns the following errors on failure: Modified: head/sys/kern/subr_sglist.c ============================================================================== --- head/sys/kern/subr_sglist.c Fri Aug 21 01:12:06 2009 (r196416) +++ head/sys/kern/subr_sglist.c Fri Aug 21 02:59:07 2009 (r196417) @@ -48,6 +48,32 @@ __FBSDID("$FreeBSD$"); static MALLOC_DEFINE(M_SGLIST, "sglist", "scatter/gather lists"); /* + * Convenience macros to save the state of an sglist so it can be restored + * if an append attempt fails. Since sglist's only grow we only need to + * save the current count of segments and the length of the ending segment. + * Earlier segments will not be changed by an append, and the only change + * that can occur to the ending segment is that it can be extended. + */ +struct sgsave { + u_short sg_nseg; + size_t ss_len; +}; + +#define SGLIST_SAVE(sg, sgsave) do { \ + (sgsave).sg_nseg = (sg)->sg_nseg; \ + if ((sgsave).sg_nseg > 0) \ + (sgsave).ss_len = (sg)->sg_segs[(sgsave).sg_nseg - 1].ss_len; \ + else \ + (sgsave).ss_len = 0; \ +} while (0) + +#define SGLIST_RESTORE(sg, sgsave) do { \ + (sg)->sg_nseg = (sgsave).sg_nseg; \ + if ((sgsave).sg_nseg > 0) \ + (sg)->sg_segs[(sgsave).sg_nseg - 1].ss_len = (sgsave).ss_len; \ +} while (0) + +/* * Append a single (paddr, len) to a sglist. sg is the list and ss is * the current segment in the list. If we run out of segments then * EFBIG will be returned. @@ -62,10 +88,8 @@ _sglist_append_range(struct sglist *sg, if (ss->ss_paddr + ss->ss_len == paddr) ss->ss_len += len; else { - if (sg->sg_nseg == sg->sg_maxseg) { - sg->sg_nseg = 0; + if (sg->sg_nseg == sg->sg_maxseg) return (EFBIG); - } ss++; ss->ss_paddr = paddr; ss->ss_len = len; @@ -107,26 +131,33 @@ _sglist_append_buf(struct sglist *sg, vo ss->ss_paddr = paddr; ss->ss_len = seglen; sg->sg_nseg = 1; - error = 0; } else { ss = &sg->sg_segs[sg->sg_nseg - 1]; error = _sglist_append_range(sg, &ss, paddr, seglen); + if (error) + return (error); } + vaddr += seglen; + len -= seglen; + if (donep) + *donep += seglen; - while (error == 0 && len > seglen) { - vaddr += seglen; - len -= seglen; - if (donep) - *donep += seglen; + while (len > 0) { seglen = MIN(len, PAGE_SIZE); if (pmap != NULL) paddr = pmap_extract(pmap, vaddr); else paddr = pmap_kextract(vaddr); error = _sglist_append_range(sg, &ss, paddr, seglen); + if (error) + return (error); + vaddr += seglen; + len -= seglen; + if (donep) + *donep += seglen; } - return (error); + return (0); } /* @@ -195,10 +226,16 @@ sglist_free(struct sglist *sg) int sglist_append(struct sglist *sg, void *buf, size_t len) { + struct sgsave save; + int error; if (sg->sg_maxseg == 0) return (EINVAL); - return (_sglist_append_buf(sg, buf, len, NULL, NULL)); + SGLIST_SAVE(sg, save); + error = _sglist_append_buf(sg, buf, len, NULL, NULL); + if (error) + SGLIST_RESTORE(sg, save); + return (error); } /* @@ -209,6 +246,8 @@ int sglist_append_phys(struct sglist *sg, vm_paddr_t paddr, size_t len) { struct sglist_seg *ss; + struct sgsave save; + int error; if (sg->sg_maxseg == 0) return (EINVAL); @@ -222,7 +261,11 @@ sglist_append_phys(struct sglist *sg, vm return (0); } ss = &sg->sg_segs[sg->sg_nseg - 1]; - return (_sglist_append_range(sg, &ss, paddr, len)); + SGLIST_SAVE(sg, save); + error = _sglist_append_range(sg, &ss, paddr, len); + if (error) + SGLIST_RESTORE(sg, save); + return (error); } /* @@ -233,6 +276,7 @@ sglist_append_phys(struct sglist *sg, vm int sglist_append_mbuf(struct sglist *sg, struct mbuf *m0) { + struct sgsave save; struct mbuf *m; int error; @@ -240,11 +284,14 @@ sglist_append_mbuf(struct sglist *sg, st return (EINVAL); error = 0; + SGLIST_SAVE(sg, save); for (m = m0; m != NULL; m = m->m_next) { if (m->m_len > 0) { error = sglist_append(sg, m->m_data, m->m_len); - if (error) + if (error) { + SGLIST_RESTORE(sg, save); return (error); + } } } return (0); @@ -258,11 +305,17 @@ sglist_append_mbuf(struct sglist *sg, st int sglist_append_user(struct sglist *sg, void *buf, size_t len, struct thread *td) { + struct sgsave save; + int error; if (sg->sg_maxseg == 0) return (EINVAL); - return (_sglist_append_buf(sg, buf, len, - vmspace_pmap(td->td_proc->p_vmspace), NULL)); + SGLIST_SAVE(sg, save); + error = _sglist_append_buf(sg, buf, len, + vmspace_pmap(td->td_proc->p_vmspace), NULL); + if (error) + SGLIST_RESTORE(sg, save); + return (error); } /* @@ -274,6 +327,7 @@ int sglist_append_uio(struct sglist *sg, struct uio *uio) { struct iovec *iov; + struct sgsave save; size_t resid, minlen; pmap_t pmap; int error, i; @@ -292,6 +346,7 @@ sglist_append_uio(struct sglist *sg, str pmap = NULL; error = 0; + SGLIST_SAVE(sg, save); for (i = 0; i < uio->uio_iovcnt && resid != 0; i++) { /* * Now at the first iovec to load. Load each iovec @@ -301,8 +356,10 @@ sglist_append_uio(struct sglist *sg, str if (minlen > 0) { error = _sglist_append_buf(sg, iov[i].iov_base, minlen, pmap, NULL); - if (error) + if (error) { + SGLIST_RESTORE(sg, save); return (error); + } resid -= minlen; } } @@ -397,6 +454,7 @@ sglist_clone(struct sglist *sg, int mfla new = sglist_alloc(sg->sg_maxseg, mflags); if (new == NULL) return (NULL); + new->sg_nseg = sg->sg_nseg; bcopy(sg->sg_segs, new->sg_segs, sizeof(struct sglist_seg) * sg->sg_nseg); return (new);