[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20150731193107.GQ21689@ZenIV.linux.org.uk>
Date: Fri, 31 Jul 2015 20:31:07 +0100
From: Joel Becker <jlbec@...lplan.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Alasdair Kergon <agk@...hat.com>,
Mike Snitzer <snitzer@...hat.com>, Neil Brown <neilb@...e.com>,
Alessandro Zummo <a.zummo@...ertech.it>,
Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
Oleg Drokin <oleg.drokin@...el.com>,
Andreas Dilger <andreas.dilger@...el.com>,
"Michael S. Tsirkin" <mst@...hat.com>,
Wim Van Sebroeck <wim@...ana.be>,
Christine Caulfield <ccaulfie@...hat.com>,
David Teigland <teigland@...hat.com>,
Mark Fasheh <mfasheh@...e.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] char: make misc_deregister a void function
Acked-by: Joel Becker <jlbec@...lplan.org>
On Thu, Jul 30, 2015 at 03:59:57PM -0700, Greg Kroah-Hartman wrote:
> From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>
> With well over 200+ users of this api, there are a mere 12 users that
> actually cheked the return value of this function. And all of them
> really didn't do anything with that information as the system or module
> was shutting down no matter what.
>
> So stop pretending like it matters, and just return void from
> misc_deregister(). If something goes wrong in the call, you will get a
> WARNING splat in the syslog so you know how to fix up your driver.
> Other than that, there's nothing that can go wrong.
>
> Cc: Alasdair Kergon <agk@...hat.com>
> Cc: Mike Snitzer <snitzer@...hat.com>
> Cc: Neil Brown <neilb@...e.com>
> Cc: Alessandro Zummo <a.zummo@...ertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@...e-electrons.com>
> Cc: Oleg Drokin <oleg.drokin@...el.com>
> Cc: Andreas Dilger <andreas.dilger@...el.com>
> Cc: "Michael S. Tsirkin" <mst@...hat.com>
> Cc: Wim Van Sebroeck <wim@...ana.be>
> Cc: Christine Caulfield <ccaulfie@...hat.com>
> Cc: David Teigland <teigland@...hat.com>
> Cc: Mark Fasheh <mfasheh@...e.com>
> Cc: Joel Becker <jlbec@...lplan.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>
> ---
>
> If the different subsystem maintainers want to give me an ack for this,
> I'd appreciate it. I'd like to just take the single patch in through
> the char-misc tree in one piece.
>
>
> drivers/char/misc.c | 9 +++------
> drivers/md/dm-ioctl.c | 4 +---
> drivers/misc/vmw_vmci/vmci_host.c | 5 +----
> drivers/rtc/rtc-ds1374.c | 5 ++---
> drivers/staging/android/ashmem.c | 5 +----
> drivers/staging/android/ion/ion_test.c | 3 ++-
> drivers/staging/lustre/lustre/libcfs/module.c | 4 +---
> drivers/vhost/scsi.c | 4 ++--
> drivers/watchdog/at91rm9200_wdt.c | 5 ++---
> drivers/watchdog/ks8695_wdt.c | 9 +++------
> drivers/watchdog/ts72xx_wdt.c | 3 ++-
> fs/dlm/user.c | 9 +++------
> fs/ocfs2/stack_user.c | 9 +--------
> include/linux/miscdevice.h | 2 +-
> 14 files changed, 25 insertions(+), 51 deletions(-)
>
>
> --- a/drivers/char/misc.c
> +++ b/drivers/char/misc.c
> @@ -243,17 +243,15 @@ int misc_register(struct miscdevice * mi
> * @misc: device to unregister
> *
> * Unregister a miscellaneous device that was previously
> - * successfully registered with misc_register(). Success
> - * is indicated by a zero return, a negative errno code
> - * indicates an error.
> + * successfully registered with misc_register().
> */
>
> -int misc_deregister(struct miscdevice *misc)
> +void misc_deregister(struct miscdevice *misc)
> {
> int i = DYNAMIC_MINORS - misc->minor - 1;
>
> if (WARN_ON(list_empty(&misc->list)))
> - return -EINVAL;
> + return;
>
> mutex_lock(&misc_mtx);
> list_del(&misc->list);
> @@ -261,7 +259,6 @@ int misc_deregister(struct miscdevice *m
> if (i < DYNAMIC_MINORS && i >= 0)
> clear_bit(i, misc_minors);
> mutex_unlock(&misc_mtx);
> - return 0;
> }
>
> EXPORT_SYMBOL(misc_register);
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1919,9 +1919,7 @@ int __init dm_interface_init(void)
>
> void dm_interface_exit(void)
> {
> - if (misc_deregister(&_dm_misc) < 0)
> - DMERR("misc_deregister failed for control device");
> -
> + misc_deregister(&_dm_misc);
> dm_hash_exit();
> }
>
> --- a/drivers/misc/vmw_vmci/vmci_host.c
> +++ b/drivers/misc/vmw_vmci/vmci_host.c
> @@ -1035,10 +1035,7 @@ void __exit vmci_host_exit(void)
>
> vmci_host_device_initialized = false;
>
> - error = misc_deregister(&vmci_host_miscdev);
> - if (error)
> - pr_warn("Error unregistering character device: %d\n", error);
> -
> + misc_deregister(&vmci_host_miscdev);
> vmci_ctx_destroy(host_context);
> vmci_qp_broker_exit();
>
> --- a/drivers/rtc/rtc-ds1374.c
> +++ b/drivers/rtc/rtc-ds1374.c
> @@ -666,9 +666,8 @@ static int ds1374_remove(struct i2c_clie
> #ifdef CONFIG_RTC_DRV_DS1374_WDT
> int res;
>
> - res = misc_deregister(&ds1374_miscdev);
> - if (!res)
> - ds1374_miscdev.parent = NULL;
> + misc_deregister(&ds1374_miscdev);
> + ds1374_miscdev.parent = NULL;
> unregister_reboot_notifier(&ds1374_wdt_notifier);
> #endif
>
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -867,10 +867,7 @@ static void __exit ashmem_exit(void)
>
> unregister_shrinker(&ashmem_shrinker);
>
> - ret = misc_deregister(&ashmem_misc);
> - if (unlikely(ret))
> - pr_err("failed to unregister misc device!\n");
> -
> + misc_deregister(&ashmem_misc);
> kmem_cache_destroy(ashmem_range_cachep);
> kmem_cache_destroy(ashmem_area_cachep);
>
> --- a/drivers/staging/android/ion/ion_test.c
> +++ b/drivers/staging/android/ion/ion_test.c
> @@ -269,7 +269,8 @@ static int ion_test_remove(struct platfo
> if (!testdev)
> return -ENODATA;
>
> - return misc_deregister(&testdev->misc);
> + misc_deregister(&testdev->misc);
> + return 0;
> }
>
> static struct platform_device *ion_test_pdev;
> --- a/drivers/staging/lustre/lustre/libcfs/module.c
> +++ b/drivers/staging/lustre/lustre/libcfs/module.c
> @@ -467,9 +467,7 @@ static void exit_libcfs_module(void)
> cfs_crypto_unregister();
> cfs_wi_shutdown();
>
> - rc = misc_deregister(&libcfs_dev);
> - if (rc)
> - CERROR("misc_deregister error %d\n", rc);
> + misc_deregister(&libcfs_dev);
>
> cfs_cpu_fini();
>
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1573,9 +1573,9 @@ static int __init vhost_scsi_register(vo
> return misc_register(&vhost_scsi_misc);
> }
>
> -static int vhost_scsi_deregister(void)
> +static void vhost_scsi_deregister(void)
> {
> - return misc_deregister(&vhost_scsi_misc);
> + misc_deregister(&vhost_scsi_misc);
> }
>
> static char *vhost_scsi_dump_proto_id(struct vhost_scsi_tport *tport)
> --- a/drivers/watchdog/at91rm9200_wdt.c
> +++ b/drivers/watchdog/at91rm9200_wdt.c
> @@ -269,9 +269,8 @@ static int at91wdt_remove(struct platfor
> if (res)
> dev_warn(dev, "failed to unregister restart handler\n");
>
> - res = misc_deregister(&at91wdt_miscdev);
> - if (!res)
> - at91wdt_miscdev.parent = NULL;
> + misc_deregister(&at91wdt_miscdev);
> + at91wdt_miscdev.parent = NULL;
>
> return res;
> }
> --- a/drivers/watchdog/ks8695_wdt.c
> +++ b/drivers/watchdog/ks8695_wdt.c
> @@ -254,13 +254,10 @@ static int ks8695wdt_probe(struct platfo
>
> static int ks8695wdt_remove(struct platform_device *pdev)
> {
> - int res;
> + misc_deregister(&ks8695wdt_miscdev);
> + ks8695wdt_miscdev.parent = NULL;
>
> - res = misc_deregister(&ks8695wdt_miscdev);
> - if (!res)
> - ks8695wdt_miscdev.parent = NULL;
> -
> - return res;
> + return 0;
> }
>
> static void ks8695wdt_shutdown(struct platform_device *pdev)
> --- a/drivers/watchdog/ts72xx_wdt.c
> +++ b/drivers/watchdog/ts72xx_wdt.c
> @@ -428,7 +428,8 @@ static int ts72xx_wdt_probe(struct platf
>
> static int ts72xx_wdt_remove(struct platform_device *pdev)
> {
> - return misc_deregister(&ts72xx_wdt_miscdev);
> + misc_deregister(&ts72xx_wdt_miscdev);
> + return 0;
> }
>
> static struct platform_driver ts72xx_wdt_driver = {
> --- a/fs/dlm/user.c
> +++ b/fs/dlm/user.c
> @@ -362,18 +362,15 @@ fail:
>
> int dlm_device_deregister(struct dlm_ls *ls)
> {
> - int error;
> -
> /* The device is not registered. This happens when the lockspace
> was never used from userspace, or when device_create_lockspace()
> calls dlm_release_lockspace() after the register fails. */
> if (!ls->ls_device.name)
> return 0;
>
> - error = misc_deregister(&ls->ls_device);
> - if (!error)
> - kfree(ls->ls_device.name);
> - return error;
> + misc_deregister(&ls->ls_device);
> + kfree(ls->ls_device.name);
> + return 0;
> }
>
> static int device_user_purge(struct dlm_user_proc *proc,
> --- a/fs/ocfs2/stack_user.c
> +++ b/fs/ocfs2/stack_user.c
> @@ -655,14 +655,7 @@ static int ocfs2_control_init(void)
>
> static void ocfs2_control_exit(void)
> {
> - int rc;
> -
> - rc = misc_deregister(&ocfs2_control_device);
> - if (rc)
> - printk(KERN_ERR
> - "ocfs2: Unable to deregister ocfs2_control device "
> - "(errno %d)\n",
> - -rc);
> + misc_deregister(&ocfs2_control_device);
> }
>
> static void fsdlm_lock_ast_wrapper(void *astarg)
> --- a/include/linux/miscdevice.h
> +++ b/include/linux/miscdevice.h
> @@ -67,7 +67,7 @@ struct miscdevice {
> };
>
> extern int misc_register(struct miscdevice *misc);
> -extern int misc_deregister(struct miscdevice *misc);
> +extern void misc_deregister(struct miscdevice *misc);
>
> #define MODULE_ALIAS_MISCDEV(minor) \
> MODULE_ALIAS("char-major-" __stringify(MISC_MAJOR) \
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists