Date: Fri, 9 May 2014 03:59:13 +0000 (UTC) From: "Kenneth D. Merry" <ken@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r265730 - stable/10/sys/dev/mpr Message-ID: <201405090359.s493xDS9069313@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ken Date: Fri May 9 03:59:12 2014 New Revision: 265730 URL: http://svnweb.freebsd.org/changeset/base/265730 Log: MFC mpr(4) driver changes. This includes r265386, r265424, and r265473. ------------------------------------------------------------------------ r265386 | ken | 2014-05-05 13:53:03 -0600 (Mon, 05 May 2014) | 15 lines Adjust #if statements inside mprsas_send_smpcmd() to more accurately reflect when unmapped I/O support was added. For FreeBSD 10, it arrived just prior to __FreeBSD_version 1000028. For FreeBSD 9, it arrived just prior to __FreeBSD_version 902001. Also, fix compiler warnings in mprsas_send_smpcmd() that happen in the i386 PAE build for non-unmapped I/O builds. These were fixed in mps(4) in revision 241145, but didn't make it into the mpr(4) driver. This change should only affect FreeBSD versions outside the above revisions, and thus doesn't affect head. Sponsored by: Spectra Logic Corporation ------------------------------------------------------------------------ ------------------------------------------------------------------------ r265424 | ken | 2014-05-06 00:18:43 -0600 (Tue, 06 May 2014) | 33 lines Fix a problem with async notifications in the mpr(4) driver. This problem only occurs on versions of FreeBSD prior to the recent CAM locking changes. (i.e. stable/9 and older versions of stable/10) This change should be a no-op for head and stable/10. If a path isn't specified, xpt_register_async() will create a fully wildcarded path and acquire a lock (the XPT lock in older versions, and via xpt_path_lock() in newer versions) to call xpt_action() for the XPT_SASYNC_CB CCB. It will then drop the lock and if the requested event includes AC_FOUND_DEVICE or AC_PATH_REGISTERED, it will get the caller up to date with any device arrivals or path registrations. The issue is that before the locking changes, each SIM lock would get acquired in turn during the EDT tree traversal process. If a path is specified for xpt_register_async(), it won't acquire and drop its own lock, but instead expects the caller to hold its own SIM lock. That works for the first part of xpt_register_async(), but causes a recursive lock acquisition once the EDT traversal happens and it comes to the SIM in question. And it isn't possible to call xpt_action() without holding a SIM lock. The locking changes fix this by using the XPT topology lock for EDT traversal, so it is no longer an issue to hold the SIM lock while calling xpt_register_async(). The solution for FreeBSD versions before the locking changes is to request notification of all device arrivals (so we pass a NULL path into xpt_register_async()) and then filter out the arrivals that are not ours. Sponsored by: Spectra Logic Corporation ------------------------------------------------------------------------ ------------------------------------------------------------------------ r265473 | ken | 2014-05-06 16:13:38 -0600 (Tue, 06 May 2014) | 7 lines Change the device name for mpr(4) from /dev/mpr_N to /dev/mprN. This is more consistent with the existing mps(4) behavior. Reviewed by: Steve McConnell <stephen.mcconnell@avagotech.com> ------------------------------------------------------------------------ Sponsored by: LSI, Spectra Logic Modified: stable/10/sys/dev/mpr/mpr_sas.c stable/10/sys/dev/mpr/mpr_user.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/dev/mpr/mpr_sas.c ============================================================================== --- stable/10/sys/dev/mpr/mpr_sas.c Fri May 9 03:52:10 2014 (r265729) +++ stable/10/sys/dev/mpr/mpr_sas.c Fri May 9 03:59:12 2014 (r265730) @@ -813,8 +813,49 @@ mpr_attach_sas(struct mpr_softc *sc) #else event = AC_FOUND_DEVICE; #endif + + /* + * Prior to the CAM locking improvements, we can't call + * xpt_register_async() with a particular path specified. + * + * If a path isn't specified, xpt_register_async() will + * generate a wildcard path and acquire the XPT lock while + * it calls xpt_action() to execute the XPT_SASYNC_CB CCB. + * It will then drop the XPT lock once that is done. + * + * If a path is specified for xpt_register_async(), it will + * not acquire and drop the XPT lock around the call to + * xpt_action(). xpt_action() asserts that the caller + * holds the SIM lock, so the SIM lock has to be held when + * calling xpt_register_async() when the path is specified. + * + * But xpt_register_async calls xpt_for_all_devices(), + * which calls xptbustraverse(), which will acquire each + * SIM lock. When it traverses our particular bus, it will + * necessarily acquire the SIM lock, which will lead to a + * recursive lock acquisition. + * + * The CAM locking changes fix this problem by acquiring + * the XPT topology lock around bus traversal in + * xptbustraverse(), so the caller can hold the SIM lock + * and it does not cause a recursive lock acquisition. + * + * These __FreeBSD_version values are approximate, especially + * for stable/10, which is two months later than the actual + * change. + */ + +#if (__FreeBSD_version < 1000703) || \ + ((__FreeBSD_version >= 1100000) && (__FreeBSD_version < 1100002)) + mpr_unlock(sc); + status = xpt_register_async(event, mprsas_async, sc, + NULL); + mpr_lock(sc); +#else status = xpt_register_async(event, mprsas_async, sc, sassc->path); +#endif + if (status != CAM_REQ_CMP) { mpr_dprint(sc, MPR_ERROR, "Error %#x registering async handler for " @@ -2501,7 +2542,8 @@ mprsas_send_smpcmd(struct mprsas_softc * sg = NULL; error = 0; -#if __FreeBSD_version >= 1000029 +#if (__FreeBSD_version >= 1000028) || \ + ((__FreeBSD_version >= 902001) && (__FreeBSD_version < 1000000)) switch (ccb->ccb_h.flags & CAM_DATA_MASK) { case CAM_DATA_PADDR: case CAM_DATA_SG_PADDR: @@ -2561,7 +2603,7 @@ mprsas_send_smpcmd(struct mprsas_softc * xpt_done(ccb); return; } -#else //__FreeBSD_version < 1000029 +#else /* __FreeBSD_version < 1000028 */ /* * XXX We don't yet support physical addresses here. */ @@ -2604,7 +2646,7 @@ mprsas_send_smpcmd(struct mprsas_softc * bus_dma_segment_t *req_sg; req_sg = (bus_dma_segment_t *)ccb->smpio.smp_request; - request = (uint8_t *)req_sg[0].ds_addr; + request = (uint8_t *)(uintptr_t)req_sg[0].ds_addr; } else request = ccb->smpio.smp_request; @@ -2612,14 +2654,14 @@ mprsas_send_smpcmd(struct mprsas_softc * bus_dma_segment_t *rsp_sg; rsp_sg = (bus_dma_segment_t *)ccb->smpio.smp_response; - response = (uint8_t *)rsp_sg[0].ds_addr; + response = (uint8_t *)(uintptr_t)rsp_sg[0].ds_addr; } else response = ccb->smpio.smp_response; } else { request = ccb->smpio.smp_request; response = ccb->smpio.smp_response; } -#endif //__FreeBSD_version >= 1000029 +#endif /* __FreeBSD_version < 1000028 */ cm = mpr_alloc_command(sc); if (cm == NULL) { @@ -2987,6 +3029,18 @@ mprsas_async(void *callback_arg, uint32_ break; /* + * See the comment in mpr_attach_sas() for a detailed + * explanation. In these versions of FreeBSD we register + * for all events and filter out the events that don't + * apply to us. + */ +#if (__FreeBSD_version < 1000703) || \ + ((__FreeBSD_version >= 1100000) && (__FreeBSD_version < 1100002)) + if (xpt_path_path_id(path) != sassc->sim->path_id) + break; +#endif + + /* * We should have a handle for this, but check to make sure. */ KASSERT(xpt_path_target_id(path) < sassc->maxtargets, @@ -3043,8 +3097,21 @@ mprsas_async(void *callback_arg, uint32_ case AC_FOUND_DEVICE: { struct ccb_getdev *cgd; + /* + * See the comment in mpr_attach_sas() for a detailed + * explanation. In these versions of FreeBSD we register + * for all events and filter out the events that don't + * apply to us. + */ +#if (__FreeBSD_version < 1000703) || \ + ((__FreeBSD_version >= 1100000) && (__FreeBSD_version < 1100002)) + if (xpt_path_path_id(path) != sc->sassc->sim->path_id) + break; +#endif + cgd = arg; mprsas_prepare_ssu(sc, path, cgd); + #if (__FreeBSD_version < 901503) || \ ((__FreeBSD_version >= 1000000) && (__FreeBSD_version < 1000006)) mprsas_check_eedp(sc, path, cgd); Modified: stable/10/sys/dev/mpr/mpr_user.c ============================================================================== --- stable/10/sys/dev/mpr/mpr_user.c Fri May 9 03:52:10 2014 (r265729) +++ stable/10/sys/dev/mpr/mpr_user.c Fri May 9 03:59:12 2014 (r265730) @@ -212,7 +212,7 @@ mpr_attach_user(struct mpr_softc *sc) unit = device_get_unit(sc->mpr_dev); sc->mpr_cdev = make_dev(&mpr_cdevsw, unit, UID_ROOT, GID_OPERATOR, - 0640, "mpr_%d", unit); + 0640, "mpr%d", unit); if (sc->mpr_cdev == NULL) { return (ENOMEM); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201405090359.s493xDS9069313>