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", _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
