From owner-svn-src-head@freebsd.org Sat Jun 6 00:02:50 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id C40F033BBBB; Sat, 6 Jun 2020 00:02:50 +0000 (UTC) (envelope-from chs@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49f08B4YxGz4Cpb; Sat, 6 Jun 2020 00:02:50 +0000 (UTC) (envelope-from chs@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 977CD193A2; Sat, 6 Jun 2020 00:02:50 +0000 (UTC) (envelope-from chs@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 05602o2w002385; Sat, 6 Jun 2020 00:02:50 GMT (envelope-from chs@FreeBSD.org) Received: (from chs@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 05602oJW002384; Sat, 6 Jun 2020 00:02:50 GMT (envelope-from chs@FreeBSD.org) Message-Id: <202006060002.05602oJW002384@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: chs set sender to chs@FreeBSD.org using -f From: Chuck Silvers Date: Sat, 6 Jun 2020 00:02:50 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r361852 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: chs X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 361852 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Jun 2020 00:02:50 -0000 Author: chs Date: Sat Jun 6 00:02:50 2020 New Revision: 361852 URL: https://svnweb.freebsd.org/changeset/base/361852 Log: Fix hang due to missing unbusy in sendfile when an async data I/O fails. r359473 removed the page unbusy logic from sendfile_iodone() because when vm_pager_get_pages_async() would return an error after failing to start the async I/O (eg. because VOP_BMAP failed), sendfile_swapin() would also unbusy the pages, and it was wrong to unbusy twice. However this breaks the case where vm_pager_get_pages_async() succeeds in starting an async I/O and the async I/O is what fails. In this case, sendfile_iodone() must unbusy the pages, and because sendfile_iodone() doesn't know which case it is in, sendfile_iodone() must always unbusy pages and relookup pages which have been substituted with bogus_page, which in turn means that sendfile_swapin() must never do unbusy or relookup for pages which have been given to vm_pager_get_pages_async(), even if there is an error. Reviewed by: kib, markj Sponsored by: Netflix Differential Revision: https://reviews.freebsd.org/D25136 Modified: head/sys/kern/kern_sendfile.c Modified: head/sys/kern/kern_sendfile.c ============================================================================== --- head/sys/kern/kern_sendfile.c Fri Jun 5 22:07:10 2020 (r361851) +++ head/sys/kern/kern_sendfile.c Sat Jun 6 00:02:50 2020 (r361852) @@ -292,36 +292,30 @@ sendfile_iodone(void *arg, vm_page_t *pa, int count, i struct socket *so; int i; - if (error != 0) { + if (error != 0) sfio->error = error; - /* - * Restore of the pg[] elements is done by - * sendfile_swapin(). - */ - } else { - /* - * Restore the valid page pointers. They are already - * unbusied, but still wired. For error != 0 case, - * sendfile_swapin() handles unbusy. - * - * XXXKIB since pages are only wired, and we do not - * own the object lock, other users might have - * invalidated them in meantime. Similarly, after we - * unbusied the swapped-in pages, they can become - * invalid under us. - */ - MPASS(count == 0 || pa[0] != bogus_page); - for (i = 0; i < count; i++) { - if (pa[i] == bogus_page) { - sfio->pa[(pa[0]->pindex - sfio->pindex0) + i] = - pa[i] = vm_page_relookup(sfio->obj, - pa[0]->pindex + i); - KASSERT(pa[i] != NULL, - ("%s: page %p[%d] disappeared", - __func__, pa, i)); - } else { - vm_page_xunbusy_unchecked(pa[i]); - } + + /* + * Restore the valid page pointers. They are already + * unbusied, but still wired. + * + * XXXKIB since pages are only wired, and we do not + * own the object lock, other users might have + * invalidated them in meantime. Similarly, after we + * unbusied the swapped-in pages, they can become + * invalid under us. + */ + MPASS(count == 0 || pa[0] != bogus_page); + for (i = 0; i < count; i++) { + if (pa[i] == bogus_page) { + sfio->pa[(pa[0]->pindex - sfio->pindex0) + i] = + pa[i] = vm_page_relookup(sfio->obj, + pa[0]->pindex + i); + KASSERT(pa[i] != NULL, + ("%s: page %p[%d] disappeared", + __func__, pa, i)); + } else { + vm_page_xunbusy_unchecked(pa[i]); } } @@ -534,22 +528,12 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i sendfile_iowait(sfio, "sferrio"); /* - * Perform full pages recovery before returning EIO. + * Do remaining pages recovery before returning EIO. * Pages from 0 to npages are wired. - * Pages from (i + 1) to (i + count - 1) may be - * substituted to bogus page, and not busied. - * Pages from (i + count) to (i + count1 - 1) are - * not busied. - * Rest of the pages from i to npages are busied. + * Pages from (i + count1) to npages are busied. */ for (j = 0; j < npages; j++) { - if (j >= i + count && j < i + count1) - ; - else if (j > i && j < i + count - 1 && - pa[j] == bogus_page) - pa[j] = vm_page_relookup(obj, - OFF_TO_IDX(vmoff(j, off))); - else if (j >= i) + if (j >= i + count1) vm_page_xunbusy(pa[j]); KASSERT(pa[j] != NULL && pa[j] != bogus_page, ("%s: page %p[%d] I/O recovery failure",