Date: Tue, 29 Jun 2010 01:23:05 +0400 From: Chagin Dmitry <dchagin@freebsd.org> To: Matt Jacob <mjacob@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r208752 - head/sys/cam Message-ID: <20100628212305.GA2898@dchagin.static.corbina.ru> In-Reply-To: <201006021806.o52I6Wri028466@svn.freebsd.org> References: <201006021806.o52I6Wri028466@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--VbJkn9YxBvnuCH5J Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 02, 2010 at 06:06:32PM +0000, Matt Jacob wrote: > Author: mjacob > Date: Wed Jun 2 18:06:32 2010 > New Revision: 208752 > URL: http://svn.freebsd.org/changeset/base/208752 >=20 > Log: > Protect periph drivers list and rearrange things to minimize the chance= of > stepping oneself during probing. > =20 > Don't blindly decrement a periph probe count. > =20 This commit causes a kernel panic on cdrecord -scanbus: Unread portion of the kernel message buffer: panic: _mtx_lock_sleep: recursed on non-recursive mutex XPT topology lock @ /work/head/sys/cam/cam_xpt.c:4814 #11 0xffffffff80319dd9 in _mtx_lock_flags (m=3D0xffffffff808761d8, opts=3D0x0, file=3D0xffffffff8061ae4f "/work/head/sys/cam/cam_xpt.c", line=3D0x12ce) at /work/head/sys/kern/kern_mutex.c:203 #12 0xffffffff80179adc in xptpdperiphtraverse (pdrv=3D0xffffff000377e600, start_periph=3D0x0, tr_func=3D0xffffffff8017a670 <xptplistperiphfunc>, arg=3D0xffffff006b872000) at /work/head/sys/cam/cam_xpt.c:2151 #13 0xffffffff80179753 in xptpdrvtraverse (start_pdrv=3DVariable "start_pdrv" is not available. ) at /work/head/sys/cam/cam_xpt.c:2132 #14 0xffffffff8017fd09 in xpt_action_default (start_ccb=3D0xffffff006b872000) at /work/head/sys/cam/cam_xpt.c:1967 #15 0xffffffff8017bc63 in xptioctl (dev=3DVariable "dev" is not available. ) at /work/head/sys/cam/cam_xpt.c:598 #16 0xffffffff802bc3c6 in devfs_ioctl_f (fp=3D0xffffff006b4a0a00, com=3D0xc4a81502, data=3DVariable "data" is not available. ) at /work/head/sys/fs/devfs/devfs_vnops.c:669 #17 0xffffffff80379632 in kern_ioctl (td=3D0xffffff006b4b83f0, fd=3DVariable "fd" is not available. ) at file.h:254 #18 0xffffffff80379890 in ioctl (td=3D0xffffff006b4b83f0, uap=3D0xffffff80b1312bf0) at /work/head/sys/kern/sys_generic.c:678 > Reviewed by: scsi@ > Obtained from: Alexander Motin, Atillio Rao, Others > MFC after: 1 month >=20 > Modified: > head/sys/cam/cam_periph.c > head/sys/cam/cam_xpt.c >=20 > Modified: head/sys/cam/cam_periph.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/cam/cam_periph.c Wed Jun 2 17:27:23 2010 (r208751) > +++ head/sys/cam/cam_periph.c Wed Jun 2 18:06:32 2010 (r208752) > @@ -185,17 +185,6 @@ cam_periph_alloc(periph_ctor_t *periph_c > =09 > init_level++; > =20 > - xpt_lock_buses(); > - for (p_drv =3D periph_drivers; *p_drv !=3D NULL; p_drv++) { > - if (strcmp((*p_drv)->driver_name, name) =3D=3D 0) > - break; > - } > - xpt_unlock_buses(); > - if (*p_drv =3D=3D NULL) { > - printf("cam_periph_alloc: invalid periph name '%s'\n", name); > - free(periph, M_CAMPERIPH); > - return (CAM_REQ_INVALID); > - } > =20 > sim =3D xpt_path_sim(path); > path_id =3D xpt_path_path_id(path); > @@ -208,7 +197,6 @@ cam_periph_alloc(periph_ctor_t *periph_c > periph->periph_oninval =3D periph_oninvalidate; > periph->type =3D type; > periph->periph_name =3D name; > - periph->unit_number =3D camperiphunit(*p_drv, path_id, target_id, lun_i= d); > periph->immediate_priority =3D CAM_PRIORITY_NONE; > periph->refcount =3D 0; > periph->sim =3D sim; > @@ -216,26 +204,39 @@ cam_periph_alloc(periph_ctor_t *periph_c > status =3D xpt_create_path(&path, periph, path_id, target_id, lun_id); > if (status !=3D CAM_REQ_CMP) > goto failure; > - > periph->path =3D path; > - init_level++; > - > - status =3D xpt_add_periph(periph); > - > - if (status !=3D CAM_REQ_CMP) > - goto failure; > =20 > + xpt_lock_buses(); > + for (p_drv =3D periph_drivers; *p_drv !=3D NULL; p_drv++) { > + if (strcmp((*p_drv)->driver_name, name) =3D=3D 0) > + break; > + } > + if (*p_drv =3D=3D NULL) { > + printf("cam_periph_alloc: invalid periph name '%s'\n", name); > + xpt_free_path(periph->path); > + free(periph, M_CAMPERIPH); > + xpt_unlock_buses(); > + return (CAM_REQ_INVALID); > + } > + periph->unit_number =3D camperiphunit(*p_drv, path_id, target_id, lun_i= d); > cur_periph =3D TAILQ_FIRST(&(*p_drv)->units); > while (cur_periph !=3D NULL > && cur_periph->unit_number < periph->unit_number) > cur_periph =3D TAILQ_NEXT(cur_periph, unit_links); > - > - if (cur_periph !=3D NULL) > + if (cur_periph !=3D NULL) { > + KASSERT(cur_periph->unit_number !=3D periph->unit_number, ("duplicate = units on periph list")); > TAILQ_INSERT_BEFORE(cur_periph, periph, unit_links); > - else { > + } else { > TAILQ_INSERT_TAIL(&(*p_drv)->units, periph, unit_links); > (*p_drv)->generation++; > } > + xpt_unlock_buses(); > + > + init_level++; > + > + status =3D xpt_add_periph(periph); > + if (status !=3D CAM_REQ_CMP) > + goto failure; > =20 > init_level++; > =20 > @@ -250,10 +251,12 @@ failure: > /* Initialized successfully */ > break; > case 3: > - TAILQ_REMOVE(&(*p_drv)->units, periph, unit_links); > xpt_remove_periph(periph); > /* FALLTHROUGH */ > case 2: > + xpt_lock_buses(); > + TAILQ_REMOVE(&(*p_drv)->units, periph, unit_links); > + xpt_unlock_buses(); > xpt_free_path(periph->path); > /* FALLTHROUGH */ > case 1: > @@ -288,6 +291,7 @@ cam_periph_find(struct cam_path *path, c > TAILQ_FOREACH(periph, &(*p_drv)->units, unit_links) { > if (xpt_path_comp(periph->path, path) =3D=3D 0) { > xpt_unlock_buses(); > + mtx_assert(periph->sim->mtx, MA_OWNED); > return(periph); > } > } > @@ -322,8 +326,13 @@ cam_periph_release_locked(struct cam_per > return; > =20 > xpt_lock_buses(); > - if ((--periph->refcount =3D=3D 0) > - && (periph->flags & CAM_PERIPH_INVALID)) { > + if (periph->refcount !=3D 0) { > + periph->refcount--; > + } else { > + xpt_print(periph->path, "%s: release %p when refcount is zero\n ", __f= unc__, periph); > + } > + if (periph->refcount =3D=3D 0 > + && (periph->flags & CAM_PERIPH_INVALID)) { > camperiphfree(periph); > } > xpt_unlock_buses(); >=20 > Modified: head/sys/cam/cam_xpt.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/cam/cam_xpt.c Wed Jun 2 17:27:23 2010 (r208751) > +++ head/sys/cam/cam_xpt.c Wed Jun 2 18:06:32 2010 (r208752) > @@ -2138,6 +2138,7 @@ xptpdperiphtraverse(struct periph_driver > =20 > retval =3D 1; > =20 > + xpt_lock_buses(); already acquired in xpt_action_default? > for (periph =3D (start_periph ? start_periph : > TAILQ_FIRST(&(*pdrv)->units)); periph !=3D NULL; > periph =3D next_periph) { > @@ -2145,9 +2146,12 @@ xptpdperiphtraverse(struct periph_driver > next_periph =3D TAILQ_NEXT(periph, unit_links); > =20 > retval =3D tr_func(periph, arg); > - if (retval =3D=3D 0) > + if (retval =3D=3D 0) { > + xpt_unlock_buses(); > return(retval); > + } > } > + xpt_unlock_buses(); > return(retval); > } > =20 > @@ -2323,7 +2327,6 @@ xpt_action_default(union ccb *start_ccb) > =20 > CAM_DEBUG(start_ccb->ccb_h.path, CAM_DEBUG_TRACE, ("xpt_action_default\= n")); > =20 > - > switch (start_ccb->ccb_h.func_code) { > case XPT_SCSI_IO: > { > @@ -2670,7 +2673,9 @@ xpt_action_default(union ccb *start_ccb) > xptedtmatch(cdm); > break; > case CAM_DEV_POS_PDRV: > + xpt_lock_buses(); > xptperiphlistmatch(cdm); > + xpt_unlock_buses(); > break; > default: > cdm->status =3D CAM_DEV_MATCH_ERROR; --=20 Have fun! chd --VbJkn9YxBvnuCH5J Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.10 (FreeBSD) iEYEARECAAYFAkwpErgACgkQ0t2Tb3OO/O1ohgCfYxcKPmacEO1JY7DSJoMj7zje gZQAnjTg87cT66OOE85SEHG84S93B7Xc =4vTT -----END PGP SIGNATURE----- --VbJkn9YxBvnuCH5J--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100628212305.GA2898>