On Thu, Oct 28, 2010 at 5:36 AM, Robert Love <robert.w.love@xxxxxxxxx> wrote:
> On Fri, 2010-10-22 at 20:20 +0800, Hillf Danton wrote:
>> For allocating new exch from pool, Âscanning for free slot in exch
>> array fluctuates when exch pool is close to exhaustion.
>>
>> The fluctuation is smoothed, and the scan looks to be O(2).
>>
> Hi Hillf,
>
> Â I think this patch is fine, aside from a few minor nits below. I'm
> not sure how much this benefits us though. I don't think that it will
> hurt us, but I'd like to leave it in the fcoe-next tree a bit to make
> sure there aren't any adverse effects. I will fix the two issues I
> mention below and check it into fcoe-next, unless there are objections.
>
> Have you done any profiling with this patch to show the improvement?
It is O(1) when looking up exch. Very cool.
In allocating O(1) was tried but failed.
>
>> Signed-off-by: Hillf Danton <dhillf@xxxxxxxxx>
>> ---
>>
>> --- a/drivers/scsi/libfc/fc_exch.c  Â2010-09-13 07:07:38.000000000 +0800
>> +++ b/drivers/scsi/libfc/fc_exch.c  Â2010-10-22 20:02:54.000000000 +0800
>> @@ -67,6 +67,11 @@ struct workqueue_struct *fc_exch_workque
>> Âstruct fc_exch_pool {
>> Â Â Â u16 Â Â Â Â Â Â Ânext_index;
>> Â Â Â u16 Â Â Â Â Â Â Âtotal_exches;
>> +
>> + Â Â /* two cache of free slot in exch array */
>> + Â Â u16 Â Â Â Â Â Â Âleft;
>> + Â Â u16 Â Â Â Â Â Â Âright;
>> +
>>    spinlock_t    lock;
>> Â Â Â struct list_head ex_list;
>> Â};
>> @@ -397,13 +402,26 @@ static inline void fc_exch_ptr_set(struc
>> Âstatic void fc_exch_delete(struct fc_exch *ep)
>> Â{
>> Â Â Â struct fc_exch_pool *pool;
>> + Â Â u16 index;
>>
>> Â Â Â pool = ep->pool;
>> Â Â Â spin_lock_bh(&pool->lock);
>> Â Â Â WARN_ON(pool->total_exches <= 0);
>> Â Â Â pool->total_exches--;
>> - Â Â fc_exch_ptr_set(pool, (ep->xid - ep->em->min_xid) >> fc_cpu_order,
>> - Â Â Â Â Â Â Â Â Â Â NULL);
>> +
>> + Â Â /* update cache of free slot */
>> + Â Â index = (ep->xid - ep->em->min_xid) >> fc_cpu_order;
>> + Â Â if (pool->left == FC_XID_UNKNOWN)
>> + Â Â Â Â Â Â pool->left = index;
>> + Â Â else if (pool->right == FC_XID_UNKNOWN)
>> + Â Â Â Â Â Â pool->right = index;
>> + Â Â else
>> + Â Â Â Â Â Â /* XXX
>> + Â Â Â Â Â Â Â* next = entropy(index, left, right);
>> + Â Â Â Â Â Â Â**/
>
> We can remove this comment, right?
Node.
>
>> + Â Â Â Â Â Â pool->next_index = index;
>> +
>> + Â Â fc_exch_ptr_set(pool, index, NULL);
>> Â Â Â list_del(&ep->ex_list);
>> Â Â Â spin_unlock_bh(&pool->lock);
>> Â Â Â fc_exch_release(ep); Â Â/* drop hold for exch in mp */
>> @@ -679,6 +697,19 @@ static struct fc_exch *fc_exch_em_alloc(
>> Â Â Â pool = per_cpu_ptr(mp->pool, cpu);
>> Â Â Â spin_lock_bh(&pool->lock);
>> Â Â Â put_cpu();
>> +
>> + Â Â /* peek cache of free slot */
>> + Â Â if (pool->left != FC_XID_UNKNOWN) {
>> + Â Â Â Â Â Â index = pool->left;
>> + Â Â Â Â Â Â pool->left = FC_XID_UNKNOWN;
>> + Â Â Â Â Â Â goto hit;
>> + Â Â }
>> + Â Â if (pool->right != FC_XID_UNKNOWN) {
>> + Â Â Â Â Â Â index = pool->right;
>> + Â Â Â Â Â Â pool->right = FC_XID_UNKNOWN;
>> + Â Â Â Â Â Â goto hit;
>> + Â Â }
>> +
>> Â Â Â index = pool->next_index;
>> Â Â Â /* allocate new exch from pool */
>> Â Â Â while (fc_exch_ptr_get(pool, index)) {
>> @@ -687,7 +718,7 @@ static struct fc_exch *fc_exch_em_alloc(
>> Â Â Â Â Â Â Â Â Â Â Â goto err;
>> Â Â Â }
>> Â Â Â pool->next_index = index == mp->pool_max_index ? 0 : index + 1;
>> -
>> +hit:
>> Â Â Â fc_exch_hold(ep); Â Â Â /* hold for exch in mp */
>> Â Â Â spin_lock_init(&ep->ex_lock);
>> Â Â Â /*
>> @@ -2181,6 +2212,8 @@ struct fc_exch_mgr *fc_exch_mgr_alloc(st
>> Â Â Â Â Â Â Â goto free_mempool;
>> Â Â Â for_each_possible_cpu(cpu) {
>> Â Â Â Â Â Â Â pool = per_cpu_ptr(mp->pool, cpu);
>> + Â Â Â Â Â Â pool->left Â=
>
> I think we should initialize this without relying on the following line.
It looks like,
pool->left = pool->right = FC_XID_UNKNOWN;
right?
I want cache play its role after warm-up.
thanks //Hillf
>
>> + Â Â Â Â Â Â pool->right = FC_XID_UNKNOWN;
>> Â Â Â Â Â Â Â spin_lock_init(&pool->lock);
>> Â Â Â Â Â Â Â INIT_LIST_HEAD(&pool->ex_list);
>> Â Â Â }
>> _______________________________________________
>> devel mailing list
>> devel@xxxxxxxxxxxxx
>> http://www.open-fcoe.org/mailman/listinfo/devel
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
