Re: [PATCH 4/9] pstore/blk: remove __unregister_pstore_blk

From: Kees Cook
Date: Tue Dec 01 2020 - 14:21:00 EST


On Fri, Oct 16, 2020 at 03:20:42PM +0200, Christoph Hellwig wrote:
> Fold __unregister_pstore_blk into its only caller, and merge the
> two identical calls to __unregister_pstore_device that exist in the
> caller now.

Meh, I'm not a fan of this. I like it in a separate function. Pushing it
into the _exit() routine feels weird to me.

>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/pstore/blk.c | 27 ++++++---------------------
> 1 file changed, 6 insertions(+), 21 deletions(-)
>
> diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
> index 7f8368e94b3604..3a2214ecf942ae 100644
> --- a/fs/pstore/blk.c
> +++ b/fs/pstore/blk.c
> @@ -388,19 +388,6 @@ static int __register_pstore_blk(void)
> return ret;
> }
>
> -static void __unregister_pstore_blk(unsigned int major)
> -{
> - struct pstore_device_info dev = { .read = psblk_generic_blk_read };
> - void *holder = blkdev;
> -
> - lockdep_assert_held(&pstore_blk_lock);
> - if (psblk_bdev && MAJOR(psblk_bdev->bd_dev) == major) {
> - __unregister_pstore_device(&dev);
> - psblk_put_bdev(psblk_bdev, holder);
> - psblk_bdev = NULL;
> - }
> -}
> -
> /* get information of pstore/blk */
> int pstore_blk_get_config(struct pstore_blk_config *info)
> {
> @@ -430,16 +417,14 @@ late_initcall(pstore_blk_init);
>
> static void __exit pstore_blk_exit(void)
> {
> + struct pstore_device_info dev = { };
> +
> mutex_lock(&pstore_blk_lock);
> + if (pstore_zone_info)
> + dev.read = pstore_zone_info->read;
> + __unregister_pstore_device(&dev);
> if (psblk_bdev)
> - __unregister_pstore_blk(MAJOR(psblk_bdev->bd_dev));
> - else {
> - struct pstore_device_info dev = { };
> -
> - if (pstore_zone_info)
> - dev.read = pstore_zone_info->read;
> - __unregister_pstore_device(&dev);
> - }
> + psblk_put_bdev(psblk_bdev, blkdev);
> mutex_unlock(&pstore_blk_lock);
> }
> module_exit(pstore_blk_exit);
> --
> 2.28.0
>

--
Kees Cook