From 00411b7b1e3ec477b75bb83ccd417c7609832db6 Mon Sep 17 00:00:00 2001 From: Gabriel Somlo Date: Mon, 22 Feb 2016 16:18:18 -0500 Subject: firmware: fw_cfg register offsets on supported architectures only Refrain from defining default fw_cfg register offsets on unsupported architectures -- throw an error instead. If QEMU were to add fw_cfg support on additional architectures, we should add them to the FW_CFG_SYSFS depends statement in drivers/firmware/Kconfig, and provide default values for register offsets in drivers/firmware/qemu_fw_cfg.c at that time. Suggested-by: Michael S. Tsirkin Signed-off-by: Gabriel Somlo Acked-by: Michael S. Tsirkin Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/qemu_fw_cfg.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c index fedbff55a7f3..7bba76c0206c 100644 --- a/drivers/firmware/qemu_fw_cfg.c +++ b/drivers/firmware/qemu_fw_cfg.c @@ -109,9 +109,7 @@ static void fw_cfg_io_cleanup(void) # define FW_CFG_CTRL_OFF 0x00 # define FW_CFG_DATA_OFF 0x01 # else -# warning "QEMU FW_CFG may not be available on this architecture!" -# define FW_CFG_CTRL_OFF 0x00 -# define FW_CFG_DATA_OFF 0x01 +# error "QEMU FW_CFG not available on this architecture!" # endif #endif -- cgit v1.2.3 From 49db08c358873af11ba3c25401de88156fa5d365 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Fri, 19 Feb 2016 15:36:07 +0100 Subject: chrdev: emit a warning when we go below dynamic major range Currently a dynamically allocated character device major is taken from 254 and downward. This mechanism is used for RTC, IIO and a few other subsystems. The kernel currently has no check prevening these dynamic allocations from eating into the assigned numbers at 233 and downward. In a recent test it was reported that so many dynamic device majors were used on a test server, that the major number for infiniband (231) was stolen. This occurred when allocating a new major number for GPIO chips. The error messages from the kernel were not helpful. (See: https://lkml.org/lkml/2016/2/14/124) This patch adds a defined lower limit of the dynamic major allocation region will henceforth emit a warning if we start to eat into the assigned numbers. It does not do any semantic changes and will not change the kernels behaviour: numbers will still continue to be stolen, but we will know from dmesg what is going on. This also updates the Documentation/devices.txt to clearly reflect that we are using this range of major numbers for dynamic allocation. Reported-by: Ying Huang Cc: Linus Torvalds Cc: Greg Kroah-Hartman Cc: Alan Cox Cc: Arnd Bergmann Signed-off-by: Linus Walleij Signed-off-by: Greg Kroah-Hartman --- Documentation/devices.txt | 6 +++--- fs/char_dev.c | 4 ++++ include/linux/fs.h | 2 ++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Documentation/devices.txt b/Documentation/devices.txt index 87b4c5e82d39..0a3588a9798d 100644 --- a/Documentation/devices.txt +++ b/Documentation/devices.txt @@ -3099,9 +3099,9 @@ Your cooperation is appreciated. 129 = /dev/ipath_sma Device used by Subnet Management Agent 130 = /dev/ipath_diag Device used by diagnostics programs -234-239 UNASSIGNED - -240-254 char LOCAL/EXPERIMENTAL USE +234-254 char RESERVED FOR DYNAMIC ASSIGNMENT + Character devices that request a dynamic allocation of major number will + take numbers starting from 254 and downward. 240-254 block LOCAL/EXPERIMENTAL USE Allocated for local/experimental use. For devices not diff --git a/fs/char_dev.c b/fs/char_dev.c index 24b142569ca9..687471dc04a0 100644 --- a/fs/char_dev.c +++ b/fs/char_dev.c @@ -91,6 +91,10 @@ __register_chrdev_region(unsigned int major, unsigned int baseminor, break; } + if (i < CHRDEV_MAJOR_DYN_END) + pr_warn("CHRDEV \"%s\" major number %d goes below the dynamic allocation range", + name, i); + if (i == 0) { ret = -EBUSY; goto out; diff --git a/include/linux/fs.h b/include/linux/fs.h index 14a97194b34b..60082be96de8 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2385,6 +2385,8 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev, /* fs/char_dev.c */ #define CHRDEV_MAJOR_HASH_SIZE 255 +/* Marks the bottom of the first segment of free char majors */ +#define CHRDEV_MAJOR_DYN_END 234 extern int alloc_chrdev_region(dev_t *, unsigned, unsigned, const char *); extern int register_chrdev_region(dev_t, unsigned, const char *); extern int __register_chrdev(unsigned int major, unsigned int baseminor, -- cgit v1.2.3 From ebdf4040c16df572d95ea63fc1f64c156baeb947 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Fri, 19 Feb 2016 21:17:36 +0100 Subject: Documentation: update the devices.txt documentation Alan is no longer maintaining this list through the Linux assigned numbers authority. Make it a collective document by referring to "the maintainers" in plural throughout, and naming the chardev and block layer maintainers in particular as parties of involvement. Cut down and remove some sections that pertained to the process of maintaining the list at lanana.org and contacting Alan directly. Make it clear that this document, in the kernel, is the master document. Also move paragraphs around so as to emphasize dynamic major number allocation. Remove paragraph on 2.6 deprecation, that tag no longer appears anywhere in the file. Cc: Jonathan Corbet Cc: Linus Torvalds Cc: Alan Cox Cc: Arnd Bergmann Cc: Jens Axboe Signed-off-by: Linus Walleij Signed-off-by: Greg Kroah-Hartman --- CREDITS | 1 + Documentation/devices.txt | 80 +++++++++++++++-------------------------------- 2 files changed, 26 insertions(+), 55 deletions(-) diff --git a/CREDITS b/CREDITS index 4312cd076b5b..0f0bf22afe0c 100644 --- a/CREDITS +++ b/CREDITS @@ -768,6 +768,7 @@ D: Z85230 driver D: Former security contact point (please use vendor-sec@lst.de) D: ex 2.2 maintainer D: 2.1.x modular sound +D: Assigned major/minor numbers maintainer at lanana.org S: c/o Red Hat UK Ltd S: Alexandra House S: Alexandra Terrace diff --git a/Documentation/devices.txt b/Documentation/devices.txt index 0a3588a9798d..4035eca87144 100644 --- a/Documentation/devices.txt +++ b/Documentation/devices.txt @@ -1,20 +1,17 @@ - LINUX ALLOCATED DEVICES (2.6+ version) - - Maintained by Alan Cox - - Last revised: 6th April 2009 + LINUX ALLOCATED DEVICES (4.x+ version) This list is the Linux Device List, the official registry of allocated device numbers and /dev directory nodes for the Linux operating system. -The latest version of this list is available from -http://www.lanana.org/docs/device-list/ or -ftp://ftp.kernel.org/pub/linux/docs/device-list/. This version may be -newer than the one distributed with the Linux kernel. - -The LaTeX version of this document is no longer maintained. +The LaTeX version of this document is no longer maintained, nor is +the document that used to reside at lanana.org. This version in the +mainline Linux kernel is the master document. Updates shall be sent +as patches to the kernel maintainers (see the SubmittingPatches document). +Specifically explore the sections titled "CHAR and MISC DRIVERS", and +"BLOCK LAYER" in the MAINTAINERS file to find the right maintainers +to involve for character and block devices. This document is included by reference into the Filesystem Hierarchy Standard (FHS). The FHS is available from http://www.pathname.com/fhs/. @@ -23,60 +20,33 @@ Allocations marked (68k/Amiga) apply to Linux/68k on the Amiga platform only. Allocations marked (68k/Atari) apply to Linux/68k on the Atari platform only. -The symbol {2.6} means the allocation is obsolete and scheduled for -removal once kernel version 2.6 (or equivalent) is released. Some of these -allocations have already been removed. - -This document is in the public domain. The author requests, however, +This document is in the public domain. The authors requests, however, that semantically altered versions are not distributed without -permission of the author, assuming the author can be contacted without +permission of the authors, assuming the authors can be contacted without an unreasonable effort. -In particular, please don't sent patches for this list to Linus, at -least not without contacting me first. - -I do not have any information about these devices beyond what appears -on this list. Any such information requests will be deleted without -reply. - **** DEVICE DRIVERS AUTHORS PLEASE READ THIS **** -To have a major number allocated, or a minor number in situations -where that applies (e.g. busmice), please contact me with the -appropriate device information. Also, if you have additional -information regarding any of the devices listed below, or if I have -made a mistake, I would greatly appreciate a note. - -I do, however, make a few requests about the nature of your report. -This is necessary for me to be able to keep this list up to date and -correct in a timely manner. First of all, *please* send it to the -correct address... . I receive hundreds of email -messages a day, so mail sent to other addresses may very well get lost -in the avalanche. Please put in a descriptive subject, so I can find -your mail again should I need to. Too many people send me email -saying just "device number request" in the subject. - -Second, please include a description of the device *in the same format -as this list*. The reason for this is that it is the only way I have -found to ensure I have all the requisite information to publish your -device and avoid conflicts. +Linux now has extensive support for dynamic allocation of device numbering +and can use sysfs and udev (systemd) to handle the naming needs. There are +still some exceptions in the serial and boot device area. Before asking +for a device number make sure you actually need one. -Third, please don't assume that the distributed version of the list is -up to date. Due to the number of registrations I have to maintain it -in "batch mode", so there is likely additional registrations that -haven't been listed yet. +To have a major number allocated, or a minor number in situations +where that applies (e.g. busmice), please submit a patch and send to +the authors as indicated above. -Fourth, remember that Linux now has extensive support for dynamic allocation -of device numbering and can use sysfs and udev to handle the naming needs. -There are still some exceptions in the serial and boot device area. Before -asking for a device number make sure you actually need one. +Keep the description of the device *in the same format +as this list*. The reason for this is that it is the only way we have +found to ensure we have all the requisite information to publish your +device and avoid conflicts. -Finally, sometimes I have to play "namespace police." Please don't be -offended. I often get submissions for /dev names that would be bound -to cause conflicts down the road. I am trying to avoid getting in a +Finally, sometimes we have to play "namespace police." Please don't be +offended. We often get submissions for /dev names that would be bound +to cause conflicts down the road. We are trying to avoid getting in a situation where we would have to suffer an incompatible forward -change. Therefore, please consult with me *before* you make your +change. Therefore, please consult with us *before* you make your device names and numbers in any way public, at least to the point where it would be at all difficult to get them changed. -- cgit v1.2.3 From b3c1be1b789cca6d3e39c950dfed690f0511fe76 Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Fri, 22 Jan 2016 11:28:07 -0500 Subject: base: isa: Remove X86_32 dependency Many motherboards utilize a LPC to ISA bridge in order to decode ISA-style port-mapped I/O addresses. This is particularly true for embedded motherboards supporting the PC/104 bus (a bus specification derived from ISA). These motherboards are now commonly running 64-bit x86 processors. The X86_32 dependency should be removed from the ISA bus configuration option in order to support these newer motherboards. A new config option, CONFIG_ISA_BUS, is introduced to allow for the compilation of the ISA bus driver independent of the CONFIG_ISA option. Devices which communicate via ISA-compatible buses can now be supported independent of the dependencies of the CONFIG_ISA option. Signed-off-by: William Breathitt Gray Reviewed-by: Thomas Gleixner Signed-off-by: Greg Kroah-Hartman --- arch/x86/Kconfig | 6 ++++++ drivers/base/Makefile | 2 +- include/linux/isa.h | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 2dc18605831f..a5977986f38b 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -2472,10 +2472,16 @@ config ISA_DMA_API Enables ISA-style DMA support for devices requiring such controllers. If unsure, say Y. +config ISA_BUS + bool "ISA bus support" + help + Enables ISA bus support for devices requiring such controllers. + if X86_32 config ISA bool "ISA support" + depends on ISA_BUS ---help--- Find out whether you have ISA slots on your motherboard. ISA is the name of a bus system, i.e. the way the CPU talks to the other stuff diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 6b2a84e7f2be..4ebfb81cc7e9 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -10,7 +10,7 @@ obj-$(CONFIG_DMA_CMA) += dma-contiguous.o obj-y += power/ obj-$(CONFIG_HAS_DMA) += dma-mapping.o obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o -obj-$(CONFIG_ISA) += isa.o +obj-$(CONFIG_ISA_BUS) += isa.o obj-$(CONFIG_FW_LOADER) += firmware_class.o obj-$(CONFIG_NUMA) += node.o obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o diff --git a/include/linux/isa.h b/include/linux/isa.h index b0270e3814c8..2a02862775eb 100644 --- a/include/linux/isa.h +++ b/include/linux/isa.h @@ -22,7 +22,7 @@ struct isa_driver { #define to_isa_driver(x) container_of((x), struct isa_driver, driver) -#ifdef CONFIG_ISA +#ifdef CONFIG_ISA_BUS int isa_register_driver(struct isa_driver *, unsigned int); void isa_unregister_driver(struct isa_driver *); #else -- cgit v1.2.3 From a8f324a46fbe5473633af00039e81821be0ce51b Mon Sep 17 00:00:00 2001 From: Roman Pen Date: Tue, 9 Feb 2016 11:30:29 +0100 Subject: debugfs: fix inode i_nlink references for automount dentry Directory inodes should start off with i_nlink == 2 (one extra ref for "." entry). debugfs_create_automount() increases neither the i_nlink reference for current inode nor for parent inode. On attempt to remove the automount dentry, kernel complains: [ 86.288070] WARNING: CPU: 1 PID: 3616 at fs/inode.c:273 drop_nlink+0x3e/0x50() [ 86.288461] Modules linked in: debugfs_example2(O-) [ 86.288745] CPU: 1 PID: 3616 Comm: rmmod Tainted: G O 4.4.0-rc3-next-20151207+ #135 [ 86.289197] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.2-20150617_082717-anatol 04/01/2014 [ 86.289696] ffffffff81be05c9 ffff8800b9e6fda0 ffffffff81352e2c 0000000000000000 [ 86.290110] ffff8800b9e6fdd8 ffffffff81065142 ffff8801399175e8 ffff8800bb78b240 [ 86.290507] ffff8801399175e8 ffff8800b73d7898 ffff8800b73d7840 ffff8800b9e6fde8 [ 86.290933] Call Trace: [ 86.291080] [] dump_stack+0x4e/0x82 [ 86.291340] [] warn_slowpath_common+0x82/0xc0 [ 86.291640] [] warn_slowpath_null+0x1a/0x20 [ 86.291932] [] drop_nlink+0x3e/0x50 [ 86.292208] [] simple_unlink+0x4b/0x60 [ 86.292481] [] simple_rmdir+0x37/0x50 [ 86.292748] [] __debugfs_remove.part.16+0xa8/0xd0 [ 86.293082] [] debugfs_remove_recursive+0xdb/0x1c0 [ 86.293406] [] cleanup_module+0x2d/0x3b [debugfs_example2] [ 86.293762] [] SyS_delete_module+0x16b/0x220 [ 86.294077] [] entry_SYSCALL_64_fastpath+0x12/0x6a [ 86.294405] ---[ end trace c9fc53353fe14a36 ]--- [ 86.294639] ------------[ cut here ]------------ To reproduce the issue it is enough to invoke these lines: autom = debugfs_create_automount("automount", NULL, vfsmount_cb, data); BUG_ON(IS_ERR_OR_NULL(autom)); debugfs_remove(autom); The issue is fixed by increasing inode i_nlink references for current and parent inodes. Signed-off-by: Roman Pen Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/inode.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index bece948b363d..13d65f8a7b12 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -461,7 +461,11 @@ struct dentry *debugfs_create_automount(const char *name, inode->i_flags |= S_AUTOMOUNT; inode->i_private = data; dentry->d_fsdata = (void *)f; + /* directory inodes start off with i_nlink == 2 (for "." entry) */ + inc_nlink(inode); d_instantiate(dentry, inode); + inc_nlink(d_inode(dentry->d_parent)); + fsnotify_mkdir(d_inode(dentry->d_parent), dentry); return end_creating(dentry); } EXPORT_SYMBOL(debugfs_create_automount); -- cgit v1.2.3 From 1b48b530dac3e600d92854d98a2a519243661f6c Mon Sep 17 00:00:00 2001 From: Deepa Dinamani Date: Mon, 22 Feb 2016 07:17:47 -0800 Subject: fs: debugfs: Replace CURRENT_TIME by current_fs_time() CURRENT_TIME macro is not appropriate for filesystems as it doesn't use the right granularity for filesystem timestamps. Use current_fs_time() instead. Signed-off-by: Deepa Dinamani Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/inode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 13d65f8a7b12..b1e7f35f3cd4 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -39,7 +39,8 @@ static struct inode *debugfs_get_inode(struct super_block *sb) struct inode *inode = new_inode(sb); if (inode) { inode->i_ino = get_next_ino(); - inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; + inode->i_atime = inode->i_mtime = + inode->i_ctime = current_fs_time(sb); } return inode; } -- cgit v1.2.3 From 3a3a5fece6f28c14d3d05c74fb7696412e53a067 Mon Sep 17 00:00:00 2001 From: Deepa Dinamani Date: Mon, 22 Feb 2016 07:17:53 -0800 Subject: fs: kernfs: Replace CURRENT_TIME by current_fs_time() This is in preparation for the series that transitions filesystem timestamps to use 64 bit time and hence make them y2038 safe. CURRENT_TIME macro will be deleted before merging the aforementioned series. Use current_fs_time() instead of CURRENT_TIME for inode timestamps. struct kernfs_node is associated with a sysfs file/ directory. Truncate the values to appropriate time granularity when writing to inode timestamps of the files. ktime_get_real_ts() is used to obtain times for struct kernfs_iattrs. Since these times are later assigned to inode times using timespec_truncate() for all filesystem based operations, we can save the supers list traversal time here by using ktime_get_real_ts() directly. Signed-off-by: Deepa Dinamani Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/dir.c | 8 +++++--- fs/kernfs/inode.c | 15 ++++++++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 03b688d19f69..eb2c58732bcf 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -753,7 +753,8 @@ int kernfs_add_one(struct kernfs_node *kn) ps_iattr = parent->iattr; if (ps_iattr) { struct iattr *ps_iattrs = &ps_iattr->ia_iattr; - ps_iattrs->ia_ctime = ps_iattrs->ia_mtime = CURRENT_TIME; + ktime_get_real_ts(&ps_iattrs->ia_ctime); + ps_iattrs->ia_mtime = ps_iattrs->ia_ctime; } mutex_unlock(&kernfs_mutex); @@ -1279,8 +1280,9 @@ static void __kernfs_remove(struct kernfs_node *kn) /* update timestamps on the parent */ if (ps_iattr) { - ps_iattr->ia_iattr.ia_ctime = CURRENT_TIME; - ps_iattr->ia_iattr.ia_mtime = CURRENT_TIME; + ktime_get_real_ts(&ps_iattr->ia_iattr.ia_ctime); + ps_iattr->ia_iattr.ia_mtime = + ps_iattr->ia_iattr.ia_ctime; } kernfs_put(pos); diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c index 16405ae88d2d..1ac1dbae5aba 100644 --- a/fs/kernfs/inode.c +++ b/fs/kernfs/inode.c @@ -54,7 +54,10 @@ static struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn) iattrs->ia_mode = kn->mode; iattrs->ia_uid = GLOBAL_ROOT_UID; iattrs->ia_gid = GLOBAL_ROOT_GID; - iattrs->ia_atime = iattrs->ia_mtime = iattrs->ia_ctime = CURRENT_TIME; + + ktime_get_real_ts(&iattrs->ia_atime); + iattrs->ia_mtime = iattrs->ia_atime; + iattrs->ia_ctime = iattrs->ia_atime; simple_xattrs_init(&kn->iattr->xattrs); out_unlock: @@ -236,16 +239,18 @@ ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size) static inline void set_default_inode_attr(struct inode *inode, umode_t mode) { inode->i_mode = mode; - inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; + inode->i_atime = inode->i_mtime = + inode->i_ctime = current_fs_time(inode->i_sb); } static inline void set_inode_attr(struct inode *inode, struct iattr *iattr) { + struct super_block *sb = inode->i_sb; inode->i_uid = iattr->ia_uid; inode->i_gid = iattr->ia_gid; - inode->i_atime = iattr->ia_atime; - inode->i_mtime = iattr->ia_mtime; - inode->i_ctime = iattr->ia_ctime; + inode->i_atime = timespec_trunc(iattr->ia_atime, sb->s_time_gran); + inode->i_mtime = timespec_trunc(iattr->ia_mtime, sb->s_time_gran); + inode->i_ctime = timespec_trunc(iattr->ia_ctime, sb->s_time_gran); } static void kernfs_refresh_inode(struct kernfs_node *kn, struct inode *inode) -- cgit v1.2.3 From 9fd4dcece43a53e5a9e65a973df5693702ee6401 Mon Sep 17 00:00:00 2001 From: Nicolai Stange Date: Tue, 22 Mar 2016 14:11:13 +0100 Subject: debugfs: prevent access to possibly dead file_operations at file open Nothing prevents a dentry found by path lookup before a return of __debugfs_remove() to actually get opened after that return. Now, after the return of __debugfs_remove(), there are no guarantees whatsoever regarding the memory the corresponding inode's file_operations object had been kept in. Since __debugfs_remove() is seldomly invoked, usually from module exit handlers only, the race is hard to trigger and the impact is very low. A discussion of the problem outlined above as well as a suggested solution can be found in the (sub-)thread rooted at http://lkml.kernel.org/g/20130401203445.GA20862@ZenIV.linux.org.uk ("Yet another pipe related oops.") Basically, Greg KH suggests to introduce an intermediate fops and Al Viro points out that a pointer to the original ones may be stored in ->d_fsdata. Follow this line of reasoning: - Add SRCU as a reverse dependency of DEBUG_FS. - Introduce a srcu_struct object for the debugfs subsystem. - In debugfs_create_file(), store a pointer to the original file_operations object in ->d_fsdata. - Make debugfs_remove() and debugfs_remove_recursive() wait for a SRCU grace period after the dentry has been delete()'d and before they return to their callers. - Introduce an intermediate file_operations object named "debugfs_open_proxy_file_operations". It's ->open() functions checks, under the protection of a SRCU read lock, whether the dentry is still alive, i.e. has not been d_delete()'d and if so, tries to acquire a reference on the owning module. On success, it sets the file object's ->f_op to the original file_operations and forwards the ongoing open() call to the original ->open(). - For clarity, rename the former debugfs_file_operations to debugfs_noop_file_operations -- they are in no way canonical. The choice of SRCU over "normal" RCU is justified by the fact, that the former may also be used to protect ->i_private data from going away during the execution of a file's readers and writers which may (and do) sleep. Finally, introduce the fs/debugfs/internal.h header containing some declarations internal to the debugfs implementation. Signed-off-by: Nicolai Stange Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++- fs/debugfs/inode.c | 13 ++++++- fs/debugfs/internal.h | 24 +++++++++++++ include/linux/debugfs.h | 3 -- lib/Kconfig.debug | 1 + 5 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 fs/debugfs/internal.h diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index d2ba12e23ed9..736ab3c988f2 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -22,6 +22,9 @@ #include #include #include +#include + +#include "internal.h" static ssize_t default_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos) @@ -35,13 +38,99 @@ static ssize_t default_write_file(struct file *file, const char __user *buf, return count; } -const struct file_operations debugfs_file_operations = { +const struct file_operations debugfs_noop_file_operations = { .read = default_read_file, .write = default_write_file, .open = simple_open, .llseek = noop_llseek, }; +/** + * debugfs_use_file_start - mark the beginning of file data access + * @dentry: the dentry object whose data is being accessed. + * @srcu_idx: a pointer to some memory to store a SRCU index in. + * + * Up to a matching call to debugfs_use_file_finish(), any + * successive call into the file removing functions debugfs_remove() + * and debugfs_remove_recursive() will block. Since associated private + * file data may only get freed after a successful return of any of + * the removal functions, you may safely access it after a successful + * call to debugfs_use_file_start() without worrying about + * lifetime issues. + * + * If -%EIO is returned, the file has already been removed and thus, + * it is not safe to access any of its data. If, on the other hand, + * it is allowed to access the file data, zero is returned. + * + * Regardless of the return code, any call to + * debugfs_use_file_start() must be followed by a matching call + * to debugfs_use_file_finish(). + */ +static int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx) + __acquires(&debugfs_srcu) +{ + *srcu_idx = srcu_read_lock(&debugfs_srcu); + barrier(); + if (d_unlinked(dentry)) + return -EIO; + return 0; +} + +/** + * debugfs_use_file_finish - mark the end of file data access + * @srcu_idx: the SRCU index "created" by a former call to + * debugfs_use_file_start(). + * + * Allow any ongoing concurrent call into debugfs_remove() or + * debugfs_remove_recursive() blocked by a former call to + * debugfs_use_file_start() to proceed and return to its caller. + */ +static void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu) +{ + srcu_read_unlock(&debugfs_srcu, srcu_idx); +} + +#define F_DENTRY(filp) ((filp)->f_path.dentry) + +#define REAL_FOPS_DEREF(dentry) \ + ((const struct file_operations *)(dentry)->d_fsdata) + +static int open_proxy_open(struct inode *inode, struct file *filp) +{ + const struct dentry *dentry = F_DENTRY(filp); + const struct file_operations *real_fops = NULL; + int srcu_idx, r; + + r = debugfs_use_file_start(dentry, &srcu_idx); + if (r) { + r = -ENOENT; + goto out; + } + + real_fops = REAL_FOPS_DEREF(dentry); + real_fops = fops_get(real_fops); + if (!real_fops) { + /* Huh? Module did not clean up after itself at exit? */ + WARN(1, "debugfs file owner did not clean up at exit: %pd", + dentry); + r = -ENXIO; + goto out; + } + replace_fops(filp, real_fops); + + if (real_fops->open) + r = real_fops->open(inode, filp); + +out: + fops_put(real_fops); + debugfs_use_file_finish(srcu_idx); + return r; +} + +const struct file_operations debugfs_open_proxy_file_operations = { + .open = open_proxy_open, +}; + static struct dentry *debugfs_create_mode(const char *name, umode_t mode, struct dentry *parent, void *value, const struct file_operations *fops, diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index b1e7f35f3cd4..2905dd160575 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -27,9 +27,14 @@ #include #include #include +#include + +#include "internal.h" #define DEBUGFS_DEFAULT_MODE 0700 +DEFINE_SRCU(debugfs_srcu); + static struct vfsmount *debugfs_mount; static int debugfs_mount_count; static bool debugfs_registered; @@ -341,8 +346,12 @@ struct dentry *debugfs_create_file(const char *name, umode_t mode, return failed_creating(dentry); inode->i_mode = mode; - inode->i_fop = fops ? fops : &debugfs_file_operations; inode->i_private = data; + + inode->i_fop = fops ? &debugfs_open_proxy_file_operations + : &debugfs_noop_file_operations; + dentry->d_fsdata = (void *)fops; + d_instantiate(dentry, inode); fsnotify_create(d_inode(dentry->d_parent), dentry); return end_creating(dentry); @@ -570,6 +579,7 @@ void debugfs_remove(struct dentry *dentry) inode_unlock(d_inode(parent)); if (!ret) simple_release_fs(&debugfs_mount, &debugfs_mount_count); + synchronize_srcu(&debugfs_srcu); } EXPORT_SYMBOL_GPL(debugfs_remove); @@ -647,6 +657,7 @@ void debugfs_remove_recursive(struct dentry *dentry) if (!__debugfs_remove(child, parent)) simple_release_fs(&debugfs_mount, &debugfs_mount_count); inode_unlock(d_inode(parent)); + synchronize_srcu(&debugfs_srcu); } EXPORT_SYMBOL_GPL(debugfs_remove_recursive); diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h new file mode 100644 index 000000000000..c7aaa5cb6685 --- /dev/null +++ b/fs/debugfs/internal.h @@ -0,0 +1,24 @@ +/* + * internal.h - declarations internal to debugfs + * + * Copyright (C) 2016 Nicolai Stange + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + */ + +#ifndef _DEBUGFS_INTERNAL_H_ +#define _DEBUGFS_INTERNAL_H_ + +struct file_operations; +struct srcu_struct; + +/* declared over in file.c */ +extern const struct file_operations debugfs_noop_file_operations; +extern const struct file_operations debugfs_open_proxy_file_operations; + +extern struct srcu_struct debugfs_srcu; + +#endif /* _DEBUGFS_INTERNAL_H_ */ diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 981e53ab84e8..fcafe2d389f9 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -43,9 +43,6 @@ extern struct dentry *arch_debugfs_dir; #if defined(CONFIG_DEBUG_FS) -/* declared over in file.c */ -extern const struct file_operations debugfs_file_operations; - struct dentry *debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, const struct file_operations *fops); diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 1e9a607534ca..ddb0e8337aae 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -257,6 +257,7 @@ config PAGE_OWNER config DEBUG_FS bool "Debug Filesystem" + select SRCU help debugfs is a virtual file system that kernel developers use to put debugging files into. Enable this option to be able to read and -- cgit v1.2.3 From 49d200deaa680501f19a247b1fffb29301e51d2b Mon Sep 17 00:00:00 2001 From: Nicolai Stange Date: Tue, 22 Mar 2016 14:11:14 +0100 Subject: debugfs: prevent access to removed files' private data Upon return of debugfs_remove()/debugfs_remove_recursive(), it might still be attempted to access associated private file data through previously opened struct file objects. If that data has been freed by the caller of debugfs_remove*() in the meanwhile, the reading/writing process would either encounter a fault or, if the memory address in question has been reassigned again, unrelated data structures could get overwritten. However, since debugfs files are seldomly removed, usually from module exit handlers only, the impact is very low. Currently, there are ~1000 call sites of debugfs_create_file() spread throughout the whole tree and touching all of those struct file_operations in order to make them file removal aware by means of checking the result of debugfs_use_file_start() from within their methods is unfeasible. Instead, wrap the struct file_operations by a lifetime managing proxy at file open: - In debugfs_create_file(), the original fops handed in has got stashed away in ->d_fsdata already. - In debugfs_create_file(), install a proxy file_operations factory, debugfs_full_proxy_file_operations, at ->i_fop. This proxy factory has got an ->open() method only. It carries out some lifetime checks and if successful, dynamically allocates and sets up a new struct file_operations proxy at ->f_op. Afterwards, it forwards to the ->open() of the original struct file_operations in ->d_fsdata, if any. The dynamically set up proxy at ->f_op has got a lifetime managing wrapper set for each of the methods defined in the original struct file_operations in ->d_fsdata. Its ->release()er frees the proxy again and forwards to the original ->release(), if any. In order not to mislead the VFS layer, it is strictly necessary to leave those fields blank in the proxy that have been NULL in the original struct file_operations also, i.e. aren't supported. This is why there is a need for dynamically allocated proxies. The choice made not to allocate a proxy instance for every dentry at file creation, but for every struct file object instantiated thereof is justified by the expected usage pattern of debugfs, namely that in general very few files get opened more than once at a time. The wrapper methods set in the struct file_operations implement lifetime managing by means of the SRCU protection facilities already in place for debugfs: They set up a SRCU read side critical section and check whether the dentry is still alive by means of debugfs_use_file_start(). If so, they forward the call to the original struct file_operation stored in ->d_fsdata, still under the protection of the SRCU read side critical section. This SRCU read side critical section prevents any pending debugfs_remove() and friends to return to their callers. Since a file's private data must only be freed after the return of debugfs_remove(), the ongoing proxied call is guarded against any file removal race. If, on the other hand, the initial call to debugfs_use_file_start() detects that the dentry is dead, the wrapper simply returns -EIO and does not forward the call. Note that the ->poll() wrapper is special in that its signature does not allow for the return of arbitrary -EXXX values and thus, POLLHUP is returned here. In order not to pollute debugfs with wrapper definitions that aren't ever needed, I chose not to define a wrapper for every struct file_operations method possible. Instead, a wrapper is defined only for the subset of methods which are actually set by any debugfs users. Currently, these are: ->llseek() ->read() ->write() ->unlocked_ioctl() ->poll() The ->release() wrapper is special in that it does not protect the original ->release() in any way from dead files in order not to leak resources. Thus, any ->release() handed to debugfs must implement file lifetime management manually, if needed. For only 33 out of a total of 434 releasers handed in to debugfs, it could not be verified immediately whether they access data structures that might have been freed upon a debugfs_remove() return in the meanwhile. Export debugfs_use_file_start() and debugfs_use_file_finish() in order to allow any ->release() to manually implement file lifetime management. For a set of common cases of struct file_operations implemented by the debugfs_core itself, future patches will incorporate file lifetime management directly within those in order to allow for their unproxied operation. Rename the original, non-proxying "debugfs_create_file()" to "debugfs_create_file_unsafe()" and keep it for future internal use by debugfs itself. Factor out code common to both into the new __debugfs_create_file(). Signed-off-by: Nicolai Stange Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++- fs/debugfs/inode.c | 70 ++++++++++++++------- fs/debugfs/internal.h | 6 +- include/linux/debugfs.h | 20 ++++++ 4 files changed, 226 insertions(+), 27 deletions(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 736ab3c988f2..6eb58a8ed03c 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -23,9 +23,12 @@ #include #include #include +#include #include "internal.h" +struct poll_table_struct; + static ssize_t default_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos) { @@ -66,7 +69,7 @@ const struct file_operations debugfs_noop_file_operations = { * debugfs_use_file_start() must be followed by a matching call * to debugfs_use_file_finish(). */ -static int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx) +int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx) __acquires(&debugfs_srcu) { *srcu_idx = srcu_read_lock(&debugfs_srcu); @@ -75,6 +78,7 @@ static int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx) return -EIO; return 0; } +EXPORT_SYMBOL_GPL(debugfs_use_file_start); /** * debugfs_use_file_finish - mark the end of file data access @@ -85,10 +89,11 @@ static int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx) * debugfs_remove_recursive() blocked by a former call to * debugfs_use_file_start() to proceed and return to its caller. */ -static void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu) +void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu) { srcu_read_unlock(&debugfs_srcu, srcu_idx); } +EXPORT_SYMBOL_GPL(debugfs_use_file_finish); #define F_DENTRY(filp) ((filp)->f_path.dentry) @@ -131,6 +136,154 @@ const struct file_operations debugfs_open_proxy_file_operations = { .open = open_proxy_open, }; +#define PROTO(args...) args +#define ARGS(args...) args + +#define FULL_PROXY_FUNC(name, ret_type, filp, proto, args) \ +static ret_type full_proxy_ ## name(proto) \ +{ \ + const struct dentry *dentry = F_DENTRY(filp); \ + const struct file_operations *real_fops = \ + REAL_FOPS_DEREF(dentry); \ + int srcu_idx; \ + ret_type r; \ + \ + r = debugfs_use_file_start(dentry, &srcu_idx); \ + if (likely(!r)) \ + r = real_fops->name(args); \ + debugfs_use_file_finish(srcu_idx); \ + return r; \ +} + +FULL_PROXY_FUNC(llseek, loff_t, filp, + PROTO(struct file *filp, loff_t offset, int whence), + ARGS(filp, offset, whence)); + +FULL_PROXY_FUNC(read, ssize_t, filp, + PROTO(struct file *filp, char __user *buf, size_t size, + loff_t *ppos), + ARGS(filp, buf, size, ppos)); + +FULL_PROXY_FUNC(write, ssize_t, filp, + PROTO(struct file *filp, const char __user *buf, size_t size, + loff_t *ppos), + ARGS(filp, buf, size, ppos)); + +FULL_PROXY_FUNC(unlocked_ioctl, long, filp, + PROTO(struct file *filp, unsigned int cmd, unsigned long arg), + ARGS(filp, cmd, arg)); + +static unsigned int full_proxy_poll(struct file *filp, + struct poll_table_struct *wait) +{ + const struct dentry *dentry = F_DENTRY(filp); + const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry); + int srcu_idx; + unsigned int r = 0; + + if (debugfs_use_file_start(dentry, &srcu_idx)) { + debugfs_use_file_finish(srcu_idx); + return POLLHUP; + } + + r = real_fops->poll(filp, wait); + debugfs_use_file_finish(srcu_idx); + return r; +} + +static int full_proxy_release(struct inode *inode, struct file *filp) +{ + const struct dentry *dentry = F_DENTRY(filp); + const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry); + const struct file_operations *proxy_fops = filp->f_op; + int r = 0; + + /* + * We must not protect this against removal races here: the + * original releaser should be called unconditionally in order + * not to leak any resources. Releasers must not assume that + * ->i_private is still being meaningful here. + */ + if (real_fops->release) + r = real_fops->release(inode, filp); + + replace_fops(filp, d_inode(dentry)->i_fop); + kfree((void *)proxy_fops); + fops_put(real_fops); + return 0; +} + +static void __full_proxy_fops_init(struct file_operations *proxy_fops, + const struct file_operations *real_fops) +{ + proxy_fops->release = full_proxy_release; + if (real_fops->llseek) + proxy_fops->llseek = full_proxy_llseek; + if (real_fops->read) + proxy_fops->read = full_proxy_read; + if (real_fops->write) + proxy_fops->write = full_proxy_write; + if (real_fops->poll) + proxy_fops->poll = full_proxy_poll; + if (real_fops->unlocked_ioctl) + proxy_fops->unlocked_ioctl = full_proxy_unlocked_ioctl; +} + +static int full_proxy_open(struct inode *inode, struct file *filp) +{ + const struct dentry *dentry = F_DENTRY(filp); + const struct file_operations *real_fops = NULL; + struct file_operations *proxy_fops = NULL; + int srcu_idx, r; + + r = debugfs_use_file_start(dentry, &srcu_idx); + if (r) { + r = -ENOENT; + goto out; + } + + real_fops = REAL_FOPS_DEREF(dentry); + real_fops = fops_get(real_fops); + if (!real_fops) { + /* Huh? Module did not cleanup after itself at exit? */ + WARN(1, "debugfs file owner did not clean up at exit: %pd", + dentry); + r = -ENXIO; + goto out; + } + + proxy_fops = kzalloc(sizeof(*proxy_fops), GFP_KERNEL); + if (!proxy_fops) { + r = -ENOMEM; + goto free_proxy; + } + __full_proxy_fops_init(proxy_fops, real_fops); + replace_fops(filp, proxy_fops); + + if (real_fops->open) { + r = real_fops->open(inode, filp); + + if (filp->f_op != proxy_fops) { + /* No protection against file removal anymore. */ + WARN(1, "debugfs file owner replaced proxy fops: %pd", + dentry); + goto free_proxy; + } + } + + goto out; +free_proxy: + kfree(proxy_fops); + fops_put(real_fops); +out: + debugfs_use_file_finish(srcu_idx); + return r; +} + +const struct file_operations debugfs_full_proxy_file_operations = { + .open = full_proxy_open, +}; + static struct dentry *debugfs_create_mode(const char *name, umode_t mode, struct dentry *parent, void *value, const struct file_operations *fops, diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 2905dd160575..136f269f01de 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -300,6 +300,37 @@ static struct dentry *end_creating(struct dentry *dentry) return dentry; } +static struct dentry *__debugfs_create_file(const char *name, umode_t mode, + struct dentry *parent, void *data, + const struct file_operations *proxy_fops, + const struct file_operations *real_fops) +{ + struct dentry *dentry; + struct inode *inode; + + if (!(mode & S_IFMT)) + mode |= S_IFREG; + BUG_ON(!S_ISREG(mode)); + dentry = start_creating(name, parent); + + if (IS_ERR(dentry)) + return NULL; + + inode = debugfs_get_inode(dentry->d_sb); + if (unlikely(!inode)) + return failed_creating(dentry); + + inode->i_mode = mode; + inode->i_private = data; + + inode->i_fop = proxy_fops; + dentry->d_fsdata = (void *)real_fops; + + d_instantiate(dentry, inode); + fsnotify_create(d_inode(dentry->d_parent), dentry); + return end_creating(dentry); +} + /** * debugfs_create_file - create a file in the debugfs filesystem * @name: a pointer to a string containing the name of the file to create. @@ -330,33 +361,24 @@ struct dentry *debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, const struct file_operations *fops) { - struct dentry *dentry; - struct inode *inode; - - if (!(mode & S_IFMT)) - mode |= S_IFREG; - BUG_ON(!S_ISREG(mode)); - dentry = start_creating(name, parent); - - if (IS_ERR(dentry)) - return NULL; - - inode = debugfs_get_inode(dentry->d_sb); - if (unlikely(!inode)) - return failed_creating(dentry); - inode->i_mode = mode; - inode->i_private = data; + return __debugfs_create_file(name, mode, parent, data, + fops ? &debugfs_full_proxy_file_operations : + &debugfs_noop_file_operations, + fops); +} +EXPORT_SYMBOL_GPL(debugfs_create_file); - inode->i_fop = fops ? &debugfs_open_proxy_file_operations - : &debugfs_noop_file_operations; - dentry->d_fsdata = (void *)fops; +struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, + struct dentry *parent, void *data, + const struct file_operations *fops) +{ - d_instantiate(dentry, inode); - fsnotify_create(d_inode(dentry->d_parent), dentry); - return end_creating(dentry); + return __debugfs_create_file(name, mode, parent, data, + fops ? &debugfs_open_proxy_file_operations : + &debugfs_noop_file_operations, + fops); } -EXPORT_SYMBOL_GPL(debugfs_create_file); /** * debugfs_create_file_size - create a file in the debugfs filesystem @@ -579,6 +601,7 @@ void debugfs_remove(struct dentry *dentry) inode_unlock(d_inode(parent)); if (!ret) simple_release_fs(&debugfs_mount, &debugfs_mount_count); + synchronize_srcu(&debugfs_srcu); } EXPORT_SYMBOL_GPL(debugfs_remove); @@ -657,6 +680,7 @@ void debugfs_remove_recursive(struct dentry *dentry) if (!__debugfs_remove(child, parent)) simple_release_fs(&debugfs_mount, &debugfs_mount_count); inode_unlock(d_inode(parent)); + synchronize_srcu(&debugfs_srcu); } EXPORT_SYMBOL_GPL(debugfs_remove_recursive); diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index c7aaa5cb6685..bba52634b995 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -13,12 +13,14 @@ #define _DEBUGFS_INTERNAL_H_ struct file_operations; -struct srcu_struct; /* declared over in file.c */ extern const struct file_operations debugfs_noop_file_operations; extern const struct file_operations debugfs_open_proxy_file_operations; +extern const struct file_operations debugfs_full_proxy_file_operations; -extern struct srcu_struct debugfs_srcu; +struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, + struct dentry *parent, void *data, + const struct file_operations *fops); #endif /* _DEBUGFS_INTERNAL_H_ */ diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index fcafe2d389f9..a63e6ea3321c 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -19,9 +19,11 @@ #include #include +#include struct device; struct file_operations; +struct srcu_struct; struct debugfs_blob_wrapper { void *data; @@ -41,6 +43,8 @@ struct debugfs_regset32 { extern struct dentry *arch_debugfs_dir; +extern struct srcu_struct debugfs_srcu; + #if defined(CONFIG_DEBUG_FS) struct dentry *debugfs_create_file(const char *name, umode_t mode, @@ -65,6 +69,11 @@ struct dentry *debugfs_create_automount(const char *name, void debugfs_remove(struct dentry *dentry); void debugfs_remove_recursive(struct dentry *dentry); +int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx) + __acquires(&debugfs_srcu); + +void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu); + struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry, struct dentry *new_dir, const char *new_name); @@ -173,6 +182,17 @@ static inline void debugfs_remove(struct dentry *dentry) static inline void debugfs_remove_recursive(struct dentry *dentry) { } +static inline int debugfs_use_file_start(const struct dentry *dentry, + int *srcu_idx) + __acquires(&debugfs_srcu) +{ + return 0; +} + +static inline void debugfs_use_file_finish(int srcu_idx) + __releases(&debugfs_srcu) +{ } + static inline struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry, struct dentry *new_dir, char *new_name) { -- cgit v1.2.3 From c64688081490321f2d23a292ef24e60bb321f3f1 Mon Sep 17 00:00:00 2001 From: Nicolai Stange Date: Tue, 22 Mar 2016 14:11:15 +0100 Subject: debugfs: add support for self-protecting attribute file fops In order to protect them against file removal issues, debugfs_create_file() creates a lifetime managing proxy around each struct file_operations handed in. In cases where this struct file_operations is able to manage file lifetime by itself already, the proxy created by debugfs is a waste of resources. The most common class of struct file_operations given to debugfs are those defined by means of the DEFINE_SIMPLE_ATTRIBUTE() macro. Introduce a DEFINE_DEBUGFS_ATTRIBUTE() macro to allow any struct file_operations of this class to be easily made file lifetime aware and thus, to be operated unproxied. Specifically, introduce debugfs_attr_read() and debugfs_attr_write() which wrap simple_attr_read() and simple_attr_write() under the protection of a debugfs_use_file_start()/debugfs_use_file_finish() pair. Make DEFINE_DEBUGFS_ATTRIBUTE() set the defined struct file_operations' ->read() and ->write() members to these wrappers. Export debugfs_create_file_unsafe() in order to allow debugfs users to create their files in non-proxying operation mode. Signed-off-by: Nicolai Stange Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 28 ++++++++++++++++++++++++++++ fs/debugfs/inode.c | 28 ++++++++++++++++++++++++++++ include/linux/debugfs.h | 26 ++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 6eb58a8ed03c..8ef56d9499a4 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -284,6 +284,34 @@ const struct file_operations debugfs_full_proxy_file_operations = { .open = full_proxy_open, }; +ssize_t debugfs_attr_read(struct file *file, char __user *buf, + size_t len, loff_t *ppos) +{ + ssize_t ret; + int srcu_idx; + + ret = debugfs_use_file_start(F_DENTRY(file), &srcu_idx); + if (likely(!ret)) + ret = simple_attr_read(file, buf, len, ppos); + debugfs_use_file_finish(srcu_idx); + return ret; +} +EXPORT_SYMBOL_GPL(debugfs_attr_read); + +ssize_t debugfs_attr_write(struct file *file, const char __user *buf, + size_t len, loff_t *ppos) +{ + ssize_t ret; + int srcu_idx; + + ret = debugfs_use_file_start(F_DENTRY(file), &srcu_idx); + if (likely(!ret)) + ret = simple_attr_write(file, buf, len, ppos); + debugfs_use_file_finish(srcu_idx); + return ret; +} +EXPORT_SYMBOL_GPL(debugfs_attr_write); + static struct dentry *debugfs_create_mode(const char *name, umode_t mode, struct dentry *parent, void *value, const struct file_operations *fops, diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 136f269f01de..41e079a8da26 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -369,6 +369,33 @@ struct dentry *debugfs_create_file(const char *name, umode_t mode, } EXPORT_SYMBOL_GPL(debugfs_create_file); +/** + * debugfs_create_file_unsafe - create a file in the debugfs filesystem + * @name: a pointer to a string containing the name of the file to create. + * @mode: the permission that the file should have. + * @parent: a pointer to the parent dentry for this file. This should be a + * directory dentry if set. If this parameter is NULL, then the + * file will be created in the root of the debugfs filesystem. + * @data: a pointer to something that the caller will want to get to later + * on. The inode.i_private pointer will point to this value on + * the open() call. + * @fops: a pointer to a struct file_operations that should be used for + * this file. + * + * debugfs_create_file_unsafe() is completely analogous to + * debugfs_create_file(), the only difference being that the fops + * handed it will not get protected against file removals by the + * debugfs core. + * + * It is your responsibility to protect your struct file_operation + * methods against file removals by means of debugfs_use_file_start() + * and debugfs_use_file_finish(). ->open() is still protected by + * debugfs though. + * + * Any struct file_operations defined by means of + * DEFINE_DEBUGFS_ATTRIBUTE() is protected against file removals and + * thus, may be used here. + */ struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, struct dentry *parent, void *data, const struct file_operations *fops) @@ -379,6 +406,7 @@ struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, &debugfs_noop_file_operations, fops); } +EXPORT_SYMBOL_GPL(debugfs_create_file_unsafe); /** * debugfs_create_file_size - create a file in the debugfs filesystem diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index a63e6ea3321c..1438e2322d5c 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -50,6 +50,9 @@ extern struct srcu_struct debugfs_srcu; struct dentry *debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, const struct file_operations *fops); +struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, + struct dentry *parent, void *data, + const struct file_operations *fops); struct dentry *debugfs_create_file_size(const char *name, umode_t mode, struct dentry *parent, void *data, @@ -74,6 +77,26 @@ int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx) void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu); +ssize_t debugfs_attr_read(struct file *file, char __user *buf, + size_t len, loff_t *ppos); +ssize_t debugfs_attr_write(struct file *file, const char __user *buf, + size_t len, loff_t *ppos); + +#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \ +static int __fops ## _open(struct inode *inode, struct file *file) \ +{ \ + __simple_attr_check_format(__fmt, 0ull); \ + return simple_attr_open(inode, file, __get, __set, __fmt); \ +} \ +static const struct file_operations __fops = { \ + .owner = THIS_MODULE, \ + .open = __fops ## _open, \ + .release = simple_attr_release, \ + .read = debugfs_attr_read, \ + .write = debugfs_attr_write, \ + .llseek = generic_file_llseek, \ +} + struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry, struct dentry *new_dir, const char *new_name); @@ -193,6 +216,9 @@ static inline void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu) { } +#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \ + static const struct file_operations __fops = { 0 } + static inline struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry, struct dentry *new_dir, char *new_name) { -- cgit v1.2.3 From 5103068eaca290f890a30aae70085fac44cecaf6 Mon Sep 17 00:00:00 2001 From: Nicolai Stange Date: Tue, 22 Mar 2016 14:11:16 +0100 Subject: debugfs, coccinelle: check for obsolete DEFINE_SIMPLE_ATTRIBUTE() usage In order to protect against file removal races, debugfs files created via debugfs_create_file() now get wrapped by a struct file_operations at their opening. If the original struct file_operations are known to be safe against removal races by themselves already, the proxy creation may be bypassed by creating the files through debugfs_create_file_unsafe(). In order to help debugfs users who use the common DEFINE_SIMPLE_ATTRIBUTE() + debugfs_create_file() idiom to transition to removal safe struct file_operations, the helper macro DEFINE_DEBUGFS_ATTRIBUTE() has been introduced. Thus, the preferred strategy is to use DEFINE_DEBUGFS_ATTRIBUTE() + debugfs_create_file_unsafe() now. Introduce a Coccinelle script that searches for DEFINE_SIMPLE_ATTRIBUTE()-defined struct file_operations handed into debugfs_create_file(). Suggest to turn these usages into the DEFINE_DEBUGFS_ATTRIBUTE() + debugfs_create_file_unsafe() pattern. Signed-off-by: Nicolai Stange Acked-by: Julia Lawall Signed-off-by: Greg Kroah-Hartman --- .../api/debugfs/debugfs_simple_attr.cocci | 67 ++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci diff --git a/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci b/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci new file mode 100644 index 000000000000..85cf5408d378 --- /dev/null +++ b/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci @@ -0,0 +1,67 @@ +/// Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE +/// for debugfs files. +/// +//# Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() +//# imposes some significant overhead as compared to +//# DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe(). +// +// Copyright (C): 2016 Nicolai Stange +// Options: --no-includes +// + +virtual context +virtual patch +virtual org +virtual report + +@dsa@ +declarer name DEFINE_SIMPLE_ATTRIBUTE; +identifier dsa_fops; +expression dsa_get, dsa_set, dsa_fmt; +position p; +@@ +DEFINE_SIMPLE_ATTRIBUTE@p(dsa_fops, dsa_get, dsa_set, dsa_fmt); + +@dcf@ +expression name, mode, parent, data; +identifier dsa.dsa_fops; +@@ +debugfs_create_file(name, mode, parent, data, &dsa_fops) + + +@context_dsa depends on context && dcf@ +declarer name DEFINE_DEBUGFS_ATTRIBUTE; +identifier dsa.dsa_fops; +expression dsa.dsa_get, dsa.dsa_set, dsa.dsa_fmt; +@@ +* DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); + + +@patch_dcf depends on patch expression@ +expression name, mode, parent, data; +identifier dsa.dsa_fops; +@@ +- debugfs_create_file(name, mode, parent, data, &dsa_fops) ++ debugfs_create_file_unsafe(name, mode, parent, data, &dsa_fops) + +@patch_dsa depends on patch_dcf && patch@ +identifier dsa.dsa_fops; +expression dsa.dsa_get, dsa.dsa_set, dsa.dsa_fmt; +@@ +- DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); ++ DEFINE_DEBUGFS_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); + + +@script:python depends on org && dcf@ +fops << dsa.dsa_fops; +p << dsa.p; +@@ +msg="%s should be defined with DEFINE_DEBUGFS_ATTRIBUTE" % (fops) +coccilib.org.print_todo(p[0], msg) + +@script:python depends on report && dcf@ +fops << dsa.dsa_fops; +p << dsa.p; +@@ +msg="WARNING: %s should be defined with DEFINE_DEBUGFS_ATTRIBUTE" % (fops) +coccilib.report.print_report(p[0], msg) -- cgit v1.2.3 From 4909f168104b24f592fb8d502e2a6520346a3927 Mon Sep 17 00:00:00 2001 From: Nicolai Stange Date: Tue, 22 Mar 2016 14:11:17 +0100 Subject: debugfs: unproxify integer attribute files Currently, the struct file_operations associated with the integer attribute style files created through the debugfs_create_*() helpers are not file lifetime aware as they are defined by means of DEFINE_SIMPLE_ATTRIBUTE(). Thus, a lifetime managing proxy is created around the original fops each time such a file is opened which is an unnecessary waste of resources. Migrate all usages of DEFINE_SIMPLE_ATTRIBUTE() within debugfs itself to DEFINE_DEBUGFS_ATTRIBUTE() in order to implement file lifetime managing within the struct file_operations thus defined. Introduce the debugfs_create_mode_unsafe() helper, analogous to debugfs_create_mode(), but distinct in that it creates the files in non-proxying operation mode through debugfs_create_file_unsafe(). Feed all struct file_operations migrated to DEFINE_DEBUGFS_ATTRIBUTE() into debugfs_create_mode_unsafe() instead of former debugfs_create_mode(). Signed-off-by: Nicolai Stange Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 123 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 75 insertions(+), 48 deletions(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 8ef56d9499a4..4b3967e86e97 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -328,6 +328,24 @@ static struct dentry *debugfs_create_mode(const char *name, umode_t mode, return debugfs_create_file(name, mode, parent, value, fops); } +static struct dentry *debugfs_create_mode_unsafe(const char *name, umode_t mode, + struct dentry *parent, void *value, + const struct file_operations *fops, + const struct file_operations *fops_ro, + const struct file_operations *fops_wo) +{ + /* if there are no write bits set, make read only */ + if (!(mode & S_IWUGO)) + return debugfs_create_file_unsafe(name, mode, parent, value, + fops_ro); + /* if there are no read bits set, make write only */ + if (!(mode & S_IRUGO)) + return debugfs_create_file_unsafe(name, mode, parent, value, + fops_wo); + + return debugfs_create_file_unsafe(name, mode, parent, value, fops); +} + static int debugfs_u8_set(void *data, u64 val) { *(u8 *)data = val; @@ -338,9 +356,9 @@ static int debugfs_u8_get(void *data, u64 *val) *val = *(u8 *)data; return 0; } -DEFINE_SIMPLE_ATTRIBUTE(fops_u8, debugfs_u8_get, debugfs_u8_set, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_u8_ro, debugfs_u8_get, NULL, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_u8_wo, NULL, debugfs_u8_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u8, debugfs_u8_get, debugfs_u8_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u8_ro, debugfs_u8_get, NULL, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u8_wo, NULL, debugfs_u8_set, "%llu\n"); /** * debugfs_create_u8 - create a debugfs file that is used to read and write an unsigned 8-bit value @@ -369,7 +387,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u8_wo, NULL, debugfs_u8_set, "%llu\n"); struct dentry *debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_u8, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_u8, &fops_u8_ro, &fops_u8_wo); } EXPORT_SYMBOL_GPL(debugfs_create_u8); @@ -384,9 +402,9 @@ static int debugfs_u16_get(void *data, u64 *val) *val = *(u16 *)data; return 0; } -DEFINE_SIMPLE_ATTRIBUTE(fops_u16, debugfs_u16_get, debugfs_u16_set, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_u16_ro, debugfs_u16_get, NULL, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_u16_wo, NULL, debugfs_u16_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u16, debugfs_u16_get, debugfs_u16_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u16_ro, debugfs_u16_get, NULL, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u16_wo, NULL, debugfs_u16_set, "%llu\n"); /** * debugfs_create_u16 - create a debugfs file that is used to read and write an unsigned 16-bit value @@ -415,7 +433,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u16_wo, NULL, debugfs_u16_set, "%llu\n"); struct dentry *debugfs_create_u16(const char *name, umode_t mode, struct dentry *parent, u16 *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_u16, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_u16, &fops_u16_ro, &fops_u16_wo); } EXPORT_SYMBOL_GPL(debugfs_create_u16); @@ -430,9 +448,9 @@ static int debugfs_u32_get(void *data, u64 *val) *val = *(u32 *)data; return 0; } -DEFINE_SIMPLE_ATTRIBUTE(fops_u32, debugfs_u32_get, debugfs_u32_set, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_u32_ro, debugfs_u32_get, NULL, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_u32_wo, NULL, debugfs_u32_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u32, debugfs_u32_get, debugfs_u32_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u32_ro, debugfs_u32_get, NULL, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u32_wo, NULL, debugfs_u32_set, "%llu\n"); /** * debugfs_create_u32 - create a debugfs file that is used to read and write an unsigned 32-bit value @@ -461,7 +479,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u32_wo, NULL, debugfs_u32_set, "%llu\n"); struct dentry *debugfs_create_u32(const char *name, umode_t mode, struct dentry *parent, u32 *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_u32, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_u32, &fops_u32_ro, &fops_u32_wo); } EXPORT_SYMBOL_GPL(debugfs_create_u32); @@ -477,9 +495,9 @@ static int debugfs_u64_get(void *data, u64 *val) *val = *(u64 *)data; return 0; } -DEFINE_SIMPLE_ATTRIBUTE(fops_u64, debugfs_u64_get, debugfs_u64_set, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_u64_ro, debugfs_u64_get, NULL, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_u64_wo, NULL, debugfs_u64_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u64, debugfs_u64_get, debugfs_u64_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u64_ro, debugfs_u64_get, NULL, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_u64_wo, NULL, debugfs_u64_set, "%llu\n"); /** * debugfs_create_u64 - create a debugfs file that is used to read and write an unsigned 64-bit value @@ -508,7 +526,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u64_wo, NULL, debugfs_u64_set, "%llu\n"); struct dentry *debugfs_create_u64(const char *name, umode_t mode, struct dentry *parent, u64 *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_u64, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_u64, &fops_u64_ro, &fops_u64_wo); } EXPORT_SYMBOL_GPL(debugfs_create_u64); @@ -524,9 +542,10 @@ static int debugfs_ulong_get(void *data, u64 *val) *val = *(unsigned long *)data; return 0; } -DEFINE_SIMPLE_ATTRIBUTE(fops_ulong, debugfs_ulong_get, debugfs_ulong_set, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_ulong_ro, debugfs_ulong_get, NULL, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_ulong_wo, NULL, debugfs_ulong_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_ulong, debugfs_ulong_get, debugfs_ulong_set, + "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_ulong_ro, debugfs_ulong_get, NULL, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_ulong_wo, NULL, debugfs_ulong_set, "%llu\n"); /** * debugfs_create_ulong - create a debugfs file that is used to read and write @@ -556,26 +575,30 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ulong_wo, NULL, debugfs_ulong_set, "%llu\n"); struct dentry *debugfs_create_ulong(const char *name, umode_t mode, struct dentry *parent, unsigned long *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_ulong, - &fops_ulong_ro, &fops_ulong_wo); + return debugfs_create_mode_unsafe(name, mode, parent, value, + &fops_ulong, &fops_ulong_ro, + &fops_ulong_wo); } EXPORT_SYMBOL_GPL(debugfs_create_ulong); -DEFINE_SIMPLE_ATTRIBUTE(fops_x8, debugfs_u8_get, debugfs_u8_set, "0x%02llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x8_ro, debugfs_u8_get, NULL, "0x%02llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x8_wo, NULL, debugfs_u8_set, "0x%02llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x8, debugfs_u8_get, debugfs_u8_set, "0x%02llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x8_ro, debugfs_u8_get, NULL, "0x%02llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x8_wo, NULL, debugfs_u8_set, "0x%02llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x16, debugfs_u16_get, debugfs_u16_set, "0x%04llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x16_ro, debugfs_u16_get, NULL, "0x%04llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x16_wo, NULL, debugfs_u16_set, "0x%04llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x16, debugfs_u16_get, debugfs_u16_set, + "0x%04llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x16_ro, debugfs_u16_get, NULL, "0x%04llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x16_wo, NULL, debugfs_u16_set, "0x%04llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x32, debugfs_u32_get, debugfs_u32_set, "0x%08llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x32_ro, debugfs_u32_get, NULL, "0x%08llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x32_wo, NULL, debugfs_u32_set, "0x%08llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, debugfs_u32_get, debugfs_u32_set, + "0x%08llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x32_ro, debugfs_u32_get, NULL, "0x%08llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x32_wo, NULL, debugfs_u32_set, "0x%08llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x64, debugfs_u64_get, debugfs_u64_set, "0x%016llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x64_ro, debugfs_u64_get, NULL, "0x%016llx\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_x64_wo, NULL, debugfs_u64_set, "0x%016llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x64, debugfs_u64_get, debugfs_u64_set, + "0x%016llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x64_ro, debugfs_u64_get, NULL, "0x%016llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_x64_wo, NULL, debugfs_u64_set, "0x%016llx\n"); /* * debugfs_create_x{8,16,32,64} - create a debugfs file that is used to read and write an unsigned {8,16,32,64}-bit value @@ -598,7 +621,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_x64_wo, NULL, debugfs_u64_set, "0x%016llx\n"); struct dentry *debugfs_create_x8(const char *name, umode_t mode, struct dentry *parent, u8 *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_x8, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_x8, &fops_x8_ro, &fops_x8_wo); } EXPORT_SYMBOL_GPL(debugfs_create_x8); @@ -616,7 +639,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_x8); struct dentry *debugfs_create_x16(const char *name, umode_t mode, struct dentry *parent, u16 *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_x16, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_x16, &fops_x16_ro, &fops_x16_wo); } EXPORT_SYMBOL_GPL(debugfs_create_x16); @@ -634,7 +657,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_x16); struct dentry *debugfs_create_x32(const char *name, umode_t mode, struct dentry *parent, u32 *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_x32, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_x32, &fops_x32_ro, &fops_x32_wo); } EXPORT_SYMBOL_GPL(debugfs_create_x32); @@ -652,7 +675,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_x32); struct dentry *debugfs_create_x64(const char *name, umode_t mode, struct dentry *parent, u64 *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_x64, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_x64, &fops_x64_ro, &fops_x64_wo); } EXPORT_SYMBOL_GPL(debugfs_create_x64); @@ -668,10 +691,10 @@ static int debugfs_size_t_get(void *data, u64 *val) *val = *(size_t *)data; return 0; } -DEFINE_SIMPLE_ATTRIBUTE(fops_size_t, debugfs_size_t_get, debugfs_size_t_set, - "%llu\n"); /* %llu and %zu are more or less the same */ -DEFINE_SIMPLE_ATTRIBUTE(fops_size_t_ro, debugfs_size_t_get, NULL, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_size_t_wo, NULL, debugfs_size_t_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_size_t, debugfs_size_t_get, debugfs_size_t_set, + "%llu\n"); /* %llu and %zu are more or less the same */ +DEFINE_DEBUGFS_ATTRIBUTE(fops_size_t_ro, debugfs_size_t_get, NULL, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_size_t_wo, NULL, debugfs_size_t_set, "%llu\n"); /** * debugfs_create_size_t - create a debugfs file that is used to read and write an size_t value @@ -686,8 +709,9 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_size_t_wo, NULL, debugfs_size_t_set, "%llu\n"); struct dentry *debugfs_create_size_t(const char *name, umode_t mode, struct dentry *parent, size_t *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_size_t, - &fops_size_t_ro, &fops_size_t_wo); + return debugfs_create_mode_unsafe(name, mode, parent, value, + &fops_size_t, &fops_size_t_ro, + &fops_size_t_wo); } EXPORT_SYMBOL_GPL(debugfs_create_size_t); @@ -701,10 +725,12 @@ static int debugfs_atomic_t_get(void *data, u64 *val) *val = atomic_read((atomic_t *)data); return 0; } -DEFINE_SIMPLE_ATTRIBUTE(fops_atomic_t, debugfs_atomic_t_get, +DEFINE_DEBUGFS_ATTRIBUTE(fops_atomic_t, debugfs_atomic_t_get, debugfs_atomic_t_set, "%lld\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_atomic_t_ro, debugfs_atomic_t_get, NULL, "%lld\n"); -DEFINE_SIMPLE_ATTRIBUTE(fops_atomic_t_wo, NULL, debugfs_atomic_t_set, "%lld\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_atomic_t_ro, debugfs_atomic_t_get, NULL, + "%lld\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_atomic_t_wo, NULL, debugfs_atomic_t_set, + "%lld\n"); /** * debugfs_create_atomic_t - create a debugfs file that is used to read and @@ -720,8 +746,9 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_atomic_t_wo, NULL, debugfs_atomic_t_set, "%lld\n"); struct dentry *debugfs_create_atomic_t(const char *name, umode_t mode, struct dentry *parent, atomic_t *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_atomic_t, - &fops_atomic_t_ro, &fops_atomic_t_wo); + return debugfs_create_mode_unsafe(name, mode, parent, value, + &fops_atomic_t, &fops_atomic_t_ro, + &fops_atomic_t_wo); } EXPORT_SYMBOL_GPL(debugfs_create_atomic_t); -- cgit v1.2.3 From 4d45f7974ccf0aa783034fef2661573b3a28609e Mon Sep 17 00:00:00 2001 From: Nicolai Stange Date: Tue, 22 Mar 2016 14:11:18 +0100 Subject: debugfs: unproxify files created through debugfs_create_bool() Currently, the struct file_operations fops_bool associated with files created through the debugfs_create_bool() helpers are not file lifetime aware. Thus, a lifetime managing proxy is created around fops_bool each time such a file is opened which is an unnecessary waste of resources. Implement file lifetime management for the fops_bool file_operations. Namely, make debugfs_read_file_bool() and debugfs_write_file_bool() safe against file removals by means of debugfs_use_file_start() and debugfs_use_file_finish(). Make debugfs_create_bool() create its files in non-proxying operation mode through debugfs_create_mode_unsafe(). Finally, purge debugfs_create_mode() as debugfs_create_bool() had been its last user. Signed-off-by: Nicolai Stange Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 4b3967e86e97..8a548bee1b3d 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -312,22 +312,6 @@ ssize_t debugfs_attr_write(struct file *file, const char __user *buf, } EXPORT_SYMBOL_GPL(debugfs_attr_write); -static struct dentry *debugfs_create_mode(const char *name, umode_t mode, - struct dentry *parent, void *value, - const struct file_operations *fops, - const struct file_operations *fops_ro, - const struct file_operations *fops_wo) -{ - /* if there are no write bits set, make read only */ - if (!(mode & S_IWUGO)) - return debugfs_create_file(name, mode, parent, value, fops_ro); - /* if there are no read bits set, make write only */ - if (!(mode & S_IRUGO)) - return debugfs_create_file(name, mode, parent, value, fops_wo); - - return debugfs_create_file(name, mode, parent, value, fops); -} - static struct dentry *debugfs_create_mode_unsafe(const char *name, umode_t mode, struct dentry *parent, void *value, const struct file_operations *fops, @@ -756,9 +740,17 @@ ssize_t debugfs_read_file_bool(struct file *file, char __user *user_buf, size_t count, loff_t *ppos) { char buf[3]; - bool *val = file->private_data; + bool val; + int r, srcu_idx; + + r = debugfs_use_file_start(F_DENTRY(file), &srcu_idx); + if (likely(!r)) + val = *(bool *)file->private_data; + debugfs_use_file_finish(srcu_idx); + if (r) + return r; - if (*val) + if (val) buf[0] = 'Y'; else buf[0] = 'N'; @@ -774,6 +766,7 @@ ssize_t debugfs_write_file_bool(struct file *file, const char __user *user_buf, char buf[32]; size_t buf_size; bool bv; + int r, srcu_idx; bool *val = file->private_data; buf_size = min(count, (sizeof(buf)-1)); @@ -781,8 +774,14 @@ ssize_t debugfs_write_file_bool(struct file *file, const char __user *user_buf, return -EFAULT; buf[buf_size] = '\0'; - if (strtobool(buf, &bv) == 0) - *val = bv; + if (strtobool(buf, &bv) == 0) { + r = debugfs_use_file_start(F_DENTRY(file), &srcu_idx); + if (likely(!r)) + *val = bv; + debugfs_use_file_finish(srcu_idx); + if (r) + return r; + } return count; } @@ -834,7 +833,7 @@ static const struct file_operations fops_bool_wo = { struct dentry *debugfs_create_bool(const char *name, umode_t mode, struct dentry *parent, bool *value) { - return debugfs_create_mode(name, mode, parent, value, &fops_bool, + return debugfs_create_mode_unsafe(name, mode, parent, value, &fops_bool, &fops_bool_ro, &fops_bool_wo); } EXPORT_SYMBOL_GPL(debugfs_create_bool); -- cgit v1.2.3 From 83b711cbf4ff42a9996c5f092762b3967d307d73 Mon Sep 17 00:00:00 2001 From: Nicolai Stange Date: Tue, 22 Mar 2016 14:11:19 +0100 Subject: debugfs: unproxify files created through debugfs_create_blob() Currently, the struct file_operations fops_blob associated with files created through the debugfs_create_blob() helpers are not file lifetime aware. Thus, a lifetime managing proxy is created around fops_blob each time such a file is opened which is an unnecessary waste of resources. Implement file lifetime management for the fops_bool file_operations. Namely, make read_file_blob() safe gainst file removals by means of debugfs_use_file_start() and debugfs_use_file_finish(). Make debugfs_create_blob() create its files in non-proxying operation mode by means of debugfs_create_file_unsafe(). Signed-off-by: Nicolai Stange Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 8a548bee1b3d..2e86d66f7850 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -842,8 +842,15 @@ static ssize_t read_file_blob(struct file *file, char __user *user_buf, size_t count, loff_t *ppos) { struct debugfs_blob_wrapper *blob = file->private_data; - return simple_read_from_buffer(user_buf, count, ppos, blob->data, - blob->size); + ssize_t r; + int srcu_idx; + + r = debugfs_use_file_start(F_DENTRY(file), &srcu_idx); + if (likely(!r)) + r = simple_read_from_buffer(user_buf, count, ppos, blob->data, + blob->size); + debugfs_use_file_finish(srcu_idx); + return r; } static const struct file_operations fops_blob = { @@ -880,7 +887,7 @@ struct dentry *debugfs_create_blob(const char *name, umode_t mode, struct dentry *parent, struct debugfs_blob_wrapper *blob) { - return debugfs_create_file(name, mode, parent, blob, &fops_blob); + return debugfs_create_file_unsafe(name, mode, parent, blob, &fops_blob); } EXPORT_SYMBOL_GPL(debugfs_create_blob); -- cgit v1.2.3 From c4a74f63dfd2e75e7d40a9aaa4052b0ef26e617c Mon Sep 17 00:00:00 2001 From: Nicolai Stange Date: Tue, 22 Mar 2016 14:11:20 +0100 Subject: debugfs: unproxify files created through debugfs_create_u32_array() The struct file_operations u32_array_fops associated with files created through debugfs_create_u32_array() has been lifetime aware already: everything needed for subsequent operation is copied to a ->f_private buffer at file opening time in u32_array_open(). Now, ->open() is always protected against file removal issues by the debugfs core. There is no need for the debugfs core to wrap the u32_array_fops with a file lifetime managing proxy. Make debugfs_create_u32_array() create its files in non-proxying operation mode by means of debugfs_create_file_unsafe(). Signed-off-by: Nicolai Stange Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 2e86d66f7850..9c1c9a01b7e5 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -992,7 +992,8 @@ struct dentry *debugfs_create_u32_array(const char *name, umode_t mode, data->array = array; data->elements = elements; - return debugfs_create_file(name, mode, parent, data, &u32_array_fops); + return debugfs_create_file_unsafe(name, mode, parent, data, + &u32_array_fops); } EXPORT_SYMBOL_GPL(debugfs_create_u32_array); -- cgit v1.2.3 From 522566376a3f8373fbd5ff75bb8a7a2da701c1a7 Mon Sep 17 00:00:00 2001 From: Aviya Erenfeld Date: Thu, 14 Apr 2016 11:59:31 +0200 Subject: devcoredump: add scatterlist support Add scatterlist support (dev_coredumpsg) to allow drivers to avoid vmalloc() like dev_coredumpm(), while also avoiding the module reference that the latter function requires. This internally uses dev_coredumpm() with function inside the devcoredump module, requiring removing the const (which touches the driver using it.) Signed-off-by: Aviya Erenfeld Signed-off-by: Johannes Berg Signed-off-by: Greg Kroah-Hartman --- drivers/base/devcoredump.c | 83 +++++++++++++++++++++--- drivers/net/wireless/intel/iwlwifi/mvm/fw-dbg.c | 4 +- include/linux/devcoredump.h | 86 ++++++++++++++++++++++--- 3 files changed, 154 insertions(+), 19 deletions(-) diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c index 1bd120a0b084..240374fd1838 100644 --- a/drivers/base/devcoredump.c +++ b/drivers/base/devcoredump.c @@ -4,6 +4,7 @@ * GPL LICENSE SUMMARY * * Copyright(c) 2014 Intel Mobile Communications GmbH + * Copyright(c) 2015 Intel Deutschland GmbH * * This program is free software; you can redistribute it and/or modify * it under the terms of version 2 of the GNU General Public License as @@ -41,12 +42,12 @@ static bool devcd_disabled; struct devcd_entry { struct device devcd_dev; - const void *data; + void *data; size_t datalen; struct module *owner; ssize_t (*read)(char *buffer, loff_t offset, size_t count, - const void *data, size_t datalen); - void (*free)(const void *data); + void *data, size_t datalen); + void (*free)(void *data); struct delayed_work del_wk; struct device *failing_dev; }; @@ -174,7 +175,7 @@ static struct class devcd_class = { }; static ssize_t devcd_readv(char *buffer, loff_t offset, size_t count, - const void *data, size_t datalen) + void *data, size_t datalen) { if (offset > datalen) return -EINVAL; @@ -188,6 +189,11 @@ static ssize_t devcd_readv(char *buffer, loff_t offset, size_t count, return count; } +static void devcd_freev(void *data) +{ + vfree(data); +} + /** * dev_coredumpv - create device coredump with vmalloc data * @dev: the struct device for the crashed device @@ -198,10 +204,10 @@ static ssize_t devcd_readv(char *buffer, loff_t offset, size_t count, * This function takes ownership of the vmalloc'ed data and will free * it when it is no longer used. See dev_coredumpm() for more information. */ -void dev_coredumpv(struct device *dev, const void *data, size_t datalen, +void dev_coredumpv(struct device *dev, void *data, size_t datalen, gfp_t gfp) { - dev_coredumpm(dev, NULL, data, datalen, gfp, devcd_readv, vfree); + dev_coredumpm(dev, NULL, data, datalen, gfp, devcd_readv, devcd_freev); } EXPORT_SYMBOL_GPL(dev_coredumpv); @@ -212,6 +218,44 @@ static int devcd_match_failing(struct device *dev, const void *failing) return devcd->failing_dev == failing; } +/** + * devcd_free_sgtable - free all the memory of the given scatterlist table + * (i.e. both pages and scatterlist instances) + * NOTE: if two tables allocated with devcd_alloc_sgtable and then chained + * using the sg_chain function then that function should be called only once + * on the chained table + * @table: pointer to sg_table to free + */ +static void devcd_free_sgtable(void *data) +{ + _devcd_free_sgtable(data); +} + +/** + * devcd_read_from_table - copy data from sg_table to a given buffer + * and return the number of bytes read + * @buffer: the buffer to copy the data to it + * @buf_len: the length of the buffer + * @data: the scatterlist table to copy from + * @offset: start copy from @offset@ bytes from the head of the data + * in the given scatterlist + * @data_len: the length of the data in the sg_table + */ +static ssize_t devcd_read_from_sgtable(char *buffer, loff_t offset, + size_t buf_len, void *data, + size_t data_len) +{ + struct scatterlist *table = data; + + if (offset > data_len) + return -EINVAL; + + if (offset + buf_len > data_len) + buf_len = data_len - offset; + return sg_pcopy_to_buffer(table, sg_nents(table), buffer, buf_len, + offset); +} + /** * dev_coredumpm - create device coredump with read/free methods * @dev: the struct device for the crashed device @@ -228,10 +272,10 @@ static int devcd_match_failing(struct device *dev, const void *failing) * function will be called to free the data. */ void dev_coredumpm(struct device *dev, struct module *owner, - const void *data, size_t datalen, gfp_t gfp, + void *data, size_t datalen, gfp_t gfp, ssize_t (*read)(char *buffer, loff_t offset, size_t count, - const void *data, size_t datalen), - void (*free)(const void *data)) + void *data, size_t datalen), + void (*free)(void *data)) { static atomic_t devcd_count = ATOMIC_INIT(0); struct devcd_entry *devcd; @@ -291,6 +335,27 @@ void dev_coredumpm(struct device *dev, struct module *owner, } EXPORT_SYMBOL_GPL(dev_coredumpm); +/** + * dev_coredumpmsg - create device coredump that uses scatterlist as data + * parameter + * @dev: the struct device for the crashed device + * @table: the dump data + * @datalen: length of the data + * @gfp: allocation flags + * + * Creates a new device coredump for the given device. If a previous one hasn't + * been read yet, the new coredump is discarded. The data lifetime is determined + * by the device coredump framework and when it is no longer needed + * it will free the data. + */ +void dev_coredumpsg(struct device *dev, struct scatterlist *table, + size_t datalen, gfp_t gfp) +{ + dev_coredumpm(dev, NULL, table, datalen, gfp, devcd_read_from_sgtable, + devcd_free_sgtable); +} +EXPORT_SYMBOL_GPL(dev_coredumpsg); + static int __init devcoredump_init(void) { return class_register(&devcd_class); diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw-dbg.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw-dbg.c index 4856eac120f6..a4b0581d2275 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw-dbg.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw-dbg.c @@ -71,7 +71,7 @@ #include "iwl-csr.h" static ssize_t iwl_mvm_read_coredump(char *buffer, loff_t offset, size_t count, - const void *data, size_t datalen) + void *data, size_t datalen) { const struct iwl_mvm_dump_ptrs *dump_ptrs = data; ssize_t bytes_read; @@ -104,7 +104,7 @@ static ssize_t iwl_mvm_read_coredump(char *buffer, loff_t offset, size_t count, return bytes_read + bytes_read_trans; } -static void iwl_mvm_free_coredump(const void *data) +static void iwl_mvm_free_coredump(void *data) { const struct iwl_mvm_dump_ptrs *fw_error_dump = data; diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h index c0a360e99f64..269521f143ac 100644 --- a/include/linux/devcoredump.h +++ b/include/linux/devcoredump.h @@ -1,3 +1,22 @@ +/* + * This file is provided under the GPLv2 license. + * + * GPL LICENSE SUMMARY + * + * Copyright(c) 2015 Intel Deutschland GmbH + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * The full GNU General Public License is included in this distribution + * in the file called COPYING. + */ #ifndef __DEVCOREDUMP_H #define __DEVCOREDUMP_H @@ -5,17 +24,62 @@ #include #include +#include +#include + +/* + * _devcd_free_sgtable - free all the memory of the given scatterlist table + * (i.e. both pages and scatterlist instances) + * NOTE: if two tables allocated and chained using the sg_chain function then + * this function should be called only once on the first table + * @table: pointer to sg_table to free + */ +static inline void _devcd_free_sgtable(struct scatterlist *table) +{ + int i; + struct page *page; + struct scatterlist *iter; + struct scatterlist *delete_iter; + + /* free pages */ + iter = table; + for_each_sg(table, iter, sg_nents(table), i) { + page = sg_page(iter); + if (page) + __free_page(page); + } + + /* then free all chained tables */ + iter = table; + delete_iter = table; /* always points on a head of a table */ + while (!sg_is_last(iter)) { + iter++; + if (sg_is_chain(iter)) { + iter = sg_chain_ptr(iter); + kfree(delete_iter); + delete_iter = iter; + } + } + + /* free the last table */ + kfree(delete_iter); +} + + #ifdef CONFIG_DEV_COREDUMP -void dev_coredumpv(struct device *dev, const void *data, size_t datalen, +void dev_coredumpv(struct device *dev, void *data, size_t datalen, gfp_t gfp); void dev_coredumpm(struct device *dev, struct module *owner, - const void *data, size_t datalen, gfp_t gfp, + void *data, size_t datalen, gfp_t gfp, ssize_t (*read)(char *buffer, loff_t offset, size_t count, - const void *data, size_t datalen), - void (*free)(const void *data)); + void *data, size_t datalen), + void (*free)(void *data)); + +void dev_coredumpsg(struct device *dev, struct scatterlist *table, + size_t datalen, gfp_t gfp); #else -static inline void dev_coredumpv(struct device *dev, const void *data, +static inline void dev_coredumpv(struct device *dev, void *data, size_t datalen, gfp_t gfp) { vfree(data); @@ -23,13 +87,19 @@ static inline void dev_coredumpv(struct device *dev, const void *data, static inline void dev_coredumpm(struct device *dev, struct module *owner, - const void *data, size_t datalen, gfp_t gfp, + void *data, size_t datalen, gfp_t gfp, ssize_t (*read)(char *buffer, loff_t offset, size_t count, - const void *data, size_t datalen), - void (*free)(const void *data)) + void *data, size_t datalen), + void (*free)(void *data)) { free(data); } + +static inline void dev_coredumpsg(struct device *dev, struct scatterlist *table, + size_t datalen, gfp_t gfp) +{ + _devcd_free_sgtable(table); +} #endif /* CONFIG_DEV_COREDUMP */ #endif /* __DEVCOREDUMP_H */ -- cgit v1.2.3 From e4234a1fc343ca35f852bc527fae56fade879d4a Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 31 Mar 2016 11:45:06 +0100 Subject: kernfs: Move faulting copy_user operations outside of the mutex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fault in a user provided buffer may lead anywhere, and lockdep warns that we have a potential deadlock between the mm->mmap_sem and the kernfs file mutex: [ 82.811702] ====================================================== [ 82.811705] [ INFO: possible circular locking dependency detected ] [ 82.811709] 4.5.0-rc4-gfxbench+ #1 Not tainted [ 82.811711] ------------------------------------------------------- [ 82.811714] kms_setmode/5859 is trying to acquire lock: [ 82.811717] (&dev->struct_mutex){+.+.+.}, at: [] drm_gem_mmap+0x1a1/0x270 [ 82.811731] but task is already holding lock: [ 82.811734] (&mm->mmap_sem){++++++}, at: [] vm_mmap_pgoff+0x44/0xa0 [ 82.811745] which lock already depends on the new lock. [ 82.811749] the existing dependency chain (in reverse order) is: [ 82.811752] -> #3 (&mm->mmap_sem){++++++}: [ 82.811761] [] lock_acquire+0xc3/0x1d0 [ 82.811766] [] __might_fault+0x75/0xa0 [ 82.811771] [] kernfs_fop_write+0x8a/0x180 [ 82.811787] [] __vfs_write+0x23/0xe0 [ 82.811792] [] vfs_write+0xa4/0x190 [ 82.811797] [] SyS_write+0x44/0xb0 [ 82.811801] [] entry_SYSCALL_64_fastpath+0x16/0x73 [ 82.811807] -> #2 (s_active#6){++++.+}: [ 82.811814] [] lock_acquire+0xc3/0x1d0 [ 82.811819] [] __kernfs_remove+0x210/0x2f0 [ 82.811823] [] kernfs_remove_by_name_ns+0x40/0xa0 [ 82.811828] [] sysfs_remove_file_ns+0x10/0x20 [ 82.811832] [] device_del+0x124/0x250 [ 82.811837] [] device_unregister+0x19/0x60 [ 82.811841] [] cpu_cache_sysfs_exit+0x51/0xb0 [ 82.811846] [] cacheinfo_cpu_callback+0x38/0x70 [ 82.811851] [] notifier_call_chain+0x39/0xa0 [ 82.811856] [] __raw_notifier_call_chain+0x9/0x10 [ 82.811860] [] cpu_notify+0x1e/0x40 [ 82.811865] [] cpu_notify_nofail+0x9/0x20 [ 82.811869] [] _cpu_down+0x233/0x340 [ 82.811874] [] disable_nonboot_cpus+0xc9/0x350 [ 82.811878] [] suspend_devices_and_enter+0x5a1/0xb50 [ 82.811883] [] pm_suspend+0x543/0x8d0 [ 82.811888] [] state_store+0x77/0xe0 [ 82.811892] [] kobj_attr_store+0xf/0x20 [ 82.811897] [] sysfs_kf_write+0x40/0x50 [ 82.811902] [] kernfs_fop_write+0x13c/0x180 [ 82.811906] [] __vfs_write+0x23/0xe0 [ 82.811910] [] vfs_write+0xa4/0x190 [ 82.811914] [] SyS_write+0x44/0xb0 [ 82.811918] [] entry_SYSCALL_64_fastpath+0x16/0x73 [ 82.811923] -> #1 (cpu_hotplug.lock){+.+.+.}: [ 82.811929] [] lock_acquire+0xc3/0x1d0 [ 82.811933] [] mutex_lock_nested+0x62/0x3b0 [ 82.811940] [] get_online_cpus+0x61/0x80 [ 82.811944] [] stop_machine+0x1b/0xe0 [ 82.811949] [] gen8_ggtt_insert_entries__BKL+0x2d/0x30 [i915] [ 82.812009] [] ggtt_bind_vma+0x46/0x70 [i915] [ 82.812045] [] i915_vma_bind+0x140/0x290 [i915] [ 82.812081] [] i915_gem_object_do_pin+0x899/0xb00 [i915] [ 82.812117] [] i915_gem_object_pin+0x35/0x40 [i915] [ 82.812154] [] intel_init_pipe_control+0xbe/0x210 [i915] [ 82.812192] [] intel_logical_rings_init+0xe2/0xde0 [i915] [ 82.812232] [] i915_gem_init+0xf3/0x130 [i915] [ 82.812278] [] i915_driver_load+0xf2d/0x1770 [i915] [ 82.812318] [] drm_dev_register+0xa4/0xb0 [ 82.812323] [] drm_get_pci_dev+0xce/0x1e0 [ 82.812328] [] i915_pci_probe+0x2f/0x50 [i915] [ 82.812360] [] pci_device_probe+0x87/0xf0 [ 82.812366] [] driver_probe_device+0x229/0x450 [ 82.812371] [] __driver_attach+0x83/0x90 [ 82.812375] [] bus_for_each_dev+0x61/0xa0 [ 82.812380] [] driver_attach+0x19/0x20 [ 82.812384] [] bus_add_driver+0x1ef/0x290 [ 82.812388] [] driver_register+0x5b/0xe0 [ 82.812393] [] __pci_register_driver+0x5b/0x60 [ 82.812398] [] drm_pci_init+0xd6/0x100 [ 82.812402] [] 0xffffffffa027c094 [ 82.812406] [] do_one_initcall+0xae/0x1d0 [ 82.812412] [] do_init_module+0x5b/0x1cb [ 82.812417] [] load_module+0x1c20/0x2480 [ 82.812422] [] SyS_finit_module+0x7e/0xa0 [ 82.812428] [] entry_SYSCALL_64_fastpath+0x16/0x73 [ 82.812433] -> #0 (&dev->struct_mutex){+.+.+.}: [ 82.812439] [] __lock_acquire+0x1fc9/0x20f0 [ 82.812443] [] lock_acquire+0xc3/0x1d0 [ 82.812456] [] drm_gem_mmap+0x1c7/0x270 [ 82.812460] [] mmap_region+0x334/0x580 [ 82.812466] [] do_mmap+0x364/0x410 [ 82.812470] [] vm_mmap_pgoff+0x6d/0xa0 [ 82.812474] [] SyS_mmap_pgoff+0x184/0x220 [ 82.812479] [] SyS_mmap+0x1d/0x20 [ 82.812484] [] entry_SYSCALL_64_fastpath+0x16/0x73 [ 82.812489] other info that might help us debug this: [ 82.812493] Chain exists of: &dev->struct_mutex --> s_active#6 --> &mm->mmap_sem [ 82.812502] Possible unsafe locking scenario: [ 82.812506] CPU0 CPU1 [ 82.812508] ---- ---- [ 82.812510] lock(&mm->mmap_sem); [ 82.812514] lock(s_active#6); [ 82.812519] lock(&mm->mmap_sem); [ 82.812522] lock(&dev->struct_mutex); [ 82.812526] *** DEADLOCK *** [ 82.812531] 1 lock held by kms_setmode/5859: [ 82.812533] #0: (&mm->mmap_sem){++++++}, at: [] vm_mmap_pgoff+0x44/0xa0 [ 82.812541] stack backtrace: [ 82.812547] CPU: 0 PID: 5859 Comm: kms_setmode Not tainted 4.5.0-rc4-gfxbench+ #1 [ 82.812550] Hardware name: /NUC5CPYB, BIOS PYBSWCEL.86A.0040.2015.0814.1353 08/14/2015 [ 82.812553] 0000000000000000 ffff880079407bf0 ffffffff813f8505 ffffffff825fb270 [ 82.812560] ffffffff825c4190 ffff880079407c30 ffffffff810c84ac ffff880079407c90 [ 82.812566] ffff8800797ed328 ffff8800797ecb00 0000000000000001 ffff8800797ed350 [ 82.812573] Call Trace: [ 82.812578] [] dump_stack+0x67/0x92 [ 82.812582] [] print_circular_bug+0x1fc/0x310 [ 82.812586] [] __lock_acquire+0x1fc9/0x20f0 [ 82.812590] [] lock_acquire+0xc3/0x1d0 [ 82.812594] [] ? drm_gem_mmap+0x1a1/0x270 [ 82.812599] [] drm_gem_mmap+0x1c7/0x270 [ 82.812603] [] ? drm_gem_mmap+0x1a1/0x270 [ 82.812608] [] mmap_region+0x334/0x580 [ 82.812612] [] do_mmap+0x364/0x410 [ 82.812616] [] vm_mmap_pgoff+0x6d/0xa0 [ 82.812629] [] SyS_mmap_pgoff+0x184/0x220 [ 82.812633] [] SyS_mmap+0x1d/0x20 [ 82.812637] [] entry_SYSCALL_64_fastpath+0x16/0x73 Highly unlikely though this scenario is, we can avoid the issue entirely by moving the copy operation from out under the kernfs_get_active() tracking by assigning the preallocated buffer its own mutex. The temporary buffer allocation doesn't require mutex locking as it is entirely local. The locked section was extended by the addition of the preallocated buf to speed up md user operations in commit 2b75869bba676c248d8d25ae6d2bd9221dfffdb6 Author: NeilBrown Date: Mon Oct 13 16:41:28 2014 +1100 sysfs/kernfs: allow attributes to request write buffer be pre-allocated. Reported-by: Ville Syrjälä Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94350 Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Cc: Ville Syrjälä Cc: Joonas Lahtinen Cc: NeilBrown Acked-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/file.c | 51 ++++++++++++++++++++++++++++---------------------- include/linux/kernfs.h | 1 + 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index 7247252ee9b1..e1574008adc9 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -190,15 +190,16 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of, char *buf; buf = of->prealloc_buf; - if (!buf) + if (buf) + mutex_lock(&of->prealloc_mutex); + else buf = kmalloc(len, GFP_KERNEL); if (!buf) return -ENOMEM; /* * @of->mutex nests outside active ref and is used both to ensure that - * the ops aren't called concurrently for the same open file, and - * to provide exclusive access to ->prealloc_buf (when that exists). + * the ops aren't called concurrently for the same open file. */ mutex_lock(&of->mutex); if (!kernfs_get_active(of->kn)) { @@ -214,21 +215,23 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of, else len = -EINVAL; + kernfs_put_active(of->kn); + mutex_unlock(&of->mutex); + if (len < 0) - goto out_unlock; + goto out_free; if (copy_to_user(user_buf, buf, len)) { len = -EFAULT; - goto out_unlock; + goto out_free; } *ppos += len; - out_unlock: - kernfs_put_active(of->kn); - mutex_unlock(&of->mutex); out_free: - if (buf != of->prealloc_buf) + if (buf == of->prealloc_buf) + mutex_unlock(&of->prealloc_mutex); + else kfree(buf); return len; } @@ -284,15 +287,22 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf, } buf = of->prealloc_buf; - if (!buf) + if (buf) + mutex_lock(&of->prealloc_mutex); + else buf = kmalloc(len + 1, GFP_KERNEL); if (!buf) return -ENOMEM; + if (copy_from_user(buf, user_buf, len)) { + len = -EFAULT; + goto out_free; + } + buf[len] = '\0'; /* guarantee string termination */ + /* * @of->mutex nests outside active ref and is used both to ensure that - * the ops aren't called concurrently for the same open file, and - * to provide exclusive access to ->prealloc_buf (when that exists). + * the ops aren't called concurrently for the same open file. */ mutex_lock(&of->mutex); if (!kernfs_get_active(of->kn)) { @@ -301,26 +311,22 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf, goto out_free; } - if (copy_from_user(buf, user_buf, len)) { - len = -EFAULT; - goto out_unlock; - } - buf[len] = '\0'; /* guarantee string termination */ - ops = kernfs_ops(of->kn); if (ops->write) len = ops->write(of, buf, len, *ppos); else len = -EINVAL; + kernfs_put_active(of->kn); + mutex_unlock(&of->mutex); + if (len > 0) *ppos += len; -out_unlock: - kernfs_put_active(of->kn); - mutex_unlock(&of->mutex); out_free: - if (buf != of->prealloc_buf) + if (buf == of->prealloc_buf) + mutex_unlock(&of->prealloc_mutex); + else kfree(buf); return len; } @@ -687,6 +693,7 @@ static int kernfs_fop_open(struct inode *inode, struct file *file) error = -ENOMEM; if (!of->prealloc_buf) goto err_free; + mutex_init(&of->prealloc_mutex); } /* diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h index c06c44242f39..d306e282bb1d 100644 --- a/include/linux/kernfs.h +++ b/include/linux/kernfs.h @@ -177,6 +177,7 @@ struct kernfs_open_file { /* private fields, do not use outside kernfs proper */ struct mutex mutex; + struct mutex prealloc_mutex; int event; struct list_head list; char *prealloc_buf; -- cgit v1.2.3 From ded9db380d34344ee5263002570b9b8b705f7830 Mon Sep 17 00:00:00 2001 From: Yi Zhang Date: Tue, 8 Mar 2016 23:15:22 +0800 Subject: base: dd: don't remove driver_data in -EPROBE_DEFER case the driver_data may be used for sanity check, it fails the probe() if driver_data is NULL after it is re-triggered. for example, soc_probe() in sound/soc/soc-core.c Signed-off-by: Yi Zhang Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 16688f50729c..dae6cefa6060 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -402,7 +402,8 @@ pinctrl_bind_failed: devres_release_all(dev); driver_sysfs_remove(dev); dev->driver = NULL; - dev_set_drvdata(dev, NULL); + if (ret != -EPROBE_DEFER) + dev_set_drvdata(dev, NULL); if (dev->pm_domain && dev->pm_domain->dismiss) dev->pm_domain->dismiss(dev); pm_runtime_reinit(dev); -- cgit v1.2.3 From 1f62ff34a90471d1b735bac2c79e894afc7c59bc Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Thu, 24 Mar 2016 22:19:40 +0100 Subject: driver-core: use 'dev' argument in dev_dbg_ratelimited stub dev_dbg_ratelimited() is a macro that ignores its first argument when DEBUG is not set, which can lead to unused variable warnings: ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle': ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable] ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle': ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable] The macro already ensures that all its other arguments are silently ignored by the compiler without triggering a warning, through the use of the no_printk() macro, but the dev argument is not passed into that. This changes the definition to use the same trick as no_printk() with an if(0) that leads the compiler to not evaluate the side-effects but still see that 'dev' might not be unused. Signed-off-by: Arnd Bergmann Suggested-by: Andrew Lunn Fixes: 6f586e663e3b ("driver-core: Shut up dev_dbg_reatelimited() without DEBUG") Reviewed-by: Dmitry Torokhov Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/linux/device.h b/include/linux/device.h index 002c59728dbe..07f74c246cac 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1293,8 +1293,11 @@ do { \ dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \ } while (0) #else -#define dev_dbg_ratelimited(dev, fmt, ...) \ - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) +#define dev_dbg_ratelimited(dev, fmt, ...) \ +do { \ + if (0) \ + dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \ +} while (0) #endif #ifdef VERBOSE_DEBUG -- cgit v1.2.3 From 8ac0fba2da41620f4931a1007c71b0d4723eb02a Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 17:50:29 -0400 Subject: isa: Decouple X86_32 dependency from the ISA Kconfig option The introduction of the ISA_BUS option blocks the compilation of ISA drivers on non-x86 platforms. The ISA_BUS configuration option should not be necessary if the X86_32 dependency can be decoupled from the ISA configuration option. This patch both removes the ISA_BUS configuration option entirely and removes the X86_32 dependency from the ISA configuration option. Acked-by: Ingo Molnar Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- arch/x86/Kconfig | 10 ++-------- drivers/base/Makefile | 2 +- include/linux/isa.h | 2 +- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index a5977986f38b..280e5ebae2f5 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -2472,16 +2472,8 @@ config ISA_DMA_API Enables ISA-style DMA support for devices requiring such controllers. If unsure, say Y. -config ISA_BUS - bool "ISA bus support" - help - Enables ISA bus support for devices requiring such controllers. - -if X86_32 - config ISA bool "ISA support" - depends on ISA_BUS ---help--- Find out whether you have ISA slots on your motherboard. ISA is the name of a bus system, i.e. the way the CPU talks to the other stuff @@ -2489,6 +2481,8 @@ config ISA (MCA) or VESA. ISA is an older system, now being displaced by PCI; newer boards don't support it. If you have ISA, say Y, otherwise N. +if X86_32 + config EISA bool "EISA support" depends on ISA diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 4ebfb81cc7e9..6b2a84e7f2be 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -10,7 +10,7 @@ obj-$(CONFIG_DMA_CMA) += dma-contiguous.o obj-y += power/ obj-$(CONFIG_HAS_DMA) += dma-mapping.o obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o -obj-$(CONFIG_ISA_BUS) += isa.o +obj-$(CONFIG_ISA) += isa.o obj-$(CONFIG_FW_LOADER) += firmware_class.o obj-$(CONFIG_NUMA) += node.o obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o diff --git a/include/linux/isa.h b/include/linux/isa.h index 2a02862775eb..b0270e3814c8 100644 --- a/include/linux/isa.h +++ b/include/linux/isa.h @@ -22,7 +22,7 @@ struct isa_driver { #define to_isa_driver(x) container_of((x), struct isa_driver, driver) -#ifdef CONFIG_ISA_BUS +#ifdef CONFIG_ISA int isa_register_driver(struct isa_driver *, unsigned int); void isa_unregister_driver(struct isa_driver *); #else -- cgit v1.2.3 From 0619f39474b991244d0a2cbd323915c0e6167b5a Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 11:46:17 -0400 Subject: pnp: pnpbios: Add explicit X86_32 dependency to PNPBIOS The PNPBIOS driver requires preprocessor defines (located in include/asm/segment.h) only declared if the architecture is set to X86_32. If the architecture is set to X86_64, the PNPBIOS driver will not build properly. The X86 dependecy for the PNPBIOS configuration option is changed to an explicit X86_32 dependency in order to prevent an attempt to build for an unsupported architecture. Acked-by: Rafael J. Wysocki Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- drivers/pnp/pnpbios/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pnp/pnpbios/Kconfig b/drivers/pnp/pnpbios/Kconfig index 50c3dd065e03..a786086b2ec7 100644 --- a/drivers/pnp/pnpbios/Kconfig +++ b/drivers/pnp/pnpbios/Kconfig @@ -3,7 +3,7 @@ # config PNPBIOS bool "Plug and Play BIOS support" - depends on ISA && X86 + depends on ISA && X86_32 default n ---help--- Linux uses the PNPBIOS as defined in "Plug and Play BIOS -- cgit v1.2.3 From 339e6e31d2bfb40354cbfe672b357b88a88223f2 Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:42:47 -0400 Subject: isa: Implement the module_isa_driver macro The module_isa_driver macro is a helper macro for ISA drivers which do not do anything special in module init/exit. This eliminates a lot of boilerplate code. Each module may only use this macro once, and calling it replaces module_init and module_exit. Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- include/linux/isa.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/include/linux/isa.h b/include/linux/isa.h index b0270e3814c8..e394917d18c2 100644 --- a/include/linux/isa.h +++ b/include/linux/isa.h @@ -36,4 +36,25 @@ static inline void isa_unregister_driver(struct isa_driver *d) } #endif +/** + * module_isa_driver() - Helper macro for registering a ISA driver + * @__isa_driver: isa_driver struct + * @__num_isa_dev: number of devices to register + * + * Helper macro for ISA drivers which do not do anything special in module + * init/exit. This eliminates a lot of boilerplate code. Each module may only + * use this macro once, and calling it replaces module_init and module_exit. + */ +#define module_isa_driver(__isa_driver, __num_isa_dev) \ +static int __init __isa_driver##_init(void) \ +{ \ + return isa_register_driver(&(__isa_driver), __num_isa_dev); \ +} \ +module_init(__isa_driver##_init); \ +static void __exit __isa_driver##_exit(void) \ +{ \ + isa_unregister_driver(&(__isa_driver)); \ +} \ +module_exit(__isa_driver##_exit); + #endif /* __LINUX_ISA_H */ -- cgit v1.2.3 From d9a9c6172d1cec51851e9015b6c4379635c31f1a Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:43:10 -0400 Subject: isa: Implement the max_num_isa_dev macro max_num_isa_dev is a macro to determine the maximum possible number of ISA devices which may be registered in the I/O port address space given the address extent of the ISA devices. The highest base address possible for an ISA device is 0x3FF; this results in 1024 possible base addresses. Dividing the number of possible base addresses by the address extent taken by each device results in the maximum number of devices on a system. Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- include/linux/isa.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/linux/isa.h b/include/linux/isa.h index e394917d18c2..5ab85281230b 100644 --- a/include/linux/isa.h +++ b/include/linux/isa.h @@ -57,4 +57,15 @@ static void __exit __isa_driver##_exit(void) \ } \ module_exit(__isa_driver##_exit); +/** + * max_num_isa_dev() - Maximum possible number registered of an ISA device + * @__ida_dev_ext: ISA device address extent + * + * The highest base address possible for an ISA device is 0x3FF; this results in + * 1024 possible base addresses. Dividing the number of possible base addresses + * by the address extent taken by each device results in the maximum number of + * devices on a system. + */ +#define max_num_isa_dev(__isa_dev_ext) (1024 / __isa_dev_ext) + #endif /* __LINUX_ISA_H */ -- cgit v1.2.3 From ad7afc38eab37c8a71f10ad6246df6ba7d7e5bb3 Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:43:35 -0400 Subject: Documentation: Add ISA bus driver documentation This is a verbatim copy of the original commit message of the initial commit of the ISA bus driver authored by Rene Herman. Descriptions of the module_isa_driver macro and max_num_isa_dev macro are provided at the end. Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- Documentation/isa.txt | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++ MAINTAINERS | 7 +++ 2 files changed, 128 insertions(+) create mode 100644 Documentation/isa.txt diff --git a/Documentation/isa.txt b/Documentation/isa.txt new file mode 100644 index 000000000000..f232c26a40be --- /dev/null +++ b/Documentation/isa.txt @@ -0,0 +1,121 @@ +ISA Drivers +----------- + +The following text is adapted from the commit message of the initial +commit of the ISA bus driver authored by Rene Herman. + +During the recent "isa drivers using platform devices" discussion it was +pointed out that (ALSA) ISA drivers ran into the problem of not having +the option to fail driver load (device registration rather) upon not +finding their hardware due to a probe() error not being passed up +through the driver model. In the course of that, I suggested a separate +ISA bus might be best; Russell King agreed and suggested this bus could +use the .match() method for the actual device discovery. + +The attached does this. For this old non (generically) discoverable ISA +hardware only the driver itself can do discovery so as a difference with +the platform_bus, this isa_bus also distributes match() up to the +driver. + +As another difference: these devices only exist in the driver model due +to the driver creating them because it might want to drive them, meaning +that all device creation has been made internal as well. + +The usage model this provides is nice, and has been acked from the ALSA +side by Takashi Iwai and Jaroslav Kysela. The ALSA driver module_init's +now (for oldisa-only drivers) become: + +static int __init alsa_card_foo_init(void) +{ + return isa_register_driver(&snd_foo_isa_driver, SNDRV_CARDS); +} + +static void __exit alsa_card_foo_exit(void) +{ + isa_unregister_driver(&snd_foo_isa_driver); +} + +Quite like the other bus models therefore. This removes a lot of +duplicated init code from the ALSA ISA drivers. + +The passed in isa_driver struct is the regular driver struct embedding a +struct device_driver, the normal probe/remove/shutdown/suspend/resume +callbacks, and as indicated that .match callback. + +The "SNDRV_CARDS" you see being passed in is a "unsigned int ndev" +parameter, indicating how many devices to create and call our methods +with. + +The platform_driver callbacks are called with a platform_device param; +the isa_driver callbacks are being called with a "struct device *dev, +unsigned int id" pair directly -- with the device creation completely +internal to the bus it's much cleaner to not leak isa_dev's by passing +them in at all. The id is the only thing we ever want other then the +struct device * anyways, and it makes for nicer code in the callbacks as +well. + +With this additional .match() callback ISA drivers have all options. If +ALSA would want to keep the old non-load behaviour, it could stick all +of the old .probe in .match, which would only keep them registered after +everything was found to be present and accounted for. If it wanted the +behaviour of always loading as it inadvertently did for a bit after the +changeover to platform devices, it could just not provide a .match() and +do everything in .probe() as before. + +If it, as Takashi Iwai already suggested earlier as a way of following +the model from saner buses more closely, wants to load when a later bind +could conceivably succeed, it could use .match() for the prerequisites +(such as checking the user wants the card enabled and that port/irq/dma +values have been passed in) and .probe() for everything else. This is +the nicest model. + +To the code... + +This exports only two functions; isa_{,un}register_driver(). + +isa_register_driver() register's the struct device_driver, and then +loops over the passed in ndev creating devices and registering them. +This causes the bus match method to be called for them, which is: + +int isa_bus_match(struct device *dev, struct device_driver *driver) +{ + struct isa_driver *isa_driver = to_isa_driver(driver); + + if (dev->platform_data == isa_driver) { + if (!isa_driver->match || + isa_driver->match(dev, to_isa_dev(dev)->id)) + return 1; + dev->platform_data = NULL; + } + return 0; +} + +The first thing this does is check if this device is in fact one of this +driver's devices by seeing if the device's platform_data pointer is set +to this driver. Platform devices compare strings, but we don't need to +do that with everything being internal, so isa_register_driver() abuses +dev->platform_data as a isa_driver pointer which we can then check here. +I believe platform_data is available for this, but if rather not, moving +the isa_driver pointer to the private struct isa_dev is ofcourse fine as +well. + +Then, if the the driver did not provide a .match, it matches. If it did, +the driver match() method is called to determine a match. + +If it did _not_ match, dev->platform_data is reset to indicate this to +isa_register_driver which can then unregister the device again. + +If during all this, there's any error, or no devices matched at all +everything is backed out again and the error, or -ENODEV, is returned. + +isa_unregister_driver() just unregisters the matched devices and the +driver itself. + +module_isa_driver is a helper macro for ISA drivers which do not do +anything special in module init/exit. This eliminates a lot of +boilerplate code. Each module may only use this macro once, and calling +it replaces module_init and module_exit. + +max_num_isa_dev is a macro to determine the maximum possible number of +ISA devices which may be registered in the I/O port address space given +the address extent of the ISA devices. diff --git a/MAINTAINERS b/MAINTAINERS index 1d5b4becab6f..92a190675ae3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5995,6 +5995,13 @@ F: include/linux/irqdomain.h F: kernel/irq/irqdomain.c F: kernel/irq/msi.c +ISA +M: William Breathitt Gray +S: Maintained +F: Documentation/isa.txt +F: drivers/base/isa.c +F: include/linux/isa.h + ISAPNP M: Jaroslav Kysela S: Maintained -- cgit v1.2.3 From 136c9568457d7747f63c23f9acb01a0e831f351a Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:43:54 -0400 Subject: iio: stx104: Add X86 dependency to STX104 Kconfig option The Apex Embedded Systems STX104 DAC driver expects the STX104 device to be I/O port-mapped addressable on an X86 platform. This patch adds an explicit X86 dependency to the STX104 configuration option to prevent building on non-X86 architectures. Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Lars-Peter Clausen Cc: Peter Meerwald Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- drivers/iio/dac/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index a995139f907c..6abcfb8597d9 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -210,7 +210,7 @@ config MCP4922 config STX104 tristate "Apex Embedded Systems STX104 DAC driver" - depends on ISA + depends on X86 && ISA help Say yes here to build support for the 2-channel DAC on the Apex Embedded Systems STX104 integrated analog PC/104 card. The base port -- cgit v1.2.3 From 9c2cfc19e64a513a1b9e55f78e38661a1b97b91e Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:44:11 -0400 Subject: iio: stx104: Utilize the module_isa_driver and max_num_isa_dev macros The Apex Embedded Systems STX104 DAC drivers does not do anything special in module init/exit. This patch eliminates the module init/exit boilerplate code by utilizing the module_isa_driver macro. Additionally, the max_num_isa_dev macro is utilized to simplify the determination of maximum possible STX104 devices in the system. Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Lars-Peter Clausen Cc: Peter Meerwald Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- drivers/iio/dac/stx104.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/drivers/iio/dac/stx104.c b/drivers/iio/dac/stx104.c index 174f4b75ceed..27941220872f 100644 --- a/drivers/iio/dac/stx104.c +++ b/drivers/iio/dac/stx104.c @@ -33,16 +33,9 @@ } #define STX104_EXTENT 16 -/** - * The highest base address possible for an ISA device is 0x3FF; this results in - * 1024 possible base addresses. Dividing the number of possible base addresses - * by the address extent taken by each device results in the maximum number of - * devices on a system. - */ -#define MAX_NUM_STX104 (1024 / STX104_EXTENT) -static unsigned base[MAX_NUM_STX104]; -static unsigned num_stx104; +static unsigned int base[max_num_isa_dev(STX104_EXTENT)]; +static unsigned int num_stx104; module_param_array(base, uint, &num_stx104, 0); MODULE_PARM_DESC(base, "Apex Embedded Systems STX104 base addresses"); @@ -134,18 +127,7 @@ static struct isa_driver stx104_driver = { } }; -static void __exit stx104_exit(void) -{ - isa_unregister_driver(&stx104_driver); -} - -static int __init stx104_init(void) -{ - return isa_register_driver(&stx104_driver, num_stx104); -} - -module_init(stx104_init); -module_exit(stx104_exit); +module_isa_driver(stx104_driver, num_stx104); MODULE_AUTHOR("William Breathitt Gray "); MODULE_DESCRIPTION("Apex Embedded Systems STX104 DAC driver"); -- cgit v1.2.3 From 4ef1bec4e4f393aa96d2b5345bb08baabaec8ee1 Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:44:26 -0400 Subject: watchdog: ebc-c384_wdt: Utilize the ISA bus driver The WinSystems EBC-C384 watchdog timer is controlled via ISA bus communication. As such, the ISA bus driver is more appropriate than the platform driver for the WinSystems EBC-C384 watchdog timer driver. Cc: Wim Van Sebroeck Acked-by: Guenter Roeck Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- drivers/watchdog/Kconfig | 2 +- drivers/watchdog/ebc-c384_wdt.c | 43 ++++++++++------------------------------- 2 files changed, 11 insertions(+), 34 deletions(-) diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index fb947655badd..0aefe2510e42 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -738,7 +738,7 @@ config ALIM7101_WDT config EBC_C384_WDT tristate "WinSystems EBC-C384 Watchdog Timer" - depends on X86 + depends on X86 && ISA select WATCHDOG_CORE help Enables watchdog timer support for the watchdog timer on the diff --git a/drivers/watchdog/ebc-c384_wdt.c b/drivers/watchdog/ebc-c384_wdt.c index 77fda0b4b90e..4b849b8e37c2 100644 --- a/drivers/watchdog/ebc-c384_wdt.c +++ b/drivers/watchdog/ebc-c384_wdt.c @@ -16,10 +16,10 @@ #include #include #include +#include #include #include #include -#include #include #include @@ -95,9 +95,8 @@ static const struct watchdog_info ebc_c384_wdt_info = { .identity = MODULE_NAME }; -static int __init ebc_c384_wdt_probe(struct platform_device *pdev) +static int ebc_c384_wdt_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct watchdog_device *wdd; if (!devm_request_region(dev, BASE_ADDR, ADDR_EXTENT, dev_name(dev))) { @@ -122,61 +121,39 @@ static int __init ebc_c384_wdt_probe(struct platform_device *pdev) dev_warn(dev, "Invalid timeout (%u seconds), using default (%u seconds)\n", timeout, WATCHDOG_TIMEOUT); - platform_set_drvdata(pdev, wdd); + dev_set_drvdata(dev, wdd); return watchdog_register_device(wdd); } -static int ebc_c384_wdt_remove(struct platform_device *pdev) +static int ebc_c384_wdt_remove(struct device *dev, unsigned int id) { - struct watchdog_device *wdd = platform_get_drvdata(pdev); + struct watchdog_device *wdd = dev_get_drvdata(dev); watchdog_unregister_device(wdd); return 0; } -static struct platform_driver ebc_c384_wdt_driver = { +static struct isa_driver ebc_c384_wdt_driver = { + .probe = ebc_c384_wdt_probe, .driver = { .name = MODULE_NAME }, .remove = ebc_c384_wdt_remove }; -static struct platform_device *ebc_c384_wdt_device; - static int __init ebc_c384_wdt_init(void) { - int err; - if (!dmi_match(DMI_BOARD_NAME, "EBC-C384 SBC")) return -ENODEV; - ebc_c384_wdt_device = platform_device_alloc(MODULE_NAME, -1); - if (!ebc_c384_wdt_device) - return -ENOMEM; - - err = platform_device_add(ebc_c384_wdt_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&ebc_c384_wdt_driver, ebc_c384_wdt_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(ebc_c384_wdt_device); -err_platform_device: - platform_device_put(ebc_c384_wdt_device); - return err; + return isa_register_driver(&ebc_c384_wdt_driver, 1); } static void __exit ebc_c384_wdt_exit(void) { - platform_device_unregister(ebc_c384_wdt_device); - platform_driver_unregister(&ebc_c384_wdt_driver); + isa_unregister_driver(&ebc_c384_wdt_driver); } module_init(ebc_c384_wdt_init); @@ -185,4 +162,4 @@ module_exit(ebc_c384_wdt_exit); MODULE_AUTHOR("William Breathitt Gray "); MODULE_DESCRIPTION("WinSystems EBC-C384 watchdog timer driver"); MODULE_LICENSE("GPL v2"); -MODULE_ALIAS("platform:" MODULE_NAME); +MODULE_ALIAS("isa:" MODULE_NAME); -- cgit v1.2.3 From 4c23db0f9f7f5e554837f69a849de75777f3fefc Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:44:39 -0400 Subject: gpio: 104-dio-48e: Utilize the ISA bus driver The ACCES 104-DIO-48E series communicates via the ISA bus. As such, it is more appropriate to use the ISA bus driver over the platform driver to control the ACCES 104-DIO-48E GPIO driver. This patch also adds support for multiple devices via the base and irq module array parameters. Each element of the base array corresponds to a discrete device; each element of the irq array corresponds to the respective device addressed in the respective base array element. Acked-by: Linus Walleij Cc: Alexandre Courbot Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- drivers/gpio/Kconfig | 9 ++-- drivers/gpio/gpio-104-dio-48e.c | 106 ++++++++++++++-------------------------- 2 files changed, 43 insertions(+), 72 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 5f3429f0bf46..5aca476c6692 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -517,12 +517,13 @@ menu "Port-mapped I/O GPIO drivers" config GPIO_104_DIO_48E tristate "ACCES 104-DIO-48E GPIO support" + depends on ISA select GPIOLIB_IRQCHIP help - Enables GPIO support for the ACCES 104-DIO-48E family. The base port - address for the device may be configured via the dio_48e_base module - parameter. The interrupt line number for the device may be configured - via the dio_48e_irq module parameter. + Enables GPIO support for the ACCES 104-DIO-48E series (104-DIO-48E, + 104-DIO-24E). The base port addresses for the devices may be + configured via the base module parameter. The interrupt line numbers + for the devices may be configured via the irq module parameter. config GPIO_104_IDIO_16 tristate "ACCES 104-IDIO-16 GPIO support" diff --git a/drivers/gpio/gpio-104-dio-48e.c b/drivers/gpio/gpio-104-dio-48e.c index 448a903089ef..1a647c07be67 100644 --- a/drivers/gpio/gpio-104-dio-48e.c +++ b/drivers/gpio/gpio-104-dio-48e.c @@ -1,5 +1,5 @@ /* - * GPIO driver for the ACCES 104-DIO-48E + * GPIO driver for the ACCES 104-DIO-48E series * Copyright (C) 2016 William Breathitt Gray * * This program is free software; you can redistribute it and/or modify @@ -10,6 +10,9 @@ * WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * General Public License for more details. + * + * This driver supports the following ACCES devices: 104-DIO-48E and + * 104-DIO-24E. */ #include #include @@ -19,18 +22,23 @@ #include #include #include +#include #include #include #include -#include #include -static unsigned dio_48e_base; -module_param(dio_48e_base, uint, 0); -MODULE_PARM_DESC(dio_48e_base, "ACCES 104-DIO-48E base address"); -static unsigned dio_48e_irq; -module_param(dio_48e_irq, uint, 0); -MODULE_PARM_DESC(dio_48e_irq, "ACCES 104-DIO-48E interrupt line number"); +#define DIO48E_EXTENT 16 +#define MAX_NUM_DIO48E max_num_isa_dev(DIO48E_EXTENT) + +static unsigned int base[MAX_NUM_DIO48E]; +static unsigned int num_dio48e; +module_param_array(base, uint, &num_dio48e, 0); +MODULE_PARM_DESC(base, "ACCES 104-DIO-48E base addresses"); + +static unsigned int irq[MAX_NUM_DIO48E]; +module_param_array(irq, uint, NULL, 0); +MODULE_PARM_DESC(irq, "ACCES 104-DIO-48E interrupt line numbers"); /** * struct dio48e_gpio - GPIO device private data structure @@ -294,23 +302,19 @@ static irqreturn_t dio48e_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int __init dio48e_probe(struct platform_device *pdev) +static int dio48e_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct dio48e_gpio *dio48egpio; - const unsigned base = dio_48e_base; - const unsigned extent = 16; const char *const name = dev_name(dev); int err; - const unsigned irq = dio_48e_irq; dio48egpio = devm_kzalloc(dev, sizeof(*dio48egpio), GFP_KERNEL); if (!dio48egpio) return -ENOMEM; - if (!devm_request_region(dev, base, extent, name)) { + if (!devm_request_region(dev, base[id], DIO48E_EXTENT, name)) { dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", - base, base + extent); + base[id], base[id] + DIO48E_EXTENT); return -EBUSY; } @@ -324,8 +328,8 @@ static int __init dio48e_probe(struct platform_device *pdev) dio48egpio->chip.direction_output = dio48e_gpio_direction_output; dio48egpio->chip.get = dio48e_gpio_get; dio48egpio->chip.set = dio48e_gpio_set; - dio48egpio->base = base; - dio48egpio->irq = irq; + dio48egpio->base = base[id]; + dio48egpio->irq = irq[id]; spin_lock_init(&dio48egpio->lock); @@ -338,19 +342,19 @@ static int __init dio48e_probe(struct platform_device *pdev) } /* initialize all GPIO as output */ - outb(0x80, base + 3); - outb(0x00, base); - outb(0x00, base + 1); - outb(0x00, base + 2); - outb(0x00, base + 3); - outb(0x80, base + 7); - outb(0x00, base + 4); - outb(0x00, base + 5); - outb(0x00, base + 6); - outb(0x00, base + 7); + outb(0x80, base[id] + 3); + outb(0x00, base[id]); + outb(0x00, base[id] + 1); + outb(0x00, base[id] + 2); + outb(0x00, base[id] + 3); + outb(0x80, base[id] + 7); + outb(0x00, base[id] + 4); + outb(0x00, base[id] + 5); + outb(0x00, base[id] + 6); + outb(0x00, base[id] + 7); /* disable IRQ by default */ - inb(base + 0xB); + inb(base[id] + 0xB); err = gpiochip_irqchip_add(&dio48egpio->chip, &dio48e_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); @@ -359,7 +363,7 @@ static int __init dio48e_probe(struct platform_device *pdev) goto err_gpiochip_remove; } - err = request_irq(irq, dio48e_irq_handler, 0, name, dio48egpio); + err = request_irq(irq[id], dio48e_irq_handler, 0, name, dio48egpio); if (err) { dev_err(dev, "IRQ handler registering failed (%d)\n", err); goto err_gpiochip_remove; @@ -372,9 +376,9 @@ err_gpiochip_remove: return err; } -static int dio48e_remove(struct platform_device *pdev) +static int dio48e_remove(struct device *dev, unsigned int id) { - struct dio48e_gpio *const dio48egpio = platform_get_drvdata(pdev); + struct dio48e_gpio *const dio48egpio = dev_get_drvdata(dev); free_irq(dio48egpio->irq, dio48egpio); gpiochip_remove(&dio48egpio->chip); @@ -382,48 +386,14 @@ static int dio48e_remove(struct platform_device *pdev) return 0; } -static struct platform_device *dio48e_device; - -static struct platform_driver dio48e_driver = { +static struct isa_driver dio48e_driver = { + .probe = dio48e_probe, .driver = { .name = "104-dio-48e" }, .remove = dio48e_remove }; - -static void __exit dio48e_exit(void) -{ - platform_device_unregister(dio48e_device); - platform_driver_unregister(&dio48e_driver); -} - -static int __init dio48e_init(void) -{ - int err; - - dio48e_device = platform_device_alloc(dio48e_driver.driver.name, -1); - if (!dio48e_device) - return -ENOMEM; - - err = platform_device_add(dio48e_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&dio48e_driver, dio48e_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(dio48e_device); -err_platform_device: - platform_device_put(dio48e_device); - return err; -} - -module_init(dio48e_init); -module_exit(dio48e_exit); +module_isa_driver(dio48e_driver, num_dio48e); MODULE_AUTHOR("William Breathitt Gray "); MODULE_DESCRIPTION("ACCES 104-DIO-48E GPIO driver"); -- cgit v1.2.3 From 72bf7443ba618b9f7a3167c1f591a0dc00faeb2d Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:44:55 -0400 Subject: gpio: 104-idi-48: Utilize the ISA bus driver The ACCES 104-IDI-48 series communicates via the ISA bus. As such, it is more appropriate to use the ISA bus driver over the platform driver to control the ACCES 104-IDI-48 GPIO driver. This patch also adds support for multiple devices via the base and irq module array parameters. Each element of the base array corresponds to a discrete device; each element of the irq array corresponds to the respective device addressed in the respective base array element. Acked-by: Linus Walleij Cc: Alexandre Courbot Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- drivers/gpio/Kconfig | 10 +++-- drivers/gpio/gpio-104-idi-48.c | 86 ++++++++++++++---------------------------- 2 files changed, 34 insertions(+), 62 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 5aca476c6692..c96ef58ed9e1 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -536,12 +536,14 @@ config GPIO_104_IDIO_16 config GPIO_104_IDI_48 tristate "ACCES 104-IDI-48 GPIO support" + depends on ISA select GPIOLIB_IRQCHIP help - Enables GPIO support for the ACCES 104-IDI-48 family. The base port - address for the device may be configured via the idi_48_base module - parameter. The interrupt line number for the device may be configured - via the idi_48_irq module parameter. + Enables GPIO support for the ACCES 104-IDI-48 family (104-IDI-48A, + 104-IDI-48AC, 104-IDI-48B, 104-IDI-48BC). The base port addresses for + the devices may be configured via the base module parameter. The + interrupt line numbers for the devices may be configured via the irq + module parameter. config GPIO_F7188X tristate "F71869, F71869A, F71882FG, F71889F and F81866 GPIO support" diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c index e37cd4cdda35..6c75c83baf5a 100644 --- a/drivers/gpio/gpio-104-idi-48.c +++ b/drivers/gpio/gpio-104-idi-48.c @@ -10,6 +10,9 @@ * WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * General Public License for more details. + * + * This driver supports the following ACCES devices: 104-IDI-48A, + * 104-IDI-48AC, 104-IDI-48B, and 104-IDI-48BC. */ #include #include @@ -19,18 +22,23 @@ #include #include #include +#include #include #include #include -#include #include -static unsigned idi_48_base; -module_param(idi_48_base, uint, 0); -MODULE_PARM_DESC(idi_48_base, "ACCES 104-IDI-48 base address"); -static unsigned idi_48_irq; -module_param(idi_48_irq, uint, 0); -MODULE_PARM_DESC(idi_48_irq, "ACCES 104-IDI-48 interrupt line number"); +#define IDI_48_EXTENT 8 +#define MAX_NUM_IDI_48 max_num_isa_dev(IDI_48_EXTENT) + +static unsigned int base[MAX_NUM_IDI_48]; +static unsigned int num_idi_48; +module_param_array(base, uint, &num_idi_48, 0); +MODULE_PARM_DESC(base, "ACCES 104-IDI-48 base addresses"); + +static unsigned int irq[MAX_NUM_IDI_48]; +module_param_array(irq, uint, NULL, 0); +MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers"); /** * struct idi_48_gpio - GPIO device private data structure @@ -211,23 +219,19 @@ static irqreturn_t idi_48_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int __init idi_48_probe(struct platform_device *pdev) +static int idi_48_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct idi_48_gpio *idi48gpio; - const unsigned base = idi_48_base; - const unsigned extent = 8; const char *const name = dev_name(dev); int err; - const unsigned irq = idi_48_irq; idi48gpio = devm_kzalloc(dev, sizeof(*idi48gpio), GFP_KERNEL); if (!idi48gpio) return -ENOMEM; - if (!devm_request_region(dev, base, extent, name)) { + if (!devm_request_region(dev, base[id], IDI_48_EXTENT, name)) { dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", - base, base + extent); + base[id], base[id] + IDI_48_EXTENT); return -EBUSY; } @@ -239,8 +243,8 @@ static int __init idi_48_probe(struct platform_device *pdev) idi48gpio->chip.get_direction = idi_48_gpio_get_direction; idi48gpio->chip.direction_input = idi_48_gpio_direction_input; idi48gpio->chip.get = idi_48_gpio_get; - idi48gpio->base = base; - idi48gpio->irq = irq; + idi48gpio->base = base[id]; + idi48gpio->irq = irq[id]; spin_lock_init(&idi48gpio->lock); @@ -253,8 +257,8 @@ static int __init idi_48_probe(struct platform_device *pdev) } /* Disable IRQ by default */ - outb(0, base + 7); - inb(base + 7); + outb(0, base[id] + 7); + inb(base[id] + 7); err = gpiochip_irqchip_add(&idi48gpio->chip, &idi_48_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); @@ -263,7 +267,7 @@ static int __init idi_48_probe(struct platform_device *pdev) goto err_gpiochip_remove; } - err = request_irq(irq, idi_48_irq_handler, IRQF_SHARED, name, + err = request_irq(irq[id], idi_48_irq_handler, IRQF_SHARED, name, idi48gpio); if (err) { dev_err(dev, "IRQ handler registering failed (%d)\n", err); @@ -277,9 +281,9 @@ err_gpiochip_remove: return err; } -static int idi_48_remove(struct platform_device *pdev) +static int idi_48_remove(struct device *dev, unsigned int id) { - struct idi_48_gpio *const idi48gpio = platform_get_drvdata(pdev); + struct idi_48_gpio *const idi48gpio = dev_get_drvdata(dev); free_irq(idi48gpio->irq, idi48gpio); gpiochip_remove(&idi48gpio->chip); @@ -287,48 +291,14 @@ static int idi_48_remove(struct platform_device *pdev) return 0; } -static struct platform_device *idi_48_device; - -static struct platform_driver idi_48_driver = { +static struct isa_driver idi_48_driver = { + .probe = idi_48_probe, .driver = { .name = "104-idi-48" }, .remove = idi_48_remove }; - -static void __exit idi_48_exit(void) -{ - platform_device_unregister(idi_48_device); - platform_driver_unregister(&idi_48_driver); -} - -static int __init idi_48_init(void) -{ - int err; - - idi_48_device = platform_device_alloc(idi_48_driver.driver.name, -1); - if (!idi_48_device) - return -ENOMEM; - - err = platform_device_add(idi_48_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&idi_48_driver, idi_48_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(idi_48_device); -err_platform_device: - platform_device_put(idi_48_device); - return err; -} - -module_init(idi_48_init); -module_exit(idi_48_exit); +module_isa_driver(idi_48_driver, num_idi_48); MODULE_AUTHOR("William Breathitt Gray "); MODULE_DESCRIPTION("ACCES 104-IDI-48 GPIO driver"); -- cgit v1.2.3 From 86ea8a95a42f752fe0aa1c7ad1bfe8ce9be5d30e Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:45:11 -0400 Subject: gpio: 104-idio-16: Utilize the ISA bus driver The ACCES 104-IDIO-16 series communicates via the ISA bus. As such, it is more appropriate to use the ISA bus driver over the platform driver to control the ACCES 104-IDIO-16 GPIO driver. This patch also adds support for multiple devices via the base and irq module array parameters. Each element of the base array corresponds to a discrete device; each element of the irq array corresponds to the respective device addressed in the respective base array element. Acked-by: Linus Walleij Cc: Alexandre Courbot Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- drivers/gpio/Kconfig | 10 +++-- drivers/gpio/gpio-104-idio-16.c | 85 ++++++++++++++--------------------------- 2 files changed, 34 insertions(+), 61 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index c96ef58ed9e1..af3237cef81d 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -527,12 +527,14 @@ config GPIO_104_DIO_48E config GPIO_104_IDIO_16 tristate "ACCES 104-IDIO-16 GPIO support" + depends on ISA select GPIOLIB_IRQCHIP help - Enables GPIO support for the ACCES 104-IDIO-16 family. The base port - address for the device may be set via the idio_16_base module - parameter. The interrupt line number for the device may be set via the - idio_16_irq module parameter. + Enables GPIO support for the ACCES 104-IDIO-16 family (104-IDIO-16, + 104-IDIO-16E, 104-IDO-16, 104-IDIO-8, 104-IDIO-8E, 104-IDO-8). The + base port addresses for the devices may be configured via the base + module parameter. The interrupt line numbers for the devices may be + configured via the irq module parameter. config GPIO_104_IDI_48 tristate "ACCES 104-IDI-48 GPIO support" diff --git a/drivers/gpio/gpio-104-idio-16.c b/drivers/gpio/gpio-104-idio-16.c index ecc85fe9323d..6787b8fcf0d8 100644 --- a/drivers/gpio/gpio-104-idio-16.c +++ b/drivers/gpio/gpio-104-idio-16.c @@ -10,6 +10,9 @@ * WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * General Public License for more details. + * + * This driver supports the following ACCES devices: 104-IDIO-16, + * 104-IDIO-16E, 104-IDO-16, 104-IDIO-8, 104-IDIO-8E, and 104-IDO-8. */ #include #include @@ -19,18 +22,23 @@ #include #include #include +#include #include #include #include -#include #include -static unsigned idio_16_base; -module_param(idio_16_base, uint, 0); -MODULE_PARM_DESC(idio_16_base, "ACCES 104-IDIO-16 base address"); -static unsigned idio_16_irq; -module_param(idio_16_irq, uint, 0); -MODULE_PARM_DESC(idio_16_irq, "ACCES 104-IDIO-16 interrupt line number"); +#define IDIO_16_EXTENT 8 +#define MAX_NUM_IDIO_16 max_num_isa_dev(IDIO_16_EXTENT) + +static unsigned int base[MAX_NUM_IDIO_16]; +static unsigned int num_idio_16; +module_param_array(base, uint, &num_idio_16, 0); +MODULE_PARM_DESC(base, "ACCES 104-IDIO-16 base addresses"); + +static unsigned int irq[MAX_NUM_IDIO_16]; +module_param_array(irq, uint, NULL, 0); +MODULE_PARM_DESC(irq, "ACCES 104-IDIO-16 interrupt line numbers"); /** * struct idio_16_gpio - GPIO device private data structure @@ -185,23 +193,19 @@ static irqreturn_t idio_16_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int __init idio_16_probe(struct platform_device *pdev) +static int idio_16_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct idio_16_gpio *idio16gpio; - const unsigned base = idio_16_base; - const unsigned extent = 8; const char *const name = dev_name(dev); int err; - const unsigned irq = idio_16_irq; idio16gpio = devm_kzalloc(dev, sizeof(*idio16gpio), GFP_KERNEL); if (!idio16gpio) return -ENOMEM; - if (!devm_request_region(dev, base, extent, name)) { + if (!devm_request_region(dev, base[id], IDIO_16_EXTENT, name)) { dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", - base, base + extent); + base[id], base[id] + IDIO_16_EXTENT); return -EBUSY; } @@ -215,8 +219,8 @@ static int __init idio_16_probe(struct platform_device *pdev) idio16gpio->chip.direction_output = idio_16_gpio_direction_output; idio16gpio->chip.get = idio_16_gpio_get; idio16gpio->chip.set = idio_16_gpio_set; - idio16gpio->base = base; - idio16gpio->irq = irq; + idio16gpio->base = base[id]; + idio16gpio->irq = irq[id]; idio16gpio->out_state = 0xFFFF; spin_lock_init(&idio16gpio->lock); @@ -230,8 +234,8 @@ static int __init idio_16_probe(struct platform_device *pdev) } /* Disable IRQ by default */ - outb(0, base + 2); - outb(0, base + 1); + outb(0, base[id] + 2); + outb(0, base[id] + 1); err = gpiochip_irqchip_add(&idio16gpio->chip, &idio_16_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); @@ -240,7 +244,7 @@ static int __init idio_16_probe(struct platform_device *pdev) goto err_gpiochip_remove; } - err = request_irq(irq, idio_16_irq_handler, 0, name, idio16gpio); + err = request_irq(irq[id], idio_16_irq_handler, 0, name, idio16gpio); if (err) { dev_err(dev, "IRQ handler registering failed (%d)\n", err); goto err_gpiochip_remove; @@ -253,9 +257,9 @@ err_gpiochip_remove: return err; } -static int idio_16_remove(struct platform_device *pdev) +static int idio_16_remove(struct device *dev, unsigned int id) { - struct idio_16_gpio *const idio16gpio = platform_get_drvdata(pdev); + struct idio_16_gpio *const idio16gpio = dev_get_drvdata(dev); free_irq(idio16gpio->irq, idio16gpio); gpiochip_remove(&idio16gpio->chip); @@ -263,48 +267,15 @@ static int idio_16_remove(struct platform_device *pdev) return 0; } -static struct platform_device *idio_16_device; - -static struct platform_driver idio_16_driver = { +static struct isa_driver idio_16_driver = { + .probe = idio_16_probe, .driver = { .name = "104-idio-16" }, .remove = idio_16_remove }; -static void __exit idio_16_exit(void) -{ - platform_device_unregister(idio_16_device); - platform_driver_unregister(&idio_16_driver); -} - -static int __init idio_16_init(void) -{ - int err; - - idio_16_device = platform_device_alloc(idio_16_driver.driver.name, -1); - if (!idio_16_device) - return -ENOMEM; - - err = platform_device_add(idio_16_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&idio_16_driver, idio_16_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(idio_16_device); -err_platform_device: - platform_device_put(idio_16_device); - return err; -} - -module_init(idio_16_init); -module_exit(idio_16_exit); +module_isa_driver(idio_16_driver, num_idio_16); MODULE_AUTHOR("William Breathitt Gray "); MODULE_DESCRIPTION("ACCES 104-IDIO-16 GPIO driver"); -- cgit v1.2.3 From cc736607c86d39ea078519af0de6ee0fbf3096a6 Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Sun, 1 May 2016 18:45:24 -0400 Subject: gpio: ws16c48: Utilize the ISA bus driver The WinSystems WS16C48 communicates via the ISA bus. As such, it is more appropriate to use the ISA bus driver over the platform driver to control the WinSystems WS16C48 GPIO driver. This patch also adds support for multiple devices via the base and irq module array parameters. Each element of the base array corresponds to a discrete device; each element of the irq array corresponds to the respective device addressed in the respective base array element. Acked-by: Linus Walleij Cc: Alexandre Courbot Signed-off-by: William Breathitt Gray Signed-off-by: Greg Kroah-Hartman --- drivers/gpio/Kconfig | 9 ++--- drivers/gpio/gpio-ws16c48.c | 88 +++++++++++++++------------------------------ 2 files changed, 33 insertions(+), 64 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index af3237cef81d..e5c122413843 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -614,12 +614,13 @@ config GPIO_TS5500 config GPIO_WS16C48 tristate "WinSystems WS16C48 GPIO support" + depends on ISA select GPIOLIB_IRQCHIP help - Enables GPIO support for the WinSystems WS16C48. The base port address - for the device may be configured via the ws16c48_base module - parameter. The interrupt line number for the device may be configured - via the ws16c48_irq module parameter. + Enables GPIO support for the WinSystems WS16C48. The base port + addresses for the devices may be configured via the base module + parameter. The interrupt line numbers for the devices may be + configured via the irq module parameter. endmenu diff --git a/drivers/gpio/gpio-ws16c48.c b/drivers/gpio/gpio-ws16c48.c index 51f41e8fd21e..eaa71d440ccf 100644 --- a/drivers/gpio/gpio-ws16c48.c +++ b/drivers/gpio/gpio-ws16c48.c @@ -19,18 +19,23 @@ #include #include #include +#include #include #include #include -#include #include -static unsigned ws16c48_base; -module_param(ws16c48_base, uint, 0); -MODULE_PARM_DESC(ws16c48_base, "WinSystems WS16C48 base address"); -static unsigned ws16c48_irq; -module_param(ws16c48_irq, uint, 0); -MODULE_PARM_DESC(ws16c48_irq, "WinSystems WS16C48 interrupt line number"); +#define WS16C48_EXTENT 16 +#define MAX_NUM_WS16C48 max_num_isa_dev(WS16C48_EXTENT) + +static unsigned int base[MAX_NUM_WS16C48]; +static unsigned int num_ws16c48; +module_param_array(base, uint, &num_ws16c48, 0); +MODULE_PARM_DESC(base, "WinSystems WS16C48 base addresses"); + +static unsigned int irq[MAX_NUM_WS16C48]; +module_param_array(irq, uint, NULL, 0); +MODULE_PARM_DESC(irq, "WinSystems WS16C48 interrupt line numbers"); /** * struct ws16c48_gpio - GPIO device private data structure @@ -298,23 +303,19 @@ static irqreturn_t ws16c48_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int __init ws16c48_probe(struct platform_device *pdev) +static int ws16c48_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct ws16c48_gpio *ws16c48gpio; - const unsigned base = ws16c48_base; - const unsigned extent = 16; const char *const name = dev_name(dev); int err; - const unsigned irq = ws16c48_irq; ws16c48gpio = devm_kzalloc(dev, sizeof(*ws16c48gpio), GFP_KERNEL); if (!ws16c48gpio) return -ENOMEM; - if (!devm_request_region(dev, base, extent, name)) { + if (!devm_request_region(dev, base[id], WS16C48_EXTENT, name)) { dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", - base, base + extent); + base[id], base[id] + WS16C48_EXTENT); return -EBUSY; } @@ -328,8 +329,8 @@ static int __init ws16c48_probe(struct platform_device *pdev) ws16c48gpio->chip.direction_output = ws16c48_gpio_direction_output; ws16c48gpio->chip.get = ws16c48_gpio_get; ws16c48gpio->chip.set = ws16c48_gpio_set; - ws16c48gpio->base = base; - ws16c48gpio->irq = irq; + ws16c48gpio->base = base[id]; + ws16c48gpio->irq = irq[id]; spin_lock_init(&ws16c48gpio->lock); @@ -342,11 +343,11 @@ static int __init ws16c48_probe(struct platform_device *pdev) } /* Disable IRQ by default */ - outb(0x80, base + 7); - outb(0, base + 8); - outb(0, base + 9); - outb(0, base + 10); - outb(0xC0, base + 7); + outb(0x80, base[id] + 7); + outb(0, base[id] + 8); + outb(0, base[id] + 9); + outb(0, base[id] + 10); + outb(0xC0, base[id] + 7); err = gpiochip_irqchip_add(&ws16c48gpio->chip, &ws16c48_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); @@ -355,7 +356,7 @@ static int __init ws16c48_probe(struct platform_device *pdev) goto err_gpiochip_remove; } - err = request_irq(irq, ws16c48_irq_handler, IRQF_SHARED, name, + err = request_irq(irq[id], ws16c48_irq_handler, IRQF_SHARED, name, ws16c48gpio); if (err) { dev_err(dev, "IRQ handler registering failed (%d)\n", err); @@ -369,9 +370,9 @@ err_gpiochip_remove: return err; } -static int ws16c48_remove(struct platform_device *pdev) +static int ws16c48_remove(struct device *dev, unsigned int id) { - struct ws16c48_gpio *const ws16c48gpio = platform_get_drvdata(pdev); + struct ws16c48_gpio *const ws16c48gpio = dev_get_drvdata(dev); free_irq(ws16c48gpio->irq, ws16c48gpio); gpiochip_remove(&ws16c48gpio->chip); @@ -379,48 +380,15 @@ static int ws16c48_remove(struct platform_device *pdev) return 0; } -static struct platform_device *ws16c48_device; - -static struct platform_driver ws16c48_driver = { +static struct isa_driver ws16c48_driver = { + .probe = ws16c48_probe, .driver = { .name = "ws16c48" }, .remove = ws16c48_remove }; -static void __exit ws16c48_exit(void) -{ - platform_device_unregister(ws16c48_device); - platform_driver_unregister(&ws16c48_driver); -} - -static int __init ws16c48_init(void) -{ - int err; - - ws16c48_device = platform_device_alloc(ws16c48_driver.driver.name, -1); - if (!ws16c48_device) - return -ENOMEM; - - err = platform_device_add(ws16c48_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&ws16c48_driver, ws16c48_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(ws16c48_device); -err_platform_device: - platform_device_put(ws16c48_device); - return err; -} - -module_init(ws16c48_init); -module_exit(ws16c48_exit); +module_isa_driver(ws16c48_driver, num_ws16c48); MODULE_AUTHOR("William Breathitt Gray "); MODULE_DESCRIPTION("WinSystems WS16C48 GPIO driver"); -- cgit v1.2.3 From c6e360a0d9d282e9c8688dcdabdc3669912b66ef Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 3 May 2016 08:06:06 -0700 Subject: Revert "base: dd: don't remove driver_data in -EPROBE_DEFER case" This reverts commit ded9db380d34344ee5263002570b9b8b705f7830. Thierry Reding writes: This causes a boot regression on at least one board, caused by one of the drivers looking at driver data to check whether or not the driver has properly loaded. If the code encounters a non-NULL pointer it tries to dereference it, but because it's already been freed there is no memory backing it and things crash. I don't think keeping stale pointers around is a good idea. The whole point of setting this to NULL in the core is so that probe failures result in the same starting conditions no matter what. Can we please get this reverted? Reported-by: Thierry Reding Cc: Yi Zhang Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index dae6cefa6060..16688f50729c 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -402,8 +402,7 @@ pinctrl_bind_failed: devres_release_all(dev); driver_sysfs_remove(dev); dev->driver = NULL; - if (ret != -EPROBE_DEFER) - dev_set_drvdata(dev, NULL); + dev_set_drvdata(dev, NULL); if (dev->pm_domain && dev->pm_domain->dismiss) dev->pm_domain->dismiss(dev); pm_runtime_reinit(dev); -- cgit v1.2.3