From d3816b8b988038bd1c4b7fe08f130ba3783a3432 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 5 Aug 2024 17:54:01 +0900 Subject: firewire: core: use guard macro to maintain IDR of isochronous resources for userspace clients 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. Link: https://lore.kernel.org/r/20240805085408.251763-11-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-cdev.c | 131 +++++++++++++++++++------------------------ 1 file changed, 59 insertions(+), 72 deletions(-) (limited to 'drivers') 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) -- cgit v1.2.3