Hi Eric,
On Wed, 2016-09-28 at 18:42 -0700, Eric Dumazet wrote:
> On Wed, 2016-09-28 at 12:52 +0200, Paolo Abeni wrote:
>
> > +static void udp_rmem_release(struct sock *sk, int partial)
> > +{
> > + struct udp_sock *up = udp_sk(sk);
> > + int fwd, amt;
> > +
> > + if (partial && !udp_under_memory_pressure(sk))
> > + return;
> > +
> > + /* we can have concurrent release; if we catch any conflict
> > + * we let only one of them do the work
> > + */
> > + if (atomic_dec_if_positive(&up->can_reclaim) < 0)
> > + return;
> > +
> > + fwd = __udp_forward(up, atomic_read(&sk->sk_rmem_alloc));
> > + if (fwd < SK_MEM_QUANTUM + partial) {
> > + atomic_inc(&up->can_reclaim);
> > + return;
> > + }
> > +
> > + amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
> > + atomic_sub(amt, &up->mem_allocated);
> > + atomic_inc(&up->can_reclaim);
> > +
> > + __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> > + sk->sk_forward_alloc = fwd - amt;
> > +}
Thank you for reviewing this!
> This is racy...
Could you please elaborate?
> all these atomics make me nervous...
I'd like to drop some of them if possible.
atomic_inc(&up->can_reclaim);
could probably be replaced with atomic_set(&up->can_reclaim, 1) since we
don't have concurrent processes doing that and can_reclaim.counter is
known to be 0 at that point.
Performance wise the impact is minimal, since in normal condition we do
the reclaim only on socket shutdown.
Paolo
