netdev
[Top] [All Lists]

Re: atomic_dec_and_test for child dst needed in dst_destroy?

To: "David S. Miller" <davem@xxxxxxxxxxxxx>
Subject: Re: atomic_dec_and_test for child dst needed in dst_destroy?
From: Christoph Lameter <christoph@xxxxxxxxxxx>
Date: Tue, 5 Apr 2005 12:58:15 -0700 (PDT)
Cc: netdev@xxxxxxxxxxx
In-reply-to: <20050405125014.1ce46c66.davem@xxxxxxxxxxxxx>
References: <Pine.LNX.4.58.0504051155260.13697@xxxxxxxxxxxxxxxxx> <20050405123438.28f02423.davem@xxxxxxxxxxxxx> <Pine.LNX.4.58.0504051238390.14264@xxxxxxxxxxxxxxxxx> <20050405125014.1ce46c66.davem@xxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
On Tue, 5 Apr 2005, David S. Miller wrote:

> > I fail to see what the point of having a single instance of
> > atomic_dec_and_test for __refcnt is. In particular since the upper layers
> > guarantee that dst_destroy is not called multiple times for the same dst
> > entry.
>
> If this is true, what performance improvement could you possibly be
> seeing from this change?

We could make refcnt into an array of pointers that point to node specific
memory. This avoids cache line bouncing. However, you cannot atomically
dec_and_test an array. This is the only location where a dec_and_test is
used on dst->__refcnt.

> I know you are making this change for performance reasons, yet you
> aren't mentioning any details about this.  That information is
> part of what we need to know to judge this change.

I figured that it is not worth posting the patch if there is a reason
for the dec_and_test here. I was sure if my assessment of the role
of this atomic_dec_and_test here is correct.

> I've very hesistant to undo atomic operation memory barriers, after
> all of the weird problems we had in the neighbour cache.

We can put an explicit barier in if that is the only reason for the
atomic_dec_and_test.

<Prev in Thread] Current Thread [Next in Thread>