> On 16 Apr 2019, at 12.16, Igor Konopko <[email protected]> wrote: > > Currently all the target instances are removed under global nvm_lock. > This was needed to ensure that nvm_dev struct will not be freed by > hot unplug event during target removal. However, current implementation > has some drawbacks, since the same lock is used when new nvme subsystem > is registered, so we can have a situation, that due to long process of > target removal on drive A, registration (and listing in OS) of the > drive B will take a lot of time, since it will wait for that lock. > > Now when we have kref which ensures that nvm_dev will not be freed in > the meantime, we can easily get rid of this lock for a time when we are > removing nvm targets. > > Signed-off-by: Igor Konopko <[email protected]> > --- > drivers/lightnvm/core.c | 34 ++++++++++++++++------------------ > 1 file changed, 16 insertions(+), 18 deletions(-) > > diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c > index 0e9f7996..0df7454 100644 > --- a/drivers/lightnvm/core.c > +++ b/drivers/lightnvm/core.c > @@ -483,7 +483,6 @@ static void __nvm_remove_target(struct nvm_target *t, > bool graceful) > > /** > * nvm_remove_tgt - Removes a target from the media manager > - * @dev: device > * @remove: ioctl structure with target name to remove. > * > * Returns: > @@ -491,18 +490,27 @@ static void __nvm_remove_target(struct nvm_target *t, > bool graceful) > * 1: on not found > * <0: on error > */ > -static int nvm_remove_tgt(struct nvm_dev *dev, struct nvm_ioctl_remove > *remove) > +static int nvm_remove_tgt(struct nvm_ioctl_remove *remove) > { > struct nvm_target *t; > + struct nvm_dev *dev; > > - mutex_lock(&dev->mlock); > - t = nvm_find_target(dev, remove->tgtname); > - if (!t) { > + down_read(&nvm_lock); > + list_for_each_entry(dev, &nvm_devices, devices) { > + mutex_lock(&dev->mlock); > + t = nvm_find_target(dev, remove->tgtname); > + if (t) { > + mutex_unlock(&dev->mlock); > + break; > + } > mutex_unlock(&dev->mlock); > - return 1; > } > + up_read(&nvm_lock); > + > + if (!t) > + return 1; > + > __nvm_remove_target(t, true); > - mutex_unlock(&dev->mlock); > kref_put(&dev->ref, nvm_free); > > return 0; > @@ -1348,8 +1356,6 @@ static long nvm_ioctl_dev_create(struct file *file, > void __user *arg) > static long nvm_ioctl_dev_remove(struct file *file, void __user *arg) > { > struct nvm_ioctl_remove remove; > - struct nvm_dev *dev; > - int ret = 0; > > if (copy_from_user(&remove, arg, sizeof(struct nvm_ioctl_remove))) > return -EFAULT; > @@ -1361,15 +1367,7 @@ static long nvm_ioctl_dev_remove(struct file *file, > void __user *arg) > return -EINVAL; > } > > - down_read(&nvm_lock); > - list_for_each_entry(dev, &nvm_devices, devices) { > - ret = nvm_remove_tgt(dev, &remove); > - if (!ret) > - break; > - } > - up_read(&nvm_lock); > - > - return ret; > + return nvm_remove_tgt(&remove); > } > > /* kept for compatibility reasons */ > -- > 2.9.5
Looks good to me Reviewed-by: Javier González <[email protected]>
signature.asc
Description: Message signed with OpenPGP
