diff --git a/doc/scst_user_spec.sgml b/doc/scst_user_spec.sgml index 467f9e8e1..08ec6efb9 100644 --- a/doc/scst_user_spec.sgml +++ b/doc/scst_user_spec.sgml @@ -151,16 +151,8 @@ function, or close on its file descriptor. SCST_USER_UNREGISTER_DEVICE

-SCST_USER_UNREGISTER_DEVICE unregisters the corresponding virtual user -space device. It doesn't have any parameters. - -During execution of SCST_USER_UNREGISTER_DEVICE at least one another -thread must process all coming subcommands, otherwise after timeout it -will fail with EBUSY error. The processing should stop on receiving -ENODEV error. - -SCST_USER_UNREGISTER_DEVICE returns 0 on success or -1 in case of error, -and errno is set appropriately. +SCST_USER_UNREGISTER_DEVICE is obsolete and should not be used. Just +close the device's fd instead. SCST_USER_SET_OPTIONS/SCST_USER_GET_OPTIONS @@ -290,6 +282,10 @@ thread, which also handles incoming commands, otherwise there could be a but nobody handles them. This "deadlock" will be resolved only when initiator, which sent those commands, aborts them after timeout. + SCST_USER_REPLY_AND_GET_CMD diff --git a/scst/src/dev_handlers/scst_user.c b/scst/src/dev_handlers/scst_user.c index 0125f7ee6..2d45bb4f1 100644 --- a/scst/src/dev_handlers/scst_user.c +++ b/scst/src/dev_handlers/scst_user.c @@ -47,8 +47,6 @@ #define DEV_USER_ATTACH_TIMEOUT (5*HZ) struct scst_user_dev { - struct rw_semaphore dev_rwsem; - /* * Must be kept here, because it's needed on the cleanup time, * when corresponding scst_dev is already dead. @@ -58,7 +56,10 @@ struct scst_user_dev { /* Protected by udev_cmd_threads.cmd_list_lock */ struct list_head ready_cmd_list; - /* Protected by dev_rwsem or don't need any protection */ + /* + * Don't need any protection or assignment in SCST_USER_SET_OPTIONS + * supposed to be serialized by the caller + */ unsigned int blocking:1; unsigned int cleanup_done:1; unsigned int tst:3; @@ -218,8 +219,6 @@ static struct kmem_cache *user_dev_cachep; static struct kmem_cache *user_cmd_cachep; static struct kmem_cache *user_get_cmd_cachep; -static DEFINE_MUTEX(dev_priv_mutex); - static const struct file_operations dev_user_fops = { .poll = dev_user_poll, .unlocked_ioctl = dev_user_ioctl, @@ -1707,21 +1706,16 @@ static int dev_user_reply_cmd(struct file *file, void __user *arg) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (unlikely(res != 0)) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); rc = copy_from_user(&reply, arg, sizeof(reply)); if (unlikely(rc != 0)) { PRINT_ERROR("Failed to copy %d user's bytes", rc); res = -EFAULT; - goto out_up; + goto out; } TRACE_DBG("Reply for dev %s", dev->name); @@ -1730,10 +1724,7 @@ static int dev_user_reply_cmd(struct file *file, void __user *arg) res = dev_user_process_reply(dev, &reply); if (unlikely(res < 0)) - goto out_up; - -out_up: - up_read(&dev->dev_rwsem); + goto out; out: TRACE_EXIT_RES(res); @@ -1750,21 +1741,16 @@ static int dev_user_get_ext_cdb(struct file *file, void __user *arg) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (unlikely(res != 0)) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); rc = copy_from_user(&get, arg, sizeof(get)); if (unlikely(rc != 0)) { PRINT_ERROR("Failed to copy %d user's bytes", rc); res = -EFAULT; - goto out_up; + goto out; } TRACE_MGMT_DBG("Get ext cdb for dev %s", dev->name); @@ -1826,16 +1812,13 @@ out_cmd_put: out_put: ucmd_put(ucmd); -out_up: - up_read(&dev->dev_rwsem); - out: TRACE_EXIT_RES(res); return res; out_unlock: spin_unlock_irq(&dev->udev_cmd_threads.cmd_list_lock); - goto out_up; + goto out; } static int dev_user_process_scst_commands(struct scst_user_dev *dev) @@ -1979,15 +1962,10 @@ static int dev_user_reply_get_cmd(struct file *file, void __user *arg) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (unlikely(res != 0)) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); /* get_user() can't be used with 64-bit values on x86_32 */ rc = copy_from_user(&ureply, (uint64_t __user *) @@ -1996,7 +1974,7 @@ static int dev_user_reply_get_cmd(struct file *file, void __user *arg) if (unlikely(rc != 0)) { PRINT_ERROR("Failed to copy %d user's bytes", rc); res = -EFAULT; - goto out_up; + goto out; } TRACE_DBG("ureply %lld (dev %s)", (unsigned long long int)ureply, @@ -2005,7 +1983,7 @@ static int dev_user_reply_get_cmd(struct file *file, void __user *arg) cmd = kmem_cache_alloc(user_get_cmd_cachep, GFP_KERNEL); if (unlikely(cmd == NULL)) { res = -ENOMEM; - goto out_up; + goto out; } if (ureply != 0) { @@ -2069,16 +2047,13 @@ again: } else spin_unlock_irq(&dev->udev_cmd_threads.cmd_list_lock); -out_up: - up_read(&dev->dev_rwsem); - out: TRACE_EXIT_RES(res); return res; out_free: kmem_cache_free(user_get_cmd_cachep, cmd); - goto out_up; + goto out; } static long dev_user_ioctl(struct file *file, unsigned int cmd, @@ -2187,15 +2162,10 @@ static unsigned int dev_user_poll(struct file *file, poll_table *wait) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (unlikely(res != 0)) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); spin_lock_irq(&dev->udev_cmd_threads.cmd_list_lock); @@ -2222,8 +2192,6 @@ static unsigned int dev_user_poll(struct file *file, poll_table *wait) out_unlock: spin_unlock_irq(&dev->udev_cmd_threads.cmd_list_lock); - up_read(&dev->dev_rwsem); - out: TRACE_EXIT_HRES(res); return res; @@ -2977,7 +2945,6 @@ static int dev_user_register_dev(struct file *file, goto out_put; } - init_rwsem(&dev->dev_rwsem); INIT_LIST_HEAD(&dev->ready_cmd_list); if (file->f_flags & O_NONBLOCK) { TRACE_DBG("%s", "Non-blocking operations"); @@ -3086,15 +3053,19 @@ static int dev_user_register_dev(struct file *file, goto out_unreg_handler; } - mutex_lock(&dev_priv_mutex); + spin_lock(&dev_list_lock); if (file->private_data != NULL) { - mutex_unlock(&dev_priv_mutex); + spin_unlock(&dev_list_lock); PRINT_ERROR("%s", "Device already registered"); res = -EINVAL; goto out_unreg_drv; } + /* + * Assumption here is that the private_data reading is atomic, + * hence could be lockless and without ACCESS_ONCE(). + */ file->private_data = dev; - mutex_unlock(&dev_priv_mutex); + spin_unlock(&dev_list_lock); out: TRACE_EXIT_RES(res); @@ -3129,57 +3100,9 @@ out_put: static int dev_user_unregister_dev(struct file *file) { - int res; - struct scst_user_dev *dev; - - TRACE_ENTRY(); - - mutex_lock(&dev_priv_mutex); - dev = file->private_data; - res = dev_user_check_reg(dev); - if (res != 0) { - mutex_unlock(&dev_priv_mutex); - goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); - - res = scst_suspend_activity(SCST_SUSPEND_TIMEOUT_USER); - if (res != 0) - goto out_up; - - up_read(&dev->dev_rwsem); - - mutex_lock(&dev_priv_mutex); - dev = file->private_data; - if (dev == NULL) { - mutex_unlock(&dev_priv_mutex); - goto out_resume; - } - - dev->blocking = 0; - wake_up_all(&dev->udev_cmd_threads.cmd_list_waitQ); - - down_write(&dev->dev_rwsem); - file->private_data = NULL; - mutex_unlock(&dev_priv_mutex); - - dev_user_exit_dev(dev); - - up_write(&dev->dev_rwsem); /* to make lockdep happy */ - - kmem_cache_free(user_dev_cachep, dev); - -out_resume: - scst_resume_activity(); - -out: - TRACE_EXIT_RES(res); - return res; - -out_up: - up_read(&dev->dev_rwsem); - goto out; + PRINT_WARNING("SCST_USER_UNREGISTER_DEVICE is obsolete and NOOP. " + "Closing fd should be used instead."); + return 0; } static int dev_user_flush_cache(struct file *file) @@ -3189,28 +3112,20 @@ static int dev_user_flush_cache(struct file *file) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (res != 0) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); res = scst_suspend_activity(SCST_SUSPEND_TIMEOUT_USER); if (res != 0) - goto out_up; + goto out; sgv_pool_flush(dev->pool); sgv_pool_flush(dev->pool_clust); scst_resume_activity(); -out_up: - up_read(&dev->dev_rwsem); - out: TRACE_EXIT_RES(res); return res; @@ -3223,20 +3138,13 @@ static int dev_user_capacity_changed(struct file *file) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (res != 0) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); scst_capacity_data_changed(dev->sdev); - up_read(&dev->dev_rwsem); - out: TRACE_EXIT_RES(res); return res; @@ -3256,21 +3164,16 @@ static int dev_user_prealloc_buffer(struct file *file, void __user *arg) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (unlikely(res != 0)) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); rc = copy_from_user(&pre.in, arg, sizeof(pre.in)); if (unlikely(rc != 0)) { PRINT_ERROR("Failed to copy %d user's bytes", rc); res = -EFAULT; - goto out_up; + goto out; } TRACE_MEM("Prealloc buffer with size %dKB for dev %s", @@ -3283,7 +3186,7 @@ static int dev_user_prealloc_buffer(struct file *file, void __user *arg) ucmd = dev_user_alloc_ucmd(dev, GFP_KERNEL); if (ucmd == NULL) { res = -ENOMEM; - goto out_up; + goto out; } ucmd->buff_cached = 1; @@ -3336,9 +3239,6 @@ static int dev_user_prealloc_buffer(struct file *file, void __user *arg) out_put: ucmd_put(ucmd); -out_up: - up_read(&dev->dev_rwsem); - out: TRACE_EXIT_RES(res); return res; @@ -3433,27 +3333,19 @@ static int dev_user_set_opt(struct file *file, const struct scst_user_opt *opt) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (res != 0) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); res = scst_suspend_activity(SCST_SUSPEND_TIMEOUT_USER); if (res != 0) - goto out_up; + goto out; res = __dev_user_set_opt(dev, opt); scst_resume_activity(); -out_up: - up_read(&dev->dev_rwsem); - out: TRACE_EXIT_RES(res); return res; @@ -3467,15 +3359,10 @@ static int dev_user_get_opt(struct file *file, void __user *arg) TRACE_ENTRY(); - mutex_lock(&dev_priv_mutex); dev = file->private_data; res = dev_user_check_reg(dev); - if (res != 0) { - mutex_unlock(&dev_priv_mutex); + if (unlikely(res != 0)) goto out; - } - down_read(&dev->dev_rwsem); - mutex_unlock(&dev_priv_mutex); opt.parse_type = dev->parse_type; opt.on_free_cmd_type = dev->on_free_cmd_type; @@ -3501,12 +3388,9 @@ static int dev_user_get_opt(struct file *file, void __user *arg) if (unlikely(rc != 0)) { PRINT_ERROR("Failed to copy to user %d bytes", rc); res = -EFAULT; - goto out_up; + goto out; } -out_up: - up_read(&dev->dev_rwsem); - out: TRACE_EXIT_RES(res); return res; diff --git a/usr/fileio/fileio.c b/usr/fileio/fileio.c index 2b808f404..935f75d70 100644 --- a/usr/fileio/fileio.c +++ b/usr/fileio/fileio.c @@ -476,6 +476,7 @@ out_unreg: alarm(0); for (i = 0; i < num_devs; i++) { if (unreg_before_close) { + /* Just to see the obsolete call message */ res = ioctl(devs[i].scst_usr_fd, SCST_USER_UNREGISTER_DEVICE, NULL); if (res != 0) { res = errno;