On 2020/07/03 10:31, Chaitanya Kulkarni wrote:
> The nullb_list is destroyed when error occurs in the null_init() and
> when removing the module in null_exit(). The identical code is repeated
> in those functions which can be a part of helper function. This also
> removes the extra variable struct nullb *nullb in the both functions.
>
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> ---
>
> * Changes from V1:-
>
> 1. Add reviewed-by and missing signoff.
> 2. Replace list_entry() with list_first_entry() in nullb_list
> deletion sequence.
>
> ---
> drivers/block/null_blk_main.c | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
> index 907c6858aec0..5095942d9b53 100644
> --- a/drivers/block/null_blk_main.c
> +++ b/drivers/block/null_blk_main.c
> @@ -1868,11 +1868,23 @@ static int null_add_dev(struct nullb_device *dev)
> return rv;
> }
>
> +static void null_delete_nullb_list(void)
> +{
> + struct nullb_device *dev;
> + struct nullb *nullb;
> +
> + while (!list_empty(&nullb_list)) {
> + nullb = list_first_entry(nullb_list.next, struct nullb, list);
> + dev = nullb->dev;
> + null_del_dev(nullb);
> + null_free_dev(dev);
> + }
> +}
> +
> static int __init null_init(void)
> {
> int ret = 0;
> unsigned int i;
> - struct nullb *nullb;
> struct nullb_device *dev;
>
> if (g_bs > PAGE_SIZE) {
> @@ -1939,12 +1951,7 @@ static int __init null_init(void)
> return 0;
>
> err_dev:
> - while (!list_empty(&nullb_list)) {
> - nullb = list_entry(nullb_list.next, struct nullb, list);
> - dev = nullb->dev;
> - null_del_dev(nullb);
> - null_free_dev(dev);
> - }
> + null_delete_nullb_list();
> unregister_blkdev(null_major, "nullb");
> err_conf:
> configfs_unregister_subsystem(&nullb_subsys);
> @@ -1956,21 +1963,12 @@ static int __init null_init(void)
>
> static void __exit null_exit(void)
> {
> - struct nullb *nullb;
> -
> configfs_unregister_subsystem(&nullb_subsys);
>
> unregister_blkdev(null_major, "nullb");
>
> mutex_lock(&lock);
> - while (!list_empty(&nullb_list)) {
> - struct nullb_device *dev;
> -
> - nullb = list_entry(nullb_list.next, struct nullb, list);
> - dev = nullb->dev;
> - null_del_dev(nullb);
> - null_free_dev(dev);
> - }
> + null_delete_nullb_list();
> mutex_unlock(&lock);
>
> if (g_queue_mode == NULL_Q_MQ && shared_tags)
>
Looks good to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
--
Damien Le Moal
Western Digital Research
