From 4e6b9319bce7a2be878d79bcbe2cb558b619b360 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Tue, 30 Jul 2013 08:40:23 -0400 Subject: [PATCH 1/6] firewire: WQ_NON_REENTRANT is meaningless and going away dbf2576e37 ("workqueue: make all workqueues non-reentrant") made WQ_NON_REENTRANT no-op and the flag is going away. Remove its usages. This patch doesn't introduce any behavior changes. Signed-off-by: Tejun Heo Signed-off-by: Stefan Richter --- drivers/firewire/core-transaction.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 28a94c7ec6e5..e5af0e3a26ec 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -1262,8 +1262,7 @@ static int __init fw_core_init(void) { int ret; - fw_workqueue = alloc_workqueue("firewire", - WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0); + fw_workqueue = alloc_workqueue("firewire", WQ_MEM_RECLAIM, 0); if (!fw_workqueue) return -ENOMEM; From 0a41981803fcd4107fff4e943afb72940ba653d2 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Mon, 29 Apr 2013 00:16:14 +0200 Subject: [PATCH 2/6] firewire: core: typecast from gfp_t to bool more safely An idr related patch introduced the following sparse warning: drivers/firewire/core-cdev.c:488:33: warning: incorrect type in initializer (different base types) drivers/firewire/core-cdev.c:488:33: expected bool [unsigned] [usertype] preload drivers/firewire/core-cdev.c:488:33: got restricted gfp_t So let's convert from gfp_t bitfield to Boolean explicitly and safely. Signed-off-by: Stefan Richter --- drivers/firewire/core-cdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index ac1b43a04285..d7d5c8af92b9 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -486,7 +486,7 @@ static int ioctl_get_info(struct client *client, union ioctl_arg *arg) static int add_client_resource(struct client *client, struct client_resource *resource, gfp_t gfp_mask) { - bool preload = gfp_mask & __GFP_WAIT; + bool preload = !!(gfp_mask & __GFP_WAIT); unsigned long flags; int ret; From af53122a2a6239ef235e55cedc324499e31dad87 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Mon, 5 Aug 2013 15:10:38 +0200 Subject: [PATCH 3/6] firewire: ohci: change confusing name of a struct member We have got struct descriptor *descriptors; dma_addr_t descriptors_bus; dma_addr_t buffer_bus; struct descriptor buffer[0]; void *misc_buffer; dma_addr_t misc_buffer_bus; __be32 *config_rom; dma_addr_t config_rom_bus; __be32 *next_config_rom; dma_addr_t next_config_rom_bus; But then we have got __le32 *self_id_cpu; dma_addr_t self_id_bus; Better apply the pattern of xyz vs. xyz_bus to self_id vs. self_id_bus as well. The _cpu suffix looks particularly weird in conversions from little endian to CPU endian. Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index afb701ec90ca..4dc65679312c 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -235,7 +235,7 @@ struct fw_ohci { dma_addr_t next_config_rom_bus; __be32 next_header; - __le32 *self_id_cpu; + __le32 *self_id; dma_addr_t self_id_bus; struct work_struct bus_reset_work; @@ -1929,12 +1929,12 @@ static void bus_reset_work(struct work_struct *work) return; } - generation = (cond_le32_to_cpu(ohci->self_id_cpu[0]) >> 16) & 0xff; + generation = (cond_le32_to_cpu(ohci->self_id[0]) >> 16) & 0xff; rmb(); for (i = 1, j = 0; j < self_id_count; i += 2, j++) { - u32 id = cond_le32_to_cpu(ohci->self_id_cpu[i]); - u32 id2 = cond_le32_to_cpu(ohci->self_id_cpu[i + 1]); + u32 id = cond_le32_to_cpu(ohci->self_id[i]); + u32 id2 = cond_le32_to_cpu(ohci->self_id[i + 1]); if (id != ~id2) { /* @@ -3692,7 +3692,7 @@ static int pci_probe(struct pci_dev *dev, goto fail_contexts; } - ohci->self_id_cpu = ohci->misc_buffer + PAGE_SIZE/2; + ohci->self_id = ohci->misc_buffer + PAGE_SIZE/2; ohci->self_id_bus = ohci->misc_buffer_bus + PAGE_SIZE/2; bus_options = reg_read(ohci, OHCI1394_BusOptions); From 0dbe15f88be5b2cdf4ca4145797861dfb0d583a5 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Mon, 5 Aug 2013 15:14:36 +0200 Subject: [PATCH 4/6] firewire: ohci: beautify some macro definitions a) Sort device IDs by vendor -- device -- revision. b) Write quirk flags in hexadecimal. This affects the user-visible output of "modinfo firewire-ohci". Since more flags have been added recently, it is now easier to cope with them in hexadecimal represen- tation. Besides, the device-specific combination of quirk flags is shown in hexadecimal in the kernel log too. (And firewire-sbp2 presents its own quirk flags in modinfo as hexadecimals as well.) Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 4dc65679312c..04e6eb127b73 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -271,6 +271,7 @@ static inline struct fw_ohci *fw_ohci(struct fw_card *card) static char ohci_driver_name[] = KBUILD_MODNAME; +#define PCI_VENDOR_ID_PINNACLE_SYSTEMS 0x11bd #define PCI_DEVICE_ID_AGERE_FW643 0x5901 #define PCI_DEVICE_ID_CREATIVE_SB1394 0x4001 #define PCI_DEVICE_ID_JMICRON_JMB38X_FW 0x2380 @@ -278,17 +279,16 @@ static char ohci_driver_name[] = KBUILD_MODNAME; #define PCI_DEVICE_ID_TI_TSB12LV26 0x8020 #define PCI_DEVICE_ID_TI_TSB82AA2 0x8025 #define PCI_DEVICE_ID_VIA_VT630X 0x3044 -#define PCI_VENDOR_ID_PINNACLE_SYSTEMS 0x11bd #define PCI_REV_ID_VIA_VT6306 0x46 -#define QUIRK_CYCLE_TIMER 1 -#define QUIRK_RESET_PACKET 2 -#define QUIRK_BE_HEADERS 4 -#define QUIRK_NO_1394A 8 -#define QUIRK_NO_MSI 16 -#define QUIRK_TI_SLLZ059 32 -#define QUIRK_IR_WAKE 64 -#define QUIRK_PHY_LCTRL_TIMEOUT 128 +#define QUIRK_CYCLE_TIMER 0x1 +#define QUIRK_RESET_PACKET 0x2 +#define QUIRK_BE_HEADERS 0x4 +#define QUIRK_NO_1394A 0x8 +#define QUIRK_NO_MSI 0x10 +#define QUIRK_TI_SLLZ059 0x20 +#define QUIRK_IR_WAKE 0x40 +#define QUIRK_PHY_LCTRL_TIMEOUT 0x80 /* In case of multiple matches in ohci_quirks[], only the first one is used. */ static const struct { From 7a723c6ed9e92bf91db5c65542c78106030afdbe Mon Sep 17 00:00:00 2001 From: Stephan Gatzka Date: Mon, 26 Aug 2013 20:50:04 +0200 Subject: [PATCH 5/6] firewire: ohci: Change module_pci_driver to module_init/module_exit This is a prerequisite to allocate a per driver self_id workqueue. This reverts the ohci.c part of patch fe2af11c220c7bb3a67f7aec0594811e5c59e019. Signed-off-by: Stephan Gatzka Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 04e6eb127b73..80830d6fe46b 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3870,7 +3870,18 @@ static struct pci_driver fw_ohci_pci_driver = { #endif }; -module_pci_driver(fw_ohci_pci_driver); +static int __init fw_ohci_init(void) +{ + return pci_register_driver(&fw_ohci_pci_driver); +} + +static void __exit fw_ohci_cleanup(void) +{ + pci_unregister_driver(&fw_ohci_pci_driver); +} + +module_init(fw_ohci_init); +module_exit(fw_ohci_cleanup); MODULE_AUTHOR("Kristian Hoegsberg "); MODULE_DESCRIPTION("Driver for PCI OHCI IEEE1394 controllers"); From db9ae8fec7b19f0ac6c60d998cac968d801a998d Mon Sep 17 00:00:00 2001 From: Stephan Gatzka Date: Mon, 26 Aug 2013 20:50:05 +0200 Subject: [PATCH 6/6] firewire: ohci: Fix deadlock at bus reset Put bus_reset_work into its own workqueue. By doing this, forward progress of bus_reset_work() is guaranteed if the work is switched over to a rescuer thread. Switching work to a rescuer thread happens if a new worker thread could not be allocated in certain time (MAYDAY_INITIAL_TIMEOUT, typically 10 ms). This might not be possible under high memory pressure or even on a heavily loaded embedded system running a slow serial console. The former deadlock occured in the following situation: The rescuer thread ran fw_device_init->read_config_rom->read_rom->fw_run_transaction. fw_run_transaction blocked waiting for the completion object. This completion object would have been completed in bus_reset_work, but this work was never executed in the rescuer thread due to its strictly sequential behaviour. [Stefan R.: Removed WQ_NON_REENTRANT flag from allocation because it is no longer needed in current kernels. Add it back if you backport to kernels older than 3.7, i.e. one which does not contain dbf2576e37da "workqueue: make all workqueues non-reentrant". Swapped order of destroy_workqueue and pci_unregister_driver.] Signed-off-by: Stephan Gatzka Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 80830d6fe46b..6aa8a86cb83b 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -242,6 +242,8 @@ struct fw_ohci { u32 self_id_buffer[512]; }; +static struct workqueue_struct *selfid_workqueue; + static inline struct fw_ohci *fw_ohci(struct fw_card *card) { return container_of(card, struct fw_ohci, card); @@ -2087,7 +2089,7 @@ static irqreturn_t irq_handler(int irq, void *data) log_irqs(ohci, event); if (event & OHCI1394_selfIDComplete) - queue_work(fw_workqueue, &ohci->bus_reset_work); + queue_work(selfid_workqueue, &ohci->bus_reset_work); if (event & OHCI1394_RQPkt) tasklet_schedule(&ohci->ar_request_ctx.tasklet); @@ -3872,12 +3874,17 @@ static struct pci_driver fw_ohci_pci_driver = { static int __init fw_ohci_init(void) { + selfid_workqueue = alloc_workqueue(KBUILD_MODNAME, WQ_MEM_RECLAIM, 0); + if (!selfid_workqueue) + return -ENOMEM; + return pci_register_driver(&fw_ohci_pci_driver); } static void __exit fw_ohci_cleanup(void) { pci_unregister_driver(&fw_ohci_pci_driver); + destroy_workqueue(selfid_workqueue); } module_init(fw_ohci_init);