Re: [RFC PATCH v2] lightnvm: add mechanism to trigger pblk close on reboot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On 23 Mar 2019, at 02.07, Marcin Dziegielewski <marcin.dziegielewski@xxxxxxxxx> wrote:
> 
> Currently if we issue reboot to the system pblk will close
> ungracefully and in consequence it will need recovery on load.
> 
> This patch propose utilize of reboot notifier feature to trigger
> gracefull pblk shutdown on reboot.
> 
> Signed-off-by: Marcin Dziegielewski <marcin.dziegielewski@xxxxxxxxx>
> —

Please, add changes since V1 next time.

> drivers/lightnvm/core.c | 65 ++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 51 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 5f82036..4a421f5 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -27,6 +27,7 @@
> #include <linux/miscdevice.h>
> #include <linux/lightnvm.h>
> #include <linux/sched/sysctl.h>
> +#include <linux/reboot.h>
> 
> static LIST_HEAD(nvm_tgt_types);
> static DECLARE_RWSEM(nvm_tgtt_lock);
> @@ -1138,9 +1139,52 @@ struct nvm_dev *nvm_alloc_dev(int node)
> }
> EXPORT_SYMBOL(nvm_alloc_dev);
> 
> +static void _nvm_unregister(struct nvm_dev *dev, bool graceful)
> +{
> +	struct nvm_target *t, *tmp;
> +
> +	mutex_lock(&dev->mlock);
> +	list_for_each_entry_safe(t, tmp, &dev->targets, list) {
> +		if (t->dev->parent != dev)
> +			continue;
> +		__nvm_remove_target(t, graceful);
> +	}
> +	mutex_unlock(&dev->mlock);
> +
> +	list_del(&dev->devices);
> +
> +	nvm_free(dev);
> +}
> +
> +static int nvm_notify_reboot(struct notifier_block *this,
> +			    unsigned long code, void *x)
> +{
> +	struct nvm_dev *dev, *t;
> +
> +	down_write(&nvm_lock);
> +	if (list_empty(&nvm_devices)) {
> +		up_write(&nvm_lock);
> +		return NOTIFY_DONE;
> +	}
> +
> +	list_for_each_entry_safe(dev, t, &nvm_devices, devices)
> +		_nvm_unregister(dev, true);
> +
> +	up_write(&nvm_lock);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block nvm_notifier = {
> +	.notifier_call	= nvm_notify_reboot,
> +	.next		= NULL,
> +	.priority	= INT_MAX, /* before any real devices */

Nip: Can you align all values at ‘=‘?

> +};
> +
> int nvm_register(struct nvm_dev *dev)
> {
> 	int ret, exp_pool_size;
> +	bool is_first;
> 
> 	if (!dev->q || !dev->ops)
> 		return -EINVAL;
> @@ -1161,32 +1205,25 @@ int nvm_register(struct nvm_dev *dev)
> 		return -ENOMEM;
> 	}
> 
> -	/* register device with a supported media manager */
> 	down_write(&nvm_lock);
> +	is_first = list_empty(&nvm_devices);
> +
> +	/* register device with a supported media manager */
> 	list_add(&dev->devices, &nvm_devices);
> 	up_write(&nvm_lock);
> 
> +	if (is_first)
> +		register_reboot_notifier(&nvm_notifier);
> +
> 	return 0;
> }
> EXPORT_SYMBOL(nvm_register);
> 
> void nvm_unregister(struct nvm_dev *dev)
> {
> -	struct nvm_target *t, *tmp;
> -
> -	mutex_lock(&dev->mlock);
> -	list_for_each_entry_safe(t, tmp, &dev->targets, list) {
> -		if (t->dev->parent != dev)
> -			continue;
> -		__nvm_remove_target(t, false);
> -	}
> -	mutex_unlock(&dev->mlock);
> -
> 	down_write(&nvm_lock);
> -	list_del(&dev->devices);
> +	_nvm_unregister(dev, false);
> 	up_write(&nvm_lock);
> -
> -	nvm_free(dev);
> }
> EXPORT_SYMBOL(nvm_unregister);
> 
> --
> 1.8.3.1

Otherwise, it looks good to me.

Reviewed-by: Javier González <javier@xxxxxxxxxxx>


Attachment: signature.asc
Description: Message signed with OpenPGP


[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux