From 2e7189b6c774adc6b848808286b14d0825dc9d1a Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 12 Jul 2018 09:33:17 +0200 Subject: driver core: bus: convert to use BUS_ATTR_WO and RW We are trying to get rid of BUS_ATTR() and the usage of that in bus.c can be trivially converted to use BUS_ATTR_WO and RW, so use those macros instead. Cc: "Rafael J. Wysocki" Signed-off-by: Greg Kroah-Hartman --- drivers/base/bus.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/base/bus.c b/drivers/base/bus.c index e06a57936cc9..a56e865cdbfa 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -236,12 +236,12 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf, } static DRIVER_ATTR_IGNORE_LOCKDEP(bind, S_IWUSR, NULL, bind_store); -static ssize_t show_drivers_autoprobe(struct bus_type *bus, char *buf) +static ssize_t drivers_autoprobe_show(struct bus_type *bus, char *buf) { return sprintf(buf, "%d\n", bus->p->drivers_autoprobe); } -static ssize_t store_drivers_autoprobe(struct bus_type *bus, +static ssize_t drivers_autoprobe_store(struct bus_type *bus, const char *buf, size_t count) { if (buf[0] == '0') @@ -251,7 +251,7 @@ static ssize_t store_drivers_autoprobe(struct bus_type *bus, return count; } -static ssize_t store_drivers_probe(struct bus_type *bus, +static ssize_t drivers_probe_store(struct bus_type *bus, const char *buf, size_t count) { struct device *dev; @@ -586,9 +586,8 @@ static void remove_bind_files(struct device_driver *drv) driver_remove_file(drv, &driver_attr_unbind); } -static BUS_ATTR(drivers_probe, S_IWUSR, NULL, store_drivers_probe); -static BUS_ATTR(drivers_autoprobe, S_IWUSR | S_IRUGO, - show_drivers_autoprobe, store_drivers_autoprobe); +static BUS_ATTR_WO(drivers_probe); +static BUS_ATTR_RW(drivers_autoprobe); static int add_probe_files(struct bus_type *bus) { -- cgit v1.2.3 From a4723041857eaa35f189d237da769c4c63235544 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 29 Oct 2018 16:31:26 +0100 Subject: driver core: drop use of BUS_ATTR() We are trying to get rid of BUS_ATTR() so drop the last user of it from the tree. We had to "open code" it in order to prevent a function name conflict due to the use of DEVICE_ATTR_WO() earlier in the file :( Cc: "Rafael J. Wysocki" Signed-off-by: Greg Kroah-Hartman --- drivers/base/bus.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/base/bus.c b/drivers/base/bus.c index a56e865cdbfa..3ab45843da39 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -837,7 +837,14 @@ static ssize_t bus_uevent_store(struct bus_type *bus, rc = kobject_synth_uevent(&bus->p->subsys.kobj, buf, count); return rc ? rc : count; } -static BUS_ATTR(uevent, S_IWUSR, NULL, bus_uevent_store); +/* + * "open code" the old BUS_ATTR() macro here. We want to use BUS_ATTR_WO() + * here, but can not use it as earlier in the file we have + * DEVICE_ATTR_WO(uevent), which would cause a clash with the with the store + * function name. + */ +static struct bus_attribute bus_attr_uevent = __ATTR(uevent, S_IWUSR, NULL, + bus_uevent_store); /** * bus_register - register a driver-core subsystem -- cgit v1.2.3 From 4bd4e92cfe6d2af77938c2a8ac6635c363dc0ac2 Mon Sep 17 00:00:00 2001 From: Stephen Martin Date: Thu, 20 Dec 2018 13:50:28 -0800 Subject: sysfs: fix blank line coding style warning Fixed a coding style issue. Signed-off-by: Stephen Martin Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 51398457fe00..52d9235e0291 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -497,6 +497,7 @@ bool sysfs_remove_file_self(struct kobject *kobj, const struct attribute *attr) void sysfs_remove_files(struct kobject *kobj, const struct attribute * const *ptr) { int i; + for (i = 0; ptr[i]; i++) sysfs_remove_file(kobj, ptr[i]); } -- cgit v1.2.3 From 570d0200123fb4f809aa2f6226e93a458d664d70 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Fri, 18 Jan 2019 10:34:59 +0800 Subject: driver core: move device->knode_class to device_private As the description of struct device_private says, it stores data which is private to driver core. And it already has similar fields like: knode_parent, knode_driver, knode_driver and knode_bus. This look it is more proper to put knode_class together with those fields to make it private to driver core. This patch move device->knode_class to device_private to make it comply with code convention. Signed-off-by: Wei Yang Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 4 ++++ drivers/base/class.c | 14 ++++++++++---- drivers/base/core.c | 4 ++-- include/linux/device.h | 1 - 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index 7a419a7a6235..37329a668935 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -60,6 +60,7 @@ struct driver_private { * @knode_parent - node in sibling list * @knode_driver - node in driver list * @knode_bus - node in bus list + * @knode_class - node in class list * @deferred_probe - entry in deferred_probe_list which is used to retry the * binding of drivers which were unable to get all the resources needed by * the device; typically because it depends on another driver getting @@ -74,6 +75,7 @@ struct device_private { struct klist_node knode_parent; struct klist_node knode_driver; struct klist_node knode_bus; + struct klist_node knode_class; struct list_head deferred_probe; struct device *device; }; @@ -83,6 +85,8 @@ struct device_private { container_of(obj, struct device_private, knode_driver) #define to_device_private_bus(obj) \ container_of(obj, struct device_private, knode_bus) +#define to_device_private_class(obj) \ + container_of(obj, struct device_private, knode_class) /* initialisation functions */ extern int devices_init(void); diff --git a/drivers/base/class.c b/drivers/base/class.c index 54def4e02f00..d8a6a5864c2e 100644 --- a/drivers/base/class.c +++ b/drivers/base/class.c @@ -117,16 +117,22 @@ static void class_put(struct class *cls) kset_put(&cls->p->subsys); } +static struct device *klist_class_to_dev(struct klist_node *n) +{ + struct device_private *p = to_device_private_class(n); + return p->device; +} + static void klist_class_dev_get(struct klist_node *n) { - struct device *dev = container_of(n, struct device, knode_class); + struct device *dev = klist_class_to_dev(n); get_device(dev); } static void klist_class_dev_put(struct klist_node *n) { - struct device *dev = container_of(n, struct device, knode_class); + struct device *dev = klist_class_to_dev(n); put_device(dev); } @@ -277,7 +283,7 @@ void class_dev_iter_init(struct class_dev_iter *iter, struct class *class, struct klist_node *start_knode = NULL; if (start) - start_knode = &start->knode_class; + start_knode = &start->p->knode_class; klist_iter_init_node(&class->p->klist_devices, &iter->ki, start_knode); iter->type = type; } @@ -304,7 +310,7 @@ struct device *class_dev_iter_next(struct class_dev_iter *iter) knode = klist_next(&iter->ki); if (!knode) return NULL; - dev = container_of(knode, struct device, knode_class); + dev = klist_class_to_dev(knode); if (!iter->type || iter->type == dev->type) return dev; } diff --git a/drivers/base/core.c b/drivers/base/core.c index 0073b09bb99f..4a4b6f8cbc4f 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1966,7 +1966,7 @@ int device_add(struct device *dev) if (dev->class) { mutex_lock(&dev->class->p->mutex); /* tie the class to the device */ - klist_add_tail(&dev->knode_class, + klist_add_tail(&dev->p->knode_class, &dev->class->p->klist_devices); /* notify any interfaces that the device is here */ @@ -2105,7 +2105,7 @@ void device_del(struct device *dev) if (class_intf->remove_dev) class_intf->remove_dev(dev, class_intf); /* remove the device from the class list */ - klist_del(&dev->knode_class); + klist_del(&dev->p->knode_class); mutex_unlock(&dev->class->p->mutex); } device_remove_file(dev, &dev_attr_uevent); diff --git a/include/linux/device.h b/include/linux/device.h index 6cb4640b6160..d0e452fd0bff 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1035,7 +1035,6 @@ struct device { spinlock_t devres_lock; struct list_head devres_head; - struct klist_node knode_class; struct class *class; const struct attribute_group **groups; /* optional groups */ -- cgit v1.2.3 From 8092e79204e7884f4bee3584ecfe6cf4a124d129 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Thu, 20 Dec 2018 23:28:37 -0800 Subject: ihex: Share code between ihex_validate_fw() and ihex_next_binrec() Convert both ihex_validate_fw() and ihex_next_binrec() to use a helper function to calculate next record offest. This way we only have one place implementing next record offset calculation logic. No functional change intended. Cc: Chris Healy Cc: Kyle McMartin Cc: Andrew Morton Cc: Masahiro Yamada Cc: David Woodhouse Cc: Greg Kroah-Hartman Cc: linux-kernel Signed-off-by: Andrey Smirnov Signed-off-by: Greg Kroah-Hartman --- include/linux/ihex.h | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/include/linux/ihex.h b/include/linux/ihex.h index 75c194391869..9c701521176b 100644 --- a/include/linux/ihex.h +++ b/include/linux/ihex.h @@ -23,29 +23,34 @@ struct ihex_binrec { /* Find the next record, taking into account the 4-byte alignment */ static inline const struct ihex_binrec * -ihex_next_binrec(const struct ihex_binrec *rec) +__ihex_next_binrec(const struct ihex_binrec *rec) { int next = ((be16_to_cpu(rec->len) + 5) & ~3) - 2; rec = (void *)&rec->data[next]; + return rec; +} + +static inline const struct ihex_binrec * +ihex_next_binrec(const struct ihex_binrec *rec) +{ + rec = __ihex_next_binrec(rec); + return be16_to_cpu(rec->len) ? rec : NULL; } /* Check that ihex_next_binrec() won't take us off the end of the image... */ static inline int ihex_validate_fw(const struct firmware *fw) { - const struct ihex_binrec *rec; - size_t ofs = 0; + const struct ihex_binrec *end, *rec; - while (ofs <= fw->size - sizeof(*rec)) { - rec = (void *)&fw->data[ofs]; + rec = (const void *)fw->data; + end = (const void *)&fw->data[fw->size - sizeof(*end)]; + for (; rec <= end; rec = __ihex_next_binrec(rec)) { /* Zero length marks end of records */ if (!be16_to_cpu(rec->len)) return 0; - - /* Point to next record... */ - ofs += (sizeof(*rec) + be16_to_cpu(rec->len) + 3) & ~3; } return -EINVAL; } -- cgit v1.2.3 From 5158c36ec9d0b3343f58987cec7ebfd866331fd0 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Thu, 20 Dec 2018 23:28:38 -0800 Subject: ihex: Check if zero-length record is at the end of the blob When verifying the validity of IHEX file we need to make sure that zero-length record we found is located at the end of the file. Not doing that could result in an invalid file with a bogus zero-length in the middle short-circuiting the check and being reported as valid. Cc: Chris Healy Cc: Kyle McMartin Cc: Andrew Morton Cc: Masahiro Yamada Cc: David Woodhouse Cc: Greg Kroah-Hartman Cc: linux-kernel Signed-off-by: Andrey Smirnov Signed-off-by: Greg Kroah-Hartman --- include/linux/ihex.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/ihex.h b/include/linux/ihex.h index 9c701521176b..9130f307a420 100644 --- a/include/linux/ihex.h +++ b/include/linux/ihex.h @@ -49,7 +49,7 @@ static inline int ihex_validate_fw(const struct firmware *fw) for (; rec <= end; rec = __ihex_next_binrec(rec)) { /* Zero length marks end of records */ - if (!be16_to_cpu(rec->len)) + if (rec == end && !be16_to_cpu(rec->len)) return 0; } return -EINVAL; -- cgit v1.2.3 From 9fb4ab4d3dd665a62da9c176a89e7c7feaf5d9e4 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Thu, 20 Dec 2018 23:28:39 -0800 Subject: ihex: Simplify next record offset calculation Next record calucaltion can be reduced to a much more tivial ALIGN operation as follows: 1. Splitting 5 into 2 + 3 we get next = ((be16_to_cpu(rec->len) + 2 + 3) & ~3) - 2 (1) 2. Using ALIGN macro we reduce (1) to: ALIGN(be16_to_cpu(rec->len) + 2, 4) - 2 (2) 3. Subsituting 'next' in original next record calucation we get: (void *)&rec->data[ALIGN(be16_to_cpu(rec->len) + 2, 4) - 2] (3) 4. Converting array index to pointer arithmetic we convert (3) into: (void *)rec + sizeof(*rec) + ALIGN(be16_to_cpu(rec->len) + 2, 4) - 2 (4) 5. Subsituting sizeof(*rec) with its value, 6, and substracting 2, in (4) we get: (void *)rec + ALIGN(be16_to_cpu(rec->len) + 2, 4) + 4 (5) 6. Since ALIGN(X, 4) + 4 == ALIGN(X + 4, 4), (5) can be converted to: (void *)rec + ALIGN(be16_to_cpu(rec->len) + 6, 4) (6) 5. Subsituting 6 in (6) to sizeof(*rec) we get: (void *)rec + ALIGN(be16_to_cpu(rec->len) + sizeof(*rec), 4) (7) Using expression (7) should make it more clear that next record is located by adding full size of the current record (payload + auxiliary data) aligned to 4 bytes, to the location of the current one. No functional change intended. Cc: Chris Healy Cc: Kyle McMartin Cc: Andrew Morton Cc: Masahiro Yamada Cc: David Woodhouse Cc: Greg Kroah-Hartman Cc: linux-kernel Signed-off-by: Andrey Smirnov Signed-off-by: Greg Kroah-Hartman --- include/linux/ihex.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/include/linux/ihex.h b/include/linux/ihex.h index 9130f307a420..98cb5ce0b0a0 100644 --- a/include/linux/ihex.h +++ b/include/linux/ihex.h @@ -21,14 +21,18 @@ struct ihex_binrec { uint8_t data[0]; } __attribute__((packed)); +static inline uint16_t ihex_binrec_size(const struct ihex_binrec *p) +{ + return be16_to_cpu(p->len) + sizeof(*p); +} + /* Find the next record, taking into account the 4-byte alignment */ static inline const struct ihex_binrec * __ihex_next_binrec(const struct ihex_binrec *rec) { - int next = ((be16_to_cpu(rec->len) + 5) & ~3) - 2; - rec = (void *)&rec->data[next]; + const void *p = rec; - return rec; + return p + ALIGN(ihex_binrec_size(rec), 4); } static inline const struct ihex_binrec * -- cgit v1.2.3 From 2ef8179bb7a6817de3fc9407ab55aa357f2d1e4d Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Thu, 20 Dec 2018 23:28:40 -0800 Subject: tools/firmware/ihex2fw: Simplify next record offset calculation We can convert original expression for 'writelen" to use ALIGN as follows: (p->len + 9) & ~3 => (p->len + 6 + 3) & ~3 => ALIGN(p->len + 6, 4) Now, subsituting "p->len + 6" with "p->len + sizeof(p->addr) + sizeof(p->len)" we end up with the same expression as used by kernel couterpart in linux/ihex.h: ALIGN(p->len + sizeof(p->addr) + sizeof(p->len), 4) That is a full size of the record, aligned to 4 bytes. No functional change intended. Cc: Chris Healy Cc: Kyle McMartin Cc: Andrew Morton Cc: Masahiro Yamada Cc: David Woodhouse Cc: Greg Kroah-Hartman Cc: linux-kernel Signed-off-by: Andrey Smirnov Signed-off-by: Greg Kroah-Hartman --- tools/firmware/ihex2fw.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tools/firmware/ihex2fw.c b/tools/firmware/ihex2fw.c index b58dd061e978..e081cef730d8 100644 --- a/tools/firmware/ihex2fw.c +++ b/tools/firmware/ihex2fw.c @@ -24,6 +24,10 @@ #include +#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) + struct ihex_binrec { struct ihex_binrec *next; /* not part of the real data structure */ uint32_t addr; @@ -259,13 +263,18 @@ static void file_record(struct ihex_binrec *record) *p = record; } +static uint16_t ihex_binrec_size(struct ihex_binrec *p) +{ + return p->len + sizeof(p->addr) + sizeof(p->len); +} + static int output_records(int outfd) { unsigned char zeroes[6] = {0, 0, 0, 0, 0, 0}; struct ihex_binrec *p = records; while (p) { - uint16_t writelen = (p->len + 9) & ~3; + uint16_t writelen = ALIGN(ihex_binrec_size(p), 4); p->addr = htonl(p->addr); p->len = htons(p->len); -- cgit v1.2.3 From 925f8d4aad5ca1bf18987a3cdcb0e176bddddcf1 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Thu, 20 Dec 2018 23:28:41 -0800 Subject: tools/firmware/ihex2fw: Replace explicit alignment with ALIGN (X + 3) & ~3 is the same as ALIGN(X, 4), so replace all of the instances of the formwer in the code with the latter. While at it, introduce a helper variable 'record_size' to avoid duplicating length calculatin code. No functional change intended. Cc: Chris Healy Cc: Kyle McMartin Cc: Andrew Morton Cc: Masahiro Yamada Cc: David Woodhouse Cc: Greg Kroah-Hartman Cc: linux-kernel Signed-off-by: Andrey Smirnov Signed-off-by: Greg Kroah-Hartman --- tools/firmware/ihex2fw.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/firmware/ihex2fw.c b/tools/firmware/ihex2fw.c index e081cef730d8..8925b60e51f5 100644 --- a/tools/firmware/ihex2fw.c +++ b/tools/firmware/ihex2fw.c @@ -135,6 +135,7 @@ int main(int argc, char **argv) static int process_ihex(uint8_t *data, ssize_t size) { struct ihex_binrec *record; + size_t record_size; uint32_t offset = 0; uint32_t data32; uint8_t type, crc = 0, crcbyte = 0; @@ -161,12 +162,13 @@ next_record: len <<= 8; len += hex(data + i, &crc); i += 2; } - record = malloc((sizeof (*record) + len + 3) & ~3); + record_size = ALIGN(sizeof(*record) + len, 4); + record = malloc(record_size); if (!record) { fprintf(stderr, "out of memory for records\n"); return -ENOMEM; } - memset(record, 0, (sizeof(*record) + len + 3) & ~3); + memset(record, 0, record_size); record->len = len; /* now check if we have enough data to read everything */ -- cgit v1.2.3 From 91f382a46822c7a2913e6fabb2b882cc0f23144a Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 11 Jan 2019 18:51:32 +0900 Subject: firmware_loader: move CONFIG_FW_LOADER_USER_HELPER switch to Makefile The whole code of fallback_table.c is surrounded by #ifdef of CONFIG_FW_LOADER_USER_HELPER. Move the CONFIG_FW_LOADER_USER_HELPER switch to Makefile so that it is not compiled at all when this CONFIG option is disabled. I also removed the confusing comment, "Module or buit-in [sic]". CONFIG_FW_LOADER_USER_HELPER is a boolean option. (If it were a module, CONFIG_FW_LOADER_USER_HELPER_MODULE would be defined instead.) Signed-off-by: Masahiro Yamada Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/Makefile | 2 +- drivers/base/firmware_loader/fallback_table.c | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile index a97eeb0be1d8..5c7f422f7528 100644 --- a/drivers/base/firmware_loader/Makefile +++ b/drivers/base/firmware_loader/Makefile @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 # Makefile for the Linux firmware loader -obj-y := fallback_table.o +obj-$(CONFIG_FW_LOADER_USER_HELPER) += fallback_table.o obj-$(CONFIG_FW_LOADER) += firmware_class.o firmware_class-objs := main.o firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o diff --git a/drivers/base/firmware_loader/fallback_table.c b/drivers/base/firmware_loader/fallback_table.c index 7428659d8df9..776dd69cf5be 100644 --- a/drivers/base/firmware_loader/fallback_table.c +++ b/drivers/base/firmware_loader/fallback_table.c @@ -16,9 +16,6 @@ * firmware fallback configuration table */ -/* Module or buit-in */ -#ifdef CONFIG_FW_LOADER_USER_HELPER - static unsigned int zero; static unsigned int one = 1; @@ -51,5 +48,3 @@ struct ctl_table firmware_config_table[] = { { } }; EXPORT_SYMBOL_GPL(firmware_config_table); - -#endif -- cgit v1.2.3 From f96182e959a41e35df0adae9ae09a49ff8a618a8 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 11 Jan 2019 18:52:00 +0900 Subject: firmware_loader: move firmware/ to drivers/base/firmware_loader/builtin/ Currently, the 'firmware' directory only contains a single Makefile to embed extra firmware into the kernel. Move it to the more relevant place. Signed-off-by: Masahiro Yamada Signed-off-by: Greg Kroah-Hartman --- Makefile | 2 +- drivers/base/firmware_loader/Makefile | 2 ++ drivers/base/firmware_loader/builtin/.gitignore | 1 + drivers/base/firmware_loader/builtin/Makefile | 40 +++++++++++++++++++++++++ firmware/.gitignore | 1 - firmware/Makefile | 40 ------------------------- 6 files changed, 44 insertions(+), 42 deletions(-) create mode 100644 drivers/base/firmware_loader/builtin/.gitignore create mode 100644 drivers/base/firmware_loader/builtin/Makefile delete mode 100644 firmware/.gitignore delete mode 100644 firmware/Makefile diff --git a/Makefile b/Makefile index 499b96810995..a9b9369b4bdd 100644 --- a/Makefile +++ b/Makefile @@ -583,7 +583,7 @@ export KBUILD_MODULES KBUILD_BUILTIN ifeq ($(KBUILD_EXTMOD),) # Objects we will link into vmlinux / subdirs we need to visit init-y := init/ -drivers-y := drivers/ sound/ firmware/ +drivers-y := drivers/ sound/ net-y := net/ libs-y := lib/ core-y := usr/ diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile index 5c7f422f7528..0b2dfa6259c9 100644 --- a/drivers/base/firmware_loader/Makefile +++ b/drivers/base/firmware_loader/Makefile @@ -5,3 +5,5 @@ obj-$(CONFIG_FW_LOADER_USER_HELPER) += fallback_table.o obj-$(CONFIG_FW_LOADER) += firmware_class.o firmware_class-objs := main.o firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o + +obj-y += builtin/ diff --git a/drivers/base/firmware_loader/builtin/.gitignore b/drivers/base/firmware_loader/builtin/.gitignore new file mode 100644 index 000000000000..9c8bdb9fdcc3 --- /dev/null +++ b/drivers/base/firmware_loader/builtin/.gitignore @@ -0,0 +1 @@ +*.gen.S diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile new file mode 100644 index 000000000000..37e5ae387400 --- /dev/null +++ b/drivers/base/firmware_loader/builtin/Makefile @@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0 + +# Create $(fwdir) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't have a +# leading /, it's relative to $(srctree). +fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR)) +fwdir := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir)) + +obj-y := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE))) + +FWNAME = $(patsubst $(obj)/%.gen.S,%,$@) +FWSTR = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME)))) +ASM_WORD = $(if $(CONFIG_64BIT),.quad,.long) +ASM_ALIGN = $(if $(CONFIG_64BIT),3,2) +PROGBITS = $(if $(CONFIG_ARM),%,@)progbits + +filechk_fwbin = \ + echo "/* Generated by $(src)/Makefile */" ;\ + echo " .section .rodata" ;\ + echo " .p2align $(ASM_ALIGN)" ;\ + echo "_fw_$(FWSTR)_bin:" ;\ + echo " .incbin \"$(fwdir)/$(FWNAME)\"" ;\ + echo "_fw_end:" ;\ + echo " .section .rodata.str,\"aMS\",$(PROGBITS),1" ;\ + echo " .p2align $(ASM_ALIGN)" ;\ + echo "_fw_$(FWSTR)_name:" ;\ + echo " .string \"$(FWNAME)\"" ;\ + echo " .section .builtin_fw,\"a\",$(PROGBITS)" ;\ + echo " .p2align $(ASM_ALIGN)" ;\ + echo " $(ASM_WORD) _fw_$(FWSTR)_name" ;\ + echo " $(ASM_WORD) _fw_$(FWSTR)_bin" ;\ + echo " $(ASM_WORD) _fw_end - _fw_$(FWSTR)_bin" + +$(obj)/%.gen.S: FORCE + $(call filechk,fwbin) + +# The .o files depend on the binaries directly; the .S files don't. +$(addprefix $(obj)/, $(obj-y)): $(obj)/%.gen.o: $(fwdir)/% + +targets := $(patsubst $(obj)/%,%, \ + $(shell find $(obj) -name \*.gen.S 2>/dev/null)) diff --git a/firmware/.gitignore b/firmware/.gitignore deleted file mode 100644 index 9c8bdb9fdcc3..000000000000 --- a/firmware/.gitignore +++ /dev/null @@ -1 +0,0 @@ -*.gen.S diff --git a/firmware/Makefile b/firmware/Makefile deleted file mode 100644 index 37e5ae387400..000000000000 --- a/firmware/Makefile +++ /dev/null @@ -1,40 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0 - -# Create $(fwdir) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't have a -# leading /, it's relative to $(srctree). -fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR)) -fwdir := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir)) - -obj-y := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE))) - -FWNAME = $(patsubst $(obj)/%.gen.S,%,$@) -FWSTR = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME)))) -ASM_WORD = $(if $(CONFIG_64BIT),.quad,.long) -ASM_ALIGN = $(if $(CONFIG_64BIT),3,2) -PROGBITS = $(if $(CONFIG_ARM),%,@)progbits - -filechk_fwbin = \ - echo "/* Generated by $(src)/Makefile */" ;\ - echo " .section .rodata" ;\ - echo " .p2align $(ASM_ALIGN)" ;\ - echo "_fw_$(FWSTR)_bin:" ;\ - echo " .incbin \"$(fwdir)/$(FWNAME)\"" ;\ - echo "_fw_end:" ;\ - echo " .section .rodata.str,\"aMS\",$(PROGBITS),1" ;\ - echo " .p2align $(ASM_ALIGN)" ;\ - echo "_fw_$(FWSTR)_name:" ;\ - echo " .string \"$(FWNAME)\"" ;\ - echo " .section .builtin_fw,\"a\",$(PROGBITS)" ;\ - echo " .p2align $(ASM_ALIGN)" ;\ - echo " $(ASM_WORD) _fw_$(FWSTR)_name" ;\ - echo " $(ASM_WORD) _fw_$(FWSTR)_bin" ;\ - echo " $(ASM_WORD) _fw_end - _fw_$(FWSTR)_bin" - -$(obj)/%.gen.S: FORCE - $(call filechk,fwbin) - -# The .o files depend on the binaries directly; the .S files don't. -$(addprefix $(obj)/, $(obj-y)): $(obj)/%.gen.o: $(fwdir)/% - -targets := $(patsubst $(obj)/%,%, \ - $(shell find $(obj) -name \*.gen.S 2>/dev/null)) -- cgit v1.2.3 From 0eeb27311f3a06b39ed51027260fb46c8b04357c Mon Sep 17 00:00:00 2001 From: Sergey Senozhatsky Date: Sun, 30 Dec 2018 12:46:52 +0900 Subject: debugfs: debugfs_use_start/finish do not exist anymore debugfs_use_file_start() and debugfs_use_file_finish() do not exist since commit c9afbec27089 ("debugfs: purge obsolete SRCU based removal protection"); tweak debugfs_create_file_unsafe() comment. Signed-off-by: Sergey Senozhatsky Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 13b01351dd1c..4354069101b4 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -422,8 +422,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_file); * 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 + * methods against file removals by means of debugfs_file_get() + * and debugfs_file_put(). ->open() is still protected by * debugfs though. * * Any struct file_operations defined by means of -- cgit v1.2.3 From 21acc07d33a968a74111a98c8221069dcabf5d95 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 4 Jan 2019 14:26:18 +0100 Subject: f2fs: no need to check return value of debugfs_create functions When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Jaegeuk Kim Reviewed-by: Chao Yu Cc: linux-f2fs-devel@lists.sourceforge.net Signed-off-by: Greg Kroah-Hartman --- fs/f2fs/debug.c | 20 +++----------------- fs/f2fs/f2fs.h | 4 ++-- fs/f2fs/super.c | 5 +---- 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c index ebcc121920ba..fd7f170e2f2d 100644 --- a/fs/f2fs/debug.c +++ b/fs/f2fs/debug.c @@ -506,30 +506,16 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi) kvfree(si); } -int __init f2fs_create_root_stats(void) +void __init f2fs_create_root_stats(void) { - struct dentry *file; - f2fs_debugfs_root = debugfs_create_dir("f2fs", NULL); - if (!f2fs_debugfs_root) - return -ENOMEM; - file = debugfs_create_file("status", S_IRUGO, f2fs_debugfs_root, - NULL, &stat_fops); - if (!file) { - debugfs_remove(f2fs_debugfs_root); - f2fs_debugfs_root = NULL; - return -ENOMEM; - } - - return 0; + debugfs_create_file("status", S_IRUGO, f2fs_debugfs_root, NULL, + &stat_fops); } void f2fs_destroy_root_stats(void) { - if (!f2fs_debugfs_root) - return; - debugfs_remove_recursive(f2fs_debugfs_root); f2fs_debugfs_root = NULL; } diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 12fabd6735dd..8f23ee6e8eb9 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3328,7 +3328,7 @@ static inline struct f2fs_stat_info *F2FS_STAT(struct f2fs_sb_info *sbi) int f2fs_build_stats(struct f2fs_sb_info *sbi); void f2fs_destroy_stats(struct f2fs_sb_info *sbi); -int __init f2fs_create_root_stats(void); +void __init f2fs_create_root_stats(void); void f2fs_destroy_root_stats(void); #else #define stat_inc_cp_count(si) do { } while (0) @@ -3366,7 +3366,7 @@ void f2fs_destroy_root_stats(void); static inline int f2fs_build_stats(struct f2fs_sb_info *sbi) { return 0; } static inline void f2fs_destroy_stats(struct f2fs_sb_info *sbi) { } -static inline int __init f2fs_create_root_stats(void) { return 0; } +static inline void __init f2fs_create_root_stats(void) { } static inline void f2fs_destroy_root_stats(void) { } #endif diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index c46a1d4318d4..3d3ce9eb6d13 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -3545,9 +3545,7 @@ static int __init init_f2fs_fs(void) err = register_filesystem(&f2fs_fs_type); if (err) goto free_shrinker; - err = f2fs_create_root_stats(); - if (err) - goto free_filesystem; + f2fs_create_root_stats(); err = f2fs_init_post_read_processing(); if (err) goto free_root_stats; @@ -3555,7 +3553,6 @@ static int __init init_f2fs_fs(void) free_root_stats: f2fs_destroy_root_stats(); -free_filesystem: unregister_filesystem(&f2fs_fs_type); free_shrinker: unregister_shrinker(&f2fs_shrinker_info); -- cgit v1.2.3 From d61dfafc30b44dd55d886cda54543dd37cea3e13 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 21 Dec 2018 08:54:33 +0100 Subject: PCI: pci.c: convert to use BUS_ATTR_RW We are trying to get rid of BUS_ATTR() and the usage of that in pci.c can be trivially converted to use BUS_ATTR_RW(), so use that instead. Cc: Bjorn Helgaas Signed-off-by: Greg Kroah-Hartman --- drivers/pci/pci.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index c9d8e3c837de..fda84538de79 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -6034,19 +6034,18 @@ static ssize_t pci_get_resource_alignment_param(char *buf, size_t size) return count; } -static ssize_t pci_resource_alignment_show(struct bus_type *bus, char *buf) +static ssize_t resource_alignment_show(struct bus_type *bus, char *buf) { return pci_get_resource_alignment_param(buf, PAGE_SIZE); } -static ssize_t pci_resource_alignment_store(struct bus_type *bus, +static ssize_t resource_alignment_store(struct bus_type *bus, const char *buf, size_t count) { return pci_set_resource_alignment_param(buf, count); } -static BUS_ATTR(resource_alignment, 0644, pci_resource_alignment_show, - pci_resource_alignment_store); +static BUS_ATTR_RW(resource_alignment); static int __init pci_resource_alignment_sysfs_init(void) { -- cgit v1.2.3 From 1094f6d06703bd489cea444f9ba03bd4ef123acc Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 21 Dec 2018 08:54:34 +0100 Subject: PCI: pci-sysfs.c: convert to use BUS_ATTR_WO We are trying to get rid of BUS_ATTR() and the usage of that in pci-sysfs.c can be trivially converted to use BUS_ATTR_WO(), so use that instead. Cc: Bjorn Helgaas Signed-off-by: Greg Kroah-Hartman --- drivers/pci/pci-sysfs.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 9ecfe13157c0..25794c27c7a4 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -412,8 +412,7 @@ static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RW(msi_bus); -static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf, - size_t count) +static ssize_t rescan_store(struct bus_type *bus, const char *buf, size_t count) { unsigned long val; struct pci_bus *b = NULL; @@ -429,7 +428,7 @@ static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf, } return count; } -static BUS_ATTR(rescan, (S_IWUSR|S_IWGRP), NULL, bus_rescan_store); +static BUS_ATTR_WO(rescan); static struct attribute *pci_bus_attrs[] = { &bus_attr_rescan.attr, -- cgit v1.2.3 From c1507ea8349c04681b9b288bc81ed786199ee181 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 21 Dec 2018 08:54:36 +0100 Subject: pseries: ibmebus.c: convert to use BUS_ATTR_WO We are trying to get rid of BUS_ATTR() and the usage of that in ibmebus.c can be trivially converted to use BUS_ATTR_WO(), so use that instead. Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Tyrel Datwyler Acked-by: Michael Ellerman (powerpc) Signed-off-by: Greg Kroah-Hartman --- arch/powerpc/platforms/pseries/ibmebus.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/platforms/pseries/ibmebus.c b/arch/powerpc/platforms/pseries/ibmebus.c index 5b4a56131904..84e8ec4011ba 100644 --- a/arch/powerpc/platforms/pseries/ibmebus.c +++ b/arch/powerpc/platforms/pseries/ibmebus.c @@ -261,8 +261,7 @@ static char *ibmebus_chomp(const char *in, size_t count) return out; } -static ssize_t ibmebus_store_probe(struct bus_type *bus, - const char *buf, size_t count) +static ssize_t probe_store(struct bus_type *bus, const char *buf, size_t count) { struct device_node *dn = NULL; struct device *dev; @@ -298,10 +297,9 @@ out: return rc; return count; } -static BUS_ATTR(probe, 0200, NULL, ibmebus_store_probe); +static BUS_ATTR_WO(probe); -static ssize_t ibmebus_store_remove(struct bus_type *bus, - const char *buf, size_t count) +static ssize_t remove_store(struct bus_type *bus, const char *buf, size_t count) { struct device *dev; char *path; @@ -325,7 +323,7 @@ static ssize_t ibmebus_store_remove(struct bus_type *bus, return -ENODEV; } } -static BUS_ATTR(remove, 0200, NULL, ibmebus_store_remove); +static BUS_ATTR_WO(remove); static struct attribute *ibmbus_bus_attrs[] = { &bus_attr_probe.attr, -- cgit v1.2.3 From c9fbe769d0f220f6b766e38c323ff52e680bc9e2 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 21 Dec 2018 08:54:37 +0100 Subject: rapidio: rio-sysfs.c: convert to use BUS_ATTR_WO We are trying to get rid of BUS_ATTR() and the usage of that in rio-sysfs.c can be trivially converted to use BUS_ATTR_WO(), so use that instead. Cc: Matt Porter Cc: Alexandre Bounine Signed-off-by: Greg Kroah-Hartman --- drivers/rapidio/rio-sysfs.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/rapidio/rio-sysfs.c b/drivers/rapidio/rio-sysfs.c index 73e4b407f162..ad5e303dda05 100644 --- a/drivers/rapidio/rio-sysfs.c +++ b/drivers/rapidio/rio-sysfs.c @@ -290,8 +290,7 @@ const struct attribute_group *rio_dev_groups[] = { NULL, }; -static ssize_t bus_scan_store(struct bus_type *bus, const char *buf, - size_t count) +static ssize_t scan_store(struct bus_type *bus, const char *buf, size_t count) { long val; int rc; @@ -314,7 +313,7 @@ exit: return rc; } -static BUS_ATTR(scan, (S_IWUSR|S_IWGRP), NULL, bus_scan_store); +static BUS_ATTR_WO(scan); static struct attribute *rio_bus_attrs[] = { &bus_attr_scan.attr, -- cgit v1.2.3 From 7e9586bab2caf182eb93980a6bbe7bcbc00ddd53 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 21 Dec 2018 08:54:38 +0100 Subject: block: rbd: convert to use BUS_ATTR_WO and RO We are trying to get rid of BUS_ATTR() and the usage of that in rbd.c can be trivially converted to use BUS_ATTR_WO and RO, so use those macros instead. Cc: Sage Weil Cc: Alex Elder Cc: Jens Axboe Acked-by: Ilya Dryomov Signed-off-by: Greg Kroah-Hartman --- drivers/block/rbd.c | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 1e92b61d0bd5..282e2e82d849 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -428,14 +428,13 @@ static bool single_major = true; module_param(single_major, bool, 0444); MODULE_PARM_DESC(single_major, "Use a single major number for all rbd devices (default: true)"); -static ssize_t rbd_add(struct bus_type *bus, const char *buf, - size_t count); -static ssize_t rbd_remove(struct bus_type *bus, const char *buf, - size_t count); -static ssize_t rbd_add_single_major(struct bus_type *bus, const char *buf, - size_t count); -static ssize_t rbd_remove_single_major(struct bus_type *bus, const char *buf, - size_t count); +static ssize_t add_store(struct bus_type *bus, const char *buf, size_t count); +static ssize_t remove_store(struct bus_type *bus, const char *buf, + size_t count); +static ssize_t add_single_major_store(struct bus_type *bus, const char *buf, + size_t count); +static ssize_t remove_single_major_store(struct bus_type *bus, const char *buf, + size_t count); static int rbd_dev_image_probe(struct rbd_device *rbd_dev, int depth); static int rbd_dev_id_to_minor(int dev_id) @@ -464,16 +463,16 @@ static bool rbd_is_lock_owner(struct rbd_device *rbd_dev) return is_lock_owner; } -static ssize_t rbd_supported_features_show(struct bus_type *bus, char *buf) +static ssize_t supported_features_show(struct bus_type *bus, char *buf) { return sprintf(buf, "0x%llx\n", RBD_FEATURES_SUPPORTED); } -static BUS_ATTR(add, 0200, NULL, rbd_add); -static BUS_ATTR(remove, 0200, NULL, rbd_remove); -static BUS_ATTR(add_single_major, 0200, NULL, rbd_add_single_major); -static BUS_ATTR(remove_single_major, 0200, NULL, rbd_remove_single_major); -static BUS_ATTR(supported_features, 0444, rbd_supported_features_show, NULL); +static BUS_ATTR_WO(add); +static BUS_ATTR_WO(remove); +static BUS_ATTR_WO(add_single_major); +static BUS_ATTR_WO(remove_single_major); +static BUS_ATTR_RO(supported_features); static struct attribute *rbd_bus_attrs[] = { &bus_attr_add.attr, @@ -5934,9 +5933,7 @@ err_out_args: goto out; } -static ssize_t rbd_add(struct bus_type *bus, - const char *buf, - size_t count) +static ssize_t add_store(struct bus_type *bus, const char *buf, size_t count) { if (single_major) return -EINVAL; @@ -5944,9 +5941,8 @@ static ssize_t rbd_add(struct bus_type *bus, return do_rbd_add(bus, buf, count); } -static ssize_t rbd_add_single_major(struct bus_type *bus, - const char *buf, - size_t count) +static ssize_t add_single_major_store(struct bus_type *bus, const char *buf, + size_t count) { return do_rbd_add(bus, buf, count); } @@ -6049,9 +6045,7 @@ static ssize_t do_rbd_remove(struct bus_type *bus, return count; } -static ssize_t rbd_remove(struct bus_type *bus, - const char *buf, - size_t count) +static ssize_t remove_store(struct bus_type *bus, const char *buf, size_t count) { if (single_major) return -EINVAL; @@ -6059,9 +6053,8 @@ static ssize_t rbd_remove(struct bus_type *bus, return do_rbd_remove(bus, buf, count); } -static ssize_t rbd_remove_single_major(struct bus_type *bus, - const char *buf, - size_t count) +static ssize_t remove_single_major_store(struct bus_type *bus, const char *buf, + size_t count) { return do_rbd_remove(bus, buf, count); } -- cgit v1.2.3 From 7ab35a14de257d5fdac9c7e44c4a1a69967f9fa2 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 8 Jan 2019 23:36:14 -0800 Subject: kobject: make kset_get_ownership() 'static' kset_get_ownership() is only used in lib/kobject.c, so make it 'static'. Signed-off-by: Eric Biggers Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kobject.c b/lib/kobject.c index b72e00fd7d09..aa89edcd2b63 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -887,7 +887,7 @@ static void kset_release(struct kobject *kobj) kfree(kset); } -void kset_get_ownership(struct kobject *kobj, kuid_t *uid, kgid_t *gid) +static void kset_get_ownership(struct kobject *kobj, kuid_t *uid, kgid_t *gid) { if (kobj->parent) kobject_get_ownership(kobj->parent, uid, gid); -- cgit v1.2.3 From b3fa29ad83770c764909d8497be3866ad341a3a6 Mon Sep 17 00:00:00 2001 From: Bo YU Date: Wed, 9 Jan 2019 04:17:00 -0500 Subject: kobject: to repalce printk with pr_* style Repalce printk with pr_warn in kobject_synth_uevent and replace printk with pr_err in uevent_net_init to make both consistent with other code in kobject_uevent.c Signed-off-by: Bo YU Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- lib/kobject_uevent.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c index 27c6118afd1c..c87a96c4800e 100644 --- a/lib/kobject_uevent.c +++ b/lib/kobject_uevent.c @@ -224,7 +224,7 @@ int kobject_synth_uevent(struct kobject *kobj, const char *buf, size_t count) out: if (r) { devpath = kobject_get_path(kobj, GFP_KERNEL); - printk(KERN_WARNING "synth uevent: %s: %s", + pr_warn("synth uevent: %s: %s", devpath ?: "unknown device", msg ?: "failed to send uevent"); kfree(devpath); @@ -765,8 +765,7 @@ static int uevent_net_init(struct net *net) ue_sk->sk = netlink_kernel_create(net, NETLINK_KOBJECT_UEVENT, &cfg); if (!ue_sk->sk) { - printk(KERN_ERR - "kobject_uevent: unable to create netlink socket!\n"); + pr_err("kobject_uevent: unable to create netlink socket!\n"); kfree(ue_sk); return -ENODEV; } -- cgit v1.2.3 From 549ad24374c0f796a0d3676a8962596915921c68 Mon Sep 17 00:00:00 2001 From: Bo YU Date: Wed, 9 Jan 2019 04:17:48 -0500 Subject: kobject: drop newline from msg string There is currently a missing terminating newline in non-switch case match when msg == NULL Signed-off-by: Bo YU Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- lib/kobject_uevent.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c index c87a96c4800e..f05802687ba4 100644 --- a/lib/kobject_uevent.c +++ b/lib/kobject_uevent.c @@ -200,7 +200,7 @@ int kobject_synth_uevent(struct kobject *kobj, const char *buf, size_t count) r = kobject_action_type(buf, count, &action, &action_args); if (r) { - msg = "unknown uevent action string\n"; + msg = "unknown uevent action string"; goto out; } @@ -212,7 +212,7 @@ int kobject_synth_uevent(struct kobject *kobj, const char *buf, size_t count) r = kobject_action_args(action_args, count - (action_args - buf), &env); if (r == -EINVAL) { - msg = "incorrect uevent action arguments\n"; + msg = "incorrect uevent action arguments"; goto out; } @@ -224,7 +224,7 @@ int kobject_synth_uevent(struct kobject *kobj, const char *buf, size_t count) out: if (r) { devpath = kobject_get_path(kobj, GFP_KERNEL); - pr_warn("synth uevent: %s: %s", + pr_warn("synth uevent: %s: %s\n", devpath ?: "unknown device", msg ?: "failed to send uevent"); kfree(devpath); -- cgit v1.2.3 From 8a4b32691120b8fd6b029f5ad37d742a97ec82c1 Mon Sep 17 00:00:00 2001 From: Jerome Brunet Date: Fri, 21 Dec 2018 17:23:41 +0100 Subject: driver core: silence device link messages unless debugging On platforms making a fair use of regulators, the dev_info() messages coming from the device link function are a bit too verbose. The amount of message will increase further with the clock framework joining the device link party. These messages looks valuable for people debugging device link related issues, so dev_dbg() looks more appropriate than dev_info(). Signed-off-by: Jerome Brunet Acked-by: Kevin Hilman Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 4a4b6f8cbc4f..4e57e1632f87 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -302,7 +302,7 @@ struct device_link *device_link_add(struct device *consumer, list_add_tail_rcu(&link->s_node, &supplier->links.consumers); list_add_tail_rcu(&link->c_node, &consumer->links.suppliers); - dev_info(consumer, "Linked as a consumer to %s\n", dev_name(supplier)); + dev_dbg(consumer, "Linked as a consumer to %s\n", dev_name(supplier)); out: device_pm_unlock(); @@ -328,8 +328,8 @@ static void __device_link_del(struct kref *kref) { struct device_link *link = container_of(kref, struct device_link, kref); - dev_info(link->consumer, "Dropping the link to %s\n", - dev_name(link->supplier)); + dev_dbg(link->consumer, "Dropping the link to %s\n", + dev_name(link->supplier)); if (link->flags & DL_FLAG_PM_RUNTIME) pm_runtime_drop_link(link->consumer); -- cgit v1.2.3 From 0fe6f7874d467456da6f6a221dd92499a3ab1780 Mon Sep 17 00:00:00 2001 From: Yong Wu Date: Tue, 1 Jan 2019 12:51:05 +0800 Subject: driver core: Remove the link if there is no driver with AUTO flag DL_FLAG_AUTOREMOVE_CONSUMER/SUPPLIER means "Remove the link automatically on consumer/supplier driver unbind", that means we should remove whole the device_link when there is no this driver no matter what the ref_count of the link is. CC: Greg Kroah-Hartman Signed-off-by: Yong Wu Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 4e57e1632f87..26f8cd6dd0b3 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -512,7 +512,7 @@ static void __device_links_no_driver(struct device *dev) continue; if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) - kref_put(&link->kref, __device_link_del); + __device_link_del(&link->kref); else if (link->status != DL_STATE_SUPPLIER_UNBIND) WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } @@ -557,7 +557,7 @@ void device_links_driver_cleanup(struct device *dev) */ if (link->status == DL_STATE_SUPPLIER_UNBIND && link->flags & DL_FLAG_AUTOREMOVE_SUPPLIER) - kref_put(&link->kref, __device_link_del); + __device_link_del(&link->kref); WRITE_ONCE(link->status, DL_STATE_DORMANT); } -- cgit v1.2.3 From 3451a495ef244a88ed6317a035299d835554d579 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:10 -0800 Subject: driver core: Establish order of operations for device_add and device_del via bitflag Add an additional bit flag to the device_private struct named "dead". This additional flag provides a guarantee that when a device_del is executed on a given interface an async worker will not attempt to attach the driver following the earlier device_del call. Previously this guarantee was not present and could result in the device_del call attempting to remove a driver from an interface only to have the async worker attempt to probe the driver later when it finally completes the asynchronous probe call. One additional change added was that I pulled the check for dev->driver out of the __device_attach_driver call and instead placed it in the __device_attach_async_helper call. This was motivated by the fact that the only other caller of this, __device_attach, had already taken the device_lock() and checked for dev->driver. Instead of testing for this twice in this path it makes more sense to just consolidate the dev->dead and dev->driver checks together into one set of checks. Reviewed-by: Dan Williams Reviewed-by: Rafael J. Wysocki Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 4 ++++ drivers/base/core.c | 11 +++++++++++ drivers/base/dd.c | 22 +++++++++++----------- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index 37329a668935..7ca475af8953 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -67,6 +67,9 @@ struct driver_private { * probed first. * @device - pointer back to the struct device that this structure is * associated with. + * @dead - This device is currently either in the process of or has been + * removed from the system. Any asynchronous events scheduled for this + * device should exit without taking any action. * * Nothing outside of the driver core should ever touch these fields. */ @@ -78,6 +81,7 @@ struct device_private { struct klist_node knode_class; struct list_head deferred_probe; struct device *device; + u8 dead:1; }; #define to_device_private_parent(obj) \ container_of(obj, struct device_private, knode_parent) diff --git a/drivers/base/core.c b/drivers/base/core.c index 26f8cd6dd0b3..1febfd2a353a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2080,6 +2080,17 @@ void device_del(struct device *dev) struct kobject *glue_dir = NULL; struct class_interface *class_intf; + /* + * Hold the device lock and set the "dead" flag to guarantee that + * the update behavior is consistent with the other bitfields near + * it and that we cannot have an asynchronous probe routine trying + * to run while we are tearing out the bus/class/sysfs from + * underneath the device. + */ + device_lock(dev); + dev->p->dead = true; + device_unlock(dev); + /* Notify clients of device removal. This call must come * before dpm_sysfs_remove(). */ diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 8ac10af17c00..636cd16b1b62 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -731,15 +731,6 @@ static int __device_attach_driver(struct device_driver *drv, void *_data) bool async_allowed; int ret; - /* - * Check if device has already been claimed. This may - * happen with driver loading, device discovery/registration, - * and deferred probe processing happens all at once with - * multiple threads. - */ - if (dev->driver) - return -EBUSY; - ret = driver_match_device(drv, dev); if (ret == 0) { /* no match */ @@ -774,6 +765,15 @@ static void __device_attach_async_helper(void *_dev, async_cookie_t cookie) device_lock(dev); + /* + * Check if device has already been removed or claimed. This may + * happen with driver loading, device discovery/registration, + * and deferred probe processing happens all at once with + * multiple threads. + */ + if (dev->p->dead || dev->driver) + goto out_unlock; + if (dev->parent) pm_runtime_get_sync(dev->parent); @@ -784,7 +784,7 @@ static void __device_attach_async_helper(void *_dev, async_cookie_t cookie) if (dev->parent) pm_runtime_put(dev->parent); - +out_unlock: device_unlock(dev); put_device(dev); @@ -897,7 +897,7 @@ static int __driver_attach(struct device *dev, void *data) if (dev->parent && dev->bus->need_parent_lock) device_lock(dev->parent); device_lock(dev); - if (!dev->driver) + if (!dev->p->dead && !dev->driver) driver_probe_device(drv, dev); device_unlock(dev); if (dev->parent && dev->bus->need_parent_lock) -- cgit v1.2.3 From ed88747c6c4a2fc2f961a36d4c50cb0868c30229 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:16 -0800 Subject: device core: Consolidate locking and unlocking of parent and device Try to consolidate all of the locking and unlocking of both the parent and device when attaching or removing a driver from a given device. To do that I first consolidated the lock pattern into two functions __device_driver_lock and __device_driver_unlock. After doing that I then created functions specific to attaching and detaching the driver while acquiring these locks. By doing this I was able to reduce the number of spots where we touch need_parent_lock from 12 down to 4. This patch should produce no functional changes, it is meant to be a code clean-up/consolidation only. Reviewed-by: Luis Chamberlain Reviewed-by: Bart Van Assche Reviewed-by: Dan Williams Reviewed-by: Rafael J. Wysocki Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 2 ++ drivers/base/bus.c | 23 +++---------- drivers/base/dd.c | 95 ++++++++++++++++++++++++++++++++++++++++++----------- 3 files changed, 81 insertions(+), 39 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index 7ca475af8953..b6966511b0ca 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -132,6 +132,8 @@ extern int driver_add_groups(struct device_driver *drv, const struct attribute_group **groups); extern void driver_remove_groups(struct device_driver *drv, const struct attribute_group **groups); +int device_driver_attach(struct device_driver *drv, struct device *dev); +void device_driver_detach(struct device *dev); extern char *make_class_name(const char *name, struct kobject *kobj); diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 3ab45843da39..cfa5b63b5582 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -187,11 +187,7 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == drv) { - if (dev->parent && dev->bus->need_parent_lock) - device_lock(dev->parent); - device_release_driver(dev); - if (dev->parent && dev->bus->need_parent_lock) - device_unlock(dev->parent); + device_driver_detach(dev); err = count; } put_device(dev); @@ -214,13 +210,7 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf, dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == NULL && driver_match_device(drv, dev)) { - if (dev->parent && bus->need_parent_lock) - device_lock(dev->parent); - device_lock(dev); - err = driver_probe_device(drv, dev); - device_unlock(dev); - if (dev->parent && bus->need_parent_lock) - device_unlock(dev->parent); + err = device_driver_attach(drv, dev); if (err > 0) { /* success */ @@ -773,13 +763,8 @@ EXPORT_SYMBOL_GPL(bus_rescan_devices); */ int device_reprobe(struct device *dev) { - if (dev->driver) { - if (dev->parent && dev->bus->need_parent_lock) - device_lock(dev->parent); - device_release_driver(dev); - if (dev->parent && dev->bus->need_parent_lock) - device_unlock(dev->parent); - } + if (dev->driver) + device_driver_detach(dev); return bus_rescan_devices_helper(dev, NULL); } EXPORT_SYMBOL_GPL(device_reprobe); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 636cd16b1b62..34556359c7da 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -867,6 +867,64 @@ void device_initial_probe(struct device *dev) __device_attach(dev, true); } +/* + * __device_driver_lock - acquire locks needed to manipulate dev->drv + * @dev: Device we will update driver info for + * @parent: Parent device. Needed if the bus requires parent lock + * + * This function will take the required locks for manipulating dev->drv. + * Normally this will just be the @dev lock, but when called for a USB + * interface, @parent lock will be held as well. + */ +static void __device_driver_lock(struct device *dev, struct device *parent) +{ + if (parent && dev->bus->need_parent_lock) + device_lock(parent); + device_lock(dev); +} + +/* + * __device_driver_unlock - release locks needed to manipulate dev->drv + * @dev: Device we will update driver info for + * @parent: Parent device. Needed if the bus requires parent lock + * + * This function will release the required locks for manipulating dev->drv. + * Normally this will just be the the @dev lock, but when called for a + * USB interface, @parent lock will be released as well. + */ +static void __device_driver_unlock(struct device *dev, struct device *parent) +{ + device_unlock(dev); + if (parent && dev->bus->need_parent_lock) + device_unlock(parent); +} + +/** + * device_driver_attach - attach a specific driver to a specific device + * @drv: Driver to attach + * @dev: Device to attach it to + * + * Manually attach driver to a device. Will acquire both @dev lock and + * @dev->parent lock if needed. + */ +int device_driver_attach(struct device_driver *drv, struct device *dev) +{ + int ret = 0; + + __device_driver_lock(dev, dev->parent); + + /* + * If device has been removed or someone has already successfully + * bound a driver before us just skip the driver probe call. + */ + if (!dev->p->dead && !dev->driver) + ret = driver_probe_device(drv, dev); + + __device_driver_unlock(dev, dev->parent); + + return ret; +} + static int __driver_attach(struct device *dev, void *data) { struct device_driver *drv = data; @@ -894,14 +952,7 @@ static int __driver_attach(struct device *dev, void *data) return ret; } /* ret > 0 means positive match */ - if (dev->parent && dev->bus->need_parent_lock) - device_lock(dev->parent); - device_lock(dev); - if (!dev->p->dead && !dev->driver) - driver_probe_device(drv, dev); - device_unlock(dev); - if (dev->parent && dev->bus->need_parent_lock) - device_unlock(dev->parent); + device_driver_attach(drv, dev); return 0; } @@ -932,15 +983,11 @@ static void __device_release_driver(struct device *dev, struct device *parent) drv = dev->driver; if (drv) { while (device_links_busy(dev)) { - device_unlock(dev); - if (parent && dev->bus->need_parent_lock) - device_unlock(parent); + __device_driver_unlock(dev, parent); device_links_unbind_consumers(dev); - if (parent && dev->bus->need_parent_lock) - device_lock(parent); - device_lock(dev); + __device_driver_lock(dev, parent); /* * A concurrent invocation of the same function might * have released the driver successfully while this one @@ -993,16 +1040,12 @@ void device_release_driver_internal(struct device *dev, struct device_driver *drv, struct device *parent) { - if (parent && dev->bus->need_parent_lock) - device_lock(parent); + __device_driver_lock(dev, parent); - device_lock(dev); if (!drv || drv == dev->driver) __device_release_driver(dev, parent); - device_unlock(dev); - if (parent && dev->bus->need_parent_lock) - device_unlock(parent); + __device_driver_unlock(dev, parent); } /** @@ -1027,6 +1070,18 @@ void device_release_driver(struct device *dev) } EXPORT_SYMBOL_GPL(device_release_driver); +/** + * device_driver_detach - detach driver from a specific device + * @dev: device to detach driver from + * + * Detach driver from device. Will acquire both @dev lock and @dev->parent + * lock if needed. + */ +void device_driver_detach(struct device *dev) +{ + device_release_driver_internal(dev, NULL, dev->parent); +} + /** * driver_detach - detach driver from all devices it controls. * @drv: driver. -- cgit v1.2.3 From ef0ff68351be4fd83bec2d797f0efdc0174a55a4 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:21 -0800 Subject: driver core: Probe devices asynchronously instead of the driver Probe devices asynchronously instead of the driver. This results in us seeing the same behavior if the device is registered before the driver or after. This way we can avoid serializing the initialization should the driver not be loaded until after the devices have already been added. The motivation behind this is that if we have a set of devices that take a significant amount of time to load we can greatly reduce the time to load by processing them in parallel instead of one at a time. In addition, each device can exist on a different node so placing a single thread on one CPU to initialize all of the devices for a given driver can result in poor performance on a system with multiple nodes. This approach can reduce the time needed to scan SCSI LUNs significantly. The only way to realize that speedup is by enabling more concurrency which is what is achieved with this patch. To achieve this it was necessary to add a new member "async_driver" to the device_private structure to store the driver pointer while we wait on the deferred probe call. Reviewed-by: Bart Van Assche Reviewed-by: Dan Williams Signed-off-by: Alexander Duyck Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 2 ++ drivers/base/bus.c | 23 +++-------------------- drivers/base/dd.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 20 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index b6966511b0ca..b405436ee28e 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -65,6 +65,7 @@ struct driver_private { * binding of drivers which were unable to get all the resources needed by * the device; typically because it depends on another driver getting * probed first. + * @async_driver - pointer to device driver awaiting probe via async_probe * @device - pointer back to the struct device that this structure is * associated with. * @dead - This device is currently either in the process of or has been @@ -80,6 +81,7 @@ struct device_private { struct klist_node knode_bus; struct klist_node knode_class; struct list_head deferred_probe; + struct device_driver *async_driver; struct device *device; u8 dead:1; }; diff --git a/drivers/base/bus.c b/drivers/base/bus.c index cfa5b63b5582..0a58e969f8b7 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -610,17 +610,6 @@ static ssize_t uevent_store(struct device_driver *drv, const char *buf, } static DRIVER_ATTR_WO(uevent); -static void driver_attach_async(void *_drv, async_cookie_t cookie) -{ - struct device_driver *drv = _drv; - int ret; - - ret = driver_attach(drv); - - pr_debug("bus: '%s': driver %s async attach completed: %d\n", - drv->bus->name, drv->name, ret); -} - /** * bus_add_driver - Add a driver to the bus. * @drv: driver. @@ -653,15 +642,9 @@ int bus_add_driver(struct device_driver *drv) klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers); if (drv->bus->p->drivers_autoprobe) { - if (driver_allows_async_probing(drv)) { - pr_debug("bus: '%s': probing driver %s asynchronously\n", - drv->bus->name, drv->name); - async_schedule(driver_attach_async, drv); - } else { - error = driver_attach(drv); - if (error) - goto out_unregister; - } + error = driver_attach(drv); + if (error) + goto out_unregister; } module_add_driver(drv->owner, drv); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 34556359c7da..627ad05064e0 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -925,6 +925,30 @@ int device_driver_attach(struct device_driver *drv, struct device *dev) return ret; } +static void __driver_attach_async_helper(void *_dev, async_cookie_t cookie) +{ + struct device *dev = _dev; + struct device_driver *drv; + int ret = 0; + + __device_driver_lock(dev, dev->parent); + + drv = dev->p->async_driver; + + /* + * If device has been removed or someone has already successfully + * bound a driver before us just skip the driver probe call. + */ + if (!dev->p->dead && !dev->driver) + ret = driver_probe_device(drv, dev); + + __device_driver_unlock(dev, dev->parent); + + dev_dbg(dev, "driver %s async attach completed: %d\n", drv->name, ret); + + put_device(dev); +} + static int __driver_attach(struct device *dev, void *data) { struct device_driver *drv = data; @@ -952,6 +976,25 @@ static int __driver_attach(struct device *dev, void *data) return ret; } /* ret > 0 means positive match */ + if (driver_allows_async_probing(drv)) { + /* + * Instead of probing the device synchronously we will + * probe it asynchronously to allow for more parallelism. + * + * We only take the device lock here in order to guarantee + * that the dev->driver and async_driver fields are protected + */ + dev_dbg(dev, "probing driver %s asynchronously\n", drv->name); + device_lock(dev); + if (!dev->driver) { + get_device(dev); + dev->p->async_driver = drv; + async_schedule(__driver_attach_async_helper, dev); + } + device_unlock(dev); + return 0; + } + device_driver_attach(drv, dev); return 0; -- cgit v1.2.3 From 8204e0c1113d6b7f599bcd7ebfbfde72e76c102f Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:26 -0800 Subject: workqueue: Provide queue_work_node to queue work near a given NUMA node Provide a new function, queue_work_node, which is meant to schedule work on a "random" CPU of the requested NUMA node. The main motivation for this is to help assist asynchronous init to better improve boot times for devices that are local to a specific node. For now we just default to the first CPU that is in the intersection of the cpumask of the node and the online cpumask. The only exception is if the CPU is local to the node we will just use the current CPU. This should work for our purposes as we are currently only using this for unbound work so the CPU will be translated to a node anyway instead of being directly used. As we are only using the first CPU to represent the NUMA node for now I am limiting the scope of the function so that it can only be used with unbound workqueues. Acked-by: Tejun Heo Reviewed-by: Bart Van Assche Acked-by: Dan Williams Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- include/linux/workqueue.h | 2 ++ kernel/workqueue.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 60d673e15632..1f50c1e586e7 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -463,6 +463,8 @@ int workqueue_set_unbound_cpumask(cpumask_var_t cpumask); extern bool queue_work_on(int cpu, struct workqueue_struct *wq, struct work_struct *work); +extern bool queue_work_node(int node, struct workqueue_struct *wq, + struct work_struct *work); extern bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq, struct delayed_work *work, unsigned long delay); extern bool mod_delayed_work_on(int cpu, struct workqueue_struct *wq, diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 392be4b252f6..d5a26e456f7a 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -1492,6 +1492,90 @@ bool queue_work_on(int cpu, struct workqueue_struct *wq, } EXPORT_SYMBOL(queue_work_on); +/** + * workqueue_select_cpu_near - Select a CPU based on NUMA node + * @node: NUMA node ID that we want to select a CPU from + * + * This function will attempt to find a "random" cpu available on a given + * node. If there are no CPUs available on the given node it will return + * WORK_CPU_UNBOUND indicating that we should just schedule to any + * available CPU if we need to schedule this work. + */ +static int workqueue_select_cpu_near(int node) +{ + int cpu; + + /* No point in doing this if NUMA isn't enabled for workqueues */ + if (!wq_numa_enabled) + return WORK_CPU_UNBOUND; + + /* Delay binding to CPU if node is not valid or online */ + if (node < 0 || node >= MAX_NUMNODES || !node_online(node)) + return WORK_CPU_UNBOUND; + + /* Use local node/cpu if we are already there */ + cpu = raw_smp_processor_id(); + if (node == cpu_to_node(cpu)) + return cpu; + + /* Use "random" otherwise know as "first" online CPU of node */ + cpu = cpumask_any_and(cpumask_of_node(node), cpu_online_mask); + + /* If CPU is valid return that, otherwise just defer */ + return cpu < nr_cpu_ids ? cpu : WORK_CPU_UNBOUND; +} + +/** + * queue_work_node - queue work on a "random" cpu for a given NUMA node + * @node: NUMA node that we are targeting the work for + * @wq: workqueue to use + * @work: work to queue + * + * We queue the work to a "random" CPU within a given NUMA node. The basic + * idea here is to provide a way to somehow associate work with a given + * NUMA node. + * + * This function will only make a best effort attempt at getting this onto + * the right NUMA node. If no node is requested or the requested node is + * offline then we just fall back to standard queue_work behavior. + * + * Currently the "random" CPU ends up being the first available CPU in the + * intersection of cpu_online_mask and the cpumask of the node, unless we + * are running on the node. In that case we just use the current CPU. + * + * Return: %false if @work was already on a queue, %true otherwise. + */ +bool queue_work_node(int node, struct workqueue_struct *wq, + struct work_struct *work) +{ + unsigned long flags; + bool ret = false; + + /* + * This current implementation is specific to unbound workqueues. + * Specifically we only return the first available CPU for a given + * node instead of cycling through individual CPUs within the node. + * + * If this is used with a per-cpu workqueue then the logic in + * workqueue_select_cpu_near would need to be updated to allow for + * some round robin type logic. + */ + WARN_ON_ONCE(!(wq->flags & WQ_UNBOUND)); + + local_irq_save(flags); + + if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))) { + int cpu = workqueue_select_cpu_near(node); + + __queue_work(cpu, wq, work); + ret = true; + } + + local_irq_restore(flags); + return ret; +} +EXPORT_SYMBOL_GPL(queue_work_node); + void delayed_work_timer_fn(struct timer_list *t) { struct delayed_work *dwork = from_timer(dwork, t, timer); -- cgit v1.2.3 From 6be9238e5cb64741ff95c3ae440b112753ad93de Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:31 -0800 Subject: async: Add support for queueing on specific NUMA node Introduce four new variants of the async_schedule_ functions that allow scheduling on a specific NUMA node. The first two functions are async_schedule_near and async_schedule_near_domain end up mapping to async_schedule and async_schedule_domain, but provide NUMA node specific functionality. They replace the original functions which were moved to inline function definitions that call the new functions while passing NUMA_NO_NODE. The second two functions are async_schedule_dev and async_schedule_dev_domain which provide NUMA specific functionality when passing a device as the data member and that device has a NUMA node other than NUMA_NO_NODE. The main motivation behind this is to address the need to be able to schedule device specific init work on specific NUMA nodes in order to improve performance of memory initialization. I have seen a significant improvement in initialziation time for persistent memory as a result of this approach. In the case of 3TB of memory on a single node the initialization time in the worst case went from 36s down to about 26s for a 10s improvement. As such the data shows a general benefit for affinitizing the async work to the node local to the device. Reviewed-by: Bart Van Assche Reviewed-by: Dan Williams Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- include/linux/async.h | 82 +++++++++++++++++++++++++++++++++++++++++++++++++-- kernel/async.c | 53 ++++++++++++++++++--------------- 2 files changed, 108 insertions(+), 27 deletions(-) diff --git a/include/linux/async.h b/include/linux/async.h index 6b0226bdaadc..f81d6dbffe68 100644 --- a/include/linux/async.h +++ b/include/linux/async.h @@ -14,6 +14,8 @@ #include #include +#include +#include typedef u64 async_cookie_t; typedef void (*async_func_t) (void *data, async_cookie_t cookie); @@ -37,9 +39,83 @@ struct async_domain { struct async_domain _name = { .pending = LIST_HEAD_INIT(_name.pending), \ .registered = 0 } -extern async_cookie_t async_schedule(async_func_t func, void *data); -extern async_cookie_t async_schedule_domain(async_func_t func, void *data, - struct async_domain *domain); +async_cookie_t async_schedule_node(async_func_t func, void *data, + int node); +async_cookie_t async_schedule_node_domain(async_func_t func, void *data, + int node, + struct async_domain *domain); + +/** + * async_schedule - schedule a function for asynchronous execution + * @func: function to execute asynchronously + * @data: data pointer to pass to the function + * + * Returns an async_cookie_t that may be used for checkpointing later. + * Note: This function may be called from atomic or non-atomic contexts. + */ +static inline async_cookie_t async_schedule(async_func_t func, void *data) +{ + return async_schedule_node(func, data, NUMA_NO_NODE); +} + +/** + * async_schedule_domain - schedule a function for asynchronous execution within a certain domain + * @func: function to execute asynchronously + * @data: data pointer to pass to the function + * @domain: the domain + * + * Returns an async_cookie_t that may be used for checkpointing later. + * @domain may be used in the async_synchronize_*_domain() functions to + * wait within a certain synchronization domain rather than globally. + * Note: This function may be called from atomic or non-atomic contexts. + */ +static inline async_cookie_t +async_schedule_domain(async_func_t func, void *data, + struct async_domain *domain) +{ + return async_schedule_node_domain(func, data, NUMA_NO_NODE, domain); +} + +/** + * async_schedule_dev - A device specific version of async_schedule + * @func: function to execute asynchronously + * @dev: device argument to be passed to function + * + * Returns an async_cookie_t that may be used for checkpointing later. + * @dev is used as both the argument for the function and to provide NUMA + * context for where to run the function. By doing this we can try to + * provide for the best possible outcome by operating on the device on the + * CPUs closest to the device. + * Note: This function may be called from atomic or non-atomic contexts. + */ +static inline async_cookie_t +async_schedule_dev(async_func_t func, struct device *dev) +{ + return async_schedule_node(func, dev, dev_to_node(dev)); +} + +/** + * async_schedule_dev_domain - A device specific version of async_schedule_domain + * @func: function to execute asynchronously + * @dev: device argument to be passed to function + * @domain: the domain + * + * Returns an async_cookie_t that may be used for checkpointing later. + * @dev is used as both the argument for the function and to provide NUMA + * context for where to run the function. By doing this we can try to + * provide for the best possible outcome by operating on the device on the + * CPUs closest to the device. + * @domain may be used in the async_synchronize_*_domain() functions to + * wait within a certain synchronization domain rather than globally. + * Note: This function may be called from atomic or non-atomic contexts. + */ +static inline async_cookie_t +async_schedule_dev_domain(async_func_t func, struct device *dev, + struct async_domain *domain) +{ + return async_schedule_node_domain(func, dev, dev_to_node(dev), domain); +} + void async_unregister_domain(struct async_domain *domain); extern void async_synchronize_full(void); extern void async_synchronize_full_domain(struct async_domain *domain); diff --git a/kernel/async.c b/kernel/async.c index a893d6170944..f6bd0d9885e1 100644 --- a/kernel/async.c +++ b/kernel/async.c @@ -149,7 +149,25 @@ static void async_run_entry_fn(struct work_struct *work) wake_up(&async_done); } -static async_cookie_t __async_schedule(async_func_t func, void *data, struct async_domain *domain) +/** + * async_schedule_node_domain - NUMA specific version of async_schedule_domain + * @func: function to execute asynchronously + * @data: data pointer to pass to the function + * @node: NUMA node that we want to schedule this on or close to + * @domain: the domain + * + * Returns an async_cookie_t that may be used for checkpointing later. + * @domain may be used in the async_synchronize_*_domain() functions to + * wait within a certain synchronization domain rather than globally. + * + * Note: This function may be called from atomic or non-atomic contexts. + * + * The node requested will be honored on a best effort basis. If the node + * has no CPUs associated with it then the work is distributed among all + * available CPUs. + */ +async_cookie_t async_schedule_node_domain(async_func_t func, void *data, + int node, struct async_domain *domain) { struct async_entry *entry; unsigned long flags; @@ -195,43 +213,30 @@ static async_cookie_t __async_schedule(async_func_t func, void *data, struct asy current->flags |= PF_USED_ASYNC; /* schedule for execution */ - queue_work(system_unbound_wq, &entry->work); + queue_work_node(node, system_unbound_wq, &entry->work); return newcookie; } +EXPORT_SYMBOL_GPL(async_schedule_node_domain); /** - * async_schedule - schedule a function for asynchronous execution + * async_schedule_node - NUMA specific version of async_schedule * @func: function to execute asynchronously * @data: data pointer to pass to the function + * @node: NUMA node that we want to schedule this on or close to * * Returns an async_cookie_t that may be used for checkpointing later. * Note: This function may be called from atomic or non-atomic contexts. - */ -async_cookie_t async_schedule(async_func_t func, void *data) -{ - return __async_schedule(func, data, &async_dfl_domain); -} -EXPORT_SYMBOL_GPL(async_schedule); - -/** - * async_schedule_domain - schedule a function for asynchronous execution within a certain domain - * @func: function to execute asynchronously - * @data: data pointer to pass to the function - * @domain: the domain * - * Returns an async_cookie_t that may be used for checkpointing later. - * @domain may be used in the async_synchronize_*_domain() functions to - * wait within a certain synchronization domain rather than globally. A - * synchronization domain is specified via @domain. Note: This function - * may be called from atomic or non-atomic contexts. + * The node requested will be honored on a best effort basis. If the node + * has no CPUs associated with it then the work is distributed among all + * available CPUs. */ -async_cookie_t async_schedule_domain(async_func_t func, void *data, - struct async_domain *domain) +async_cookie_t async_schedule_node(async_func_t func, void *data, int node) { - return __async_schedule(func, data, domain); + return async_schedule_node_domain(func, data, node, &async_dfl_domain); } -EXPORT_SYMBOL_GPL(async_schedule_domain); +EXPORT_SYMBOL_GPL(async_schedule_node); /** * async_synchronize_full - synchronize all asynchronous function calls -- cgit v1.2.3 From c37e20eaf4b21125898fd454f3ea6b212865d0a6 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:37 -0800 Subject: driver core: Attach devices on CPU local to device node Call the asynchronous probe routines on a CPU local to the device node. By doing this we should be able to improve our initialization time significantly as we can avoid having to access the device from a remote node which may introduce higher latency. For example, in the case of initializing memory for NVDIMM this can have a significant impact as initialing 3TB on remote node can take up to 39 seconds while initialing it on a local node only takes 23 seconds. It is situations like this where we will see the biggest improvement. Reviewed-by: Dan Williams Reviewed-by: Bart Van Assche Signed-off-by: Alexander Duyck Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 627ad05064e0..aa6a9c613595 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -829,7 +829,7 @@ static int __device_attach(struct device *dev, bool allow_async) */ dev_dbg(dev, "scheduling asynchronous probe\n"); get_device(dev); - async_schedule(__device_attach_async_helper, dev); + async_schedule_dev(__device_attach_async_helper, dev); } else { pm_request_idle(dev); } @@ -989,7 +989,7 @@ static int __driver_attach(struct device *dev, void *data) if (!dev->driver) { get_device(dev); dev->p->async_driver = drv; - async_schedule(__driver_attach_async_helper, dev); + async_schedule_dev(__driver_attach_async_helper, dev); } device_unlock(dev); return 0; -- cgit v1.2.3 From 8b9ec6b732775849f506aa6c2649e626e82a297c Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:42 -0800 Subject: PM core: Use new async_schedule_dev command Use the device specific version of the async_schedule commands to defer various tasks related to power management. By doing this we should see a slight improvement in performance as any device that is sensitive to latency/locality in the setup will now be initializing on the node closest to the device. Reviewed-by: Dan Williams Reviewed-by: Bart Van Assche Reviewed-by: Rafael J. Wysocki Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/base/power/main.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 0992e67e862b..93ddbcebcece 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -727,7 +727,7 @@ void dpm_noirq_resume_devices(pm_message_t state) reinit_completion(&dev->power.completion); if (is_async(dev)) { get_device(dev); - async_schedule(async_resume_noirq, dev); + async_schedule_dev(async_resume_noirq, dev); } } @@ -884,7 +884,7 @@ void dpm_resume_early(pm_message_t state) reinit_completion(&dev->power.completion); if (is_async(dev)) { get_device(dev); - async_schedule(async_resume_early, dev); + async_schedule_dev(async_resume_early, dev); } } @@ -1048,7 +1048,7 @@ void dpm_resume(pm_message_t state) reinit_completion(&dev->power.completion); if (is_async(dev)) { get_device(dev); - async_schedule(async_resume, dev); + async_schedule_dev(async_resume, dev); } } @@ -1368,7 +1368,7 @@ static int device_suspend_noirq(struct device *dev) if (is_async(dev)) { get_device(dev); - async_schedule(async_suspend_noirq, dev); + async_schedule_dev(async_suspend_noirq, dev); return 0; } return __device_suspend_noirq(dev, pm_transition, false); @@ -1571,7 +1571,7 @@ static int device_suspend_late(struct device *dev) if (is_async(dev)) { get_device(dev); - async_schedule(async_suspend_late, dev); + async_schedule_dev(async_suspend_late, dev); return 0; } @@ -1835,7 +1835,7 @@ static int device_suspend(struct device *dev) if (is_async(dev)) { get_device(dev); - async_schedule(async_suspend, dev); + async_schedule_dev(async_suspend, dev); return 0; } -- cgit v1.2.3 From af87b9a7863c7bb47f8bd015c0ce4a37d70c5225 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:47 -0800 Subject: libnvdimm: Schedule device registration on node local to the device Force the device registration for nvdimm devices to be closer to the actual device. This is achieved by using either the NUMA node ID of the region, or of the parent. By doing this we can have everything above the region based on the region, and everything below the region based on the nvdimm bus. By guaranteeing NUMA locality I see an improvement of as high as 25% for per-node init of a system with 12TB of persistent memory. Reviewed-by: Bart Van Assche Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/nvdimm/bus.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c index dca5f7a805cb..7bbff0af29b2 100644 --- a/drivers/nvdimm/bus.c +++ b/drivers/nvdimm/bus.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -534,11 +535,15 @@ void __nd_device_register(struct device *dev) set_dev_node(dev, to_nd_region(dev)->numa_node); dev->bus = &nvdimm_bus_type; - if (dev->parent) + if (dev->parent) { get_device(dev->parent); + if (dev_to_node(dev) == NUMA_NO_NODE) + set_dev_node(dev, dev_to_node(dev->parent)); + } get_device(dev); - async_schedule_domain(nd_async_device_register, dev, - &nd_async_domain); + + async_schedule_dev_domain(nd_async_device_register, dev, + &nd_async_domain); } void nd_device_register(struct device *dev) -- cgit v1.2.3 From 57ea974fb8717864e8b7ec679363c5a3298a165e Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:52 -0800 Subject: driver core: Rewrite test_async_driver_probe to cover serialization and NUMA affinity The current async_probe test code is only testing one device allocated prior to driver load and only loading one device afterwards. Instead of doing things this way it makes much more sense to load one device per CPU in order to actually stress the async infrastructure. By doing this we should see delays significantly increase in the event of devices being serialized. In addition I have updated the test to verify that we are trying to place the work on the correct NUMA node when we are running in async mode. By doing this we can verify the best possible outcome for device and driver load times. I have added a timeout value that is used to disable the sleep and instead cause the probe routine to report an error indicating it timed out. By doing this we limit the maximum runtime for the test to 20 seconds or less. The last major change in this set is that I have gone through and tuned it for handling the massive number of possible events that will be scheduled. Instead of reporting the sleep for each individual device it is moved to only being displayed if we enable debugging. With this patch applied below are what a failing test and a passing test should look like. I elided a few hundred lines in the failing test that were duplicated since the system I was testing on had a massive number of CPU cores: -- Failing -- [ 243.524697] test_async_driver_probe: registering first set of asynchronous devices... [ 243.535625] test_async_driver_probe: registering asynchronous driver... [ 243.543038] test_async_driver_probe: registration took 0 msecs [ 243.549559] test_async_driver_probe: registering second set of asynchronous devices... [ 243.568350] platform test_async_driver.447: registration took 9 msecs [ 243.575544] test_async_driver_probe: registering first synchronous device... [ 243.583454] test_async_driver_probe: registering synchronous driver... [ 248.825920] test_async_driver_probe: registration took 5235 msecs [ 248.825922] test_async_driver_probe: registering second synchronous device... [ 248.825928] test_async_driver test_async_driver.443: NUMA node mismatch 3 != 1 [ 248.825932] test_async_driver test_async_driver.445: NUMA node mismatch 3 != 1 [ 248.825935] test_async_driver test_async_driver.446: NUMA node mismatch 3 != 1 [ 248.825939] test_async_driver test_async_driver.440: NUMA node mismatch 3 != 1 [ 248.825943] test_async_driver test_async_driver.441: NUMA node mismatch 3 != 1 ... [ 248.827150] test_async_driver test_async_driver.229: NUMA node mismatch 0 != 1 [ 248.827158] test_async_driver test_async_driver.228: NUMA node mismatch 0 != 1 [ 248.827220] test_async_driver test_async_driver.281: NUMA node mismatch 2 != 1 [ 248.827229] test_async_driver test_async_driver.282: NUMA node mismatch 2 != 1 [ 248.827240] test_async_driver test_async_driver.280: NUMA node mismatch 2 != 1 [ 253.945834] test_async_driver test_async_driver.1: NUMA node mismatch 0 != 1 [ 253.945878] test_sync_driver test_sync_driver.1: registration took 5119 msecs [ 253.961693] test_async_driver_probe: async events still pending, forcing timeout and synchronize [ 259.065839] test_async_driver test_async_driver.2: NUMA node mismatch 0 != 1 [ 259.073786] test_async_driver test_async_driver.3: async probe took too long [ 259.081669] test_async_driver test_async_driver.3: NUMA node mismatch 0 != 1 [ 259.089569] test_async_driver test_async_driver.4: async probe took too long [ 259.097451] test_async_driver test_async_driver.4: NUMA node mismatch 0 != 1 [ 259.105338] test_async_driver test_async_driver.5: async probe took too long [ 259.113204] test_async_driver test_async_driver.5: NUMA node mismatch 0 != 1 [ 259.121089] test_async_driver test_async_driver.6: async probe took too long [ 259.128961] test_async_driver test_async_driver.6: NUMA node mismatch 0 != 1 [ 259.136850] test_async_driver test_async_driver.7: async probe took too long ... [ 262.124062] test_async_driver test_async_driver.221: async probe took too long [ 262.132130] test_async_driver test_async_driver.221: NUMA node mismatch 3 != 1 [ 262.140206] test_async_driver test_async_driver.222: async probe took too long [ 262.148277] test_async_driver test_async_driver.222: NUMA node mismatch 3 != 1 [ 262.156351] test_async_driver test_async_driver.223: async probe took too long [ 262.164419] test_async_driver test_async_driver.223: NUMA node mismatch 3 != 1 [ 262.172630] test_async_driver_probe: Test failed with 222 errors and 336 warnings -- Passing -- [ 105.419247] test_async_driver_probe: registering first set of asynchronous devices... [ 105.432040] test_async_driver_probe: registering asynchronous driver... [ 105.439718] test_async_driver_probe: registration took 0 msecs [ 105.446239] test_async_driver_probe: registering second set of asynchronous devices... [ 105.477986] platform test_async_driver.447: registration took 22 msecs [ 105.485276] test_async_driver_probe: registering first synchronous device... [ 105.493169] test_async_driver_probe: registering synchronous driver... [ 110.597981] test_async_driver_probe: registration took 5097 msecs [ 110.604806] test_async_driver_probe: registering second synchronous device... [ 115.707490] test_sync_driver test_sync_driver.1: registration took 5094 msecs [ 115.715478] test_async_driver_probe: completed successfully Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/base/test/test_async_driver_probe.c | 261 +++++++++++++++++++++------- 1 file changed, 201 insertions(+), 60 deletions(-) diff --git a/drivers/base/test/test_async_driver_probe.c b/drivers/base/test/test_async_driver_probe.c index e7f145d662f0..f4b1d8e54daf 100644 --- a/drivers/base/test/test_async_driver_probe.c +++ b/drivers/base/test/test_async_driver_probe.c @@ -11,16 +11,47 @@ #include #include #include +#include +#include +#include #define TEST_PROBE_DELAY (5 * 1000) /* 5 sec */ #define TEST_PROBE_THRESHOLD (TEST_PROBE_DELAY / 2) +static atomic_t warnings, errors, timeout, async_completed; + static int test_probe(struct platform_device *pdev) { - dev_info(&pdev->dev, "sleeping for %d msecs in probe\n", - TEST_PROBE_DELAY); - msleep(TEST_PROBE_DELAY); - dev_info(&pdev->dev, "done sleeping\n"); + struct device *dev = &pdev->dev; + + /* + * Determine if we have hit the "timeout" limit for the test if we + * have then report it as an error, otherwise we wil sleep for the + * required amount of time and then report completion. + */ + if (atomic_read(&timeout)) { + dev_err(dev, "async probe took too long\n"); + atomic_inc(&errors); + } else { + dev_dbg(&pdev->dev, "sleeping for %d msecs in probe\n", + TEST_PROBE_DELAY); + msleep(TEST_PROBE_DELAY); + dev_dbg(&pdev->dev, "done sleeping\n"); + } + + /* + * Report NUMA mismatch if device node is set and we are not + * performing an async init on that node. + */ + if (dev->driver->probe_type == PROBE_PREFER_ASYNCHRONOUS) { + if (dev_to_node(dev) != numa_node_id()) { + dev_warn(dev, "NUMA node mismatch %d != %d\n", + dev_to_node(dev), numa_node_id()); + atomic_inc(&warnings); + } + + atomic_inc(&async_completed); + } return 0; } @@ -41,31 +72,64 @@ static struct platform_driver sync_driver = { .probe = test_probe, }; -static struct platform_device *async_dev_1, *async_dev_2; -static struct platform_device *sync_dev_1; +static struct platform_device *async_dev[NR_CPUS * 2]; +static struct platform_device *sync_dev[2]; + +static struct platform_device * +test_platform_device_register_node(char *name, int id, int nid) +{ + struct platform_device *pdev; + int ret; + + pdev = platform_device_alloc(name, id); + if (!pdev) + return NULL; + + if (nid != NUMA_NO_NODE) + set_dev_node(&pdev->dev, nid); + + ret = platform_device_add(pdev); + if (ret) { + platform_device_put(pdev); + return ERR_PTR(ret); + } + + return pdev; + +} static int __init test_async_probe_init(void) { - ktime_t calltime, delta; + struct platform_device **pdev = NULL; + int async_id = 0, sync_id = 0; unsigned long long duration; - int error; + ktime_t calltime, delta; + int err, nid, cpu; + + pr_info("registering first set of asynchronous devices...\n"); - pr_info("registering first asynchronous device...\n"); + for_each_online_cpu(cpu) { + nid = cpu_to_node(cpu); + pdev = &async_dev[async_id]; + *pdev = test_platform_device_register_node("test_async_driver", + async_id, + nid); + if (IS_ERR(*pdev)) { + err = PTR_ERR(*pdev); + *pdev = NULL; + pr_err("failed to create async_dev: %d\n", err); + goto err_unregister_async_devs; + } - async_dev_1 = platform_device_register_simple("test_async_driver", 1, - NULL, 0); - if (IS_ERR(async_dev_1)) { - error = PTR_ERR(async_dev_1); - pr_err("failed to create async_dev_1: %d\n", error); - return error; + async_id++; } pr_info("registering asynchronous driver...\n"); calltime = ktime_get(); - error = platform_driver_register(&async_driver); - if (error) { - pr_err("Failed to register async_driver: %d\n", error); - goto err_unregister_async_dev_1; + err = platform_driver_register(&async_driver); + if (err) { + pr_err("Failed to register async_driver: %d\n", err); + goto err_unregister_async_devs; } delta = ktime_sub(ktime_get(), calltime); @@ -73,86 +137,163 @@ static int __init test_async_probe_init(void) pr_info("registration took %lld msecs\n", duration); if (duration > TEST_PROBE_THRESHOLD) { pr_err("test failed: probe took too long\n"); - error = -ETIMEDOUT; + err = -ETIMEDOUT; goto err_unregister_async_driver; } - pr_info("registering second asynchronous device...\n"); + pr_info("registering second set of asynchronous devices...\n"); calltime = ktime_get(); - async_dev_2 = platform_device_register_simple("test_async_driver", 2, - NULL, 0); - if (IS_ERR(async_dev_2)) { - error = PTR_ERR(async_dev_2); - pr_err("failed to create async_dev_2: %d\n", error); - goto err_unregister_async_driver; + for_each_online_cpu(cpu) { + nid = cpu_to_node(cpu); + pdev = &sync_dev[sync_id]; + + *pdev = test_platform_device_register_node("test_async_driver", + async_id, + nid); + if (IS_ERR(*pdev)) { + err = PTR_ERR(*pdev); + *pdev = NULL; + pr_err("failed to create async_dev: %d\n", err); + goto err_unregister_async_driver; + } + + async_id++; } delta = ktime_sub(ktime_get(), calltime); duration = (unsigned long long) ktime_to_ms(delta); - pr_info("registration took %lld msecs\n", duration); + dev_info(&(*pdev)->dev, + "registration took %lld msecs\n", duration); if (duration > TEST_PROBE_THRESHOLD) { - pr_err("test failed: probe took too long\n"); - error = -ETIMEDOUT; - goto err_unregister_async_dev_2; + dev_err(&(*pdev)->dev, + "test failed: probe took too long\n"); + err = -ETIMEDOUT; + goto err_unregister_async_driver; } - pr_info("registering synchronous driver...\n"); - error = platform_driver_register(&sync_driver); - if (error) { - pr_err("Failed to register async_driver: %d\n", error); - goto err_unregister_async_dev_2; + pr_info("registering first synchronous device...\n"); + nid = cpu_to_node(cpu); + pdev = &sync_dev[sync_id]; + + *pdev = test_platform_device_register_node("test_sync_driver", + sync_id, + NUMA_NO_NODE); + if (IS_ERR(*pdev)) { + err = PTR_ERR(*pdev); + *pdev = NULL; + pr_err("failed to create sync_dev: %d\n", err); + goto err_unregister_async_driver; } - pr_info("registering synchronous device...\n"); + sync_id++; + + pr_info("registering synchronous driver...\n"); calltime = ktime_get(); - sync_dev_1 = platform_device_register_simple("test_sync_driver", 1, - NULL, 0); - if (IS_ERR(sync_dev_1)) { - error = PTR_ERR(sync_dev_1); - pr_err("failed to create sync_dev_1: %d\n", error); - goto err_unregister_sync_driver; + err = platform_driver_register(&sync_driver); + if (err) { + pr_err("Failed to register async_driver: %d\n", err); + goto err_unregister_sync_devs; } delta = ktime_sub(ktime_get(), calltime); duration = (unsigned long long) ktime_to_ms(delta); pr_info("registration took %lld msecs\n", duration); if (duration < TEST_PROBE_THRESHOLD) { - pr_err("test failed: probe was too quick\n"); - error = -ETIMEDOUT; - goto err_unregister_sync_dev_1; + dev_err(&(*pdev)->dev, + "test failed: probe was too quick\n"); + err = -ETIMEDOUT; + goto err_unregister_sync_driver; } - pr_info("completed successfully"); + pr_info("registering second synchronous device...\n"); + pdev = &sync_dev[sync_id]; + calltime = ktime_get(); - return 0; + *pdev = test_platform_device_register_node("test_sync_driver", + sync_id, + NUMA_NO_NODE); + if (IS_ERR(*pdev)) { + err = PTR_ERR(*pdev); + *pdev = NULL; + pr_err("failed to create sync_dev: %d\n", err); + goto err_unregister_sync_driver; + } -err_unregister_sync_dev_1: - platform_device_unregister(sync_dev_1); + sync_id++; -err_unregister_sync_driver: - platform_driver_unregister(&sync_driver); + delta = ktime_sub(ktime_get(), calltime); + duration = (unsigned long long) ktime_to_ms(delta); + dev_info(&(*pdev)->dev, + "registration took %lld msecs\n", duration); + if (duration < TEST_PROBE_THRESHOLD) { + dev_err(&(*pdev)->dev, + "test failed: probe was too quick\n"); + err = -ETIMEDOUT; + goto err_unregister_sync_driver; + } -err_unregister_async_dev_2: - platform_device_unregister(async_dev_2); + /* + * The async events should have completed while we were taking care + * of the synchronous events. We will now terminate any outstanding + * asynchronous probe calls remaining by forcing timeout and remove + * the driver before we return which should force the flush of the + * pending asynchronous probe calls. + * + * Otherwise if they completed without errors or warnings then + * report successful completion. + */ + if (atomic_read(&async_completed) != async_id) { + pr_err("async events still pending, forcing timeout\n"); + atomic_inc(&timeout); + err = -ETIMEDOUT; + } else if (!atomic_read(&errors) && !atomic_read(&warnings)) { + pr_info("completed successfully\n"); + return 0; + } +err_unregister_sync_driver: + platform_driver_unregister(&sync_driver); +err_unregister_sync_devs: + while (sync_id--) + platform_device_unregister(sync_dev[sync_id]); err_unregister_async_driver: platform_driver_unregister(&async_driver); +err_unregister_async_devs: + while (async_id--) + platform_device_unregister(async_dev[async_id]); + + /* + * If err is already set then count that as an additional error for + * the test. Otherwise we will report an invalid argument error and + * not count that as we should have reached here as a result of + * errors or warnings being reported by the probe routine. + */ + if (err) + atomic_inc(&errors); + else + err = -EINVAL; -err_unregister_async_dev_1: - platform_device_unregister(async_dev_1); + pr_err("Test failed with %d errors and %d warnings\n", + atomic_read(&errors), atomic_read(&warnings)); - return error; + return err; } module_init(test_async_probe_init); static void __exit test_async_probe_exit(void) { + int id = 2; + platform_driver_unregister(&async_driver); platform_driver_unregister(&sync_driver); - platform_device_unregister(async_dev_1); - platform_device_unregister(async_dev_2); - platform_device_unregister(sync_dev_1); + + while (id--) + platform_device_unregister(sync_dev[id]); + + id = NR_CPUS * 2; + while (id--) + platform_device_unregister(async_dev[id]); } module_exit(test_async_probe_exit); -- cgit v1.2.3 From 095ff29d2b882847f608cf7192361dedc22c0623 Mon Sep 17 00:00:00 2001 From: Richard Gong Date: Wed, 23 Jan 2019 12:06:05 -0600 Subject: firmware: intel_stratix10_service: add hardware dependency Add a Kconfig dependency to ensure Intel Stratix10 service layer driver can be built only on the platform that supports it. Signed-off-by: Richard Gong Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index f754578414f0..7e5491aed5c8 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -218,7 +218,7 @@ config FW_CFG_SYSFS_CMDLINE config INTEL_STRATIX10_SERVICE tristate "Intel Stratix10 Service Layer" - depends on HAVE_ARM_SMCCC + depends on (ARCH_STRATIX10 && HAVE_ARM_SMCCC) || COMPILE_TEST default n help Intel Stratix10 service layer runs at privileged exception level, -- cgit v1.2.3 From fa548d79d87fd1ad2a2c4e26f829984a106c6ca5 Mon Sep 17 00:00:00 2001 From: Mathieu Malaterre Date: Wed, 23 Jan 2019 20:42:00 +0100 Subject: drivers: base: Use __printf markup to silence compiler Silence warnings (triggered at W=1) by adding relevant __printf attributes. drivers/base/cpu.c:432:2: warning: function '__cpu_device_create' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format] Signed-off-by: Mathieu Malaterre Signed-off-by: Greg Kroah-Hartman --- drivers/base/cpu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index eb9443d5bae1..de74d7d93558 100644 --- a/drivers/base/cpu.c +++ b/drivers/base/cpu.c @@ -409,6 +409,7 @@ static void device_create_release(struct device *dev) kfree(dev); } +__printf(4, 0) static struct device * __cpu_device_create(struct device *parent, void *drvdata, const struct attribute_group **groups, -- cgit v1.2.3 From 8d84b18f5678d3adfdb9375dfb0d968da2dc753d Mon Sep 17 00:00:00 2001 From: Sergei Shtylyov Date: Tue, 29 Jan 2019 13:56:19 +0300 Subject: devres: always use dev_name() in devm_ioremap_resource() devm_ioremap_resource() prefers calling devm_request_mem_region() with a resource name instead of a device name -- this looks pretty iff a resource name isn't specified via a device tree with a "reg-names" property (in this case, a resource name is set to a device node's full name), but if it is, it doesn't really scale since these names are only unique to a given device node, not globally; so, looking at the output of 'cat /proc/iomem', you do not have an idea which memory region belongs to which device (see "dirmap", "regs", and "wbuf" lines below): 08000000-0bffffff : dirmap 48000000-bfffffff : System RAM 48000000-48007fff : reserved 48080000-48b0ffff : Kernel code 48b10000-48b8ffff : reserved 48b90000-48c7afff : Kernel data bc6a4000-bcbfffff : reserved bcc0f000-bebfffff : reserved bec0e000-bec0efff : reserved bec11000-bec11fff : reserved bec12000-bec14fff : reserved bec15000-bfffffff : reserved e6050000-e605004f : gpio@e6050000 e6051000-e605104f : gpio@e6051000 e6052000-e605204f : gpio@e6052000 e6053000-e605304f : gpio@e6053000 e6054000-e605404f : gpio@e6054000 e6055000-e605504f : gpio@e6055000 e6060000-e606050b : pin-controller@e6060000 e6e60000-e6e6003f : e6e60000.serial e7400000-e7400fff : ethernet@e7400000 ee200000-ee2001ff : regs ee208000-ee2080ff : wbuf I think that devm_request_mem_region() should be called with dev_name() despite the region names won't look as pretty as before (however, we gain more consistency with e.g. the serial driver: 08000000-0bffffff : ee200000.rpc 48000000-bfffffff : System RAM 48000000-48007fff : reserved 48080000-48b0ffff : Kernel code 48b10000-48b8ffff : reserved 48b90000-48c7afff : Kernel data bc6a4000-bcbfffff : reserved bcc0f000-bebfffff : reserved bec0e000-bec0efff : reserved bec11000-bec11fff : reserved bec12000-bec14fff : reserved bec15000-bfffffff : reserved e6050000-e605004f : e6050000.gpio e6051000-e605104f : e6051000.gpio e6052000-e605204f : e6052000.gpio e6053000-e605304f : e6053000.gpio e6054000-e605404f : e6054000.gpio e6055000-e605504f : e6055000.gpio e6060000-e606050b : e6060000.pin-controller e6e60000-e6e6003f : e6e60000.serial e7400000-e7400fff : e7400000.ethernet ee200000-ee2001ff : ee200000.rpc ee208000-ee2080ff : ee200000.rpc Fixes: 72f8c0bfa0de ("lib: devres: add convenience function to remap a resource") Signed-off-by: Sergei Shtylyov Signed-off-by: Greg Kroah-Hartman --- lib/devres.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/devres.c b/lib/devres.c index faccf1a037d0..69bed2f38306 100644 --- a/lib/devres.c +++ b/lib/devres.c @@ -134,7 +134,6 @@ EXPORT_SYMBOL(devm_iounmap); void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res) { resource_size_t size; - const char *name; void __iomem *dest_ptr; BUG_ON(!dev); @@ -145,9 +144,8 @@ void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res) } size = resource_size(res); - name = res->name ?: dev_name(dev); - if (!devm_request_mem_region(dev, res->start, size, name)) { + if (!devm_request_mem_region(dev, res->start, size, dev_name(dev))) { dev_err(dev, "can't request region for resource %pR\n", res); return IOMEM_ERR_PTR(-EBUSY); } -- cgit v1.2.3 From c8d50986da5d74ddfc233b13b91d0a13369fa164 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:45:55 +0100 Subject: driver core: Fix DL_FLAG_AUTOREMOVE_SUPPLIER device link flag handling Change the list walk in device_links_driver_cleanup() to a safe one to avoid use-after-free when dropping a link from the list during the walk. Also, while at it, fix device_link_add() to refuse to create stateless device links with DL_FLAG_AUTOREMOVE_SUPPLIER set, which is an invalid combination (setting that flag means that the driver core should manage the link, so it cannot be stateless), and extend the kerneldoc comment of device_link_add() to cover the DL_FLAG_AUTOREMOVE_SUPPLIER flag properly too. Fixes: 1689cac5b32a ("driver core: Add flag to autoremove device link on supplier unbind") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 1febfd2a353a..f24eb2daad00 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -179,10 +179,14 @@ void device_pm_move_to_tail(struct device *dev) * of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be * ignored. * - * If the DL_FLAG_AUTOREMOVE_CONSUMER is set, the link will be removed - * automatically when the consumer device driver unbinds from it. - * The combination of both DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_STATELESS - * set is invalid and will cause NULL to be returned. + * If the DL_FLAG_AUTOREMOVE_CONSUMER flag is set, the link will be removed + * automatically when the consumer device driver unbinds from it. Analogously, + * if DL_FLAG_AUTOREMOVE_SUPPLIER is set in @flags, the link will be removed + * automatically when the supplier device driver unbinds from it. + * + * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER + * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and + * will cause NULL to be returned upfront. * * A side effect of the link creation is re-ordering of dpm_list and the * devices_kset list by moving the consumer device and all devices depending @@ -199,8 +203,8 @@ struct device_link *device_link_add(struct device *consumer, struct device_link *link; if (!consumer || !supplier || - ((flags & DL_FLAG_STATELESS) && - (flags & DL_FLAG_AUTOREMOVE_CONSUMER))) + (flags & DL_FLAG_STATELESS && + flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) return NULL; device_links_write_lock(); @@ -539,11 +543,11 @@ void device_links_no_driver(struct device *dev) */ void device_links_driver_cleanup(struct device *dev) { - struct device_link *link; + struct device_link *link, *ln; device_links_write_lock(); - list_for_each_entry(link, &dev->links.consumers, s_node) { + list_for_each_entry_safe(link, ln, &dev->links.consumers, s_node) { if (link->flags & DL_FLAG_STATELESS) continue; -- cgit v1.2.3 From f265df550a4350dce0a4d721a77c52e4b847ea40 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:46:54 +0100 Subject: driver core: Avoid careless re-use of existing device links After commit ead18c23c263 ("driver core: Introduce device links reference counting"), if there is a link between the given supplier and the given consumer already, device_link_add() will refcount it and return it unconditionally. However, if the flags passed to it on the second (or any subsequent) attempt to create a device link between the same consumer-supplier pair are not compatible with the existing link's flags, that is incorrect. First off, if the existing link is stateless and the next caller of device_link_add() for the same consumer-supplier pair wants a stateful one, or the other way around, the existing link cannot be returned, because it will not match the expected behavior, so make device_link_add() dump the stack and return NULL in that case. Moreover, if the DL_FLAG_AUTOREMOVE_CONSUMER flag is passed to device_link_add(), its caller will expect its reference to the link to be dropped automatically on consumer driver removal, which will not happen if that flag is not set in the link's flags (and analogously for DL_FLAG_AUTOREMOVE_SUPPLIER). For this reason, make device_link_add() update the existing link's flags accordingly before returning it to the caller. Fixes: ead18c23c263 ("driver core: Introduce device links reference counting") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index f24eb2daad00..a4d6f9b93088 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -221,12 +221,29 @@ struct device_link *device_link_add(struct device *consumer, goto out; } - list_for_each_entry(link, &supplier->links.consumers, s_node) - if (link->consumer == consumer) { - kref_get(&link->kref); + list_for_each_entry(link, &supplier->links.consumers, s_node) { + if (link->consumer != consumer) + continue; + + /* + * Don't return a stateless link if the caller wants a stateful + * one and vice versa. + */ + if (WARN_ON((flags & DL_FLAG_STATELESS) != (link->flags & DL_FLAG_STATELESS))) { + link = NULL; goto out; } + if (flags & DL_FLAG_AUTOREMOVE_CONSUMER) + link->flags |= DL_FLAG_AUTOREMOVE_CONSUMER; + + if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) + link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; + + kref_get(&link->kref); + goto out; + } + link = kzalloc(sizeof(*link), GFP_KERNEL); if (!link) goto out; -- cgit v1.2.3 From 5db25c9eb893df8f6b93c1d97b8006d768e1b6f5 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:47:53 +0100 Subject: driver core: Do not resume suppliers under device_links_write_lock() It is incorrect to call pm_runtime_get_sync() under device_links_write_lock(), because it may end up trying to take device_links_read_lock() while resuming the target device and that will deadlock in the non-SRCU case, so avoid that by resuming the supplier device in device_link_add() before calling device_links_write_lock(). Fixes: 21d5c57b3726 ("PM / runtime: Use device links") Fixes: baa8809f6097 ("PM / runtime: Optimize the use of device links") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index a4d6f9b93088..50610cd87e71 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -201,12 +201,21 @@ struct device_link *device_link_add(struct device *consumer, struct device *supplier, u32 flags) { struct device_link *link; + bool rpm_put_supplier = false; if (!consumer || !supplier || (flags & DL_FLAG_STATELESS && flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) return NULL; + if (flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) { + if (pm_runtime_get_sync(supplier) < 0) { + pm_runtime_put_noidle(supplier); + return NULL; + } + rpm_put_supplier = true; + } + device_links_write_lock(); device_pm_lock(); @@ -250,13 +259,8 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_PM_RUNTIME) { if (flags & DL_FLAG_RPM_ACTIVE) { - if (pm_runtime_get_sync(supplier) < 0) { - pm_runtime_put_noidle(supplier); - kfree(link); - link = NULL; - goto out; - } link->rpm_active = true; + rpm_put_supplier = false; } pm_runtime_new_link(consumer); /* @@ -328,6 +332,10 @@ struct device_link *device_link_add(struct device *consumer, out: device_pm_unlock(); device_links_write_unlock(); + + if (rpm_put_supplier) + pm_runtime_put(supplier); + return link; } EXPORT_SYMBOL_GPL(device_link_add); -- cgit v1.2.3 From e2f3cd831a280fc226118d9369bf3f77aab58c56 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:49:14 +0100 Subject: driver core: Fix handling of runtime PM flags in device_link_add() After commit ead18c23c263 ("driver core: Introduce device links reference counting"), if there is a link between the given supplier and the given consumer already, device_link_add() will refcount it and return it unconditionally without updating its flags. It is possible, however, that the second (or any subsequent) caller of device_link_add() for the same consumer-supplier pair will pass DL_FLAG_PM_RUNTIME, possibly along with DL_FLAG_RPM_ACTIVE, in flags to it and the existing link may not behave as expected then. First, if DL_FLAG_PM_RUNTIME is not set in the existing link's flags at all, it needs to be set like during the original initialization of the link. Second, if DL_FLAG_RPM_ACTIVE is passed to device_link_add() in flags (in addition to DL_FLAG_PM_RUNTIME), the existing link should to be updated to reflect the "active" runtime PM configuration of the consumer-supplier pair and extra care must be taken here to avoid possible destructive races with runtime PM of the consumer. To that end, redefine the rpm_active field in struct device_link as a refcount, initialize it to 1 and make rpm_resume() (for the consumer) and device_link_add() increment it whenever they acquire a runtime PM reference on the supplier device. Accordingly, make rpm_suspend() (for the consumer) and pm_runtime_clean_up_links() decrement it and drop runtime PM references to the supplier device in a loop until rpm_active becones 1 again. Fixes: ead18c23c263 ("driver core: Introduce device links reference counting") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 45 +++++++++++++++++++++++++++++--------------- drivers/base/power/runtime.c | 26 +++++++++++-------------- include/linux/device.h | 2 +- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 50610cd87e71..8611385e44b5 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -165,6 +165,19 @@ void device_pm_move_to_tail(struct device *dev) device_links_read_unlock(idx); } +static void device_link_rpm_prepare(struct device *consumer, + struct device *supplier) +{ + pm_runtime_new_link(consumer); + /* + * If the link is being added by the consumer driver at probe time, + * balance the decrementation of the supplier's runtime PM usage counter + * after consumer probe in driver_probe_device(). + */ + if (consumer->links.status == DL_DEV_PROBING) + pm_runtime_get_noresume(supplier); +} + /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. @@ -201,7 +214,6 @@ struct device_link *device_link_add(struct device *consumer, struct device *supplier, u32 flags) { struct device_link *link; - bool rpm_put_supplier = false; if (!consumer || !supplier || (flags & DL_FLAG_STATELESS && @@ -213,7 +225,6 @@ struct device_link *device_link_add(struct device *consumer, pm_runtime_put_noidle(supplier); return NULL; } - rpm_put_supplier = true; } device_links_write_lock(); @@ -249,6 +260,15 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; + if (flags & DL_FLAG_PM_RUNTIME) { + if (!(link->flags & DL_FLAG_PM_RUNTIME)) { + device_link_rpm_prepare(consumer, supplier); + link->flags |= DL_FLAG_PM_RUNTIME; + } + if (flags & DL_FLAG_RPM_ACTIVE) + refcount_inc(&link->rpm_active); + } + kref_get(&link->kref); goto out; } @@ -257,20 +277,15 @@ struct device_link *device_link_add(struct device *consumer, if (!link) goto out; + refcount_set(&link->rpm_active, 1); + if (flags & DL_FLAG_PM_RUNTIME) { - if (flags & DL_FLAG_RPM_ACTIVE) { - link->rpm_active = true; - rpm_put_supplier = false; - } - pm_runtime_new_link(consumer); - /* - * If the link is being added by the consumer driver at probe - * time, balance the decrementation of the supplier's runtime PM - * usage counter after consumer probe in driver_probe_device(). - */ - if (consumer->links.status == DL_DEV_PROBING) - pm_runtime_get_noresume(supplier); + if (flags & DL_FLAG_RPM_ACTIVE) + refcount_inc(&link->rpm_active); + + device_link_rpm_prepare(consumer, supplier); } + get_device(supplier); link->supplier = supplier; INIT_LIST_HEAD(&link->s_node); @@ -333,7 +348,7 @@ struct device_link *device_link_add(struct device *consumer, device_pm_unlock(); device_links_write_unlock(); - if (rpm_put_supplier) + if ((flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) && !link) pm_runtime_put(supplier); return link; diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 457be03b744d..8bc9a432de70 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -259,11 +259,8 @@ static int rpm_get_suppliers(struct device *dev) list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) { int retval; - if (!(link->flags & DL_FLAG_PM_RUNTIME)) - continue; - - if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND || - link->rpm_active) + if (!(link->flags & DL_FLAG_PM_RUNTIME) || + READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND) continue; retval = pm_runtime_get_sync(link->supplier); @@ -272,7 +269,7 @@ static int rpm_get_suppliers(struct device *dev) pm_runtime_put_noidle(link->supplier); return retval; } - link->rpm_active = true; + refcount_inc(&link->rpm_active); } return 0; } @@ -281,12 +278,13 @@ static void rpm_put_suppliers(struct device *dev) { struct device_link *link; - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->rpm_active && - READ_ONCE(link->status) != DL_STATE_SUPPLIER_UNBIND) { + list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) { + if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND) + continue; + + while (refcount_dec_not_one(&link->rpm_active)) pm_runtime_put(link->supplier); - link->rpm_active = false; - } + } } /** @@ -1539,7 +1537,7 @@ void pm_runtime_remove(struct device *dev) * * Check links from this device to any consumers and if any of them have active * runtime PM references to the device, drop the usage counter of the device - * (once per link). + * (as many times as needed). * * Links with the DL_FLAG_STATELESS flag set are ignored. * @@ -1561,10 +1559,8 @@ void pm_runtime_clean_up_links(struct device *dev) if (link->flags & DL_FLAG_STATELESS) continue; - if (link->rpm_active) { + while (refcount_dec_not_one(&link->rpm_active)) pm_runtime_put_noidle(dev); - link->rpm_active = false; - } } device_links_read_unlock(idx); diff --git a/include/linux/device.h b/include/linux/device.h index d0e452fd0bff..5f49d2eff6ed 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -853,7 +853,7 @@ struct device_link { struct list_head c_node; enum device_link_state status; u32 flags; - bool rpm_active; + refcount_t rpm_active; struct kref kref; #ifdef CONFIG_SRCU struct rcu_head rcu_head; -- cgit v1.2.3 From 15cfb094160385cc0b303c4cda483caa102af654 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:50:39 +0100 Subject: driver core: Fix adding device links to probing suppliers Currently, it is not valid to add a device link from a consumer driver ->probe callback to a supplier that is still probing too, but generally this is a valid use case. For example, if the consumer has just acquired a resource that can only be available if the supplier is functional, adding a device link to that supplier right away should be safe (and even desirable arguably), but device_link_add() doesn't handle that case correctly and the initial state of the link created by it is wrong then. To address this problem, change the initial state of device links added between a probing supplier and a probing consumer to DL_STATE_CONSUMER_PROBE and update device_links_driver_bound() to skip such links on the supplier side. With this change, if the supplier probe completes first, device_links_driver_bound() called for it will skip the link state update and when it is called for the consumer, the link state will be updated to "active". In turn, if the consumer probe completes first, device_links_driver_bound() called for it will change the state of the link to "active" and when it is called for the supplier, the link status update will be skipped. However, in principle the supplier or consumer probe may still fail after the link has been added, so modify device_links_no_driver() to change device links in the "active" or "consumer probe" state to "dormant" on the supplier side and update __device_links_no_driver() to change the link state to "available" only if it is "consumer probe" or "active". Then, if the supplier probe fails first, the leftover link to the probing consumer will become "dormant" and device_links_no_driver() called for the consumer (when its probe fails) will clean it up. In turn, if the consumer probe fails first, it will either drop the link, or change its state to "available" and, in the latter case, when device_links_no_driver() is called for the supplier, it will update the link state to "dormant". [If the supplier probe fails, but the consumer probe succeeds, which should not happen as long as the consumer driver is correct, the link still will be around, but it will be "dormant" until the supplier is probed again.] Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- Documentation/driver-api/device_link.rst | 10 +++-- drivers/base/core.c | 74 ++++++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index d6763272e747..5c7178189612 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -59,11 +59,15 @@ device ``->probe`` callback or a boot-time PCI quirk. Another example for an inconsistent state would be a device link that represents a driver presence dependency, yet is added from the consumer's -``->probe`` callback while the supplier hasn't probed yet: Had the driver -core known about the device link earlier, it wouldn't have probed the +``->probe`` callback while the supplier hasn't started to probe yet: Had the +driver core known about the device link earlier, it wouldn't have probed the consumer in the first place. The onus is thus on the consumer to check presence of the supplier after adding the link, and defer probing on -non-presence. +non-presence. [Note that it is valid to create a link from the consumer's +``->probe`` callback while the supplier is still probing, but the consumer must +know that the supplier is functional already at the link creation time (that is +the case, for instance, if the consumer has just acquired some resources that +would not have been available had the supplier not been functional then).] If a device link is added in the ``->probe`` callback of the supplier or consumer driver, it is typically deleted in its ``->remove`` callback for diff --git a/drivers/base/core.c b/drivers/base/core.c index 8611385e44b5..d58ea075f807 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -300,17 +300,26 @@ struct device_link *device_link_add(struct device *consumer, link->status = DL_STATE_NONE; } else { switch (supplier->links.status) { - case DL_DEV_DRIVER_BOUND: + case DL_DEV_PROBING: switch (consumer->links.status) { case DL_DEV_PROBING: /* - * Some callers expect the link creation during - * consumer driver probe to resume the supplier - * even without DL_FLAG_RPM_ACTIVE. + * A consumer driver can create a link to a + * supplier that has not completed its probing + * yet as long as it knows that the supplier is + * already functional (for example, it has just + * acquired some resources from the supplier). */ - if (flags & DL_FLAG_PM_RUNTIME) - pm_runtime_resume(supplier); - + link->status = DL_STATE_CONSUMER_PROBE; + break; + default: + link->status = DL_STATE_DORMANT; + break; + } + break; + case DL_DEV_DRIVER_BOUND: + switch (consumer->links.status) { + case DL_DEV_PROBING: link->status = DL_STATE_CONSUMER_PROBE; break; case DL_DEV_DRIVER_BOUND: @@ -330,6 +339,14 @@ struct device_link *device_link_add(struct device *consumer, } } + /* + * Some callers expect the link creation during consumer driver probe to + * resume the supplier even without DL_FLAG_RPM_ACTIVE. + */ + if (link->status == DL_STATE_CONSUMER_PROBE && + flags & DL_FLAG_PM_RUNTIME) + pm_runtime_resume(supplier); + /* * Move the consumer and all of the devices depending on it to the end * of dpm_list and the devices_kset list. @@ -518,6 +535,16 @@ void device_links_driver_bound(struct device *dev) if (link->flags & DL_FLAG_STATELESS) continue; + /* + * Links created during consumer probe may be in the "consumer + * probe" state to start with if the supplier is still probing + * when they are created and they may become "active" if the + * consumer probe returns first. Skip them here. + */ + if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) + continue; + WARN_ON(link->status != DL_STATE_DORMANT); WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } @@ -557,17 +584,48 @@ static void __device_links_no_driver(struct device *dev) if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) __device_link_del(&link->kref); - else if (link->status != DL_STATE_SUPPLIER_UNBIND) + else if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } dev->links.status = DL_DEV_NO_DRIVER; } +/** + * device_links_no_driver - Update links after failing driver probe. + * @dev: Device whose driver has just failed to probe. + * + * Clean up leftover links to consumers for @dev and invoke + * %__device_links_no_driver() to update links to suppliers for it as + * appropriate. + * + * Links with the DL_FLAG_STATELESS flag set are ignored. + */ void device_links_no_driver(struct device *dev) { + struct device_link *link; + device_links_write_lock(); + + list_for_each_entry(link, &dev->links.consumers, s_node) { + if (link->flags & DL_FLAG_STATELESS) + continue; + + /* + * The probe has failed, so if the status of the link is + * "consumer probe" or "active", it must have been added by + * a probing consumer while this device was still probing. + * Change its state to "dormant", as it represents a valid + * relationship, but it is not functionally meaningful. + */ + if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) + WRITE_ONCE(link->status, DL_STATE_DORMANT); + } + __device_links_no_driver(dev); + device_links_write_unlock(); } -- cgit v1.2.3 From a1fdbfbb1da2063ba98a12eb6f1bdd07451c7145 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:52:45 +0100 Subject: driver core: Do not call rpm_put_suppliers() in pm_runtime_drop_link() Calling rpm_put_suppliers() from pm_runtime_drop_link() is excessive as it affects all suppliers of the consumer device and not just the one pointed to by the device link being dropped. Worst case it may cause the consumer device to stop working unexpectedly. Moreover, in principle it is racy with respect to runtime PM of the consumer device. To avoid these problems drop runtime PM references on the particular supplier pointed to by the link in question only and do that after the link has been dropped from the consumer device's list of links to suppliers, which is in device_link_free(). Fixes: a0504aecba76 ("PM / runtime: Drop usage count for suppliers at device link removal") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 3 +++ drivers/base/power/runtime.c | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index d58ea075f807..8c7327d45406 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -374,6 +374,9 @@ EXPORT_SYMBOL_GPL(device_link_add); static void device_link_free(struct device_link *link) { + while (refcount_dec_not_one(&link->rpm_active)) + pm_runtime_put(link->supplier); + put_device(link->consumer); put_device(link->supplier); kfree(link); diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 8bc9a432de70..fd5c4a7b96f0 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1611,8 +1611,6 @@ void pm_runtime_new_link(struct device *dev) void pm_runtime_drop_link(struct device *dev) { - rpm_put_suppliers(dev); - spin_lock_irq(&dev->power.lock); WARN_ON(dev->power.links_count == 0); dev->power.links_count--; -- cgit v1.2.3 From ea4f640025183860792063ba5852aec60704ad8e Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:54:21 +0100 Subject: IOMMU: Make dwo drivers use stateless device links The device links used by rockchip-iommu and exynos-iommu are completely managed by these drivers within the IOMMU framework, so there is no reason to involve the driver core in the management of these links. For this reason, make rockchip-iommu and exynos-iommu pass DL_FLAG_STATELESS in flags to device_link_add(), so that the device links used by them are stateless. [Note that this change is requisite for a subsequent one that will rework the management of stateful device links in the driver core and it will not be compatible with the two drivers in question any more.] Signed-off-by: Rafael J. Wysocki Acked-by: Marek Szyprowski Acked-by: Joerg Roedel Signed-off-by: Greg Kroah-Hartman --- drivers/iommu/exynos-iommu.c | 1 + drivers/iommu/rockchip-iommu.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 1bd0cd7168df..05c6bc099d62 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1260,6 +1260,7 @@ static int exynos_iommu_add_device(struct device *dev) * direct calls to pm_runtime_get/put in this driver. */ data->link = device_link_add(dev, data->sysmmu, + DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME); } iommu_group_put(group); diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index c9ba9f377f63..77d4bd93fe4b 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1071,7 +1071,8 @@ static int rk_iommu_add_device(struct device *dev) iommu_group_put(group); iommu_device_link(&iommu->iommu, dev); - data->link = device_link_add(dev, iommu->dev, DL_FLAG_PM_RUNTIME); + data->link = device_link_add(dev, iommu->dev, + DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME); return 0; } -- cgit v1.2.3 From 72175d4ea4c442d95cf690c3e968eeee90fd43ca Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:58:33 +0100 Subject: driver core: Make driver core own stateful device links Even though stateful device links are managed by the driver core in principle, their creators are allowed and sometimes even expected to drop references to them via device_link_del() or device_link_remove(), but that doesn't really play well with the "persistent" link concept. If "persistent" managed device links are created from driver probe callbacks, device_link_add() called to do that will take a new reference on the link each time the callback runs and those references will never be dropped, which kind of isn't nice. This issues arises because of the link reference counting carried out by device_link_add() for existing links, but that is only done to avoid deleting device links that may still be necessary, which shouldn't be a concern for managed (stateful) links. These device links are managed by the driver core and whoever creates one of them will need it at least as long as until the consumer driver is detached from its device and deleting it may be left to the driver core just fine. For this reason, rework device_link_add() to apply the reference counting to stateless links only and make device_link_del() and device_link_remove() drop references to stateless links only too. After this change, if called to add a stateful device link for a consumer-supplier pair for which a stateful device link is present already, device_link_add() will return the existing link without incrementing its reference counter. Accordingly, device_link_del() and device_link_remove() will WARN() and do nothing when called to drop a reference to a stateful link. Thus, effectively, all stateful device links will be owned by the driver core. In addition, clean up the handling of the link management flags, DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER, so that (a) they are never set at the same time and (b) if device_link_add() is called for a consumer-supplier pair with an existing stateful link between them, the flags of that link will be combined with the flags passed to device_link_add() to ensure that the life time of the link is sufficient for all of the callers of device_link_add() for the same consumer-supplier pair. Update the device_link_add() kerneldoc comment to reflect the above changes. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- Documentation/driver-api/device_link.rst | 42 +++++++++++-------- drivers/base/core.c | 69 +++++++++++++++++++++++++------- 2 files changed, 79 insertions(+), 32 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index 5c7178189612..e249e074a8d2 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -25,8 +25,8 @@ suspend/resume and shutdown ordering. Device links allow representation of such dependencies in the driver core. -In its standard form, a device link combines *both* dependency types: -It guarantees correct suspend/resume and shutdown ordering between a +In its standard or *managed* form, a device link combines *both* dependency +types: It guarantees correct suspend/resume and shutdown ordering between a "supplier" device and its "consumer" devices, and it guarantees driver presence on the supplier. The consumer devices are not probed before the supplier is bound to a driver, and they're unbound before the supplier @@ -69,12 +69,14 @@ know that the supplier is functional already at the link creation time (that is the case, for instance, if the consumer has just acquired some resources that would not have been available had the supplier not been functional then).] -If a device link is added in the ``->probe`` callback of the supplier or -consumer driver, it is typically deleted in its ``->remove`` callback for -symmetry. That way, if the driver is compiled as a module, the device -link is added on module load and orderly deleted on unload. The same -restrictions that apply to device link addition (e.g. exclusion of a -parallel suspend/resume transition) apply equally to deletion. +If a device link with ``DL_FLAG_STATELESS`` set (i.e. a stateless device link) +is added in the ``->probe`` callback of the supplier or consumer driver, it is +typically deleted in its ``->remove`` callback for symmetry. That way, if the +driver is compiled as a module, the device link is added on module load and +orderly deleted on unload. The same restrictions that apply to device link +addition (e.g. exclusion of a parallel suspend/resume transition) apply equally +to deletion. Device links with ``DL_FLAG_STATELESS`` unset (i.e. managed +device links) are deleted automatically by the driver core. Several flags may be specified on device link addition, two of which have already been mentioned above: ``DL_FLAG_STATELESS`` to express that no @@ -87,8 +89,6 @@ link is added from the consumer's ``->probe`` callback: ``DL_FLAG_RPM_ACTIVE`` can be specified to runtime resume the supplier upon addition of the device link. ``DL_FLAG_AUTOREMOVE_CONSUMER`` causes the device link to be automatically purged when the consumer fails to probe or later unbinds. -This obviates the need to explicitly delete the link in the ``->remove`` -callback or in the error path of the ``->probe`` callback. Similarly, when the device link is added from supplier's ``->probe`` callback, ``DL_FLAG_AUTOREMOVE_SUPPLIER`` causes the device link to be automatically @@ -97,12 +97,20 @@ purged when the supplier fails to probe or later unbinds. Limitations =========== -Driver authors should be aware that a driver presence dependency (i.e. when -``DL_FLAG_STATELESS`` is not specified on link addition) may cause probing of -the consumer to be deferred indefinitely. This can become a problem if the -consumer is required to probe before a certain initcall level is reached. -Worse, if the supplier driver is blacklisted or missing, the consumer will -never be probed. +Driver authors should be aware that a driver presence dependency for managed +device links (i.e. when ``DL_FLAG_STATELESS`` is not specified on link addition) +may cause probing of the consumer to be deferred indefinitely. This can become +a problem if the consumer is required to probe before a certain initcall level +is reached. Worse, if the supplier driver is blacklisted or missing, the +consumer will never be probed. + +Moreover, managed device links cannot be deleted directly. They are deleted +by the driver core when they are not necessary any more in accordance with the +``DL_FLAG_AUTOREMOVE_CONSUMER`` and ``DL_FLAG_AUTOREMOVE_SUPPLIER`` flags. +However, stateless device links (i.e. device links with ``DL_FLAG_STATELESS`` +set) are expected to be removed by whoever called :c:func:`device_link_add()` +to add them with the help of either :c:func:`device_link_del()` or +:c:func:`device_link_remove()`. Sometimes drivers depend on optional resources. They are able to operate in a degraded mode (reduced feature set or performance) when those resources @@ -286,4 +294,4 @@ API === .. kernel-doc:: drivers/base/core.c - :functions: device_link_add device_link_del + :functions: device_link_add device_link_del device_link_remove diff --git a/drivers/base/core.c b/drivers/base/core.c index 8c7327d45406..9d49b461b1d9 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -192,10 +192,21 @@ static void device_link_rpm_prepare(struct device *consumer, * of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be * ignored. * - * If the DL_FLAG_AUTOREMOVE_CONSUMER flag is set, the link will be removed - * automatically when the consumer device driver unbinds from it. Analogously, - * if DL_FLAG_AUTOREMOVE_SUPPLIER is set in @flags, the link will be removed - * automatically when the supplier device driver unbinds from it. + * If DL_FLAG_STATELESS is set in @flags, the link is not going to be managed by + * the driver core and, in particular, the caller of this function is expected + * to drop the reference to the link acquired by it directly. + * + * If that flag is not set, however, the caller of this function is handing the + * management of the link over to the driver core entirely and its return value + * can only be used to check whether or not the link is present. In that case, + * the DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER device link + * flags can be used to indicate to the driver core when the link can be safely + * deleted. Namely, setting one of them in @flags indicates to the driver core + * that the link is not going to be used (by the given caller of this function) + * after unbinding the consumer or supplier driver, respectively, from its + * device, so the link can be deleted at that point. If none of them is set, + * the link will be maintained until one of the devices pointed to by it (either + * the consumer or the supplier) is unregistered. * * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and @@ -241,6 +252,14 @@ struct device_link *device_link_add(struct device *consumer, goto out; } + /* + * DL_FLAG_AUTOREMOVE_SUPPLIER indicates that the link will be needed + * longer than for DL_FLAG_AUTOREMOVE_CONSUMER and setting them both + * together doesn't make sense, so prefer DL_FLAG_AUTOREMOVE_SUPPLIER. + */ + if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) + flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER; + list_for_each_entry(link, &supplier->links.consumers, s_node) { if (link->consumer != consumer) continue; @@ -254,12 +273,6 @@ struct device_link *device_link_add(struct device *consumer, goto out; } - if (flags & DL_FLAG_AUTOREMOVE_CONSUMER) - link->flags |= DL_FLAG_AUTOREMOVE_CONSUMER; - - if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) - link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; - if (flags & DL_FLAG_PM_RUNTIME) { if (!(link->flags & DL_FLAG_PM_RUNTIME)) { device_link_rpm_prepare(consumer, supplier); @@ -269,7 +282,25 @@ struct device_link *device_link_add(struct device *consumer, refcount_inc(&link->rpm_active); } - kref_get(&link->kref); + if (flags & DL_FLAG_STATELESS) { + kref_get(&link->kref); + goto out; + } + + /* + * If the life time of the link following from the new flags is + * longer than indicated by the flags of the existing link, + * update the existing link to stay around longer. + */ + if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) { + if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) { + link->flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER; + link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; + } + } else if (!(flags & DL_FLAG_AUTOREMOVE_CONSUMER)) { + link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER); + } goto out; } @@ -419,8 +450,16 @@ static void __device_link_del(struct kref *kref) } #endif /* !CONFIG_SRCU */ +static void device_link_put_kref(struct device_link *link) +{ + if (link->flags & DL_FLAG_STATELESS) + kref_put(&link->kref, __device_link_del); + else + WARN(1, "Unable to drop a managed device link reference\n"); +} + /** - * device_link_del - Delete a link between two devices. + * device_link_del - Delete a stateless link between two devices. * @link: Device link to delete. * * The caller must ensure proper synchronization of this function with runtime @@ -432,14 +471,14 @@ void device_link_del(struct device_link *link) { device_links_write_lock(); device_pm_lock(); - kref_put(&link->kref, __device_link_del); + device_link_put_kref(link); device_pm_unlock(); device_links_write_unlock(); } EXPORT_SYMBOL_GPL(device_link_del); /** - * device_link_remove - remove a link between two devices. + * device_link_remove - Delete a stateless link between two devices. * @consumer: Consumer end of the link. * @supplier: Supplier end of the link. * @@ -458,7 +497,7 @@ void device_link_remove(void *consumer, struct device *supplier) list_for_each_entry(link, &supplier->links.consumers, s_node) { if (link->consumer == consumer) { - kref_put(&link->kref, __device_link_del); + device_link_put_kref(link); break; } } -- cgit v1.2.3 From e7dd40105aac9ba051e44ad711123bc53a5e4c71 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:59:42 +0100 Subject: driver core: Add device link flag DL_FLAG_AUTOPROBE_CONSUMER Add a new device link flag, DL_FLAG_AUTOPROBE_CONSUMER, to request the driver core to probe for a consumer driver automatically after binding a driver to the supplier device on a persistent managed device link. As unbinding the supplier driver on a managed device link causes the consumer driver to be detached from its device automatically, this flag provides a complementary mechanism which is needed to address some "composite device" use cases. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- Documentation/driver-api/device_link.rst | 9 +++++++++ drivers/base/core.c | 16 +++++++++++++++- drivers/base/dd.c | 2 +- include/linux/device.h | 3 +++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index e249e074a8d2..c764755121c7 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -94,6 +94,15 @@ Similarly, when the device link is added from supplier's ``->probe`` callback, ``DL_FLAG_AUTOREMOVE_SUPPLIER`` causes the device link to be automatically purged when the supplier fails to probe or later unbinds. +If neither ``DL_FLAG_AUTOREMOVE_CONSUMER`` nor ``DL_FLAG_AUTOREMOVE_SUPPLIER`` +is set, ``DL_FLAG_AUTOPROBE_CONSUMER`` can be used to request the driver core +to probe for a driver for the consumer driver on the link automatically after +a driver has been bound to the supplier device. + +Note, however, that any combinations of ``DL_FLAG_AUTOREMOVE_CONSUMER``, +``DL_FLAG_AUTOREMOVE_SUPPLIER`` or ``DL_FLAG_AUTOPROBE_CONSUMER`` with +``DL_FLAG_STATELESS`` are invalid and cannot be used. + Limitations =========== diff --git a/drivers/base/core.c b/drivers/base/core.c index 9d49b461b1d9..abfce4f613f8 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -208,6 +208,12 @@ static void device_link_rpm_prepare(struct device *consumer, * the link will be maintained until one of the devices pointed to by it (either * the consumer or the supplier) is unregistered. * + * Also, if DL_FLAG_STATELESS, DL_FLAG_AUTOREMOVE_CONSUMER and + * DL_FLAG_AUTOREMOVE_SUPPLIER are not set in @flags (that is, a persistent + * managed device link is being added), the DL_FLAG_AUTOPROBE_CONSUMER flag can + * be used to request the driver core to automaticall probe for a consmer + * driver after successfully binding a driver to the supplier device. + * * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and * will cause NULL to be returned upfront. @@ -228,7 +234,12 @@ struct device_link *device_link_add(struct device *consumer, if (!consumer || !supplier || (flags & DL_FLAG_STATELESS && - flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) + flags & (DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER | + DL_FLAG_AUTOPROBE_CONSUMER)) || + (flags & DL_FLAG_AUTOPROBE_CONSUMER && + flags & (DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER))) return NULL; if (flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) { @@ -589,6 +600,9 @@ void device_links_driver_bound(struct device *dev) WARN_ON(link->status != DL_STATE_DORMANT); WRITE_ONCE(link->status, DL_STATE_AVAILABLE); + + if (link->flags & DL_FLAG_AUTOPROBE_CONSUMER) + driver_deferred_probe_add(link->consumer); } list_for_each_entry(link, &dev->links.suppliers, c_node) { diff --git a/drivers/base/dd.c b/drivers/base/dd.c index aa6a9c613595..2e898cbba79b 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -116,7 +116,7 @@ static void deferred_probe_work_func(struct work_struct *work) } static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func); -static void driver_deferred_probe_add(struct device *dev) +void driver_deferred_probe_add(struct device *dev) { mutex_lock(&deferred_probe_mutex); if (list_empty(&dev->p->deferred_probe)) { diff --git a/include/linux/device.h b/include/linux/device.h index 5f49d2eff6ed..0ab0a3a80ec3 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -341,6 +341,7 @@ struct device *driver_find_device(struct device_driver *drv, struct device *start, void *data, int (*match)(struct device *dev, void *data)); +void driver_deferred_probe_add(struct device *dev); int driver_deferred_probe_check_state(struct device *dev); /** @@ -827,12 +828,14 @@ enum device_link_state { * PM_RUNTIME: If set, the runtime PM framework will use this link. * RPM_ACTIVE: Run pm_runtime_get_sync() on the supplier during link creation. * AUTOREMOVE_SUPPLIER: Remove the link automatically on supplier driver unbind. + * AUTOPROBE_CONSUMER: Probe consumer driver automatically after supplier binds. */ #define DL_FLAG_STATELESS BIT(0) #define DL_FLAG_AUTOREMOVE_CONSUMER BIT(1) #define DL_FLAG_PM_RUNTIME BIT(2) #define DL_FLAG_RPM_ACTIVE BIT(3) #define DL_FLAG_AUTOREMOVE_SUPPLIER BIT(4) +#define DL_FLAG_AUTOPROBE_CONSUMER BIT(5) /** * struct device_link - Device link representation. -- cgit v1.2.3 From 7cbc2b421ee33bdc17aede8bc5ff0dee72e78147 Mon Sep 17 00:00:00 2001 From: Stephen Rothwell Date: Fri, 1 Feb 2019 13:37:59 +1100 Subject: firmware: intel_stratix10_service: remove COMPILE_TEST This does not build yet ... Cc: Richard Gong Fixes: 095ff29d2b88 ("firmware: intel_stratix10_service: add hardware dependency") Signed-off-by: Stephen Rothwell Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 7e5491aed5c8..cac16c4b0df3 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -218,7 +218,7 @@ config FW_CFG_SYSFS_CMDLINE config INTEL_STRATIX10_SERVICE tristate "Intel Stratix10 Service Layer" - depends on (ARCH_STRATIX10 && HAVE_ARM_SMCCC) || COMPILE_TEST + depends on ARCH_STRATIX10 && HAVE_ARM_SMCCC default n help Intel Stratix10 service layer runs at privileged exception level, -- cgit v1.2.3 From 79a4e91d1bb2a411a4ce2baa93680fa707567003 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Sat, 2 Feb 2019 19:50:17 -0800 Subject: device.h: Add __cold to dev_ logging functions Add __cold to the dev_ logging functions similar to the use of __cold in the generic printk function. Using __cold moves all the dev_ logging functions out-of-line possibly improving code locality and runtime performance. Signed-off-by: Joe Perches Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/include/linux/device.h b/include/linux/device.h index 0ab0a3a80ec3..a36830e2d0e5 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1384,28 +1384,28 @@ void device_link_remove(void *consumer, struct device *supplier); #ifdef CONFIG_PRINTK -__printf(3, 0) +__printf(3, 0) __cold int dev_vprintk_emit(int level, const struct device *dev, const char *fmt, va_list args); -__printf(3, 4) +__printf(3, 4) __cold int dev_printk_emit(int level, const struct device *dev, const char *fmt, ...); -__printf(3, 4) +__printf(3, 4) __cold void dev_printk(const char *level, const struct device *dev, const char *fmt, ...); -__printf(2, 3) +__printf(2, 3) __cold void _dev_emerg(const struct device *dev, const char *fmt, ...); -__printf(2, 3) +__printf(2, 3) __cold void _dev_alert(const struct device *dev, const char *fmt, ...); -__printf(2, 3) +__printf(2, 3) __cold void _dev_crit(const struct device *dev, const char *fmt, ...); -__printf(2, 3) +__printf(2, 3) __cold void _dev_err(const struct device *dev, const char *fmt, ...); -__printf(2, 3) +__printf(2, 3) __cold void _dev_warn(const struct device *dev, const char *fmt, ...); -__printf(2, 3) +__printf(2, 3) __cold void _dev_notice(const struct device *dev, const char *fmt, ...); -__printf(2, 3) +__printf(2, 3) __cold void _dev_info(const struct device *dev, const char *fmt, ...); #else -- cgit v1.2.3 From 4080ab083000a1e9656b0d1607e238e7001e0c84 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 7 Feb 2019 19:38:56 +0100 Subject: PM-runtime: Take suppliers into account in __pm_runtime_set_status() If the target device has any suppliers, as reflected by device links to them, __pm_runtime_set_status() does not take them into account, which is not consistent with the other parts of the PM-runtime framework and may lead to programming mistakes. Modify __pm_runtime_set_status() to take suppliers into account by activating them upfront if the new status is RPM_ACTIVE and deactivating them on exit if the new status is RPM_SUSPENDED. If the activation of one of the suppliers fails, the new status will be RPM_SUSPENDED and the (remaining) suppliers will be deactivated on exit (the child count of the device's parent will be dropped too then). Of course, adding device links locking to __pm_runtime_set_status() means that it cannot be run fron interrupt context, so make it use spin_lock_irq() and spin_unlock_irq() instead of spin_lock_irqsave() and spin_unlock_irqrestore(), respectively. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/power/runtime.c | 45 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index fd5c4a7b96f0..a0e55065817b 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1089,20 +1089,43 @@ EXPORT_SYMBOL_GPL(pm_runtime_get_if_in_use); * and the device parent's counter of unsuspended children is modified to * reflect the new status. If the new status is RPM_SUSPENDED, an idle * notification request for the parent is submitted. + * + * If @dev has any suppliers (as reflected by device links to them), and @status + * is RPM_ACTIVE, they will be activated upfront and if the activation of one + * of them fails, the status of @dev will be changed to RPM_SUSPENDED (instead + * of the @status value) and the suppliers will be deacticated on exit. The + * error returned by the failing supplier activation will be returned in that + * case. */ int __pm_runtime_set_status(struct device *dev, unsigned int status) { struct device *parent = dev->parent; - unsigned long flags; bool notify_parent = false; int error = 0; if (status != RPM_ACTIVE && status != RPM_SUSPENDED) return -EINVAL; - spin_lock_irqsave(&dev->power.lock, flags); + /* + * If the new status is RPM_ACTIVE, the suppliers can be activated + * upfront regardless of the current status, because next time + * rpm_put_suppliers() runs, the rpm_active refcounts of the links + * involved will be dropped down to one anyway. + */ + if (status == RPM_ACTIVE) { + int idx = device_links_read_lock(); + + error = rpm_get_suppliers(dev); + if (error) + status = RPM_SUSPENDED; + + device_links_read_unlock(idx); + } + + spin_lock_irq(&dev->power.lock); if (!dev->power.runtime_error && !dev->power.disable_depth) { + status = dev->power.runtime_status; error = -EAGAIN; goto out; } @@ -1134,19 +1157,31 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) spin_unlock(&parent->power.lock); - if (error) + if (error) { + status = RPM_SUSPENDED; goto out; + } } out_set: __update_runtime_status(dev, status); - dev->power.runtime_error = 0; + if (!error) + dev->power.runtime_error = 0; + out: - spin_unlock_irqrestore(&dev->power.lock, flags); + spin_unlock_irq(&dev->power.lock); if (notify_parent) pm_request_idle(parent); + if (status == RPM_SUSPENDED) { + int idx = device_links_read_lock(); + + rpm_put_suppliers(dev); + + device_links_read_unlock(idx); + } + return error; } EXPORT_SYMBOL_GPL(__pm_runtime_set_status); -- cgit v1.2.3 From 70fb9a252317e04bd9af68ed89d4bf6caeb252e4 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 7 Feb 2019 19:41:56 +0100 Subject: driver core: Document limitation related to DL_FLAG_RPM_ACTIVE If device_link_add() is called twice in a row to create a stateless device link for the same consumer-supplier pair without an attempt to delete the link between these calls, and the second caller passes DL_FLAG_RPM_ACTIVE to it in flags, calling either device_link_del() or device_link_remove() immediately after that will leave the link's supplier device with nonzero PM-runtime usage counter, which may prevent the supplier from being runtime-suspended going forward until the link is deleted by another invocation of device_link_del() or device_link_remove() for it. Even though this is confusing and may lead to subtle issues, trying to avoid it in the framework also may cause problems to appear, so document it as a known limitation. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- Documentation/driver-api/device_link.rst | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index c764755121c7..0effb792a3af 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -86,9 +86,10 @@ integration is desired. Two other flags are specifically targeted at use cases where the device link is added from the consumer's ``->probe`` callback: ``DL_FLAG_RPM_ACTIVE`` -can be specified to runtime resume the supplier upon addition of the -device link. ``DL_FLAG_AUTOREMOVE_CONSUMER`` causes the device link to be -automatically purged when the consumer fails to probe or later unbinds. +can be specified to runtime resume the supplier and prevent it from suspending +before the consumer is runtime suspended. ``DL_FLAG_AUTOREMOVE_CONSUMER`` +causes the device link to be automatically purged when the consumer fails to +probe or later unbinds. Similarly, when the device link is added from supplier's ``->probe`` callback, ``DL_FLAG_AUTOREMOVE_SUPPLIER`` causes the device link to be automatically @@ -121,6 +122,20 @@ set) are expected to be removed by whoever called :c:func:`device_link_add()` to add them with the help of either :c:func:`device_link_del()` or :c:func:`device_link_remove()`. +Passing ``DL_FLAG_RPM_ACTIVE`` along with ``DL_FLAG_STATELESS`` to +:c:func:`device_link_add()` may cause the PM-runtime usage counter of the +supplier device to remain nonzero after a subsequent invocation of either +:c:func:`device_link_del()` or :c:func:`device_link_remove()` to remove the +device link returned by it. This happens if :c:func:`device_link_add()` is +called twice in a row for the same consumer-supplier pair without removing the +link between these calls, in which case allowing the PM-runtime usage counter +of the supplier to drop on an attempt to remove the link may cause it to be +suspended while the consumer is still PM-runtime-active and that has to be +avoided. [To work around this limitation it is sufficient to let the consumer +runtime suspend at least once, or call :c:func:`pm_runtime_set_suspended()` for +it with PM-runtime disabled, between the :c:func:`device_link_add()` and +:c:func:`device_link_del()` or :c:func:`device_link_remove()` calls.] + Sometimes drivers depend on optional resources. They are able to operate in a degraded mode (reduced feature set or performance) when those resources are not present. An example is an SPI controller that can use a DMA engine -- cgit v1.2.3 From 376991db4b6464e906d699ef07681e2ffa8ab08c Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Thu, 7 Feb 2019 20:36:53 +0100 Subject: driver core: Postpone DMA tear-down until after devres release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When unbinding the (IOMMU-enabled) R-Car SATA device on Salvator-XS (R-Car H3 ES2.0), in preparation of rebinding against vfio-platform for device pass-through for virtualization:     echo ee300000.sata > /sys/bus/platform/drivers/sata_rcar/unbind the kernel crashes with:     Unable to handle kernel paging request at virtual address ffffffbf029ffffc     Mem abort info:       ESR = 0x96000006       Exception class = DABT (current EL), IL = 32 bits       SET = 0, FnV = 0       EA = 0, S1PTW = 0     Data abort info:       ISV = 0, ISS = 0x00000006       CM = 0, WnR = 0     swapper pgtable: 4k pages, 39-bit VAs, pgdp = 000000007e8c586c     [ffffffbf029ffffc] pgd=000000073bfc6003, pud=000000073bfc6003, pmd=0000000000000000     Internal error: Oops: 96000006 [#1] SMP     Modules linked in:     CPU: 0 PID: 1098 Comm: bash Not tainted 5.0.0-rc5-salvator-x-00452-g37596f884f4318ef #287     Hardware name: Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+ (DT)     pstate: 60400005 (nZCv daif +PAN -UAO)     pc : __free_pages+0x8/0x58     lr : __dma_direct_free_pages+0x50/0x5c     sp : ffffff801268baa0     x29: ffffff801268baa0 x28: 0000000000000000     x27: ffffffc6f9c60bf0 x26: ffffffc6f9c60bf0     x25: ffffffc6f9c60810 x24: 0000000000000000     x23: 00000000fffff000 x22: ffffff8012145000     x21: 0000000000000800 x20: ffffffbf029fffc8     x19: 0000000000000000 x18: ffffffc6f86c42c8     x17: 0000000000000000 x16: 0000000000000070     x15: 0000000000000003 x14: 0000000000000000     x13: ffffff801103d7f8 x12: 0000000000000028     x11: ffffff8011117604 x10: 0000000000009ad8     x9 : ffffff80110126d0 x8 : ffffffc6f7563000     x7 : 6b6b6b6b6b6b6b6b x6 : 0000000000000018     x5 : ffffff8011cf3cc8 x4 : 0000000000004000     x3 : 0000000000080000 x2 : 0000000000000001     x1 : 0000000000000000 x0 : ffffffbf029fffc8     Process bash (pid: 1098, stack limit = 0x00000000c38e3e32)     Call trace:      __free_pages+0x8/0x58      __dma_direct_free_pages+0x50/0x5c      arch_dma_free+0x1c/0x98      dma_direct_free+0x14/0x24      dma_free_attrs+0x9c/0xdc      dmam_release+0x18/0x20      release_nodes+0x25c/0x28c      devres_release_all+0x48/0x4c      device_release_driver_internal+0x184/0x1f0      device_release_driver+0x14/0x1c      unbind_store+0x70/0xb8      drv_attr_store+0x24/0x34      sysfs_kf_write+0x4c/0x64      kernfs_fop_write+0x154/0x1c4      __vfs_write+0x34/0x164      vfs_write+0xb4/0x16c      ksys_write+0x5c/0xbc      __arm64_sys_write+0x14/0x1c      el0_svc_common+0x98/0x114      el0_svc_handler+0x1c/0x24      el0_svc+0x8/0xc     Code: d51b4234 17fffffa a9bf7bfd 910003fd (b9403404)     ---[ end trace 8c564cdd3a1a840f ]--- While I've bisected this to commit e8e683ae9a736407 ("iommu/of: Fix probe-deferral"), and reverting that commit on post-v5.0-rc4 kernels does fix the problem, this turned out to be a red herring. On arm64, arch_teardown_dma_ops() resets dev->dma_ops to NULL. Hence if a driver has used a managed DMA allocation API, the allocated DMA memory will be freed using the direct DMA ops, while it may have been allocated using a custom DMA ops (iommu_dma_ops in this case). Fix this by reversing the order of the calls to devres_release_all() and arch_teardown_dma_ops(). Signed-off-by: Geert Uytterhoeven Acked-by: Christoph Hellwig Reviewed-by: Rafael J. Wysocki Cc: stable Reviewed-by: Robin Murphy Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 2e898cbba79b..348fc4695d4d 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -1058,9 +1058,9 @@ static void __device_release_driver(struct device *dev, struct device *parent) drv->remove(dev); device_links_driver_cleanup(dev); - arch_teardown_dma_ops(dev); devres_release_all(dev); + arch_teardown_dma_ops(dev); dev->driver = NULL; dev_set_drvdata(dev, NULL); if (dev->pm_domain && dev->pm_domain->dismiss) -- cgit v1.2.3 From 5b2f2bd62e79f6bc38fda5a0ff6a699f7612a7d2 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 4 Feb 2019 15:08:00 +0100 Subject: sysfs: remove unused include of kernfs-internal.h This include is not needed (fs/sysfs/file.c builds just fine without it). Remove it. Cc: Tejun Heo Signed-off-by: Ondrej Mosnacek Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 52d9235e0291..130fc6fbcc03 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -17,7 +17,6 @@ #include #include "sysfs.h" -#include "../kernfs/kernfs-internal.h" /* * Determine ktype->sysfs_ops for the given kernfs_node. This function -- cgit v1.2.3 From 26e28d68b146ec17ecc1b1833077bc712de19637 Mon Sep 17 00:00:00 2001 From: Ayush Mittal Date: Wed, 6 Feb 2019 10:25:42 +0530 Subject: kernfs: Allocating memory for kernfs_iattrs with kmem_cache. Creating a new cache for kernfs_iattrs. Currently, memory is allocated with kzalloc() which always gives aligned memory. On ARM, this is 64 byte aligned. To avoid the wastage of memory in aligning the size requested, a new cache for kernfs_iattrs is created. Size of struct kernfs_iattrs is 80 Bytes. On ARM, it will come in kmalloc-128 slab. and it will come in kmalloc-192 slab if debug info is enabled. Extra bytes taken 48 bytes. Total number of objects created : 4096 Total saving = 48*4096 = 192 KB After creating new slab(When debug info is enabled) : sh-3.2# cat /proc/slabinfo ... kernfs_iattrs_cache 4069 4096 128 32 1 : tunables 0 0 0 : slabdata 128 128 0 ... All testing has been done on ARM target. Signed-off-by: Ayush Mittal Signed-off-by: Vaneet Narang Acked-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/dir.c | 2 +- fs/kernfs/inode.c | 2 +- fs/kernfs/kernfs-internal.h | 2 +- fs/kernfs/mount.c | 7 ++++++- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 4ca0b5c18192..b84d635567d3 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -536,8 +536,8 @@ void kernfs_put(struct kernfs_node *kn) security_release_secctx(kn->iattr->ia_secdata, kn->iattr->ia_secdata_len); simple_xattrs_free(&kn->iattr->xattrs); + kmem_cache_free(kernfs_iattrs_cache, kn->iattr); } - kfree(kn->iattr); spin_lock(&kernfs_idr_lock); idr_remove(&root->ino_idr, kn->id.ino); spin_unlock(&kernfs_idr_lock); diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c index 80cebcd94c90..0c1fd945ce42 100644 --- a/fs/kernfs/inode.c +++ b/fs/kernfs/inode.c @@ -42,7 +42,7 @@ static struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn) if (kn->iattr) goto out_unlock; - kn->iattr = kzalloc(sizeof(struct kernfs_iattrs), GFP_KERNEL); + kn->iattr = kmem_cache_zalloc(kernfs_iattrs_cache, GFP_KERNEL); if (!kn->iattr) goto out_unlock; iattrs = &kn->iattr->ia_iattr; diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h index 3d83b114bb08..dba810cd83b1 100644 --- a/fs/kernfs/kernfs-internal.h +++ b/fs/kernfs/kernfs-internal.h @@ -78,7 +78,7 @@ static inline struct kernfs_node *kernfs_dentry_node(struct dentry *dentry) } extern const struct super_operations kernfs_sops; -extern struct kmem_cache *kernfs_node_cache; +extern struct kmem_cache *kernfs_node_cache, *kernfs_iattrs_cache; /* * inode.c diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c index fdf527b6d79c..450eb388bae6 100644 --- a/fs/kernfs/mount.c +++ b/fs/kernfs/mount.c @@ -20,7 +20,7 @@ #include "kernfs-internal.h" -struct kmem_cache *kernfs_node_cache; +struct kmem_cache *kernfs_node_cache, *kernfs_iattrs_cache; static int kernfs_sop_remount_fs(struct super_block *sb, int *flags, char *data) { @@ -417,4 +417,9 @@ void __init kernfs_init(void) 0, SLAB_PANIC | SLAB_TYPESAFE_BY_RCU, NULL); + + /* Creates slab cache for kernfs inode attributes */ + kernfs_iattrs_cache = kmem_cache_create("kernfs_iattrs_cache", + sizeof(struct kernfs_iattrs), + 0, SLAB_PANIC, NULL); } -- cgit v1.2.3 From 2c6f4fc884a46b17c501e7f276e8a4ab97437b50 Mon Sep 17 00:00:00 2001 From: David Engraf Date: Tue, 5 Feb 2019 13:19:52 +0100 Subject: device: Fix comment for driver_data in struct device dev_set_drvdata/dev_get_drvdata is used to access driver_data in struct device. Signed-off-by: David Engraf Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/device.h b/include/linux/device.h index a36830e2d0e5..292b720c4bc2 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -988,7 +988,7 @@ struct device { void *platform_data; /* Platform specific data, device core doesn't touch it */ void *driver_data; /* Driver data, set and get with - dev_set/get_drvdata */ + dev_set_drvdata/dev_get_drvdata */ struct dev_links_info links; struct dev_pm_info power; struct dev_pm_domain *pm_domain; -- cgit v1.2.3 From d2b284d356e9758d2bafd505d482e3c9433ef424 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Thu, 7 Feb 2019 11:06:00 -0800 Subject: Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config" This reverts commit 7492902e8d22b568463897fa967c0886764cf034. The commit tried to address an issue discovered by Dan where he got a message saying: 'usermode helper disabled so ignoring test'. Dans's commit is forcing CONFIG_FW_LOADER_USER_HELPER_FALLBACK but just having CONFIG_FW_LOADER_USER_HELPER suffices to emulate the_FALLBACK functionality. Dan's commit is trying to fix an issue which is hidden from a previous commit. That issue will be addressed properly next. Fixes: 7492902e8d22 ("selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config") Cc: stable Signed-off-by: Luis Chamberlain Signed-off-by: Greg Kroah-Hartman --- tools/testing/selftests/firmware/config | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/testing/selftests/firmware/config b/tools/testing/selftests/firmware/config index 913a25a4a32b..bf634dda0720 100644 --- a/tools/testing/selftests/firmware/config +++ b/tools/testing/selftests/firmware/config @@ -1,6 +1,5 @@ CONFIG_TEST_FIRMWARE=y CONFIG_FW_LOADER=y CONFIG_FW_LOADER_USER_HELPER=y -CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y -- cgit v1.2.3 From 13ac7db09c914e4991a08b7ad578267d5cdd9856 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Thu, 7 Feb 2019 11:06:01 -0800 Subject: Revert "selftests: firmware: remove use of non-standard diff -Z option" This reverts commit f70b472e937bb659a7b7a14e64f07308e230888c. This breaks testing on Debian, and this patch was NACKed anyway. The proper way to address this is a quirk for busybox as that is where the issue is present. Signed-off-by: Luis Chamberlain Fixes: f70b472e937b ("selftests: firmware: remove use of non-standard diff -Z option") Cc: stable Signed-off-by: Greg Kroah-Hartman --- tools/testing/selftests/firmware/fw_filesystem.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index 466cf2f91ba0..a4320c4b44dc 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -155,8 +155,11 @@ read_firmwares() { for i in $(seq 0 3); do config_set_read_fw_idx $i - # Verify the contents match - if ! diff -q "$FW" $DIR/read_firmware 2>/dev/null ; then + # Verify the contents are what we expect. + # -Z required for now -- check for yourself, md5sum + # on $FW and DIR/read_firmware will yield the same. Even + # cmp agrees, so something is off. + if ! diff -q -Z "$FW" $DIR/read_firmware 2>/dev/null ; then echo "request #$i: firmware was not loaded" >&2 exit 1 fi @@ -168,7 +171,7 @@ read_firmwares_expect_nofile() for i in $(seq 0 3); do config_set_read_fw_idx $i # Ensures contents differ - if diff -q "$FW" $DIR/read_firmware 2>/dev/null ; then + if diff -q -Z "$FW" $DIR/read_firmware 2>/dev/null ; then echo "request $i: file was not expected to match" >&2 exit 1 fi -- cgit v1.2.3 From 344c0152d878922365464b7140c74c2a5e073d99 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Thu, 7 Feb 2019 11:06:02 -0800 Subject: selftests: firmware: fix verify_reqs() return value commit a6a9be9270c87 ("selftests: firmware: return Kselftest Skip code for skipped tests") by Shuah modified failures to return the special error code of $ksft_skip (4). We have a corner case issue where we *do* want to verify_reqs(). Cc: # >= 4.18 Fixes: a6a9be9270c87 ("selftests: firmware: return Kselftest Skip code for for skipped tests") Signed-off-by: Luis Chamberlain Signed-off-by: Greg Kroah-Hartman --- tools/testing/selftests/firmware/fw_lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh index 6c5f1b2ffb74..1cbb12e284a6 100755 --- a/tools/testing/selftests/firmware/fw_lib.sh +++ b/tools/testing/selftests/firmware/fw_lib.sh @@ -91,7 +91,7 @@ verify_reqs() if [ "$TEST_REQS_FW_SYSFS_FALLBACK" = "yes" ]; then if [ ! "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then echo "usermode helper disabled so ignoring test" - exit $ksft_skip + exit 0 fi fi } -- cgit v1.2.3 From daaef255dc96834aaaad627d3271504cba3ac2dc Mon Sep 17 00:00:00 2001 From: Enrico Granata Date: Mon, 11 Feb 2019 11:01:12 -0800 Subject: driver: platform: Support parsing GpioInt 0 in platform_get_irq() ACPI 5 added support for GpioInt resources as a way to provide information about interrupts mediated via a GPIO controller. Several device buses (e.g. SPI, I2C) have support for retrieving an IRQ specified via this type of resource, and providing it directly to the driver as an IRQ number. This is not currently done for the platform drivers, as platform_get_irq() does not try to parse GpioInt() resources. This requires drivers to either have to support only one possible IRQ resource, or to have code in place to try both as a failsafe. While there is a possibility of ambiguity for devices that exposes multiple IRQs, it is easy and feasible to support the common case of devices that only expose one IRQ which would be of either type depending on the underlying system's architecture. This commit adds support for parsing a GpioInt resource in order to fulfill a request for the index 0 IRQ for a platform device. Signed-off-by: Enrico Granata Reviewed-by: Dmitry Torokhov Acked-by: Hans de Goede Reviewed-by: Mika Westerberg Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 1c958eb33ef4..0d3611cd1b3b 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -127,7 +127,20 @@ int platform_get_irq(struct platform_device *dev, unsigned int num) irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS); } - return r ? r->start : -ENXIO; + if (r) + return r->start; + + /* + * For the index 0 interrupt, allow falling back to GpioInt + * resources. While a device could have both Interrupt and GpioInt + * resources, making this fallback ambiguous, in many common cases + * the device will only expose one IRQ, and this fallback + * allows a common code path across either kind of resource. + */ + if (num == 0 && has_acpi_companion(&dev->dev)) + return acpi_dev_gpio_irq_get(ACPI_COMPANION(&dev->dev), num); + + return -ENXIO; #endif } EXPORT_SYMBOL_GPL(platform_get_irq); -- cgit v1.2.3 From c1567f813a9992a64f8d0f6cfb912c3922812c35 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 12 Feb 2019 13:04:12 +0100 Subject: PM-runtime: Fix __pm_runtime_set_status() race with runtime resume Commit 4080ab083000 ("PM-runtime: Take suppliers into account in __pm_runtime_set_status()") introduced a race condition that may trigger if __pm_runtime_set_status() is used incorrectly (that is, if it is called when PM-runtime is enabled for the target device and working). In that case, if the original PM-runtime status of the device is RPM_SUSPENDED, a runtime resume of the device may occur after __pm_runtime_set_status() has dropped its power.lock spinlock and before deactivating its suppliers, so the suppliers may be deactivated while the device is PM-runtime-active which may lead to functional issues. To avoid that, modify __pm_runtime_set_status() to check whether or not PM-runtime is enabled for the device before activating its suppliers (if the new status is RPM_ACTIVE) and either return an error if that's the case or increment the device's disable_depth counter to prevent PM-runtime from being enabled for it while the remaining part of the function is running (disable_depth is then decremented on the way out). Fixes: 4080ab083000 ("PM-runtime: Take suppliers into account in __pm_runtime_set_status()") Signed-off-by: Rafael J. Wysocki Reviewed-by: Ulf Hansson Signed-off-by: Greg Kroah-Hartman --- drivers/base/power/runtime.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index ac54f65b6d63..af23eb327f57 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1106,6 +1106,22 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) if (status != RPM_ACTIVE && status != RPM_SUSPENDED) return -EINVAL; + spin_lock_irq(&dev->power.lock); + + /* + * Prevent PM-runtime from being enabled for the device or return an + * error if it is enabled already and working. + */ + if (dev->power.runtime_error || dev->power.disable_depth) + dev->power.disable_depth++; + else + error = -EAGAIN; + + spin_unlock_irq(&dev->power.lock); + + if (error) + return error; + /* * If the new status is RPM_ACTIVE, the suppliers can be activated * upfront regardless of the current status, because next time @@ -1124,12 +1140,6 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) spin_lock_irq(&dev->power.lock); - if (!dev->power.runtime_error && !dev->power.disable_depth) { - status = dev->power.runtime_status; - error = -EAGAIN; - goto out; - } - if (dev->power.runtime_status == status || !parent) goto out_set; @@ -1182,6 +1192,8 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) device_links_read_unlock(idx); } + pm_runtime_enable(dev); + return error; } EXPORT_SYMBOL_GPL(__pm_runtime_set_status); -- cgit v1.2.3 From 4c06c4e6cf63d7f3d5dfe62593a073253d750a59 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 12 Feb 2019 13:08:10 +0100 Subject: driver core: Fix possible supplier PM-usage counter imbalance If a stateless device link to a certain supplier with DL_FLAG_PM_RUNTIME set in the flags is added and then removed by the consumer driver's probe callback, the supplier's PM-runtime usage counter will be nonzero after that which effectively causes the supplier to remain "always on" going forward. Namely, device_link_add() called to add the link invokes device_link_rpm_prepare() which notices that the consumer driver is probing, so it increments the supplier's PM-runtime usage counter with the assumption that the link will stay around until pm_runtime_put_suppliers() is called by driver_probe_device(), but if the link goes away before that point, the supplier's PM-runtime usage counter will remain nonzero. To prevent that from happening, first rework pm_runtime_get_suppliers() and pm_runtime_put_suppliers() to use the rpm_active refounts of device links and make the latter only drop rpm_active and the supplier's PM-runtime usage counter for each link by one, unless rpm_active is one already for it. Next, modify device_link_add() to bump up the new link's rpm_active refcount and the suppliers PM-runtime usage counter by two, to prevent pm_runtime_put_suppliers(), if it is called subsequently, from suspending the supplier prematurely (in case its PM-runtime usage counter goes down to 0 in there). Due to the way rpm_put_suppliers() works, this change does not affect runtime suspend of the consumer ends of new device links (or, generally, device links for which DL_FLAG_PM_RUNTIME has just been set). Fixes: e2f3cd831a28 ("driver core: Fix handling of runtime PM flags in device_link_add()") Reported-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki Reviewed-by: Ulf Hansson Tested-by: Ulf Hansson Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 21 ++++----------------- drivers/base/power/runtime.c | 27 +++++++++++++++++++++++++-- include/linux/pm_runtime.h | 4 ++++ 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index abfce4f613f8..787190238753 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -165,19 +165,6 @@ void device_pm_move_to_tail(struct device *dev) device_links_read_unlock(idx); } -static void device_link_rpm_prepare(struct device *consumer, - struct device *supplier) -{ - pm_runtime_new_link(consumer); - /* - * If the link is being added by the consumer driver at probe time, - * balance the decrementation of the supplier's runtime PM usage counter - * after consumer probe in driver_probe_device(). - */ - if (consumer->links.status == DL_DEV_PROBING) - pm_runtime_get_noresume(supplier); -} - /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. @@ -286,11 +273,11 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_PM_RUNTIME) { if (!(link->flags & DL_FLAG_PM_RUNTIME)) { - device_link_rpm_prepare(consumer, supplier); + pm_runtime_new_link(consumer); link->flags |= DL_FLAG_PM_RUNTIME; } if (flags & DL_FLAG_RPM_ACTIVE) - refcount_inc(&link->rpm_active); + pm_runtime_active_link(link, supplier); } if (flags & DL_FLAG_STATELESS) { @@ -323,9 +310,9 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_PM_RUNTIME) { if (flags & DL_FLAG_RPM_ACTIVE) - refcount_inc(&link->rpm_active); + pm_runtime_active_link(link, supplier); - device_link_rpm_prepare(consumer, supplier); + pm_runtime_new_link(consumer); } get_device(supplier); diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index af23eb327f57..6b8aa6bed064 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1625,8 +1625,10 @@ void pm_runtime_get_suppliers(struct device *dev) idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME) + if (link->flags & DL_FLAG_PM_RUNTIME) { + refcount_inc(&link->rpm_active); pm_runtime_get_sync(link->supplier); + } device_links_read_unlock(idx); } @@ -1643,7 +1645,8 @@ void pm_runtime_put_suppliers(struct device *dev) idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME) + if (link->flags & DL_FLAG_PM_RUNTIME && + refcount_dec_not_one(&link->rpm_active)) pm_runtime_put(link->supplier); device_links_read_unlock(idx); @@ -1656,6 +1659,26 @@ void pm_runtime_new_link(struct device *dev) spin_unlock_irq(&dev->power.lock); } +/** + * pm_runtime_active_link - Set up new device link as active for PM-runtime. + * @link: Device link to be set up as active. + * @supplier: Supplier end of the link. + * + * Add 2 to the rpm_active refcount of @link and increment the PM-runtime + * usage counter of @supplier once more in case the link is being added while + * the consumer driver is probing and pm_runtime_put_suppliers() will be called + * subsequently. + * + * Note that this doesn't prevent rpm_put_suppliers() from decreasing the link's + * rpm_active refcount down to one, so runtime suspend of the consumer end of + * @link is not affected. + */ +void pm_runtime_active_link(struct device_link *link, struct device *supplier) +{ + refcount_add(2, &link->rpm_active); + pm_runtime_get_noresume(supplier); +} + void pm_runtime_drop_link(struct device *dev) { spin_lock_irq(&dev->power.lock); diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index fed5be706bc9..a27bbb5937b8 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -59,6 +59,8 @@ extern void pm_runtime_clean_up_links(struct device *dev); extern void pm_runtime_get_suppliers(struct device *dev); extern void pm_runtime_put_suppliers(struct device *dev); extern void pm_runtime_new_link(struct device *dev); +extern void pm_runtime_active_link(struct device_link *link, + struct device *supplier); extern void pm_runtime_drop_link(struct device *dev); static inline void pm_suspend_ignore_children(struct device *dev, bool enable) @@ -176,6 +178,8 @@ static inline void pm_runtime_clean_up_links(struct device *dev) {} static inline void pm_runtime_get_suppliers(struct device *dev) {} static inline void pm_runtime_put_suppliers(struct device *dev) {} static inline void pm_runtime_new_link(struct device *dev) {} +static inline void pm_runtime_active_link(struct device_link *link, + struct device *supplier) {} static inline void pm_runtime_drop_link(struct device *dev) {} #endif /* !CONFIG_PM */ -- cgit v1.2.3 From 1ea61b68d0f8685775c897c2de040c73b8d1c56a Mon Sep 17 00:00:00 2001 From: Feng Tang Date: Wed, 13 Feb 2019 15:47:36 +0800 Subject: async: Add cmdline option to specify drivers to be async probed Asynchronous driver probing can help much on kernel fastboot, and this option can provide a flexible way to optimize and quickly verify async driver probe. Also it will help in below cases: * Some driver actually covers several families of HWs, some of which could use async probing while others don't. So we can't simply turn on the PROBE_PREFER_ASYNCHRONOUS flag in driver, but use this cmdline option, like igb driver async patch discussed at https://www.spinics.net/lists/netdev/msg545986.html * For SOC (System on Chip) with multiple spi or i2c controllers, most of the slave spi/i2c devices will be assigned with fixed controller number, while async probing may make those controllers get different index for each boot, which prevents those controller drivers to be async probed. For platforms not using these spi/i2c slave devices, they can use this cmdline option to benefit from the async probing. Suggested-by: Alexander Duyck Cc: Randy Dunlap Signed-off-by: Feng Tang Signed-off-by: Greg Kroah-Hartman --- Documentation/admin-guide/kernel-parameters.txt | 4 ++++ drivers/base/dd.c | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 858b6c0b9a15..dba164bbcc1a 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -910,6 +910,10 @@ The filter can be disabled or changed to another driver later using sysfs. + driver_async_probe= [KNL] + List of driver names to be probed asynchronously. + Format: ,... + drm.edid_firmware=[:][,[:]] Broken monitors, graphic adapters, KVMs and EDIDless panels may send no or incorrect EDID data sets. diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 348fc4695d4d..a823f469e53f 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -57,6 +57,10 @@ static atomic_t deferred_trigger_count = ATOMIC_INIT(0); static struct dentry *deferred_devices; static bool initcalls_done; +/* Save the async probe drivers' name from kernel cmdline */ +#define ASYNC_DRV_NAMES_MAX_LEN 256 +static char async_probe_drv_names[ASYNC_DRV_NAMES_MAX_LEN]; + /* * In some cases, like suspend to RAM or hibernation, It might be reasonable * to prohibit probing of devices as it could be unsafe. @@ -674,6 +678,23 @@ int driver_probe_device(struct device_driver *drv, struct device *dev) return ret; } +static inline bool cmdline_requested_async_probing(const char *drv_name) +{ + return parse_option_str(async_probe_drv_names, drv_name); +} + +/* The option format is "driver_async_probe=drv_name1,drv_name2,..." */ +static int __init save_async_options(char *buf) +{ + if (strlen(buf) >= ASYNC_DRV_NAMES_MAX_LEN) + printk(KERN_WARNING + "Too long list of driver names for 'driver_async_probe'!\n"); + + strlcpy(async_probe_drv_names, buf, ASYNC_DRV_NAMES_MAX_LEN); + return 0; +} +__setup("driver_async_probe=", save_async_options); + bool driver_allows_async_probing(struct device_driver *drv) { switch (drv->probe_type) { @@ -684,6 +705,9 @@ bool driver_allows_async_probing(struct device_driver *drv) return false; default: + if (cmdline_requested_async_probing(drv->name)) + return true; + if (module_requested_async_probing(drv->owner)) return true; -- cgit v1.2.3 From e4246b05507fc6102008bac0aee848f207bd96de Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 18 Feb 2019 17:36:48 +0100 Subject: drivers/component: kerneldoc polish Polish the kerneldoc a bit with suggestions from Randy. v2: Randy found another typo: s/compent/component/ Signed-off-by: Daniel Vetter Cc: "Rafael J. Wysocki" Cc: Daniel Vetter Cc: Ramalingam C Acked-by: Randy Dunlap Signed-off-by: Greg Kroah-Hartman --- drivers/base/component.c | 14 +++++++------- include/linux/component.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/base/component.c b/drivers/base/component.c index 7dbc41cccd58..532a3a5d8f63 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -27,7 +27,7 @@ * helper fills the niche of aggregate drivers for specific hardware, where * further standardization into a subsystem would not be practical. The common * example is when a logical device (e.g. a DRM display driver) is spread around - * the SoC on various component (scanout engines, blending blocks, transcoders + * the SoC on various components (scanout engines, blending blocks, transcoders * for various outputs and so on). * * The component helper also doesn't solve runtime dependencies, e.g. for system @@ -378,7 +378,7 @@ static void __component_match_add(struct device *master, } /** - * component_match_add_release - add a component match with release callback + * component_match_add_release - add a component match entry with release callback * @master: device with the aggregate driver * @matchptr: pointer to the list of component matches * @release: release function for @compare_data @@ -408,7 +408,7 @@ void component_match_add_release(struct device *master, EXPORT_SYMBOL(component_match_add_release); /** - * component_match_add_typed - add a compent match for a typed component + * component_match_add_typed - add a component match entry for a typed component * @master: device with the aggregate driver * @matchptr: pointer to the list of component matches * @compare_typed: compare function to match against all typed components @@ -537,11 +537,11 @@ static void component_unbind(struct component *component, } /** - * component_unbind_all - unbind all component to an aggregate driver + * component_unbind_all - unbind all components of an aggregate driver * @master_dev: device with the aggregate driver * @data: opaque pointer, passed to all components * - * Unbinds all components to the aggregate @dev by passing @data to their + * Unbinds all components of the aggregate @dev by passing @data to their * &component_ops.unbind functions. Should be called from * &component_master_ops.unbind. */ @@ -619,11 +619,11 @@ static int component_bind(struct component *component, struct master *master, } /** - * component_bind_all - bind all component to an aggregate driver + * component_bind_all - bind all components of an aggregate driver * @master_dev: device with the aggregate driver * @data: opaque pointer, passed to all components * - * Binds all components to the aggregate @dev by passing @data to their + * Binds all components of the aggregate @dev by passing @data to their * &component_ops.bind functions. Should be called from * &component_master_ops.bind. */ diff --git a/include/linux/component.h b/include/linux/component.h index 30bcc7e590eb..16de18f473d7 100644 --- a/include/linux/component.h +++ b/include/linux/component.h @@ -98,7 +98,7 @@ void component_match_add_typed(struct device *master, int (*compare_typed)(struct device *, int, void *), void *compare_data); /** - * component_match_add - add a compent match + * component_match_add - add a component match entry * @master: device with the aggregate driver * @matchptr: pointer to the list of component matches * @compare: compare function to match against all components -- cgit v1.2.3 From 36003d4cf57ca431fb3f94d317bcca426a2394d6 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 19 Feb 2019 17:53:26 +0100 Subject: driver core: Fix PM-runtime for links added during consumer probe Commit 4c06c4e6cf63 ("driver core: Fix possible supplier PM-usage counter imbalance") introduced a regression that causes suppliers to be suspended prematurely for device links added during consumer driver probe if the initial PM-runtime status of the consumer is "suspended" and the consumer is resumed after adding the link and before pm_runtime_put_suppliers() is called. In that case, pm_runtime_put_suppliers() will drop the rpm_active refcount for the link by one and (since rpm_active is equal to two after the preceding consumer resume) the supplier's PM-runtime usage counter will be decremented, which may cause the supplier to suspend even though the consumer's PM-runtime status is "active". For this reason, partially revert commit 4c06c4e6cf63 as the problem it tried to fix needs to be addressed somewhat differently, and change pm_runtime_get_suppliers() and pm_runtime_put_suppliers() so that the latter only drops rpm_active references acquired by the former. [This requires adding a new field to struct device_link, but I coulnd't find a cleaner way to address the issue that would work in all cases.] This causes pm_runtime_put_suppliers() to effectively ignore device links added during consumer probe, so device_link_add() doesn't need to worry about ensuring that suppliers will remain active after pm_runtime_put_suppliers() for links created with DL_FLAG_RPM_ACTIVE set and it only needs to bump up rpm_active by one for those links, so pm_runtime_active_link() is not necessary any more. Fixes: 4c06c4e6cf63 ("driver core: Fix possible supplier PM-usage counter imbalance") Reported-by: Jon Hunter Tested-by: Jon Hunter Tested-by: Ulf Hansson Reviewed-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki Tested-by: Thierry Reding Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 4 ++-- drivers/base/power/runtime.c | 29 ++++++----------------------- include/linux/device.h | 1 + include/linux/pm_runtime.h | 4 ---- 4 files changed, 9 insertions(+), 29 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 787190238753..4aeaa0c92bda 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -277,7 +277,7 @@ struct device_link *device_link_add(struct device *consumer, link->flags |= DL_FLAG_PM_RUNTIME; } if (flags & DL_FLAG_RPM_ACTIVE) - pm_runtime_active_link(link, supplier); + refcount_inc(&link->rpm_active); } if (flags & DL_FLAG_STATELESS) { @@ -310,7 +310,7 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_PM_RUNTIME) { if (flags & DL_FLAG_RPM_ACTIVE) - pm_runtime_active_link(link, supplier); + refcount_inc(&link->rpm_active); pm_runtime_new_link(consumer); } diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 6b8aa6bed064..70d2cb188601 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1626,6 +1626,7 @@ void pm_runtime_get_suppliers(struct device *dev) list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) if (link->flags & DL_FLAG_PM_RUNTIME) { + link->supplier_preactivated = true; refcount_inc(&link->rpm_active); pm_runtime_get_sync(link->supplier); } @@ -1645,9 +1646,11 @@ void pm_runtime_put_suppliers(struct device *dev) idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME && - refcount_dec_not_one(&link->rpm_active)) - pm_runtime_put(link->supplier); + if (link->supplier_preactivated) { + link->supplier_preactivated = false; + if (refcount_dec_not_one(&link->rpm_active)) + pm_runtime_put(link->supplier); + } device_links_read_unlock(idx); } @@ -1659,26 +1662,6 @@ void pm_runtime_new_link(struct device *dev) spin_unlock_irq(&dev->power.lock); } -/** - * pm_runtime_active_link - Set up new device link as active for PM-runtime. - * @link: Device link to be set up as active. - * @supplier: Supplier end of the link. - * - * Add 2 to the rpm_active refcount of @link and increment the PM-runtime - * usage counter of @supplier once more in case the link is being added while - * the consumer driver is probing and pm_runtime_put_suppliers() will be called - * subsequently. - * - * Note that this doesn't prevent rpm_put_suppliers() from decreasing the link's - * rpm_active refcount down to one, so runtime suspend of the consumer end of - * @link is not affected. - */ -void pm_runtime_active_link(struct device_link *link, struct device *supplier) -{ - refcount_add(2, &link->rpm_active); - pm_runtime_get_noresume(supplier); -} - void pm_runtime_drop_link(struct device *dev) { spin_lock_irq(&dev->power.lock); diff --git a/include/linux/device.h b/include/linux/device.h index 292b720c4bc2..a7967a48cdc9 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -861,6 +861,7 @@ struct device_link { #ifdef CONFIG_SRCU struct rcu_head rcu_head; #endif + bool supplier_preactivated; /* Owned by consumer probe. */ }; /** diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index a27bbb5937b8..fed5be706bc9 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -59,8 +59,6 @@ extern void pm_runtime_clean_up_links(struct device *dev); extern void pm_runtime_get_suppliers(struct device *dev); extern void pm_runtime_put_suppliers(struct device *dev); extern void pm_runtime_new_link(struct device *dev); -extern void pm_runtime_active_link(struct device_link *link, - struct device *supplier); extern void pm_runtime_drop_link(struct device *dev); static inline void pm_suspend_ignore_children(struct device *dev, bool enable) @@ -178,8 +176,6 @@ static inline void pm_runtime_clean_up_links(struct device *dev) {} static inline void pm_runtime_get_suppliers(struct device *dev) {} static inline void pm_runtime_put_suppliers(struct device *dev) {} static inline void pm_runtime_new_link(struct device *dev) {} -static inline void pm_runtime_active_link(struct device_link *link, - struct device *supplier) {} static inline void pm_runtime_drop_link(struct device *dev) {} #endif /* !CONFIG_PM */ -- cgit v1.2.3 From a7013ba5a9302cbded1c45ab48003c6346584a4d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 21 Feb 2019 12:28:05 +0100 Subject: driver core: Add missing description of new struct device_link field Commit 36003d4cf57c ("driver core: Fix PM-runtime for links added during consumer probe") forgot to add a kerneldoc decription for the new struct device_link member added by it, so do that now. Fixes: 36003d4cf57c ("driver core: Fix PM-runtime for links added during consumer probe") Reported-by: kbuild test robot Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/device.h b/include/linux/device.h index a7967a48cdc9..163b5898ac78 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -848,6 +848,7 @@ enum device_link_state { * @rpm_active: Whether or not the consumer device is runtime-PM-active. * @kref: Count repeated addition of the same link. * @rcu_head: An RCU head to use for deferred execution of SRCU callbacks. + * @supplier_preactivated: Supplier has been made active before consumer probe. */ struct device_link { struct device *supplier; -- cgit v1.2.3 From eac473bce4b73a089880fb164b8badef81f57fbc Mon Sep 17 00:00:00 2001 From: John Zhao Date: Wed, 20 Feb 2019 10:39:04 +0800 Subject: firmware: hardcode the debug message for -ENOENT When no file /path was found, the error code of -ENOENT enumerated in errno-base.h, is returned. Stating clearly that the file was not found is much more useful for debugging, So let's be explicit about that. Signed-off-by: John Zhao Acked-by: Luis Chamberlain Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 8e9213b36e31..7eaaf5ee5ba6 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -328,12 +328,12 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv) rc = kernel_read_file_from_path(path, &fw_priv->data, &size, msize, id); if (rc) { - if (rc == -ENOENT) - dev_dbg(device, "loading %s failed with error %d\n", - path, rc); - else + if (rc != -ENOENT) dev_warn(device, "loading %s failed with error %d\n", path, rc); + else + dev_dbg(device, "loading %s failed for no such file or directory.\n", + path); continue; } dev_dbg(device, "direct-loading %s\n", fw_priv->fw_name); -- cgit v1.2.3 From 2c1ea6abde8884208a9b94254740ae4597c62000 Mon Sep 17 00:00:00 2001 From: Mans Rullgard Date: Thu, 21 Feb 2019 11:29:35 +0000 Subject: platform: set of_node in platform_device_register_full() If the provided fwnode is an OF node, set dev.of_node as well. Also add an of_node_reused flag to struct platform_device_info and copy this to the new device. This is needed to avoid pinctrl settings being requested twice. See 4e75e1d7dac9 ("driver core: add helper to reuse a device-tree node") for a longer explanation. Some drivers are just shims that create extra "glue" devices with the DT device as parent and have the real driver bind to these. In these cases, the glue device needs to get a reference to the original DT node in order for the main driver to access properties and child nodes. For example, the sunxi-musb driver creates such a glue device using platform_device_register_full(). Consequently, devices attached to this USB interface don't get associated with DT nodes, if present, the way they do with EHCI. This change will allow sunxi-musb and similar drivers to easily propagate the DT node to child devices as required. Signed-off-by: Mans Rullgard Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 2 ++ include/linux/platform_device.h | 1 + 2 files changed, 3 insertions(+) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 0d3611cd1b3b..fc67a325beaa 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -525,6 +525,8 @@ struct platform_device *platform_device_register_full( pdev->dev.parent = pdevinfo->parent; pdev->dev.fwnode = pdevinfo->fwnode; + pdev->dev.of_node = of_node_get(to_of_node(pdev->dev.fwnode)); + pdev->dev.of_node_reused = pdevinfo->of_node_reused; if (pdevinfo->dma_mask) { /* diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index c7c081dc6034..466a8d02e298 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -63,6 +63,7 @@ extern int platform_add_devices(struct platform_device **, int); struct platform_device_info { struct device *parent; struct fwnode_handle *fwnode; + bool of_node_reused; const char *name; int id; -- cgit v1.2.3 From 36cf3b1363f464c40f6ce647d3ac0ae9617d5fbc Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 1 Mar 2019 13:24:47 +0100 Subject: driver core: platform: remove misleading err_alloc label In platform_device_register_full() the err_alloc label is misleading, we only ever jump to it if the pdev is NULL, but it then proceeds to free it, which is a no-op. Remove the label and simply exit the function immediately. Signed-off-by: Johannes Berg Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index fc67a325beaa..4e45ac21d672 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -521,7 +521,7 @@ struct platform_device *platform_device_register_full( pdev = platform_device_alloc(pdevinfo->name, pdevinfo->id); if (!pdev) - goto err_alloc; + return ERR_PTR(-ENOMEM); pdev->dev.parent = pdevinfo->parent; pdev->dev.fwnode = pdevinfo->fwnode; @@ -568,8 +568,6 @@ struct platform_device *platform_device_register_full( err: ACPI_COMPANION_SET(&pdev->dev, NULL); kfree(pdev->dev.dma_mask); - -err_alloc: platform_device_put(pdev); return ERR_PTR(ret); } -- cgit v1.2.3