From 839ee02531b5a3f7e52188011c355a6044748de3 Mon Sep 17 00:00:00 2001 From: Kashyap D Desai Date: Wed, 8 Oct 2014 09:37:47 +0000 Subject: [PATCH] In the passthru IOCTL path, the mfi command pool was freely accessible N times where as there are limited number(32) of mfi commands in the pool. The mfi command pool is now restricted to 27 simultaneous accesses by using a counting semaphore while calling the passthru function. In the mrsas_cam.c source file there was a same function name mrsas_poll(), which was same as the mrsas_poll() implemented in the mrsas.c file for the polling interface. To clearly distinguish the functionality by usage we have renamed the former as mrsas_cam_poll(). In the passthru function let's say it has got an mfi command from the pool but it has failed in one of the DMA function call which will lead to leak an mfi command because in the ERROR case it directly returns and not freeing up the occupied mfi command. Reviewed by: ambrisko MFC after: 2 weeks Sponsored by: AVAGO Technologies --- sys/dev/mrsas/mrsas.c | 29 +++++++++++++++++++++++++++-- sys/dev/mrsas/mrsas.h | 6 ++++++ sys/dev/mrsas/mrsas_cam.c | 10 +++++----- sys/dev/mrsas/mrsas_ioctl.c | 35 ++++++++++++++++++++++++----------- 4 files changed, 62 insertions(+), 18 deletions(-) diff --git a/sys/dev/mrsas/mrsas.c b/sys/dev/mrsas/mrsas.c index c4914040814b..8fce46668487 100644 --- a/sys/dev/mrsas/mrsas.c +++ b/sys/dev/mrsas/mrsas.c @@ -823,6 +823,9 @@ static int mrsas_attach(device_t dev) mtx_init(&sc->mfi_cmd_pool_lock, "mrsas_mfi_cmd_pool_lock", NULL, MTX_DEF); mtx_init(&sc->raidmap_lock, "mrsas_raidmap_lock", NULL, MTX_DEF); + /* Intialize a counting Semaphore to take care no. of concurrent IOCTLs */ + sema_init(&sc->ioctl_count_sema, MRSAS_MAX_MFI_CMDS-5, IOCTL_SEMA_DESCRIPTION); + /* Intialize linked list */ TAILQ_INIT(&sc->mrsas_mpt_cmd_list_head); TAILQ_INIT(&sc->mrsas_mfi_cmd_list_head); @@ -912,6 +915,8 @@ static int mrsas_attach(device_t dev) mtx_destroy(&sc->mpt_cmd_pool_lock); mtx_destroy(&sc->mfi_cmd_pool_lock); mtx_destroy(&sc->raidmap_lock); + /* Destroy the counting semaphore created for Ioctl */ + sema_destroy(&sc->ioctl_count_sema); attach_fail: destroy_dev(sc->mrsas_cdev); if (sc->reg_res){ @@ -937,10 +942,13 @@ static int mrsas_detach(device_t dev) sc = device_get_softc(dev); sc->remove_in_progress = 1; + /* Destroy the character device so no other IOCTL will be handled */ + destroy_dev(sc->mrsas_cdev); + /* * Take the instance off the instance array. Note that we will not * decrement the max_index. We let this array be sparse array - */ + */ for (i = 0; i < mrsas_mgmt_info.max_index; i++) { if (mrsas_mgmt_info.sc_ptr[i] == sc) { mrsas_mgmt_info.count--; @@ -984,13 +992,22 @@ static int mrsas_detach(device_t dev) mtx_destroy(&sc->mpt_cmd_pool_lock); mtx_destroy(&sc->mfi_cmd_pool_lock); mtx_destroy(&sc->raidmap_lock); + + /* Wait for all the semaphores to be released */ + while (sema_value(&sc->ioctl_count_sema) != (MRSAS_MAX_MFI_CMDS-5)) + pause("mr_shutdown", hz); + + /* Destroy the counting semaphore created for Ioctl */ + sema_destroy(&sc->ioctl_count_sema); + if (sc->reg_res){ bus_release_resource(sc->mrsas_dev, SYS_RES_MEMORY, sc->reg_res_id, sc->reg_res); } - destroy_dev(sc->mrsas_cdev); + if (sc->sysctl_tree != NULL) sysctl_ctx_free(&sc->sysctl_ctx); + return (0); } @@ -1254,13 +1271,21 @@ mrsas_ioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flag, d_thread_t *td) #ifdef COMPAT_FREEBSD32 case MRSAS_IOC_FIRMWARE_PASS_THROUGH32: #endif + /* Decrement the Ioctl counting Semaphore before getting an mfi command */ + sema_wait(&sc->ioctl_count_sema); + ret = mrsas_passthru(sc, (void *)arg, cmd); + + /* Increment the Ioctl counting semaphore value */ + sema_post(&sc->ioctl_count_sema); + break; case MRSAS_IOC_SCAN_BUS: ret = mrsas_bus_scan(sc); break; default: mrsas_dprint(sc, MRSAS_TRACE, "IOCTL command 0x%lx is not handled\n", cmd); + ret = ENOENT; } return (ret); diff --git a/sys/dev/mrsas/mrsas.h b/sys/dev/mrsas/mrsas.h index 19f119bc0652..5830f8bc54f8 100644 --- a/sys/dev/mrsas/mrsas.h +++ b/sys/dev/mrsas/mrsas.h @@ -67,12 +67,16 @@ __FBSDID("$FreeBSD$"); #include #include +#include +#include #include #include #include #include #include +#define IOCTL_SEMA_DESCRIPTION "mrsas semaphore for MFI pool" + /* * Device IDs and PCI */ @@ -2494,6 +2498,8 @@ struct mrsas_softc { struct selinfo mrsas_select; // poll select interface for application uint32_t mrsas_aen_triggered; uint32_t mrsas_poll_waiting; + + struct sema ioctl_count_sema; // counting semaphore for ioctl uint32_t max_fw_cmds; // Max commands from FW uint32_t max_num_sge; // Max number of SGEs struct resource *mrsas_irq[MAX_MSIX_COUNT]; // interrupt interface window diff --git a/sys/dev/mrsas/mrsas_cam.c b/sys/dev/mrsas/mrsas_cam.c index f4d083889b92..03f05b0d6c81 100644 --- a/sys/dev/mrsas/mrsas_cam.c +++ b/sys/dev/mrsas/mrsas_cam.c @@ -86,7 +86,7 @@ void mrsas_set_pd_lba(MRSAS_RAID_SCSI_IO_REQUEST *io_request, u_int8_t cdb_len, MR_DRV_RAID_MAP_ALL *local_map_ptr, u_int32_t ref_tag, u_int32_t ld_block_size); static void mrsas_freeze_simq(struct mrsas_mpt_cmd *cmd, struct cam_sim *sim); -static void mrsas_poll(struct cam_sim *sim); +static void mrsas_cam_poll(struct cam_sim *sim); static void mrsas_action(struct cam_sim *sim, union ccb *ccb); static void mrsas_scsiio_timeout(void *data); static void mrsas_data_load_cb(void *arg, bus_dma_segment_t *segs, @@ -137,7 +137,7 @@ int mrsas_cam_attach(struct mrsas_softc *sc) /* * Create SIM for bus 0 and register, also create path */ - sc->sim_0 = cam_sim_alloc(mrsas_action, mrsas_poll, "mrsas", sc, + sc->sim_0 = cam_sim_alloc(mrsas_action, mrsas_cam_poll, "mrsas", sc, device_get_unit(sc->mrsas_dev), &sc->sim_lock, mrsas_cam_depth, mrsas_cam_depth, devq); if (sc->sim_0 == NULL){ @@ -173,7 +173,7 @@ int mrsas_cam_attach(struct mrsas_softc *sc) /* * Create SIM for bus 1 and register, also create path */ - sc->sim_1 = cam_sim_alloc(mrsas_action, mrsas_poll, "mrsas", sc, + sc->sim_1 = cam_sim_alloc(mrsas_action, mrsas_cam_poll, "mrsas", sc, device_get_unit(sc->mrsas_dev), &sc->sim_lock, mrsas_cam_depth, mrsas_cam_depth, devq); if (sc->sim_1 == NULL){ @@ -1110,12 +1110,12 @@ void mrsas_cmd_done(struct mrsas_softc *sc, struct mrsas_mpt_cmd *cmd) } /** - * mrsas_poll: Polling entry point + * mrsas_cam_poll: Polling entry point * input: Pointer to SIM * * This is currently a stub function. */ -static void mrsas_poll(struct cam_sim *sim) +static void mrsas_cam_poll(struct cam_sim *sim) { struct mrsas_softc *sc = (struct mrsas_softc *)cam_sim_softc(sim); mrsas_isr((void *) sc); diff --git a/sys/dev/mrsas/mrsas_ioctl.c b/sys/dev/mrsas/mrsas_ioctl.c index 1e77fd87dec2..f0066b670f79 100644 --- a/sys/dev/mrsas/mrsas_ioctl.c +++ b/sys/dev/mrsas/mrsas_ioctl.c @@ -168,18 +168,21 @@ int mrsas_passthru( struct mrsas_softc *sc, void *arg, u_long ioctlCmd ) NULL, NULL, // lockfunc, lockarg &ioctl_data_tag[i])) { device_printf(sc->mrsas_dev, "Cannot allocate ioctl data tag\n"); - return (ENOMEM); + ret = ENOMEM; + goto out; } if (bus_dmamem_alloc(ioctl_data_tag[i], (void **)&ioctl_data_mem[i], (BUS_DMA_NOWAIT | BUS_DMA_ZERO), &ioctl_data_dmamap[i])) { device_printf(sc->mrsas_dev, "Cannot allocate ioctl data mem\n"); - return (ENOMEM); + ret = ENOMEM; + goto out; } if (bus_dmamap_load(ioctl_data_tag[i], ioctl_data_dmamap[i], ioctl_data_mem[i], ioctl_data_size, mrsas_alloc_cb, &ioctl_data_phys_addr[i], BUS_DMA_NOWAIT)) { device_printf(sc->mrsas_dev, "Cannot load ioctl data mem\n"); - return (ENOMEM); + ret = ENOMEM; + goto out; } /* Save the physical address and length */ @@ -222,18 +225,21 @@ int mrsas_passthru( struct mrsas_softc *sc, void *arg, u_long ioctlCmd ) NULL, NULL, // lockfunc, lockarg &ioctl_sense_tag)) { device_printf(sc->mrsas_dev, "Cannot allocate ioctl sense tag\n"); - return (ENOMEM); + ret = ENOMEM; + goto out; } if (bus_dmamem_alloc(ioctl_sense_tag, (void **)&ioctl_sense_mem, (BUS_DMA_NOWAIT | BUS_DMA_ZERO), &ioctl_sense_dmamap)) { - device_printf(sc->mrsas_dev, "Cannot allocate ioctl data mem\n"); - return (ENOMEM); + device_printf(sc->mrsas_dev, "Cannot allocate ioctl sense mem\n"); + ret = ENOMEM; + goto out; } if (bus_dmamap_load(ioctl_sense_tag, ioctl_sense_dmamap, ioctl_sense_mem, ioctl_sense_size, mrsas_alloc_cb, &ioctl_sense_phys_addr, BUS_DMA_NOWAIT)) { device_printf(sc->mrsas_dev, "Cannot load ioctl sense mem\n"); - return (ENOMEM); + ret = ENOMEM; + goto out; } sense_ptr = (unsigned long *)((unsigned long)cmd->frame + user_ioc->sense_off); @@ -299,17 +305,24 @@ int mrsas_passthru( struct mrsas_softc *sc, void *arg, u_long ioctlCmd ) */ if (ioctl_sense_phys_addr) bus_dmamap_unload(ioctl_sense_tag, ioctl_sense_dmamap); - if (ioctl_sense_mem) + if (ioctl_sense_mem != NULL) bus_dmamem_free(ioctl_sense_tag, ioctl_sense_mem, ioctl_sense_dmamap); - if (ioctl_sense_tag) + if (ioctl_sense_tag != NULL) bus_dma_tag_destroy(ioctl_sense_tag); /* * Release data buffers */ for (i = 0; i < user_ioc->sge_count; i++) { - if (!user_ioc->sgl[i].iov_len) - continue; + if (ioctlCmd == MRSAS_IOC_FIRMWARE_PASS_THROUGH64) { + if (!user_ioc->sgl[i].iov_len) + continue; +#ifdef COMPAT_FREEBSD32 + } else { + if (!user_ioc32->sgl[i].iov_len) + continue; +#endif + } if (ioctl_data_phys_addr[i]) bus_dmamap_unload(ioctl_data_tag[i], ioctl_data_dmamap[i]); if (ioctl_data_mem[i] != NULL)