Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1624622

Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume

From Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Newsgroups linux.kernel
Subject Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume
Date 2017-04-17 11:50 +0200
Message-ID <txbse-7uP-11@gated-at.bofh.it> (permalink)
References <tx1Cy-1op-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hello!

On 4/17/2017 2:19 AM, Michael S. Tsirkin wrote:

> Applications that consume a batch of entries in one go
> can benefit from ability to return some of them back
> into the ring.
>
> Add an API for that - assuming there's space. If there's no space
> naturally we can't do this and have to drop entries, but this implies
> ring is full so we'd likely drop some anyway.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Jason, in my mind the biggest issue with your batching patchset is the
> backet drops on disconnect.  This API will help avoid that in the common

    Packet?

[...]
> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
> index 783e7f5..5fbeab4 100644
> --- a/include/linux/ptr_ring.h
> +++ b/include/linux/ptr_ring.h
> @@ -457,6 +457,63 @@ static inline int ptr_ring_init(struct ptr_ring *r, int size, gfp_t gfp)
>  	return 0;
>  }
>
> +/*
> + * Return entries into ring. Destroy entries that don't fit.
> + *
> + * Note: this is expected to be a rare slow path operation.
> + *
> + * Note: producer lock is nested within consumer lock, so if you
> + * resize you must make sure all uses nest correctly.
> + * In particular if you consume ring in interrupt or BH context, you must
> + * disable interrupts/BH when doing so.
> + */
> +static inline void ptr_ring_unconsume(struct ptr_ring *r, void **batch, int n,
> +				      void (*destroy)(void *))
> +{
> +	unsigned long flags;
> +	int head;
> +
> +	spin_lock_irqsave(&(r)->consumer_lock, flags);
> +	spin_lock(&(r)->producer_lock);

    The innermost parens seem pointless here....

[...]
> +done:
> +	/* Destroy all entries left in the batch. */
> +	while (n--) {
> +		destroy(batch[n]);
> +	}

    Braces not needed here.

> +	spin_unlock(&(r)->producer_lock);
> +	spin_unlock_irqrestore(&(r)->consumer_lock, flags);

    Same comment about the innermost parens...

[...]

MBR, Sergei

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH RFC] ptr_ring: add ptr_ring_unconsume "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-17 01:20 +0200
  Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-04-17 11:50 +0200
  Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume Jason Wang <jasowang@redhat.com> - 2017-04-18 05:10 +0200
    Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-24 01:30 +0200
      Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume Jason Wang <jasowang@redhat.com> - 2017-04-24 14:00 +0200
        Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-24 14:10 +0200

csiph-web