Date: Sat, 8 Dec 2012 04:16:07 +0000 (UTC) From: "Kenneth D. Merry" <ken@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r244015 - head/sys/cam/ctl Message-ID: <201212080416.qB84G7qs060770@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ken Date: Sat Dec 8 04:16:07 2012 New Revision: 244015 URL: http://svnweb.freebsd.org/changeset/base/244015 Log: Fix the CTL OOA queue dumping code so that it does not hold a mutex while doing a copyout. That can cause a panic, because copyout can trigger VM faults, and we can't handle VM faults while holding a mutex. The solution here is to malloc a separate buffer to hold the OOA queue entries, so that we don't risk a VM fault while filling up the buffer and we don't have to drop the lock. The other solution would be to wire the user's memory while filling their buffer with copyout, but that would have been a little more complex. Also fix a debugging parenthesis issue in ctl_abort_task() pointed out by Chuck Tuffli. Sponsored by: Spectra Logic Corporation MFC after: 1 week Modified: head/sys/cam/ctl/ctl.c Modified: head/sys/cam/ctl/ctl.c ============================================================================== --- head/sys/cam/ctl/ctl.c Sat Dec 8 04:03:04 2012 (r244014) +++ head/sys/cam/ctl/ctl.c Sat Dec 8 04:16:07 2012 (r244015) @@ -351,7 +351,8 @@ static void ctl_ioctl_hard_startstop_cal struct cfi_metatask *metatask); static void ctl_ioctl_bbrread_callback(void *arg,struct cfi_metatask *metatask); static int ctl_ioctl_fill_ooa(struct ctl_lun *lun, uint32_t *cur_fill_num, - struct ctl_ooa *ooa_hdr); + struct ctl_ooa *ooa_hdr, + struct ctl_ooa_entry *kern_entries); static int ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag, struct thread *td); uint32_t ctl_get_resindex(struct ctl_nexus *nexus); @@ -1960,7 +1961,7 @@ ctl_ioctl_bbrread_callback(void *arg, st */ static int ctl_ioctl_fill_ooa(struct ctl_lun *lun, uint32_t *cur_fill_num, - struct ctl_ooa *ooa_hdr) + struct ctl_ooa *ooa_hdr, struct ctl_ooa_entry *kern_entries) { union ctl_io *io; int retval; @@ -1970,7 +1971,7 @@ ctl_ioctl_fill_ooa(struct ctl_lun *lun, for (io = (union ctl_io *)TAILQ_FIRST(&lun->ooa_queue); (io != NULL); (*cur_fill_num)++, io = (union ctl_io *)TAILQ_NEXT(&io->io_hdr, ooa_links)) { - struct ctl_ooa_entry *cur_entry, entry; + struct ctl_ooa_entry *entry; /* * If we've got more than we can fit, just count the @@ -1979,36 +1980,29 @@ ctl_ioctl_fill_ooa(struct ctl_lun *lun, if (*cur_fill_num >= ooa_hdr->alloc_num) continue; - cur_entry = &ooa_hdr->entries[*cur_fill_num]; + entry = &kern_entries[*cur_fill_num]; - bzero(&entry, sizeof(entry)); - - entry.tag_num = io->scsiio.tag_num; - entry.lun_num = lun->lun; + entry->tag_num = io->scsiio.tag_num; + entry->lun_num = lun->lun; #ifdef CTL_TIME_IO - entry.start_bt = io->io_hdr.start_bt; + entry->start_bt = io->io_hdr.start_bt; #endif - bcopy(io->scsiio.cdb, entry.cdb, io->scsiio.cdb_len); - entry.cdb_len = io->scsiio.cdb_len; + bcopy(io->scsiio.cdb, entry->cdb, io->scsiio.cdb_len); + entry->cdb_len = io->scsiio.cdb_len; if (io->io_hdr.flags & CTL_FLAG_BLOCKED) - entry.cmd_flags |= CTL_OOACMD_FLAG_BLOCKED; + entry->cmd_flags |= CTL_OOACMD_FLAG_BLOCKED; if (io->io_hdr.flags & CTL_FLAG_DMA_INPROG) - entry.cmd_flags |= CTL_OOACMD_FLAG_DMA; + entry->cmd_flags |= CTL_OOACMD_FLAG_DMA; if (io->io_hdr.flags & CTL_FLAG_ABORT) - entry.cmd_flags |= CTL_OOACMD_FLAG_ABORT; + entry->cmd_flags |= CTL_OOACMD_FLAG_ABORT; if (io->io_hdr.flags & CTL_FLAG_IS_WAS_ON_RTR) - entry.cmd_flags |= CTL_OOACMD_FLAG_RTR; + entry->cmd_flags |= CTL_OOACMD_FLAG_RTR; if (io->io_hdr.flags & CTL_FLAG_DMA_QUEUED) - entry.cmd_flags |= CTL_OOACMD_FLAG_DMA_QUEUED; - - retval = copyout(&entry, cur_entry, sizeof(entry)); - - if (retval != 0) - break; + entry->cmd_flags |= CTL_OOACMD_FLAG_DMA_QUEUED; } return (retval); @@ -2391,6 +2385,7 @@ ctl_ioctl(struct cdev *dev, u_long cmd, case CTL_GET_OOA: { struct ctl_lun *lun; struct ctl_ooa *ooa_hdr; + struct ctl_ooa_entry *entries; uint32_t cur_fill_num; ooa_hdr = (struct ctl_ooa *)addr; @@ -2414,11 +2409,20 @@ ctl_ioctl(struct cdev *dev, u_long cmd, break; } + entries = malloc(ooa_hdr->alloc_len, M_CTL, M_WAITOK | M_ZERO); + if (entries == NULL) { + printf("%s: could not allocate %d bytes for OOA " + "dump\n", __func__, ooa_hdr->alloc_len); + retval = ENOMEM; + break; + } + mtx_lock(&softc->ctl_lock); if (((ooa_hdr->flags & CTL_OOA_FLAG_ALL_LUNS) == 0) && ((ooa_hdr->lun_num > CTL_MAX_LUNS) || (softc->ctl_luns[ooa_hdr->lun_num] == NULL))) { mtx_unlock(&softc->ctl_lock); + free(entries, M_CTL); printf("%s: CTL_GET_OOA: invalid LUN %ju\n", __func__, (uintmax_t)ooa_hdr->lun_num); retval = EINVAL; @@ -2430,24 +2434,31 @@ ctl_ioctl(struct cdev *dev, u_long cmd, if (ooa_hdr->flags & CTL_OOA_FLAG_ALL_LUNS) { STAILQ_FOREACH(lun, &softc->lun_list, links) { retval = ctl_ioctl_fill_ooa(lun, &cur_fill_num, - ooa_hdr); + ooa_hdr, entries); if (retval != 0) break; } if (retval != 0) { mtx_unlock(&softc->ctl_lock); + free(entries, M_CTL); break; } } else { lun = softc->ctl_luns[ooa_hdr->lun_num]; - retval = ctl_ioctl_fill_ooa(lun, &cur_fill_num,ooa_hdr); + retval = ctl_ioctl_fill_ooa(lun, &cur_fill_num,ooa_hdr, + entries); } mtx_unlock(&softc->ctl_lock); ooa_hdr->fill_num = min(cur_fill_num, ooa_hdr->alloc_num); ooa_hdr->fill_len = ooa_hdr->fill_num * sizeof(struct ctl_ooa_entry); + retval = copyout(entries, ooa_hdr->entries, ooa_hdr->fill_len); + if (retval != 0) { + printf("%s: error copying out %d bytes for OOA dump\n", + __func__, ooa_hdr->fill_len); + } getbintime(&ooa_hdr->cur_bt); @@ -2458,6 +2469,8 @@ ctl_ioctl(struct cdev *dev, u_long cmd, ooa_hdr->dropped_num = 0; ooa_hdr->status = CTL_OOA_OK; } + + free(entries, M_CTL); break; } case CTL_CHECK_OOA: { @@ -10782,9 +10795,9 @@ ctl_abort_task(union ctl_io *io) (xio->io_hdr.flags & CTL_FLAG_DMA_INPROG) ? " DMA" : "", (xio->io_hdr.flags & - CTL_FLAG_ABORT) ? " ABORT" : ""), + CTL_FLAG_ABORT) ? " ABORT" : "", (xio->io_hdr.flags & - CTL_FLAG_IS_WAS_ON_RTR ? " RTR" : ""); + CTL_FLAG_IS_WAS_ON_RTR ? " RTR" : "")); ctl_scsi_command_string(&xio->scsiio, NULL, &sb); sbuf_finish(&sb); printf("%s\n", sbuf_data(&sb));
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201212080416.qB84G7qs060770>