hpsa: remove unneeded loop
Subsystems (2)
PATCH v2 1/1 hpsa: remove unneeded loop
Originally this was first patch in a series, but while the other patches were accepted, this one was forgotten.
The code below is adapted for the latest sources.
The cmd_pool_bits is protected everywhere with a spinlock, we don't need the test_and_set_bit, set_bit is enough and the loop can be removed too.
--- drivers/scsi/hpsa.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
drivers/scsi/hpsa.c +6 -9
@@ -4680,15 +4680,12 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)unsigned long flags;spin_lock_irqsave (&h->lock, flags); -do { - i =find_first_zero_bit (h->cmd_pool_bits, h->nr_cmds); -if (i == h->nr_cmds) { -spin_unlock_irqrestore (&h->lock, flags); -return NULL ; - } - }while (test_and_set_bit - (i & (BITS_PER_LONG -1 ), - h->cmd_pool_bits + (i /BITS_PER_LONG )) !=0 ); + i =find_first_zero_bit (h->cmd_pool_bits, h->nr_cmds); +if (i == h->nr_cmds) { +spin_unlock_irqrestore (&h->lock, flags); +return NULL ; + } +set_bit (i & (BITS_PER_LONG -1 ), h->cmd_pool_bits + (i /BITS_PER_LONG ));spin_unlock_irqrestore (&h->lock, flags); c = h->cmd_pool + i;
Signature · 1 line
1.8.3.1
Quoted 42 lines · Tomas Henzl
Originally this was first patch in a series, but while the other patches were accepted, this one was forgotten.
The code below is adapted for the latest sources.The
cmd_pool_bitsis protected everywhere with a spinlock, we don't need thetest_and_set_bit,set_bitis enough and the loop can be removed too.Signed-off-by Tomas Henzl <[email protected]>--- drivers/scsi/hpsa.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)drivers/scsi/hpsa.c +6
-9@@ -4680,15 +4680,12 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)unsigned long flags;spin_lock_irqsave (&h->lock, flags); -do { - i =find_first_zero_bit (h->cmd_pool_bits, h->nr_cmds); -if (i == h->nr_cmds) { -spin_unlock_irqrestore (&h->lock, flags); -return NULL ; - } - }while (test_and_set_bit - (i & (BITS_PER_LONG -1 ), - h->cmd_pool_bits + (i /BITS_PER_LONG )) !=0 ); + i =find_first_zero_bit (h->cmd_pool_bits, h->nr_cmds); +if (i == h->nr_cmds) { +spin_unlock_irqrestore (&h->lock, flags); +return NULL ; + } +set_bit (i & (BITS_PER_LONG -1 ), h->cmd_pool_bits + (i /BITS_PER_LONG ));spin_unlock_irqrestore (&h->lock, flags); c = h->cmd_pool + i;Signature · 1 line
1.8.3.1
I don't quite remember, but I think it was forgotten because I became more interested in removing all the locks from the main i/o path of hpsa for performance reasons (I have had a rather large patch series in the works for quite a while now to do that, maybe it will be ready for 3.17, we'll see.) So this looks ok, but if things go according to plan it won't be relevant for very long.
-- steve
Quoted 39 lines · On 05/29/2014 04:51 PM, [email protected] wrote:
Quoted 38 lines · Tomas Henzl
Originally this was first patch in a series, but while the other patches were accepted, this one was forgotten.
The code below is adapted for the latest sources.The
cmd_pool_bitsis protected everywhere with a spinlock, we don't need thetest_and_set_bit,set_bitis enough and the loop can be removed too.Signed-off-by Tomas Henzl <[email protected]>--- drivers/scsi/hpsa.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)drivers/scsi/hpsa.c +6
-9@@ -4680,15 +4680,12 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)unsigned long flags;spin_lock_irqsave (&h->lock, flags); -do { - i =find_first_zero_bit (h->cmd_pool_bits, h->nr_cmds); -if (i == h->nr_cmds) { -spin_unlock_irqrestore (&h->lock, flags); -return NULL ; - } - }while (test_and_set_bit - (i & (BITS_PER_LONG -1 ), - h->cmd_pool_bits + (i /BITS_PER_LONG )) !=0 ); + i =find_first_zero_bit (h->cmd_pool_bits, h->nr_cmds); +if (i == h->nr_cmds) { +spin_unlock_irqrestore (&h->lock, flags); +return NULL ; + } +set_bit (i & (BITS_PER_LONG -1 ), h->cmd_pool_bits + (i /BITS_PER_LONG ));spin_unlock_irqrestore (&h->lock, flags);
Quoted 10 lines
Quoted 4 lines
drivers/scsi/hpsa.c +0
-0c = h->cmd_pool + i;Signature · 1 line
1.8.3.1I don't quite remember, but I think it was forgotten because I became more interested in removing all the locks from the main i/o path of hpsa for performance reasons (I have had a rather large patch series in the works for quite a while now to do that, maybe it will be ready for 3.17, we'll see.) So this looks ok, but if things go according to plan it won't be relevant for very long.
We have discussed it quite long ago - http://www.spinics.net/lists/linux-scsi/msg67782.html
the conclusion was "I think we should go with your patch as is -- leave the spin lock, but get rid of the unnecessary loop." That's why I've reposted it.
This patch it's just a smallish simplification and maybe performace impovement, (again very small), so when there is something being prepared which rewrites the function anyway we can skip it, I'll leave the decision on you.
tomash
Quoted 7 lines
-- steve
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html