[PATCH] checkpatch: Add kmap and kmap_atomic to the deprecated list
From: Ira Weiny kmap() and kmap_atomic() are being deprecated in favor of kmap_local_page(). There are two main problems with kmap(): (1) It comes with an overhead as mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap’s pool wraps and it might block when the mapping space is fully utilized until a slot becomes available. kmap_local_page() is safe from any context and is therefore redundant with kmap_atomic() with the exception of any pagefault or preemption disable requirements. However, using kmap_atomic() for these side effects makes the code less clear. So any requirement for pagefault or preemption disable should be made explicitly. With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored. Suggested-by: Thomas Gleixner Suggested-by: Fabio M. De Francesco Signed-off-by: Ira Weiny --- Suggested by credits. Thomas: Idea to keep from growing more kmap/kmap_atomic calls. Fabio: Stole some of his boiler plate commit message. Notes on tree-wide conversions: I've cc'ed mailing lists for subsystems which currently contains either kmap() or kmap_atomic() calls. As some of you already know Fabio and I have been working through converting kmap() calls to kmap_local_page(). But there is a lot more work to be done. Help from the community is always welcome, especially with kmap_atomic() conversions. To keep from stepping on each others toes I've created a spreadsheet of the current calls[1]. Please let me or Fabio know if you plan on tacking one of the conversions so we can mark it off the list. [1] https://docs.google.com/spreadsheets/d/1i_ckZ10p90bH_CkxD2bYNi05S2Qz84E2OFPv8zq__0w/edit#gid=1679714357 --- scripts/checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 79e759aac543..9ff219e0a9d5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -807,6 +807,8 @@ our %deprecated_apis = ( "rcu_barrier_sched" => "rcu_barrier", "get_state_synchronize_sched" => "get_state_synchronize_rcu", "cond_synchronize_sched"=> "cond_synchronize_rcu", + "kmap" => "kmap_local_page", + "kmap_atomic" => "kmap_local_page", ); #Create a search pattern for all these strings to speed up a loop below base-commit: 4a9350597aff50bbd0f4b80ccf49d2e02df5 -- 2.35.3 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/9] treewide: eliminate anonymous module_init & module_exit
On Wed, Mar 16, 2022 at 12:20:01PM -0700, Randy Dunlap wrote: > There are a number of drivers that use "module_init(init)" and > "module_exit(exit)", which are anonymous names and can lead to > confusion or ambiguity when reading System.map, crashes/oops/bugs, > or an initcall_debug log. > > Give each of these init and exit functions unique driver-specific > names to eliminate the anonymous names. I'm not fully sure about the Fixes tags but I don't see that it hurts anything. For the series: Reviewed-by: Ira Weiny > > Example 1: (System.map) > 832fc78c t init > 832fc79e t init > 832fc8f8 t init > 832fca05 t init > 832fcbd2 t init > 83328f0e t init > 8332c5b1 t init > 8332d9eb t init > 8332f0aa t init > 83330e25 t init > 833317a5 t init > 8333dd6b t init > > Example 2: (initcall_debug log) > calling init+0x0/0x12 @ 1 > initcall init+0x0/0x12 returned 0 after 15 usecs > calling init+0x0/0x60 @ 1 > initcall init+0x0/0x60 returned 0 after 2 usecs > calling init+0x0/0x9a @ 1 > initcall init+0x0/0x9a returned 0 after 74 usecs > calling init+0x0/0x73 @ 1 > initcall init+0x0/0x73 returned 0 after 6 usecs > calling init+0x0/0x73 @ 1 > initcall init+0x0/0x73 returned 0 after 4 usecs > calling init+0x0/0xf5 @ 1 > initcall init+0x0/0xf5 returned 0 after 27 usecs > calling init+0x0/0x7d @ 1 > initcall init+0x0/0x7d returned 0 after 11 usecs > calling init+0x0/0xc9 @ 1 > initcall init+0x0/0xc9 returned 0 after 19 usecs > calling init+0x0/0x9d @ 1 > initcall init+0x0/0x9d returned 0 after 37 usecs > calling init+0x0/0x63f @ 1 > initcall init+0x0/0x63f returned 0 after 411 usecs > calling init+0x0/0x171 @ 1 > initcall init+0x0/0x171 returned 0 after 61 usecs > calling init+0x0/0xef @ 1 > initcall init+0x0/0xef returned 0 after 3 usecs > > Cc: "Michael S. Tsirkin" > Cc: Jason Wang > Cc: Paolo Bonzini > Cc: Stefan Hajnoczi > Cc: Jens Axboe > Cc: Amit Shah > Cc: Arnd Bergmann > Cc: Greg Kroah-Hartman > Cc: Eli Cohen > Cc: Saeed Mahameed > Cc: Leon Romanovsky > Cc: Pablo Neira Ayuso > Cc: Jozsef Kadlecsik > Cc: Florian Westphal > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: "James E.J. Bottomley" > Cc: "Martin K. Petersen" > Cc: Felipe Balbi > Cc: Michał Mirosław > Cc: Sebastian Andrzej Siewior > Cc: Krzysztof Opasiak > Cc: Igor Kotrasinski > Cc: Valentina Manea > Cc: Shuah Khan > Cc: Shuah Khan > Cc: Jussi Kivilinna > Cc: Joachim Fritschi > Cc: Herbert Xu > Cc: Thomas Gleixner > Cc: Steven Rostedt > Cc: Ingo Molnar > Cc: Karol Herbst > Cc: Pekka Paalanen > Cc: Dave Hansen > Cc: Andy Lutomirski > Cc: Peter Zijlstra > Cc: Borislav Petkov > Cc: "H. Peter Anvin" > Cc: netfilter-de...@vger.kernel.org > Cc: coret...@netfilter.org > Cc: net...@vger.kernel.org > Cc: linux-bl...@vger.kernel.org > Cc: linux-cry...@vger.kernel.org > Cc: linux-r...@vger.kernel.org > Cc: linux-s...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: nouv...@lists.freedesktop.org > Cc: virtualization@lists.linux-foundation.org > Cc: x...@kernel.org > > patches: > [PATCH 1/9] virtio_blk: eliminate anonymous module_init & module_exit > [PATCH 2/9] virtio_console: eliminate anonymous module_init & module_exit > [PATCH 3/9] net: mlx5: eliminate anonymous module_init & module_exit > [PATCH 4/9] netfilter: h323: eliminate anonymous module_init & module_exit > [PATCH 5/9] virtio-scsi: eliminate anonymous module_init & module_exit > [PATCH 6/9] usb: gadget: eliminate anonymous module_init & module_exit > [PATCH 7/9] usb: usbip: eliminate anonymous module_init & module_exit > [PATCH 8/9] x86/crypto: eliminate anonymous module_init & module_exit > [PATCH 9/9] testmmiotrace: eliminate anonymous module_init & module_exit > > diffstat: > arch/x86/crypto/blowfish_glue.c|8 > arch/x86/crypto/camellia_glue.c|8 > arch/x86/crypto/serpent_avx2_glue.c|8 > arch/x86/crypto/twofish_glue.c |8 > arch/x86/crypto/twofish_glue_3way.c|8 > arch/x86/mm/testmmiotrace.c|8 > drivers/block/virtio_blk.c |8 > drivers/char/virtio_console.c |8 > drivers/net/ethernet/mellanox/mlx5/core/main.c |8 > drivers/scsi/virtio_scsi.c |8 > dr
Re: [PATCH 6/9] usb: gadget: eliminate anonymous module_init & module_exit
On Wed, Mar 16, 2022 at 12:20:07PM -0700, Randy Dunlap wrote:
> Eliminate anonymous module_init() and module_exit(), which can lead to
> confusion or ambiguity when reading System.map, crashes/oops/bugs,
> or an initcall_debug log.
>
> Give each of these init and exit functions unique driver-specific
> names to eliminate the anonymous names.
>
> Example 1: (System.map)
> 832fc78c t init
> 832fc79e t init
> 832fc8f8 t init
>
> Example 2: (initcall_debug log)
> calling init+0x0/0x12 @ 1
> initcall init+0x0/0x12 returned 0 after 15 usecs
> calling init+0x0/0x60 @ 1
> initcall init+0x0/0x60 returned 0 after 2 usecs
> calling init+0x0/0x9a @ 1
> initcall init+0x0/0x9a returned 0 after 74 usecs
>
> Fixes: bd25a14edb75 ("usb: gadget: legacy/serial: allow dynamic removal")
> Fixes: 7bb5ea54be47 ("usb gadget serial: use composite gadget framework")
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I continue to be confused about the latest rules for the Fixes tag but this one
in particular seems completely useless. This is the 'beginning of time' commit
by Linus AFAICT. So do any of these Fixes tags need to be in this series?
Regardless:
Reviewed-by: Ira Weiny
> Signed-off-by: Randy Dunlap
> Cc: Felipe Balbi
> Cc: Michał Mirosław
> Cc: Greg Kroah-Hartman
> Cc: Sebastian Andrzej Siewior
> Cc: linux-...@vger.kernel.org
> ---
> drivers/usb/gadget/legacy/inode.c |8
> drivers/usb/gadget/legacy/serial.c | 10 +-
> drivers/usb/gadget/udc/dummy_hcd.c |8
> 3 files changed, 13 insertions(+), 13 deletions(-)
>
> --- lnx-517-rc8.orig/drivers/usb/gadget/legacy/serial.c
> +++ lnx-517-rc8/drivers/usb/gadget/legacy/serial.c
> @@ -273,7 +273,7 @@ static struct usb_composite_driver gseri
> static int switch_gserial_enable(bool do_enable)
> {
> if (!serial_config_driver.label)
> - /* init() was not called, yet */
> + /* gserial_init() was not called, yet */
> return 0;
>
> if (do_enable)
> @@ -283,7 +283,7 @@ static int switch_gserial_enable(bool do
> return 0;
> }
>
> -static int __init init(void)
> +static int __init gserial_init(void)
> {
> /* We *could* export two configs; that'd be much cleaner...
>* but neither of these product IDs was defined that way.
> @@ -314,11 +314,11 @@ static int __init init(void)
>
> return usb_composite_probe(_driver);
> }
> -module_init(init);
> +module_init(gserial_init);
>
> -static void __exit cleanup(void)
> +static void __exit gserial_cleanup(void)
> {
> if (enable)
> usb_composite_unregister(_driver);
> }
> -module_exit(cleanup);
> +module_exit(gserial_cleanup);
> --- lnx-517-rc8.orig/drivers/usb/gadget/udc/dummy_hcd.c
> +++ lnx-517-rc8/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -2765,7 +2765,7 @@ static struct platform_driver dummy_hcd_
> static struct platform_device *the_udc_pdev[MAX_NUM_UDC];
> static struct platform_device *the_hcd_pdev[MAX_NUM_UDC];
>
> -static int __init init(void)
> +static int __init dummy_hcd_init(void)
> {
> int retval = -ENOMEM;
> int i;
> @@ -2887,9 +2887,9 @@ err_alloc_udc:
> platform_device_put(the_hcd_pdev[i]);
> return retval;
> }
> -module_init(init);
> +module_init(dummy_hcd_init);
>
> -static void __exit cleanup(void)
> +static void __exit dummy_hcd_cleanup(void)
> {
> int i;
>
> @@ -2905,4 +2905,4 @@ static void __exit cleanup(void)
> platform_driver_unregister(_udc_driver);
> platform_driver_unregister(_hcd_driver);
> }
> -module_exit(cleanup);
> +module_exit(dummy_hcd_cleanup);
> --- lnx-517-rc8.orig/drivers/usb/gadget/legacy/inode.c
> +++ lnx-517-rc8/drivers/usb/gadget/legacy/inode.c
> @@ -2101,7 +2101,7 @@ MODULE_ALIAS_FS("gadgetfs");
>
> /*--*/
>
> -static int __init init (void)
> +static int __init gadgetfs_init (void)
> {
> int status;
>
> @@ -2111,12 +2111,12 @@ static int __init init (void)
> shortname, driver_desc);
> return status;
> }
> -module_init (init);
> +module_init (gadgetfs_init);
>
> -static void __exit cleanup (void)
> +static void __exit gadgetfs_cleanup (void)
> {
> pr_debug ("unregister %s\n", shortname);
> unregister_filesystem (_type);
> }
> -module_exit (cleanup);
> +module_exit (gadgetfs_cleanup);
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 03/11] dax: simplify the dax_device <-> gendisk association
On Mon, Oct 18, 2021 at 06:40:46AM +0200, Christoph Hellwig wrote:
> Replace the dax_host_hash with an xarray indexed by the pointer value
> of the gendisk, and require explicitl calls from the block drivers that
> want to associate their gendisk with a dax_device.
>
> Signed-off-by: Christoph Hellwig
> ---
> drivers/dax/bus.c| 2 +-
> drivers/dax/super.c | 106 +--
> drivers/md/dm.c | 6 +-
> drivers/nvdimm/pmem.c| 8 ++-
> drivers/s390/block/dcssblk.c | 11 +++-
> fs/fuse/virtio_fs.c | 2 +-
> include/linux/dax.h | 19 +--
> 7 files changed, 60 insertions(+), 94 deletions(-)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 6cc4da4c713d9..6d91b0186e3be 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -1326,7 +1326,7 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data
> *data)
>* No 'host' or dax_operations since there is no access to this
>* device outside of mmap of the resulting character device.
>*/
NIT: this comment needs to be updated as well.
Ira
> - dax_dev = alloc_dax(dev_dax, NULL, NULL, DAXDEV_F_SYNC);
> + dax_dev = alloc_dax(dev_dax, NULL, DAXDEV_F_SYNC);
> if (IS_ERR(dax_dev)) {
> rc = PTR_ERR(dax_dev);
> goto err_alloc_dax;
[snip]
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
