linux1394-devel Mailing List for IEEE 1394 for Linux (Page 3)
Brought to you by:
aeb,
bencollins
You can subscribe to this list here.
2000 |
Jan
|
Feb
|
Mar
(39) |
Apr
(154) |
May
(172) |
Jun
(237) |
Jul
(127) |
Aug
(135) |
Sep
(193) |
Oct
(175) |
Nov
(173) |
Dec
(148) |
---|---|---|---|---|---|---|---|---|---|---|---|---|
2001 |
Jan
(161) |
Feb
(225) |
Mar
(193) |
Apr
(158) |
May
(179) |
Jun
(292) |
Jul
(146) |
Aug
(134) |
Sep
(185) |
Oct
(190) |
Nov
(149) |
Dec
(161) |
2002 |
Jan
(186) |
Feb
(236) |
Mar
(254) |
Apr
(207) |
May
(189) |
Jun
(182) |
Jul
(202) |
Aug
(155) |
Sep
(149) |
Oct
(449) |
Nov
(191) |
Dec
(108) |
2003 |
Jan
(174) |
Feb
(242) |
Mar
(243) |
Apr
(255) |
May
(202) |
Jun
(290) |
Jul
(237) |
Aug
(178) |
Sep
(101) |
Oct
(153) |
Nov
(144) |
Dec
(95) |
2004 |
Jan
(162) |
Feb
(278) |
Mar
(282) |
Apr
(152) |
May
(127) |
Jun
(138) |
Jul
(94) |
Aug
(63) |
Sep
(64) |
Oct
(150) |
Nov
(102) |
Dec
(197) |
2005 |
Jan
(102) |
Feb
(172) |
Mar
(89) |
Apr
(158) |
May
(139) |
Jun
(160) |
Jul
(288) |
Aug
(89) |
Sep
(201) |
Oct
(92) |
Nov
(190) |
Dec
(139) |
2006 |
Jan
(121) |
Feb
(204) |
Mar
(133) |
Apr
(134) |
May
(91) |
Jun
(226) |
Jul
(122) |
Aug
(101) |
Sep
(144) |
Oct
(141) |
Nov
|
Dec
|
2023 |
Jan
(19) |
Feb
(1) |
Mar
(5) |
Apr
(5) |
May
(33) |
Jun
(17) |
Jul
|
Aug
|
Sep
(3) |
Oct
(1) |
Nov
(5) |
Dec
(40) |
2024 |
Jan
(26) |
Feb
(14) |
Mar
(26) |
Apr
(46) |
May
(17) |
Jun
(47) |
Jul
(23) |
Aug
(72) |
Sep
(33) |
Oct
|
Nov
|
Dec
|
From: Takashi S. <o-t...@sa...> - 2024-08-12 23:52:28
|
It depends on the function assigned to release member to identify resource structure. This commit adds a helper function to identify iso_resource structure. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index e72f91cc3711..6fe2a2ea9869 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -161,6 +161,11 @@ static struct iso_resource *to_iso_resource(struct client_resource *resource) static void release_iso_resource(struct client *, struct client_resource *); +static int is_iso_resource(const struct client_resource *resource) +{ + return resource->release == release_iso_resource; +} + static void schedule_iso_resource(struct iso_resource *r, unsigned long delay) { client_get(r->client); @@ -170,7 +175,7 @@ static void schedule_iso_resource(struct iso_resource *r, unsigned long delay) static void schedule_if_iso_resource(struct client_resource *resource) { - if (resource->release == release_iso_resource) + if (is_iso_resource(resource)) schedule_iso_resource(to_iso_resource(resource), 0); } -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-12 23:52:26
|
Current implementation checks and validates the result to find resource entry two times. It is redundant. This commit refactors the redundancy. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index c211bb19c94e..81fdb2be9063 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -512,15 +512,14 @@ static int release_client_resource(struct client *client, u32 handle, scoped_guard(spinlock_irq, &client->lock) { if (client->in_shutdown) - resource = NULL; - else - resource = idr_find(&client->resource_idr, handle); - if (resource && resource->release == release) - idr_remove(&client->resource_idr, handle); - } + return -EINVAL; - if (!(resource && resource->release == release)) - return -EINVAL; + resource = idr_find(&client->resource_idr, handle); + if (!resource || resource->release != release) + return -EINVAL; + + idr_remove(&client->resource_idr, handle); + } if (return_resource) *return_resource = resource; -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-12 23:52:22
|
All of local resource structure commonly have data of client_resource type in its first member. This design sometimes requires usage of container_of to retrieve parent structure by the first member. This commit adds some helper functions for this purpose. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 38 ++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 81fdb2be9063..e72f91cc3711 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -139,6 +139,26 @@ struct iso_resource { struct iso_resource_event *e_alloc, *e_dealloc; }; +static struct address_handler_resource *to_address_handler_resource(struct client_resource *resource) +{ + return container_of(resource, struct address_handler_resource, resource); +} + +static struct inbound_transaction_resource *to_inbound_transaction_resource(struct client_resource *resource) +{ + return container_of(resource, struct inbound_transaction_resource, resource); +} + +static struct descriptor_resource *to_descriptor_resource(struct client_resource *resource) +{ + return container_of(resource, struct descriptor_resource, resource); +} + +static struct iso_resource *to_iso_resource(struct client_resource *resource) +{ + return container_of(resource, struct iso_resource, resource); +} + static void release_iso_resource(struct client *, struct client_resource *); static void schedule_iso_resource(struct iso_resource *r, unsigned long delay) @@ -151,8 +171,7 @@ static void schedule_iso_resource(struct iso_resource *r, unsigned long delay) static void schedule_if_iso_resource(struct client_resource *resource) { if (resource->release == release_iso_resource) - schedule_iso_resource(container_of(resource, - struct iso_resource, resource), 0); + schedule_iso_resource(to_iso_resource(resource), 0); } /* @@ -682,8 +701,7 @@ static int ioctl_send_request(struct client *client, union ioctl_arg *arg) static void release_request(struct client *client, struct client_resource *resource) { - struct inbound_transaction_resource *r = container_of(resource, - struct inbound_transaction_resource, resource); + struct inbound_transaction_resource *r = to_inbound_transaction_resource(resource); if (r->is_fcp) fw_request_put(r->request); @@ -793,8 +811,7 @@ static void handle_request(struct fw_card *card, struct fw_request *request, static void release_address_handler(struct client *client, struct client_resource *resource) { - struct address_handler_resource *r = - container_of(resource, struct address_handler_resource, resource); + struct address_handler_resource *r = to_address_handler_resource(resource); fw_core_remove_address_handler(&r->handler); kfree(r); @@ -858,8 +875,7 @@ static int ioctl_send_response(struct client *client, union ioctl_arg *arg) release_request, &resource) < 0) return -EINVAL; - r = container_of(resource, struct inbound_transaction_resource, - resource); + r = to_inbound_transaction_resource(resource); if (r->is_fcp) { fw_request_put(r->request); goto out; @@ -893,8 +909,7 @@ static int ioctl_initiate_bus_reset(struct client *client, union ioctl_arg *arg) static void release_descriptor(struct client *client, struct client_resource *resource) { - struct descriptor_resource *r = - container_of(resource, struct descriptor_resource, resource); + struct descriptor_resource *r = to_descriptor_resource(resource); fw_core_remove_descriptor(&r->descriptor); kfree(r); @@ -1387,8 +1402,7 @@ static void iso_resource_work(struct work_struct *work) static void release_iso_resource(struct client *client, struct client_resource *resource) { - struct iso_resource *r = - container_of(resource, struct iso_resource, resource); + struct iso_resource *r = to_iso_resource(resource); guard(spinlock_irq)(&client->lock); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-12 23:52:20
|
Hi, In core function, the instances of several types of client resources are maintained by IDR, and the index value of each resource is passed to user space application as handle. As of kernel v6.0, IDR has been superseded by XArray and deprecated. This series of changes is to obsolete the usage of IDR with XArray. Takashi Sakamoto (5): firewire: core: minor code refactoring to release client resource firewire: core: add helper functions to convert to parent resource structure firewire: core: add helper function to detect data of iso resource structure firewire: core: code refactoring to use idr_for_each_entry() macro instead of idr_for_each() function firewire: core: use xarray instead of idr to maintain client resource drivers/firewire/core-cdev.c | 170 +++++++++++++++++++---------------- drivers/firewire/core.h | 1 - 2 files changed, 94 insertions(+), 77 deletions(-) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-12 01:43:10
|
The data of XArray structure includes spinlock and requires no external lock, while the data is still under the critical section by fw_device_rwsem. This commit deletes the critical section. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-device.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 9f3276aa463a..32ac0f115793 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -806,7 +806,6 @@ static int shutdown_unit(struct device *device, void *data) /* * fw_device_rwsem acts as dual purpose mutex: - * - serializes accesses to fw_device_idr, * - serializes accesses to fw_device.config_rom/.config_rom_length and * fw_unit.directory, unless those accesses happen at safe occasions */ @@ -819,8 +818,6 @@ struct fw_device *fw_device_get_by_devt(dev_t devt) { struct fw_device *device; - guard(rwsem_read)(&fw_device_rwsem); - device = xa_load(&fw_device_xa, MINOR(devt)); if (device) fw_device_get(device); @@ -874,8 +871,7 @@ static void fw_device_shutdown(struct work_struct *work) device_for_each_child(&device->device, NULL, shutdown_unit); device_unregister(&device->device); - scoped_guard(rwsem_write, &fw_device_rwsem) - xa_erase(&fw_device_xa, MINOR(device->device.devt)); + xa_erase(&fw_device_xa, MINOR(device->device.devt)); fw_device_put(device); } @@ -1087,12 +1083,10 @@ static void fw_device_init(struct work_struct *work) fw_device_get(device); - scoped_guard(rwsem_write, &fw_device_rwsem) { - // The index of allocated entry is used for minor identifier of device node. - ret = xa_alloc(&fw_device_xa, &minor, device, XA_LIMIT(0, MINORMASK), GFP_KERNEL); - if (ret < 0) - goto error; - } + // The index of allocated entry is used for minor identifier of device node. + ret = xa_alloc(&fw_device_xa, &minor, device, XA_LIMIT(0, MINORMASK), GFP_KERNEL); + if (ret < 0) + goto error; device->device.bus = &fw_bus_type; device->device.type = &fw_device_type; @@ -1152,8 +1146,7 @@ static void fw_device_init(struct work_struct *work) return; error_with_cdev: - scoped_guard(rwsem_write, &fw_device_rwsem) - xa_erase(&fw_device_xa, minor); + xa_erase(&fw_device_xa, minor); error: fw_device_put(device); // fw_device_xa's reference. -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-12 01:43:08
|
Hi, In core function, the instances of fw_device corresponding to firewire device node in system are maintained by IDR. As of kernel v6.0, IDR has been superseded by XArray and deprecated. This series of changes is to obsolete the usage of IDR with XArray. Takashi Sakamoto (2): firewire: core: replace IDR with XArray to maintain fw_device firewire: core: use lock in Xarray instead of local R/W semaphore drivers/firewire/core-device.c | 30 ++++++++++++----------------- drivers/firewire/core-transaction.c | 3 +-- drivers/firewire/core.h | 3 ++- 3 files changed, 15 insertions(+), 21 deletions(-) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-12 01:43:08
|
In core function, the instances of fw_device corresponding to firewire device node in system are maintained by IDR. As of kernel v6.0, IDR has been superseded by XArray and deprecated. This commit replaces the usage of IDR with XArray to maintain the device instances. The instance of XArray is allocated statically, and initialized with XA_FLAGS_ALLOC so that the index of allocated entry starts with zero and available as the minor identifier of device node. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-device.c | 21 +++++++++++---------- drivers/firewire/core-transaction.c | 3 +-- drivers/firewire/core.h | 3 ++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index bec7e05f6ab8..9f3276aa463a 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -12,7 +12,6 @@ #include <linux/errno.h> #include <linux/firewire.h> #include <linux/firewire-constants.h> -#include <linux/idr.h> #include <linux/jiffies.h> #include <linux/kobject.h> #include <linux/list.h> @@ -813,7 +812,7 @@ static int shutdown_unit(struct device *device, void *data) */ DECLARE_RWSEM(fw_device_rwsem); -DEFINE_IDR(fw_device_idr); +DEFINE_XARRAY_ALLOC(fw_device_xa); int fw_cdev_major; struct fw_device *fw_device_get_by_devt(dev_t devt) @@ -822,7 +821,7 @@ struct fw_device *fw_device_get_by_devt(dev_t devt) guard(rwsem_read)(&fw_device_rwsem); - device = idr_find(&fw_device_idr, MINOR(devt)); + device = xa_load(&fw_device_xa, MINOR(devt)); if (device) fw_device_get(device); @@ -858,7 +857,6 @@ static void fw_device_shutdown(struct work_struct *work) { struct fw_device *device = container_of(work, struct fw_device, work.work); - int minor = MINOR(device->device.devt); if (time_before64(get_jiffies_64(), device->card->reset_jiffies + SHUTDOWN_DELAY) @@ -877,7 +875,7 @@ static void fw_device_shutdown(struct work_struct *work) device_unregister(&device->device); scoped_guard(rwsem_write, &fw_device_rwsem) - idr_remove(&fw_device_idr, minor); + xa_erase(&fw_device_xa, MINOR(device->device.devt)); fw_device_put(device); } @@ -1049,7 +1047,8 @@ static void fw_device_init(struct work_struct *work) container_of(work, struct fw_device, work.work); struct fw_card *card = device->card; struct device *revived_dev; - int minor, ret; + u32 minor; + int ret; /* * All failure paths here set node->data to NULL, so that we @@ -1087,9 +1086,11 @@ static void fw_device_init(struct work_struct *work) device_initialize(&device->device); fw_device_get(device); + scoped_guard(rwsem_write, &fw_device_rwsem) { - minor = idr_alloc(&fw_device_idr, device, 0, 1 << MINORBITS, GFP_KERNEL); - if (minor < 0) + // The index of allocated entry is used for minor identifier of device node. + ret = xa_alloc(&fw_device_xa, &minor, device, XA_LIMIT(0, MINORMASK), GFP_KERNEL); + if (ret < 0) goto error; } @@ -1152,9 +1153,9 @@ static void fw_device_init(struct work_struct *work) error_with_cdev: scoped_guard(rwsem_write, &fw_device_rwsem) - idr_remove(&fw_device_idr, minor); + xa_erase(&fw_device_xa, minor); error: - fw_device_put(device); /* fw_device_idr's reference */ + fw_device_put(device); // fw_device_xa's reference. put_device(&device->device); /* our reference */ } diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 14af84541e83..e141d24a7644 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -13,7 +13,6 @@ #include <linux/firewire-constants.h> #include <linux/fs.h> #include <linux/init.h> -#include <linux/idr.h> #include <linux/jiffies.h> #include <linux/kernel.h> #include <linux/list.h> @@ -1359,7 +1358,7 @@ static void __exit fw_core_cleanup(void) unregister_chrdev(fw_cdev_major, "firewire"); bus_unregister(&fw_bus_type); destroy_workqueue(fw_workqueue); - idr_destroy(&fw_device_idr); + xa_destroy(&fw_device_xa); } module_init(fw_core_init); diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index 189e15e6ba82..8cace026090c 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -8,6 +8,7 @@ #include <linux/fs.h> #include <linux/list.h> #include <linux/idr.h> +#include <linux/xarray.h> #include <linux/mm_types.h> #include <linux/rwsem.h> #include <linux/slab.h> @@ -133,7 +134,7 @@ void fw_cdev_handle_phy_packet(struct fw_card *card, struct fw_packet *p); /* -device */ extern struct rw_semaphore fw_device_rwsem; -extern struct idr fw_device_idr; +extern struct xarray fw_device_xa; extern int fw_cdev_major; static inline struct fw_device *fw_device_get(struct fw_device *device) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-10 22:33:34
|
On Sat, Aug 10, 2024 at 04:04:03PM +0900, Takashi Sakamoto wrote: > A commit d8527cab6c31 ("firewire: cdev: implement new event to notify > response subaction with time stamp") adds an additional case, > FW_CDEV_EVENT_RESPONSE2, into switch statement in complete_transaction(). > However, the range of block is beyond to the case label and reaches > neibour default label. > > This commit corrects the range of block. Fortunately, it has few impacts > in practice since the local variable in the scope under the label is not > used in codes under default label. > > Fixes: d8527cab6c31 ("firewire: cdev: implement new event to notify response subaction with time stamp") > Signed-off-by: Takashi Sakamoto <o-t...@sa...> > --- > 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 672a37fa8343..c211bb19c94e 100644 > --- a/drivers/firewire/core-cdev.c > +++ b/drivers/firewire/core-cdev.c > @@ -589,11 +589,11 @@ static void complete_transaction(struct fw_card *card, int rcode, u32 request_ts > queue_event(client, &e->event, rsp, sizeof(*rsp) + rsp->length, NULL, 0); > > break; > + } > default: > WARN_ON(1); > break; > } > - } > > /* Drop the idr's reference */ > client_put(client); Applied for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-08-10 07:04:19
|
A commit d8527cab6c31 ("firewire: cdev: implement new event to notify response subaction with time stamp") adds an additional case, FW_CDEV_EVENT_RESPONSE2, into switch statement in complete_transaction(). However, the range of block is beyond to the case label and reaches neibour default label. This commit corrects the range of block. Fortunately, it has few impacts in practice since the local variable in the scope under the label is not used in codes under default label. Fixes: d8527cab6c31 ("firewire: cdev: implement new event to notify response subaction with time stamp") Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- 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 672a37fa8343..c211bb19c94e 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -589,11 +589,11 @@ static void complete_transaction(struct fw_card *card, int rcode, u32 request_ts queue_event(client, &e->event, rsp, sizeof(*rsp) + rsp->length, NULL, 0); break; + } default: WARN_ON(1); break; } - } /* Drop the idr's reference */ client_put(client); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-06 00:06:43
|
On Mon, Aug 05, 2024 at 05:53:51PM +0900, Takashi Sakamoto wrote: > Hi, > > This patchset is a revised version of the previous one. > > https://lore.kernel.org/lkml/202...@sa.../ > > The guard macro was firstly introduced in v6.5 kernel, and already > available for spin_lock, mutex, RCU, and R/W semaphore. It is useful to > ensure releasing lock in block. > > This patchset includes changes to replace lock/release codes with the guard > macro. > > * Changes in v2: > * use scoped_guard() instead of guard() just after label so that > statements are expanded there instead of declarations. > > Takashi Sakamoto (17): > firewire: core: use guard macro to maintain static packet data for phy > configuration > firewire: core: use guard macro to maintain the list of card > firewire: core: use guard macro to maintain the list of cdev clients > firewire: ohci: use guard macro to serialize accesses to phy registers > firewire: core: use guard macro to maintain RCU scope for transaction > address handler > firewire: core: use guard macro to access to IDR for fw_device > firewire: core: use guard macro to maintain the list of address > handler for transaction > firewire: core: use guard macro to disable local IRQ > firewire: core: use guard macro to maintain list of events for > userspace clients > firewire: core: use guard macro to maintain IDR of isochronous > resources for userspace clients > firewire: core: use guard macro to maintain isochronous context for > userspace client > firewire: core: use guard macro to maintain list of receivers for phy > configuration packets > firewire: core: use guard macro to maintain list of asynchronous > transaction > firewire: core: use guard macro to maintain properties of fw_card > firewire: ohci: use guard macro to maintain bus time > firewire: ohci: use guard macro to maintain image of configuration ROM > firewire: ohci: use guard macro to serialize operations for > isochronous contexts > > drivers/firewire/core-card.c | 60 ++--- > drivers/firewire/core-cdev.c | 252 ++++++++---------- > drivers/firewire/core-device.c | 83 +++--- > drivers/firewire/core-iso.c | 5 +- > drivers/firewire/core-topology.c | 5 +- > drivers/firewire/core-transaction.c | 146 ++++------ > drivers/firewire/ohci.c | 399 ++++++++++++---------------- > 7 files changed, 403 insertions(+), 547 deletions(-) Applied to for-next branch. Regards Takashi Sakamoto |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:45
|
The 1394 OHCI driver uses spinlock for the process to update local configuration ROM. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 116 +++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 67 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 5cb7c7603c2c..368420e4b414 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2139,53 +2139,42 @@ static void bus_reset_work(struct work_struct *work) at_context_flush(&ohci->at_request_ctx); at_context_flush(&ohci->at_response_ctx); - spin_lock_irq(&ohci->lock); - - ohci->generation = generation; - reg_write(ohci, OHCI1394_IntEventClear, OHCI1394_busReset); - reg_write(ohci, OHCI1394_IntMaskSet, OHCI1394_busReset); - - if (ohci->quirks & QUIRK_RESET_PACKET) - ohci->request_generation = generation; - - /* - * This next bit is unrelated to the AT context stuff but we - * have to do it under the spinlock also. If a new config rom - * was set up before this reset, the old one is now no longer - * in use and we can free it. Update the config rom pointers - * to point to the current config rom and clear the - * next_config_rom pointer so a new update can take place. - */ - - if (ohci->next_config_rom != NULL) { - if (ohci->next_config_rom != ohci->config_rom) { - free_rom = ohci->config_rom; - free_rom_bus = ohci->config_rom_bus; + scoped_guard(spinlock_irq, &ohci->lock) { + ohci->generation = generation; + reg_write(ohci, OHCI1394_IntEventClear, OHCI1394_busReset); + reg_write(ohci, OHCI1394_IntMaskSet, OHCI1394_busReset); + + if (ohci->quirks & QUIRK_RESET_PACKET) + ohci->request_generation = generation; + + // This next bit is unrelated to the AT context stuff but we have to do it under the + // spinlock also. If a new config rom was set up before this reset, the old one is + // now no longer in use and we can free it. Update the config rom pointers to point + // to the current config rom and clear the next_config_rom pointer so a new update + // can take place. + if (ohci->next_config_rom != NULL) { + if (ohci->next_config_rom != ohci->config_rom) { + free_rom = ohci->config_rom; + free_rom_bus = ohci->config_rom_bus; + } + ohci->config_rom = ohci->next_config_rom; + ohci->config_rom_bus = ohci->next_config_rom_bus; + ohci->next_config_rom = NULL; + + // Restore config_rom image and manually update config_rom registers. + // Writing the header quadlet will indicate that the config rom is ready, + // so we do that last. + reg_write(ohci, OHCI1394_BusOptions, be32_to_cpu(ohci->config_rom[2])); + ohci->config_rom[0] = ohci->next_header; + reg_write(ohci, OHCI1394_ConfigROMhdr, be32_to_cpu(ohci->next_header)); } - ohci->config_rom = ohci->next_config_rom; - ohci->config_rom_bus = ohci->next_config_rom_bus; - ohci->next_config_rom = NULL; - /* - * Restore config_rom image and manually update - * config_rom registers. Writing the header quadlet - * will indicate that the config rom is ready, so we - * do that last. - */ - reg_write(ohci, OHCI1394_BusOptions, - be32_to_cpu(ohci->config_rom[2])); - ohci->config_rom[0] = ohci->next_header; - reg_write(ohci, OHCI1394_ConfigROMhdr, - be32_to_cpu(ohci->next_header)); - } - - if (param_remote_dma) { - reg_write(ohci, OHCI1394_PhyReqFilterHiSet, ~0); - reg_write(ohci, OHCI1394_PhyReqFilterLoSet, ~0); + if (param_remote_dma) { + reg_write(ohci, OHCI1394_PhyReqFilterHiSet, ~0); + reg_write(ohci, OHCI1394_PhyReqFilterLoSet, ~0); + } } - spin_unlock_irq(&ohci->lock); - if (free_rom) dmam_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, free_rom, free_rom_bus); @@ -2626,33 +2615,26 @@ static int ohci_set_config_rom(struct fw_card *card, if (next_config_rom == NULL) return -ENOMEM; - spin_lock_irq(&ohci->lock); - - /* - * If there is not an already pending config_rom update, - * push our new allocation into the ohci->next_config_rom - * and then mark the local variable as null so that we - * won't deallocate the new buffer. - * - * OTOH, if there is a pending config_rom update, just - * use that buffer with the new config_rom data, and - * let this routine free the unused DMA allocation. - */ - - if (ohci->next_config_rom == NULL) { - ohci->next_config_rom = next_config_rom; - ohci->next_config_rom_bus = next_config_rom_bus; - next_config_rom = NULL; - } + scoped_guard(spinlock_irq, &ohci->lock) { + // If there is not an already pending config_rom update, push our new allocation + // into the ohci->next_config_rom and then mark the local variable as null so that + // we won't deallocate the new buffer. + // + // OTOH, if there is a pending config_rom update, just use that buffer with the new + // config_rom data, and let this routine free the unused DMA allocation. + if (ohci->next_config_rom == NULL) { + ohci->next_config_rom = next_config_rom; + ohci->next_config_rom_bus = next_config_rom_bus; + next_config_rom = NULL; + } - copy_config_rom(ohci->next_config_rom, config_rom, length); + copy_config_rom(ohci->next_config_rom, config_rom, length); - ohci->next_header = config_rom[0]; - ohci->next_config_rom[0] = 0; + ohci->next_header = config_rom[0]; + ohci->next_config_rom[0] = 0; - reg_write(ohci, OHCI1394_ConfigROMmap, ohci->next_config_rom_bus); - - spin_unlock_irq(&ohci->lock); + reg_write(ohci, OHCI1394_ConfigROMmap, ohci->next_config_rom_bus); + } /* If we didn't use the DMA allocation, delete it. */ if (next_config_rom != NULL) { -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:43
|
The 1394 OHCI driver maintains bus time to respond to querying request. The concurrent access to the bus time is protected by spinlock. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 1461e008d265..5cb7c7603c2c 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2300,9 +2300,8 @@ static irqreturn_t irq_handler(int irq, void *data) handle_dead_contexts(ohci); if (event & OHCI1394_cycle64Seconds) { - spin_lock(&ohci->lock); + guard(spinlock)(&ohci->lock); update_bus_time(ohci); - spin_unlock(&ohci->lock); } else flush_writes(ohci); @@ -2762,7 +2761,6 @@ static int ohci_enable_phys_dma(struct fw_card *card, static u32 ohci_read_csr(struct fw_card *card, int csr_offset) { struct fw_ohci *ohci = fw_ohci(card); - unsigned long flags; u32 value; switch (csr_offset) { @@ -2786,16 +2784,14 @@ static u32 ohci_read_csr(struct fw_card *card, int csr_offset) return get_cycle_time(ohci); case CSR_BUS_TIME: - /* - * We might be called just after the cycle timer has wrapped - * around but just before the cycle64Seconds handler, so we - * better check here, too, if the bus time needs to be updated. - */ - spin_lock_irqsave(&ohci->lock, flags); - value = update_bus_time(ohci); - spin_unlock_irqrestore(&ohci->lock, flags); - return value; + { + // We might be called just after the cycle timer has wrapped around but just before + // the cycle64Seconds handler, so we better check here, too, if the bus time needs + // to be updated. + guard(spinlock_irqsave)(&ohci->lock); + return update_bus_time(ohci); + } case CSR_BUSY_TIMEOUT: value = reg_read(ohci, OHCI1394_ATRetries); return (value >> 4) & 0x0ffff00f; @@ -2813,7 +2809,6 @@ static u32 ohci_read_csr(struct fw_card *card, int csr_offset) static void ohci_write_csr(struct fw_card *card, int csr_offset, u32 value) { struct fw_ohci *ohci = fw_ohci(card); - unsigned long flags; switch (csr_offset) { case CSR_STATE_CLEAR: @@ -2849,12 +2844,11 @@ static void ohci_write_csr(struct fw_card *card, int csr_offset, u32 value) break; case CSR_BUS_TIME: - spin_lock_irqsave(&ohci->lock, flags); - ohci->bus_time = (update_bus_time(ohci) & 0x40) | - (value & ~0x7f); - spin_unlock_irqrestore(&ohci->lock, flags); + { + guard(spinlock_irqsave)(&ohci->lock); + ohci->bus_time = (update_bus_time(ohci) & 0x40) | (value & ~0x7f); break; - + } case CSR_BUSY_TIMEOUT: value = (value & 0xf) | ((value & 0xf) << 4) | ((value & 0xf) << 8) | ((value & 0x0ffff000) << 4); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:41
|
The 1394 OHCI driver uses spinlock to serialize operations for isochronous contexts. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/ohci.c | 182 +++++++++++++++++----------------------- 1 file changed, 77 insertions(+), 105 deletions(-) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 368420e4b414..e1d24e0ec991 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1173,13 +1173,11 @@ static void context_tasklet(unsigned long data) break; if (old_desc != desc) { - /* If we've advanced to the next buffer, move the - * previous buffer to the free list. */ - unsigned long flags; + // If we've advanced to the next buffer, move the previous buffer to the + // free list. old_desc->used = 0; - spin_lock_irqsave(&ctx->ohci->lock, flags); + guard(spinlock_irqsave)(&ctx->ohci->lock); list_move_tail(&old_desc->list, &ctx->buffer_list); - spin_unlock_irqrestore(&ctx->ohci->lock, flags); } ctx->last = last; } @@ -2122,14 +2120,12 @@ static void bus_reset_work(struct work_struct *work) return; } - /* FIXME: Document how the locking works. */ - spin_lock_irq(&ohci->lock); - - ohci->generation = -1; /* prevent AT packet queueing */ - context_stop(&ohci->at_request_ctx); - context_stop(&ohci->at_response_ctx); - - spin_unlock_irq(&ohci->lock); + // FIXME: Document how the locking works. + scoped_guard(spinlock_irq, &ohci->lock) { + ohci->generation = -1; // prevent AT packet queueing + context_stop(&ohci->at_request_ctx); + context_stop(&ohci->at_response_ctx); + } /* * Per OHCI 1.2 draft, clause 7.2.3.3, hardware may leave unsent @@ -2704,7 +2700,6 @@ static int ohci_enable_phys_dma(struct fw_card *card, int node_id, int generation) { struct fw_ohci *ohci = fw_ohci(card); - unsigned long flags; int n, ret = 0; if (param_remote_dma) @@ -2715,12 +2710,10 @@ static int ohci_enable_phys_dma(struct fw_card *card, * interrupt bit. Clear physReqResourceAllBuses on bus reset. */ - spin_lock_irqsave(&ohci->lock, flags); + guard(spinlock_irqsave)(&ohci->lock); - if (ohci->generation != generation) { - ret = -ESTALE; - goto out; - } + if (ohci->generation != generation) + return -ESTALE; /* * Note, if the node ID contains a non-local bus ID, physical DMA is @@ -2734,8 +2727,6 @@ static int ohci_enable_phys_dma(struct fw_card *card, reg_write(ohci, OHCI1394_PhyReqFilterHiSet, 1 << (n - 32)); flush_writes(ohci); - out: - spin_unlock_irqrestore(&ohci->lock, flags); return ret; } @@ -3076,55 +3067,53 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card, u32 *mask, regs; int index, ret = -EBUSY; - spin_lock_irq(&ohci->lock); + scoped_guard(spinlock_irq, &ohci->lock) { + switch (type) { + case FW_ISO_CONTEXT_TRANSMIT: + mask = &ohci->it_context_mask; + callback = handle_it_packet; + index = ffs(*mask) - 1; + if (index >= 0) { + *mask &= ~(1 << index); + regs = OHCI1394_IsoXmitContextBase(index); + ctx = &ohci->it_context_list[index]; + } + break; - switch (type) { - case FW_ISO_CONTEXT_TRANSMIT: - mask = &ohci->it_context_mask; - callback = handle_it_packet; - index = ffs(*mask) - 1; - if (index >= 0) { - *mask &= ~(1 << index); - regs = OHCI1394_IsoXmitContextBase(index); - ctx = &ohci->it_context_list[index]; - } - break; + case FW_ISO_CONTEXT_RECEIVE: + channels = &ohci->ir_context_channels; + mask = &ohci->ir_context_mask; + callback = handle_ir_packet_per_buffer; + index = *channels & 1ULL << channel ? ffs(*mask) - 1 : -1; + if (index >= 0) { + *channels &= ~(1ULL << channel); + *mask &= ~(1 << index); + regs = OHCI1394_IsoRcvContextBase(index); + ctx = &ohci->ir_context_list[index]; + } + break; - case FW_ISO_CONTEXT_RECEIVE: - channels = &ohci->ir_context_channels; - mask = &ohci->ir_context_mask; - callback = handle_ir_packet_per_buffer; - index = *channels & 1ULL << channel ? ffs(*mask) - 1 : -1; - if (index >= 0) { - *channels &= ~(1ULL << channel); - *mask &= ~(1 << index); - regs = OHCI1394_IsoRcvContextBase(index); - ctx = &ohci->ir_context_list[index]; - } - break; + case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: + mask = &ohci->ir_context_mask; + callback = handle_ir_buffer_fill; + index = !ohci->mc_allocated ? ffs(*mask) - 1 : -1; + if (index >= 0) { + ohci->mc_allocated = true; + *mask &= ~(1 << index); + regs = OHCI1394_IsoRcvContextBase(index); + ctx = &ohci->ir_context_list[index]; + } + break; - case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: - mask = &ohci->ir_context_mask; - callback = handle_ir_buffer_fill; - index = !ohci->mc_allocated ? ffs(*mask) - 1 : -1; - if (index >= 0) { - ohci->mc_allocated = true; - *mask &= ~(1 << index); - regs = OHCI1394_IsoRcvContextBase(index); - ctx = &ohci->ir_context_list[index]; + default: + index = -1; + ret = -ENOSYS; } - break; - default: - index = -1; - ret = -ENOSYS; + if (index < 0) + return ERR_PTR(ret); } - spin_unlock_irq(&ohci->lock); - - if (index < 0) - return ERR_PTR(ret); - memset(ctx, 0, sizeof(*ctx)); ctx->header_length = 0; ctx->header = (void *) __get_free_page(GFP_KERNEL); @@ -3146,20 +3135,18 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card, out_with_header: free_page((unsigned long)ctx->header); out: - spin_lock_irq(&ohci->lock); - - switch (type) { - case FW_ISO_CONTEXT_RECEIVE: - *channels |= 1ULL << channel; - break; + scoped_guard(spinlock_irq, &ohci->lock) { + switch (type) { + case FW_ISO_CONTEXT_RECEIVE: + *channels |= 1ULL << channel; + break; - case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: - ohci->mc_allocated = false; - break; + case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: + ohci->mc_allocated = false; + break; + } + *mask |= 1 << index; } - *mask |= 1 << index; - - spin_unlock_irq(&ohci->lock); return ERR_PTR(ret); } @@ -3243,14 +3230,13 @@ static void ohci_free_iso_context(struct fw_iso_context *base) { struct fw_ohci *ohci = fw_ohci(base->card); struct iso_context *ctx = container_of(base, struct iso_context, base); - unsigned long flags; int index; ohci_stop_iso(base); context_release(&ctx->context); free_page((unsigned long)ctx->header); - spin_lock_irqsave(&ohci->lock, flags); + guard(spinlock_irqsave)(&ohci->lock); switch (base->type) { case FW_ISO_CONTEXT_TRANSMIT: @@ -3272,38 +3258,29 @@ static void ohci_free_iso_context(struct fw_iso_context *base) ohci->mc_allocated = false; break; } - - spin_unlock_irqrestore(&ohci->lock, flags); } static int ohci_set_iso_channels(struct fw_iso_context *base, u64 *channels) { struct fw_ohci *ohci = fw_ohci(base->card); - unsigned long flags; - int ret; switch (base->type) { case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: + { + guard(spinlock_irqsave)(&ohci->lock); - spin_lock_irqsave(&ohci->lock, flags); - - /* Don't allow multichannel to grab other contexts' channels. */ + // Don't allow multichannel to grab other contexts' channels. if (~ohci->ir_context_channels & ~ohci->mc_channels & *channels) { *channels = ohci->ir_context_channels; - ret = -EBUSY; + return -EBUSY; } else { set_multichannel_mask(ohci, *channels); - ret = 0; + return 0; } - - spin_unlock_irqrestore(&ohci->lock, flags); - - break; + } default: - ret = -EINVAL; + return -EINVAL; } - - return ret; } #ifdef CONFIG_PM @@ -3573,24 +3550,19 @@ static int ohci_queue_iso(struct fw_iso_context *base, unsigned long payload) { struct iso_context *ctx = container_of(base, struct iso_context, base); - unsigned long flags; - int ret = -ENOSYS; - spin_lock_irqsave(&ctx->context.ohci->lock, flags); + guard(spinlock_irqsave)(&ctx->context.ohci->lock); + switch (base->type) { case FW_ISO_CONTEXT_TRANSMIT: - ret = queue_iso_transmit(ctx, packet, buffer, payload); - break; + return queue_iso_transmit(ctx, packet, buffer, payload); case FW_ISO_CONTEXT_RECEIVE: - ret = queue_iso_packet_per_buffer(ctx, packet, buffer, payload); - break; + return queue_iso_packet_per_buffer(ctx, packet, buffer, payload); case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: - ret = queue_iso_buffer_fill(ctx, packet, buffer, payload); - break; + return queue_iso_buffer_fill(ctx, packet, buffer, payload); + default: + return -ENOSYS; } - spin_unlock_irqrestore(&ctx->context.ohci->lock, flags); - - return ret; } static void ohci_flush_queue_iso(struct fw_iso_context *base) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:39
|
The core function maintains clients to receive phy configuration packets by list in the instance of fw_card. The concurrent access to the list is protected by spinlock in the instance. This commit uses guard macro to maintain the list. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index ac6f9ad9e88e..f32f8667ef2c 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1659,26 +1659,22 @@ static int ioctl_receive_phy_packets(struct client *client, union ioctl_arg *arg if (!client->device->is_local) return -ENOSYS; - spin_lock_irq(&card->lock); + guard(spinlock_irq)(&card->lock); list_move_tail(&client->phy_receiver_link, &card->phy_receiver_list); client->phy_receiver_closure = a->closure; - spin_unlock_irq(&card->lock); - return 0; } void fw_cdev_handle_phy_packet(struct fw_card *card, struct fw_packet *p) { struct client *client; - struct inbound_phy_packet_event *e; - unsigned long flags; - spin_lock_irqsave(&card->lock, flags); + guard(spinlock_irqsave)(&card->lock); list_for_each_entry(client, &card->phy_receiver_list, phy_receiver_link) { - e = kmalloc(sizeof(*e) + 8, GFP_ATOMIC); + struct inbound_phy_packet_event *e = kmalloc(sizeof(*e) + 8, GFP_ATOMIC); if (e == NULL) break; @@ -1706,8 +1702,6 @@ void fw_cdev_handle_phy_packet(struct fw_card *card, struct fw_packet *p) queue_event(client, &e->event, &e->phy_packet, sizeof(*pp) + 8, NULL, 0); } } - - spin_unlock_irqrestore(&card->lock, flags); } static int (* const ioctl_handlers[])(struct client *, union ioctl_arg *) = { @@ -1855,9 +1849,8 @@ static int fw_device_op_release(struct inode *inode, struct file *file) struct client *client = file->private_data; struct event *event, *next_event; - spin_lock_irq(&client->device->card->lock); - list_del(&client->phy_receiver_link); - spin_unlock_irq(&client->device->card->lock); + scoped_guard(spinlock_irq, &client->device->card->lock) + list_del(&client->phy_receiver_link); scoped_guard(mutex, &client->device->client_list_mutex) list_del(&client->link); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:37
|
The core functions uses spinlock in instance of fw_card structure to protect concurrent access to properties in the instance. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-card.c | 16 +++++++--------- drivers/firewire/core-cdev.c | 4 +--- drivers/firewire/core-device.c | 10 +++------- drivers/firewire/core-iso.c | 5 ++--- drivers/firewire/core-topology.c | 5 +---- drivers/firewire/core-transaction.c | 12 +++++------- 6 files changed, 19 insertions(+), 33 deletions(-) diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index 79a5b19e9d18..e80b762888fa 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -374,11 +374,11 @@ static void bm_work(struct work_struct *work) bm_id = be32_to_cpu(transaction_data[0]); - spin_lock_irq(&card->lock); - if (rcode == RCODE_COMPLETE && generation == card->generation) - card->bm_node_id = - bm_id == 0x3f ? local_id : 0xffc0 | bm_id; - spin_unlock_irq(&card->lock); + scoped_guard(spinlock_irq, &card->lock) { + if (rcode == RCODE_COMPLETE && generation == card->generation) + card->bm_node_id = + bm_id == 0x3f ? local_id : 0xffc0 | bm_id; + } if (rcode == RCODE_COMPLETE && bm_id != 0x3f) { /* Somebody else is BM. Only act as IRM. */ @@ -707,7 +707,6 @@ EXPORT_SYMBOL_GPL(fw_card_release); void fw_core_remove_card(struct fw_card *card) { struct fw_card_driver dummy_driver = dummy_driver_template; - unsigned long flags; card->driver->update_phy_reg(card, 4, PHY_LINK_ACTIVE | PHY_CONTENDER, 0); @@ -721,9 +720,8 @@ void fw_core_remove_card(struct fw_card *card) dummy_driver.stop_iso = card->driver->stop_iso; card->driver = &dummy_driver; - spin_lock_irqsave(&card->lock, flags); - fw_destroy_nodes(card); - spin_unlock_irqrestore(&card->lock, flags); + scoped_guard(spinlock_irqsave, &card->lock) + fw_destroy_nodes(card); /* Wait for all users, especially device workqueue jobs, to finish. */ fw_card_put(card); diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index f32f8667ef2c..672a37fa8343 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -354,7 +354,7 @@ static void fill_bus_reset_event(struct fw_cdev_event_bus_reset *event, { struct fw_card *card = client->device->card; - spin_lock_irq(&card->lock); + guard(spinlock_irq)(&card->lock); event->closure = client->bus_reset_closure; event->type = FW_CDEV_EVENT_BUS_RESET; @@ -364,8 +364,6 @@ static void fill_bus_reset_event(struct fw_cdev_event_bus_reset *event, event->bm_node_id = card->bm_node_id; event->irm_node_id = card->irm_node->node_id; event->root_node_id = card->root_node->node_id; - - spin_unlock_irq(&card->lock); } static void for_each_client(struct fw_device *device, diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index d695ec2f1efe..bec7e05f6ab8 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -886,16 +886,14 @@ static void fw_device_release(struct device *dev) { struct fw_device *device = fw_device(dev); struct fw_card *card = device->card; - unsigned long flags; /* * Take the card lock so we don't set this to NULL while a * FW_NODE_UPDATED callback is being handled or while the * bus manager work looks at this node. */ - spin_lock_irqsave(&card->lock, flags); - device->node->data = NULL; - spin_unlock_irqrestore(&card->lock, flags); + scoped_guard(spinlock_irqsave, &card->lock) + device->node->data = NULL; fw_node_put(device->node); kfree(device->config_rom); @@ -952,7 +950,7 @@ static int lookup_existing_device(struct device *dev, void *data) return 0; guard(rwsem_read)(&fw_device_rwsem); // serialize config_rom access - spin_lock_irq(&card->lock); /* serialize node access */ + guard(spinlock_irq)(&card->lock); // serialize node access if (memcmp(old->config_rom, new->config_rom, 6 * 4) == 0 && atomic_cmpxchg(&old->state, @@ -982,8 +980,6 @@ static int lookup_existing_device(struct device *dev, void *data) match = 1; } - spin_unlock_irq(&card->lock); - return match; } diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index b3eda38a36f3..101433b8bb51 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -375,9 +375,8 @@ void fw_iso_resource_manage(struct fw_card *card, int generation, u32 channels_lo = channels_mask >> 32; /* channels 63...32 */ int irm_id, ret, c = -EINVAL; - spin_lock_irq(&card->lock); - irm_id = card->irm_node->node_id; - spin_unlock_irq(&card->lock); + scoped_guard(spinlock_irq, &card->lock) + irm_id = card->irm_node->node_id; if (channels_hi) c = manage_channel(card, irm_id, generation, channels_hi, diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index 46e6eb287d24..6adadb11962e 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -455,11 +455,10 @@ void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation, int self_id_count, u32 *self_ids, bool bm_abdicate) { struct fw_node *local_node; - unsigned long flags; trace_bus_reset_handle(card->index, generation, node_id, bm_abdicate, self_ids, self_id_count); - spin_lock_irqsave(&card->lock, flags); + guard(spinlock_irqsave)(&card->lock); /* * If the selfID buffer is not the immediate successor of the @@ -500,7 +499,5 @@ void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation, } else { update_tree(card, local_node); } - - spin_unlock_irqrestore(&card->lock, flags); } EXPORT_SYMBOL(fw_core_handle_bus_reset); diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 0f58a5d13d28..14af84541e83 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -1160,7 +1160,6 @@ static void handle_registers(struct fw_card *card, struct fw_request *request, int reg = offset & ~CSR_REGISTER_BASE; __be32 *data = payload; int rcode = RCODE_COMPLETE; - unsigned long flags; switch (reg) { case CSR_PRIORITY_BUDGET: @@ -1202,10 +1201,10 @@ static void handle_registers(struct fw_card *card, struct fw_request *request, if (tcode == TCODE_READ_QUADLET_REQUEST) { *data = cpu_to_be32(card->split_timeout_hi); } else if (tcode == TCODE_WRITE_QUADLET_REQUEST) { - spin_lock_irqsave(&card->lock, flags); + guard(spinlock_irqsave)(&card->lock); + card->split_timeout_hi = be32_to_cpu(*data) & 7; update_split_timeout(card); - spin_unlock_irqrestore(&card->lock, flags); } else { rcode = RCODE_TYPE_ERROR; } @@ -1215,11 +1214,10 @@ static void handle_registers(struct fw_card *card, struct fw_request *request, if (tcode == TCODE_READ_QUADLET_REQUEST) { *data = cpu_to_be32(card->split_timeout_lo); } else if (tcode == TCODE_WRITE_QUADLET_REQUEST) { - spin_lock_irqsave(&card->lock, flags); - card->split_timeout_lo = - be32_to_cpu(*data) & 0xfff80000; + guard(spinlock_irqsave)(&card->lock); + + card->split_timeout_lo = be32_to_cpu(*data) & 0xfff80000; update_split_timeout(card); - spin_unlock_irqrestore(&card->lock, flags); } else { rcode = RCODE_TYPE_ERROR; } -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:37
|
The core function provides UAPI to maintain isochronous resources allocated by userspace clients across bus resets automatically. The resources are maintained by IDR and the concurrent access to it is protected by spinlock in the instance of client. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 131 ++++++++++++++++------------------- 1 file changed, 59 insertions(+), 72 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 2e2199eaa05b..c2d24cc5c1f1 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -399,9 +399,9 @@ static void queue_bus_reset_event(struct client *client) queue_event(client, &e->event, &e->reset, sizeof(e->reset), NULL, 0); - spin_lock_irq(&client->lock); + guard(spinlock_irq)(&client->lock); + idr_for_each(&client->resource_idr, schedule_reallocations, client); - spin_unlock_irq(&client->lock); } void fw_device_cdev_update(struct fw_device *device) @@ -483,25 +483,23 @@ static int add_client_resource(struct client *client, struct client_resource *resource, gfp_t gfp_mask) { bool preload = gfpflags_allow_blocking(gfp_mask); - unsigned long flags; int ret; if (preload) idr_preload(gfp_mask); - spin_lock_irqsave(&client->lock, flags); - if (client->in_shutdown) - ret = -ECANCELED; - else - ret = idr_alloc(&client->resource_idr, resource, 0, 0, - GFP_NOWAIT); - if (ret >= 0) { - resource->handle = ret; - client_get(client); - schedule_if_iso_resource(resource); + scoped_guard(spinlock_irqsave, &client->lock) { + if (client->in_shutdown) + ret = -ECANCELED; + else + ret = idr_alloc(&client->resource_idr, resource, 0, 0, GFP_NOWAIT); + if (ret >= 0) { + resource->handle = ret; + client_get(client); + schedule_if_iso_resource(resource); + } } - spin_unlock_irqrestore(&client->lock, flags); if (preload) idr_preload_end(); @@ -514,14 +512,14 @@ static int release_client_resource(struct client *client, u32 handle, { struct client_resource *resource; - spin_lock_irq(&client->lock); - if (client->in_shutdown) - resource = NULL; - else - resource = idr_find(&client->resource_idr, handle); - if (resource && resource->release == release) - idr_remove(&client->resource_idr, handle); - spin_unlock_irq(&client->lock); + scoped_guard(spinlock_irq, &client->lock) { + if (client->in_shutdown) + resource = NULL; + else + resource = idr_find(&client->resource_idr, handle); + if (resource && resource->release == release) + idr_remove(&client->resource_idr, handle); + } if (!(resource && resource->release == release)) return -EINVAL; @@ -546,13 +544,12 @@ static void complete_transaction(struct fw_card *card, int rcode, u32 request_ts { struct outbound_transaction_event *e = data; struct client *client = e->client; - unsigned long flags; - spin_lock_irqsave(&client->lock, flags); - idr_remove(&client->resource_idr, e->r.resource.handle); - if (client->in_shutdown) - wake_up(&client->tx_flush_wait); - spin_unlock_irqrestore(&client->lock, flags); + scoped_guard(spinlock_irqsave, &client->lock) { + idr_remove(&client->resource_idr, e->r.resource.handle); + if (client->in_shutdown) + wake_up(&client->tx_flush_wait); + } switch (e->rsp.without_tstamp.type) { case FW_CDEV_EVENT_RESPONSE: @@ -1307,25 +1304,24 @@ static void iso_resource_work(struct work_struct *work) int generation, channel, bandwidth, todo; bool skip, free, success; - spin_lock_irq(&client->lock); - generation = client->device->generation; - todo = r->todo; - /* Allow 1000ms grace period for other reallocations. */ - if (todo == ISO_RES_ALLOC && - time_before64(get_jiffies_64(), - client->device->card->reset_jiffies + HZ)) { - schedule_iso_resource(r, DIV_ROUND_UP(HZ, 3)); - skip = true; - } else { - /* We could be called twice within the same generation. */ - skip = todo == ISO_RES_REALLOC && - r->generation == generation; + scoped_guard(spinlock_irq, &client->lock) { + generation = client->device->generation; + todo = r->todo; + // Allow 1000ms grace period for other reallocations. + if (todo == ISO_RES_ALLOC && + time_before64(get_jiffies_64(), client->device->card->reset_jiffies + HZ)) { + schedule_iso_resource(r, DIV_ROUND_UP(HZ, 3)); + skip = true; + } else { + // We could be called twice within the same generation. + skip = todo == ISO_RES_REALLOC && + r->generation == generation; + } + free = todo == ISO_RES_DEALLOC || + todo == ISO_RES_ALLOC_ONCE || + todo == ISO_RES_DEALLOC_ONCE; + r->generation = generation; } - free = todo == ISO_RES_DEALLOC || - todo == ISO_RES_ALLOC_ONCE || - todo == ISO_RES_DEALLOC_ONCE; - r->generation = generation; - spin_unlock_irq(&client->lock); if (skip) goto out; @@ -1348,24 +1344,20 @@ static void iso_resource_work(struct work_struct *work) success = channel >= 0 || bandwidth > 0; - spin_lock_irq(&client->lock); - /* - * Transit from allocation to reallocation, except if the client - * requested deallocation in the meantime. - */ - if (r->todo == ISO_RES_ALLOC) - r->todo = ISO_RES_REALLOC; - /* - * Allocation or reallocation failure? Pull this resource out of the - * idr and prepare for deletion, unless the client is shutting down. - */ - if (r->todo == ISO_RES_REALLOC && !success && - !client->in_shutdown && - idr_remove(&client->resource_idr, r->resource.handle)) { - client_put(client); - free = true; + scoped_guard(spinlock_irq, &client->lock) { + // Transit from allocation to reallocation, except if the client + // requested deallocation in the meantime. + if (r->todo == ISO_RES_ALLOC) + r->todo = ISO_RES_REALLOC; + // Allocation or reallocation failure? Pull this resource out of the + // idr and prepare for deletion, unless the client is shutting down. + if (r->todo == ISO_RES_REALLOC && !success && + !client->in_shutdown && + idr_remove(&client->resource_idr, r->resource.handle)) { + client_put(client); + free = true; + } } - spin_unlock_irq(&client->lock); if (todo == ISO_RES_ALLOC && channel >= 0) r->channels = 1ULL << channel; @@ -1403,10 +1395,10 @@ static void release_iso_resource(struct client *client, struct iso_resource *r = container_of(resource, struct iso_resource, resource); - spin_lock_irq(&client->lock); + guard(spinlock_irq)(&client->lock); + r->todo = ISO_RES_DEALLOC; schedule_iso_resource(r, 0); - spin_unlock_irq(&client->lock); } static int init_iso_resource(struct client *client, @@ -1845,14 +1837,9 @@ static int is_outbound_transaction_resource(int id, void *p, void *data) static int has_outbound_transactions(struct client *client) { - int ret; + guard(spinlock_irq)(&client->lock); - spin_lock_irq(&client->lock); - ret = idr_for_each(&client->resource_idr, - is_outbound_transaction_resource, NULL); - spin_unlock_irq(&client->lock); - - return ret; + return idr_for_each(&client->resource_idr, is_outbound_transaction_resource, NULL); } static int shutdown_resource(int id, void *p, void *data) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:36
|
The core function maintains pending asynchronous transactions by list in the instance of fw_card. The concurrent access to the list is protected by spinlock in the instance. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-transaction.c | 85 ++++++++++++----------------- 1 file changed, 34 insertions(+), 51 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index a006daf385e9..0f58a5d13d28 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -49,35 +49,31 @@ static int close_transaction(struct fw_transaction *transaction, struct fw_card u32 response_tstamp) { struct fw_transaction *t = NULL, *iter; - unsigned long flags; - spin_lock_irqsave(&card->lock, flags); - list_for_each_entry(iter, &card->transaction_list, link) { - if (iter == transaction) { - if (!try_cancel_split_timeout(iter)) { - spin_unlock_irqrestore(&card->lock, flags); - goto timed_out; + scoped_guard(spinlock_irqsave, &card->lock) { + list_for_each_entry(iter, &card->transaction_list, link) { + if (iter == transaction) { + if (try_cancel_split_timeout(iter)) { + list_del_init(&iter->link); + card->tlabel_mask &= ~(1ULL << iter->tlabel); + t = iter; + } + break; } - list_del_init(&iter->link); - card->tlabel_mask &= ~(1ULL << iter->tlabel); - t = iter; - break; } } - spin_unlock_irqrestore(&card->lock, flags); - if (t) { - if (!t->with_tstamp) { - t->callback.without_tstamp(card, rcode, NULL, 0, t->callback_data); - } else { - t->callback.with_tstamp(card, rcode, t->packet.timestamp, response_tstamp, - NULL, 0, t->callback_data); - } - return 0; + if (!t) + return -ENOENT; + + if (!t->with_tstamp) { + t->callback.without_tstamp(card, rcode, NULL, 0, t->callback_data); + } else { + t->callback.with_tstamp(card, rcode, t->packet.timestamp, response_tstamp, NULL, 0, + t->callback_data); } - timed_out: - return -ENOENT; + return 0; } /* @@ -121,16 +117,13 @@ static void split_transaction_timeout_callback(struct timer_list *timer) { struct fw_transaction *t = from_timer(t, timer, split_timeout_timer); struct fw_card *card = t->card; - unsigned long flags; - spin_lock_irqsave(&card->lock, flags); - if (list_empty(&t->link)) { - spin_unlock_irqrestore(&card->lock, flags); - return; + scoped_guard(spinlock_irqsave, &card->lock) { + if (list_empty(&t->link)) + return; + list_del(&t->link); + card->tlabel_mask &= ~(1ULL << t->tlabel); } - list_del(&t->link); - card->tlabel_mask &= ~(1ULL << t->tlabel); - spin_unlock_irqrestore(&card->lock, flags); if (!t->with_tstamp) { t->callback.without_tstamp(card, RCODE_CANCELLED, NULL, 0, t->callback_data); @@ -143,20 +136,14 @@ static void split_transaction_timeout_callback(struct timer_list *timer) static void start_split_transaction_timeout(struct fw_transaction *t, struct fw_card *card) { - unsigned long flags; + guard(spinlock_irqsave)(&card->lock); - spin_lock_irqsave(&card->lock, flags); - - if (list_empty(&t->link) || WARN_ON(t->is_split_transaction)) { - spin_unlock_irqrestore(&card->lock, flags); + if (list_empty(&t->link) || WARN_ON(t->is_split_transaction)) return; - } t->is_split_transaction = true; mod_timer(&t->split_timeout_timer, jiffies + card->split_timeout_jiffies); - - spin_unlock_irqrestore(&card->lock, flags); } static u32 compute_split_timeout_timestamp(struct fw_card *card, u32 request_timestamp); @@ -1015,7 +1002,6 @@ EXPORT_SYMBOL(fw_core_handle_request); void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) { struct fw_transaction *t = NULL, *iter; - unsigned long flags; u32 *data; size_t data_length; int tcode, tlabel, source, rcode; @@ -1054,26 +1040,23 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) break; } - spin_lock_irqsave(&card->lock, flags); - list_for_each_entry(iter, &card->transaction_list, link) { - if (iter->node_id == source && iter->tlabel == tlabel) { - if (!try_cancel_split_timeout(iter)) { - spin_unlock_irqrestore(&card->lock, flags); - goto timed_out; + scoped_guard(spinlock_irqsave, &card->lock) { + list_for_each_entry(iter, &card->transaction_list, link) { + if (iter->node_id == source && iter->tlabel == tlabel) { + if (try_cancel_split_timeout(iter)) { + list_del_init(&iter->link); + card->tlabel_mask &= ~(1ULL << iter->tlabel); + t = iter; + } + break; } - list_del_init(&iter->link); - card->tlabel_mask &= ~(1ULL << iter->tlabel); - t = iter; - break; } } - spin_unlock_irqrestore(&card->lock, flags); trace_async_response_inbound((uintptr_t)t, card->index, p->generation, p->speed, p->ack, p->timestamp, p->header, data, data_length / 4); if (!t) { - timed_out: fw_notice(card, "unsolicited response (source %x, tlabel %x)\n", source, tlabel); return; -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:35
|
The core function maintains events to userspace by list in the instance of client. The concurrent access to the list is protected by spinlock in the instance. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 90e9dfed8681..2e2199eaa05b 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -287,19 +287,17 @@ static int fw_device_op_open(struct inode *inode, struct file *file) static void queue_event(struct client *client, struct event *event, void *data0, size_t size0, void *data1, size_t size1) { - unsigned long flags; - event->v[0].data = data0; event->v[0].size = size0; event->v[1].data = data1; event->v[1].size = size1; - spin_lock_irqsave(&client->lock, flags); - if (client->in_shutdown) - kfree(event); - else - list_add_tail(&event->link, &client->event_list); - spin_unlock_irqrestore(&client->lock, flags); + scoped_guard(spinlock_irqsave, &client->lock) { + if (client->in_shutdown) + kfree(event); + else + list_add_tail(&event->link, &client->event_list); + } wake_up_interruptible(&client->wait); } @@ -321,10 +319,10 @@ static int dequeue_event(struct client *client, fw_device_is_shutdown(client->device)) return -ENODEV; - spin_lock_irq(&client->lock); - event = list_first_entry(&client->event_list, struct event, link); - list_del(&event->link); - spin_unlock_irq(&client->lock); + scoped_guard(spinlock_irq, &client->lock) { + event = list_first_entry(&client->event_list, struct event, link); + list_del(&event->link); + } total = 0; for (i = 0; i < ARRAY_SIZE(event->v) && total < count; i++) { @@ -1887,9 +1885,8 @@ static int fw_device_op_release(struct inode *inode, struct file *file) fw_iso_buffer_destroy(&client->buffer, client->device->card); /* Freeze client->resource_idr and client->event_list */ - spin_lock_irq(&client->lock); - client->in_shutdown = true; - spin_unlock_irq(&client->lock); + scoped_guard(spinlock_irq, &client->lock) + client->in_shutdown = true; wait_event(client->tx_flush_wait, !has_outbound_transactions(client)); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:34
|
The core function provides an operation for userspace application to retrieve current value of CYCLE_TIMER register with several types of system time. In the operation, local interrupt is disables so that the access of the register and ktime are done atomically. This commit uses guard macro to disable/enable local interrupts. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index c3baf688bb70..90e9dfed8681 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1263,29 +1263,27 @@ static int ioctl_get_cycle_timer2(struct client *client, union ioctl_arg *arg) struct fw_card *card = client->device->card; struct timespec64 ts = {0, 0}; u32 cycle_time = 0; - int ret = 0; + int ret; - local_irq_disable(); + guard(irq)(); ret = fw_card_read_cycle_time(card, &cycle_time); if (ret < 0) - goto end; + return ret; switch (a->clk_id) { case CLOCK_REALTIME: ktime_get_real_ts64(&ts); break; case CLOCK_MONOTONIC: ktime_get_ts64(&ts); break; case CLOCK_MONOTONIC_RAW: ktime_get_raw_ts64(&ts); break; default: - ret = -EINVAL; + return -EINVAL; } -end: - local_irq_enable(); a->tv_sec = ts.tv_sec; a->tv_nsec = ts.tv_nsec; a->cycle_timer = cycle_time; - return ret; + return 0; } static int ioctl_get_cycle_timer(struct client *client, union ioctl_arg *arg) -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:33
|
The core function allows one isochronous contexts per userspace client. The concurrent access to the context is protected by spinlock in the instance of client. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index c2d24cc5c1f1..ac6f9ad9e88e 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1062,10 +1062,10 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg) if (client->version < FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW) context->drop_overflow_headers = true; - /* We only support one context at this time. */ - spin_lock_irq(&client->lock); + // We only support one context at this time. + guard(spinlock_irq)(&client->lock); + if (client->iso_context != NULL) { - spin_unlock_irq(&client->lock); fw_iso_context_destroy(context); return -EBUSY; @@ -1075,7 +1075,6 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg) client->device->card, iso_dma_direction(context)); if (ret < 0) { - spin_unlock_irq(&client->lock); fw_iso_context_destroy(context); return ret; @@ -1084,7 +1083,6 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg) } client->iso_closure = a->closure; client->iso_context = context; - spin_unlock_irq(&client->lock); a->handle = 0; @@ -1806,16 +1804,15 @@ static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma) if (ret < 0) return ret; - spin_lock_irq(&client->lock); - if (client->iso_context) { - ret = fw_iso_buffer_map_dma(&client->buffer, - client->device->card, - iso_dma_direction(client->iso_context)); - client->buffer_is_mapped = (ret == 0); + scoped_guard(spinlock_irq, &client->lock) { + if (client->iso_context) { + ret = fw_iso_buffer_map_dma(&client->buffer, client->device->card, + iso_dma_direction(client->iso_context)); + if (ret < 0) + goto fail; + client->buffer_is_mapped = true; + } } - spin_unlock_irq(&client->lock); - if (ret < 0) - goto fail; ret = vm_map_pages_zero(vma, client->buffer.pages, client->buffer.page_count); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:32
|
The core function maintains registered cards by list. The concurrent access to the list is protected by static mutex. This commit uses guard macro to maintain the mutex. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-card.c | 44 +++++++++++++++--------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index f8b99dd6cd82..79a5b19e9d18 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -168,7 +168,6 @@ static size_t required_space(struct fw_descriptor *desc) int fw_core_add_descriptor(struct fw_descriptor *desc) { size_t i; - int ret; /* * Check descriptor is valid; the length of all blocks in the @@ -182,29 +181,25 @@ int fw_core_add_descriptor(struct fw_descriptor *desc) if (i != desc->length) return -EINVAL; - mutex_lock(&card_mutex); + guard(mutex)(&card_mutex); - if (config_rom_length + required_space(desc) > 256) { - ret = -EBUSY; - } else { - list_add_tail(&desc->link, &descriptor_list); - config_rom_length += required_space(desc); - descriptor_count++; - if (desc->immediate > 0) - descriptor_count++; - update_config_roms(); - ret = 0; - } + if (config_rom_length + required_space(desc) > 256) + return -EBUSY; - mutex_unlock(&card_mutex); + list_add_tail(&desc->link, &descriptor_list); + config_rom_length += required_space(desc); + descriptor_count++; + if (desc->immediate > 0) + descriptor_count++; + update_config_roms(); - return ret; + return 0; } EXPORT_SYMBOL(fw_core_add_descriptor); void fw_core_remove_descriptor(struct fw_descriptor *desc) { - mutex_lock(&card_mutex); + guard(mutex)(&card_mutex); list_del(&desc->link); config_rom_length -= required_space(desc); @@ -212,8 +207,6 @@ void fw_core_remove_descriptor(struct fw_descriptor *desc) if (desc->immediate > 0) descriptor_count--; update_config_roms(); - - mutex_unlock(&card_mutex); } EXPORT_SYMBOL(fw_core_remove_descriptor); @@ -587,16 +580,16 @@ int fw_card_add(struct fw_card *card, card->link_speed = link_speed; card->guid = guid; - mutex_lock(&card_mutex); + guard(mutex)(&card_mutex); generate_config_rom(card, tmp_config_rom); ret = card->driver->enable(card, tmp_config_rom, config_rom_length); - if (ret == 0) - list_add_tail(&card->link, &card_list); + if (ret < 0) + return ret; - mutex_unlock(&card_mutex); + list_add_tail(&card->link, &card_list); - return ret; + return 0; } EXPORT_SYMBOL(fw_card_add); @@ -720,9 +713,8 @@ void fw_core_remove_card(struct fw_card *card) PHY_LINK_ACTIVE | PHY_CONTENDER, 0); fw_schedule_bus_reset(card, false, true); - mutex_lock(&card_mutex); - list_del_init(&card->link); - mutex_unlock(&card_mutex); + scoped_guard(mutex, &card_mutex) + list_del_init(&card->link); /* Switch off most of the card driver interface. */ dummy_driver.free_iso_context = card->driver->free_iso_context; -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:32
|
The core function maintains the instance of fw_device structure by IDR. The concurrent access to IDR is protected by static read/write semaphore. The semaphore is also utilized to protect concurrent access to the content of configuration ROM cached to the instance so that the cache is swapped to the latest one. This commit uses guard macro to maintain the mutex. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 25 +++++------- drivers/firewire/core-device.c | 73 +++++++++++++++------------------- 2 files changed, 42 insertions(+), 56 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index a51aabb963fb..c3baf688bb70 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -454,21 +454,18 @@ static int ioctl_get_info(struct client *client, union ioctl_arg *arg) a->version = FW_CDEV_KERNEL_VERSION; a->card = client->device->card->index; - down_read(&fw_device_rwsem); - - if (a->rom != 0) { - size_t want = a->rom_length; - size_t have = client->device->config_rom_length * 4; - - ret = copy_to_user(u64_to_uptr(a->rom), - client->device->config_rom, min(want, have)); + scoped_guard(rwsem_read, &fw_device_rwsem) { + if (a->rom != 0) { + size_t want = a->rom_length; + size_t have = client->device->config_rom_length * 4; + + ret = copy_to_user(u64_to_uptr(a->rom), client->device->config_rom, + min(want, have)); + if (ret != 0) + return -EFAULT; + } + a->rom_length = client->device->config_rom_length * 4; } - a->rom_length = client->device->config_rom_length * 4; - - up_read(&fw_device_rwsem); - - if (ret != 0) - return -EFAULT; guard(mutex)(&client->device->client_list_mutex); diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 00e9a13e6c45..d695ec2f1efe 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -288,7 +288,7 @@ static ssize_t show_immediate(struct device *dev, const u32 *directories[] = {NULL, NULL}; int i, value = -1; - down_read(&fw_device_rwsem); + guard(rwsem_read)(&fw_device_rwsem); if (is_fw_unit(dev)) { directories[0] = fw_unit(dev)->directory; @@ -317,8 +317,6 @@ static ssize_t show_immediate(struct device *dev, } } - up_read(&fw_device_rwsem); - if (value < 0) return -ENOENT; @@ -339,7 +337,7 @@ static ssize_t show_text_leaf(struct device *dev, char dummy_buf[2]; int i, ret = -ENOENT; - down_read(&fw_device_rwsem); + guard(rwsem_read)(&fw_device_rwsem); if (is_fw_unit(dev)) { directories[0] = fw_unit(dev)->directory; @@ -382,15 +380,14 @@ static ssize_t show_text_leaf(struct device *dev, } } - if (ret >= 0) { - /* Strip trailing whitespace and add newline. */ - while (ret > 0 && isspace(buf[ret - 1])) - ret--; - strcpy(buf + ret, "\n"); - ret++; - } + if (ret < 0) + return ret; - up_read(&fw_device_rwsem); + // Strip trailing whitespace and add newline. + while (ret > 0 && isspace(buf[ret - 1])) + ret--; + strcpy(buf + ret, "\n"); + ret++; return ret; } @@ -466,10 +463,10 @@ static ssize_t config_rom_show(struct device *dev, struct fw_device *device = fw_device(dev); size_t length; - down_read(&fw_device_rwsem); + guard(rwsem_read)(&fw_device_rwsem); + length = device->config_rom_length * 4; memcpy(buf, device->config_rom, length); - up_read(&fw_device_rwsem); return length; } @@ -478,13 +475,10 @@ static ssize_t guid_show(struct device *dev, struct device_attribute *attr, char *buf) { struct fw_device *device = fw_device(dev); - int ret; - down_read(&fw_device_rwsem); - ret = sysfs_emit(buf, "0x%08x%08x\n", device->config_rom[3], device->config_rom[4]); - up_read(&fw_device_rwsem); + guard(rwsem_read)(&fw_device_rwsem); - return ret; + return sysfs_emit(buf, "0x%08x%08x\n", device->config_rom[3], device->config_rom[4]); } static ssize_t is_local_show(struct device *dev, @@ -524,7 +518,8 @@ static ssize_t units_show(struct device *dev, struct fw_csr_iterator ci; int key, value, i = 0; - down_read(&fw_device_rwsem); + guard(rwsem_read)(&fw_device_rwsem); + fw_csr_iterator_init(&ci, &device->config_rom[ROOT_DIR_OFFSET]); while (fw_csr_iterator_next(&ci, &key, &value)) { if (key != (CSR_UNIT | CSR_DIRECTORY)) @@ -533,7 +528,6 @@ static ssize_t units_show(struct device *dev, if (i >= PAGE_SIZE - (8 + 1 + 8 + 1)) break; } - up_read(&fw_device_rwsem); if (i) buf[i - 1] = '\n'; @@ -729,10 +723,10 @@ static int read_config_rom(struct fw_device *device, int generation) goto out; } - down_write(&fw_device_rwsem); - device->config_rom = new_rom; - device->config_rom_length = length; - up_write(&fw_device_rwsem); + scoped_guard(rwsem_write, &fw_device_rwsem) { + device->config_rom = new_rom; + device->config_rom_length = length; + } kfree(old_rom); ret = RCODE_COMPLETE; @@ -826,11 +820,11 @@ struct fw_device *fw_device_get_by_devt(dev_t devt) { struct fw_device *device; - down_read(&fw_device_rwsem); + guard(rwsem_read)(&fw_device_rwsem); + device = idr_find(&fw_device_idr, MINOR(devt)); if (device) fw_device_get(device); - up_read(&fw_device_rwsem); return device; } @@ -882,9 +876,8 @@ static void fw_device_shutdown(struct work_struct *work) device_for_each_child(&device->device, NULL, shutdown_unit); device_unregister(&device->device); - down_write(&fw_device_rwsem); - idr_remove(&fw_device_idr, minor); - up_write(&fw_device_rwsem); + scoped_guard(rwsem_write, &fw_device_rwsem) + idr_remove(&fw_device_idr, minor); fw_device_put(device); } @@ -958,7 +951,7 @@ static int lookup_existing_device(struct device *dev, void *data) if (!is_fw_device(dev)) return 0; - down_read(&fw_device_rwsem); /* serialize config_rom access */ + guard(rwsem_read)(&fw_device_rwsem); // serialize config_rom access spin_lock_irq(&card->lock); /* serialize node access */ if (memcmp(old->config_rom, new->config_rom, 6 * 4) == 0 && @@ -990,7 +983,6 @@ static int lookup_existing_device(struct device *dev, void *data) } spin_unlock_irq(&card->lock); - up_read(&fw_device_rwsem); return match; } @@ -1099,13 +1091,11 @@ static void fw_device_init(struct work_struct *work) device_initialize(&device->device); fw_device_get(device); - down_write(&fw_device_rwsem); - minor = idr_alloc(&fw_device_idr, device, 0, 1 << MINORBITS, - GFP_KERNEL); - up_write(&fw_device_rwsem); - - if (minor < 0) - goto error; + scoped_guard(rwsem_write, &fw_device_rwsem) { + minor = idr_alloc(&fw_device_idr, device, 0, 1 << MINORBITS, GFP_KERNEL); + if (minor < 0) + goto error; + } device->device.bus = &fw_bus_type; device->device.type = &fw_device_type; @@ -1165,9 +1155,8 @@ static void fw_device_init(struct work_struct *work) return; error_with_cdev: - down_write(&fw_device_rwsem); - idr_remove(&fw_device_idr, minor); - up_write(&fw_device_rwsem); + scoped_guard(rwsem_write, &fw_device_rwsem) + idr_remove(&fw_device_idr, minor); error: fw_device_put(device); /* fw_device_idr's reference */ -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:32
|
The core function maintains address handlers by list. RCU is utilized for efficient read access to any entries in the list. This commit uses guard macro to maintain RCU locking and releasing. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-transaction.c | 35 +++++++++++++---------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 2a2cbd6e2f9b..a0224d4d8e11 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -925,16 +925,14 @@ static void handle_exclusive_region_request(struct fw_card *card, if (tcode == TCODE_LOCK_REQUEST) tcode = 0x10 + async_header_get_extended_tcode(p->header); - rcu_read_lock(); - handler = lookup_enclosing_address_handler(&address_handler_list, - offset, request->length); - if (handler) - handler->address_callback(card, request, - tcode, destination, source, - p->generation, offset, - request->data, request->length, - handler->callback_data); - rcu_read_unlock(); + scoped_guard(rcu) { + handler = lookup_enclosing_address_handler(&address_handler_list, offset, + request->length); + if (handler) + handler->address_callback(card, request, tcode, destination, source, + p->generation, offset, request->data, + request->length, handler->callback_data); + } if (!handler) fw_send_response(card, request, RCODE_ADDRESS_ERROR); @@ -967,17 +965,14 @@ static void handle_fcp_region_request(struct fw_card *card, return; } - rcu_read_lock(); - list_for_each_entry_rcu(handler, &address_handler_list, link) { - if (is_enclosing_handler(handler, offset, request->length)) - handler->address_callback(card, request, tcode, - destination, source, - p->generation, offset, - request->data, - request->length, - handler->callback_data); + scoped_guard(rcu) { + list_for_each_entry_rcu(handler, &address_handler_list, link) { + if (is_enclosing_handler(handler, offset, request->length)) + handler->address_callback(card, request, tcode, destination, source, + p->generation, offset, request->data, + request->length, handler->callback_data); + } } - rcu_read_unlock(); fw_send_response(card, request, RCODE_COMPLETE); } -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:32
|
The core function maintains userspace clients by the list in fw_device object associated to the operated character device. The concurrent access to the list is protected by mutex in the object. This commit uses guard macro to maintain the mutex. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-cdev.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 619048dcfd72..a51aabb963fb 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -375,10 +375,10 @@ static void for_each_client(struct fw_device *device, { struct client *c; - mutex_lock(&device->client_list_mutex); + guard(mutex)(&device->client_list_mutex); + list_for_each_entry(c, &device->client_list, link) callback(c); - mutex_unlock(&device->client_list_mutex); } static int schedule_reallocations(int id, void *p, void *data) @@ -470,7 +470,7 @@ static int ioctl_get_info(struct client *client, union ioctl_arg *arg) if (ret != 0) return -EFAULT; - mutex_lock(&client->device->client_list_mutex); + guard(mutex)(&client->device->client_list_mutex); client->bus_reset_closure = a->bus_reset_closure; if (a->bus_reset != 0) { @@ -481,8 +481,6 @@ static int ioctl_get_info(struct client *client, union ioctl_arg *arg) if (ret == 0 && list_empty(&client->link)) list_add_tail(&client->link, &client->device->client_list); - mutex_unlock(&client->device->client_list_mutex); - return ret ? -EFAULT : 0; } @@ -1884,9 +1882,8 @@ static int fw_device_op_release(struct inode *inode, struct file *file) list_del(&client->phy_receiver_link); spin_unlock_irq(&client->device->card->lock); - mutex_lock(&client->device->client_list_mutex); - list_del(&client->link); - mutex_unlock(&client->device->client_list_mutex); + scoped_guard(mutex, &client->device->client_list_mutex) + list_del(&client->link); if (client->iso_context) fw_iso_context_destroy(client->iso_context); -- 2.43.0 |
From: Takashi S. <o-t...@sa...> - 2024-08-05 08:54:32
|
The core function maintains address handlers by list. It is protected by spinlock to insert and remove entry to the list. This commit uses guard macro to maintain the spinlock. Signed-off-by: Takashi Sakamoto <o-t...@sa...> --- drivers/firewire/core-transaction.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index a0224d4d8e11..a006daf385e9 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -596,7 +596,7 @@ int fw_core_add_address_handler(struct fw_address_handler *handler, handler->length == 0) return -EINVAL; - spin_lock(&address_handler_list_lock); + guard(spinlock)(&address_handler_list_lock); handler->offset = region->start; while (handler->offset + handler->length <= region->end) { @@ -615,8 +615,6 @@ int fw_core_add_address_handler(struct fw_address_handler *handler, } } - spin_unlock(&address_handler_list_lock); - return ret; } EXPORT_SYMBOL(fw_core_add_address_handler); @@ -632,9 +630,9 @@ EXPORT_SYMBOL(fw_core_add_address_handler); */ void fw_core_remove_address_handler(struct fw_address_handler *handler) { - spin_lock(&address_handler_list_lock); - list_del_rcu(&handler->link); - spin_unlock(&address_handler_list_lock); + scoped_guard(spinlock, &address_handler_list_lock) + list_del_rcu(&handler->link); + synchronize_rcu(); } EXPORT_SYMBOL(fw_core_remove_address_handler); -- 2.43.0 |