Messages in this thread | | | Subject | Re: [PATCH 2/3] staging: greybus: loopback.c: do insertion in O(n) instead of O(n lg n) | From | Bryan O'Donoghue <> | Date | Tue, 23 Oct 2018 23:30:01 +0100 |
| |
On 23/10/2018 17:20, Rasmus Villemoes wrote: > On 2018-10-11 01:03, Bryan O'Donoghue wrote: >> On 05/10/2018 15:28, Rasmus Villemoes wrote: >>> Signed-off-by: Rasmus Villemoes<linux@rasmusvillemoes.dk> >>> --- >>> I have no idea if the performance matters (it probably doesn't). Feel >>> free to ignore this and the followup cleanup. >> >> What's the problem you're fixing here ? >> >> Is it tested ? > > I got curious why one would want to keep a linked list sorted in the > first place (it can't be for doing binary searches). But it seems that > gb_loopback_device::list is unused, along with the list_op_async. Given > that the below compiles, doesn't that prove that the code is > dead/unused, or what am I missing? > > diff --git a/drivers/staging/greybus/loopback.c > b/drivers/staging/greybus/loopback.c > index 7080294f705c..e4d42c1dc284 100644 > --- a/drivers/staging/greybus/loopback.c > +++ b/drivers/staging/greybus/loopback.c > @@ -47,8 +47,6 @@ struct gb_loopback_device { > > /* We need to take a lock in atomic context */ > spinlock_t lock; > - struct list_head list; > - struct list_head list_op_async; > wait_queue_head_t wq; > }; > > @@ -68,7 +66,6 @@ struct gb_loopback { > struct kfifo kfifo_lat; > struct mutex mutex; > struct task_struct *task; > - struct list_head entry; > struct device *dev; > wait_queue_head_t wq; > wait_queue_head_t wq_completion; > @@ -987,37 +984,6 @@ static const struct file_operations > gb_loopback_debugfs_latency_ops = { > .release = single_release, > }; > > -static int gb_loopback_bus_id_compare(void *priv, struct list_head *lha, > - struct list_head *lhb) > -{ > - struct gb_loopback *a = list_entry(lha, struct gb_loopback, entry); > - struct gb_loopback *b = list_entry(lhb, struct gb_loopback, entry); > - struct gb_connection *ca = a->connection; > - struct gb_connection *cb = b->connection; > - > - if (ca->bundle->intf->interface_id < cb->bundle->intf->interface_id) > - return -1; > - if (cb->bundle->intf->interface_id < ca->bundle->intf->interface_id) > - return 1; > - if (ca->bundle->id < cb->bundle->id) > - return -1; > - if (cb->bundle->id < ca->bundle->id) > - return 1; > - if (ca->intf_cport_id < cb->intf_cport_id) > - return -1; > - else if (cb->intf_cport_id < ca->intf_cport_id) > - return 1; > - > - return 0; > -} > - > -static void gb_loopback_insert_id(struct gb_loopback *gb) > -{ > - /* perform an insertion sort */ > - list_add_tail(&gb->entry, &gb_dev.list); > - list_sort(NULL, &gb_dev.list, gb_loopback_bus_id_compare); > -} > - > #define DEBUGFS_NAMELEN 32 > > static int gb_loopback_probe(struct gb_bundle *bundle, > @@ -1113,7 +1079,6 @@ static int gb_loopback_probe(struct gb_bundle *bundle, > } > > spin_lock_irqsave(&gb_dev.lock, flags); > - gb_loopback_insert_id(gb); > gb_dev.count++; > spin_unlock_irqrestore(&gb_dev.lock, flags); > > @@ -1169,7 +1134,6 @@ static void gb_loopback_disconnect(struct > gb_bundle *bundle) > > spin_lock_irqsave(&gb_dev.lock, flags); > gb_dev.count--; > - list_del(&gb->entry); > spin_unlock_irqrestore(&gb_dev.lock, flags); > > device_unregister(gb->dev); > @@ -1196,8 +1160,6 @@ static int loopback_init(void) > { > int retval; > > - INIT_LIST_HEAD(&gb_dev.list); > - INIT_LIST_HEAD(&gb_dev.list_op_async); > spin_lock_init(&gb_dev.lock); > gb_dev.root = debugfs_create_dir("gb_loopback", NULL); >
Looks like dead code alright.
Reviewed-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
| |