DPDK patches and discussions
 help / color / mirror / Atom feed
From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
To: Wathsala Vithanage <wathsala.vithanage@arm.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	Andre Muezerie <andremue@linux.microsoft.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	David Marchand <david.marchand@redhat.com>,
	 Bruce Richardson <bruce.richardson@intel.com>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	nd <nd@arm.com>
Subject: RE: [PATCH] rcu: add deprecation notice about limit on defer queue element size
Date: Mon, 18 Aug 2025 13:17:49 +0000	[thread overview]
Message-ID: <3d00b2eba2a0481dada17c7139c40d2f@huawei.com> (raw)
In-Reply-To: <PAWPR08MB890987187E300A80DC4FD36B9F2CA@PAWPR08MB8909.eurprd08.prod.outlook.com>


> > >
> > > 10/07/2025 16:37, Andre Muezerie:
> > > > On Tue, Jul 01, 2025 at 04:17:20PM +0200, Thomas Monjalon wrote:
> > > > > 23/05/2025 01:37, Andre Muezerie:
> > > > > > The functions rte_rcu_qsbr_dq_create and rte_rcu_qsbr_dq_reclaim
> > > > > > establish no limit on the size of each element in the defer queue.
> > > > >
> > > > > Very good, we need more unlimited API in DPDK.
> > > > >
> > > > > > With DPDK 25.11 a hard limit will be set (``RTE_QSBR_ESIZE_MAX``).
> > > > >
> > > > > I think it is a step in the wrong direction.
> > > > > I prefer having no limit.
> > > > >
> > > > > > This will allow fixed C arrays to be used in the functions'
> > > > > > implementations, avoiding VLAs and use of alloca().
> > > > >
> > > > > I don't understand this justification.
> > > > > Why trying to remove the 2 alloca() in the lib RCU?
> > > > >
> > > >
> > > > Only because other developer expressed concerns that using alloca()
> > > > allows ill-intended callers to cause a stack overflow.
> > > > I personally also prefer to have no hardcoded limits.
> > >
> > > Yes I vote for keeping alloca().
> > >
> >
> > Probably it was me who expressed some concerns, sorry for late reply.
> > I can only repeat what I already replied to David:
> >
> > For that particular case, my reasons are mostly conceptual:
> > using alloca() doesn't really differ from simply using VLA, in fact it makes code
> > looks uglier.
> > I understand that we do want MSVC enabled, and in many cases such
> > mechanical replacement is ok, but probably better to avoid  it whenever
> > possible.
> >
> >  suppose we have 3 options:
> > 1) use predefined max value (it could be quite big to fit any reasonable usage,
> > let say 1KB or so).
> > 2) use alloca().
> > 3) come-up with some smarter approach.
> >
> > For 3) - I don't have any good ideas.
> > One option would be to create that ring RING_F_MP_HTS_ENQ  flags, then we
> > can use peek API  for enqueue part too (rte_ring_enqueue_bulk_elem_start).
> > That would solve an issue, as in that case we wouldn't need to make temp
> > copy of data on the stack.
> > My preference would be either 1) or 3), but I could leave with 2) too - specially
> > that I don't really use that part of RCU lib.
> > Would be really good to hear opinion of RCU lib maintainer.
> >
> > Konstantin
> 
> Just my 2c on the 3 options.
> 1) What's the right max size? I don't know, so I would rather leave this for the user.
> 2) I prefer this option over (1) due to above reason.
> 3) ring itself is tricky specially under relaxed memory, RCU Is already complex. So, I would rather keep them separate.

Not sure, what you are talking about?
Ring is already there, it was added as part of:
commit 706d306ea39c982193a0cedb37fd4bf8cae84972
Author: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Date:   Tue Apr 21 22:30:03 2020 -0500

    rcu: add resource reclamation APIs

    Add resource reclamation using defer queues to make it simple for
    applications and libraries to integrate rte_rcu library.

commit.
My suggestion was to change enq flags, that would allow us to use peek API.
I.E.:
       /* Decide the flags for the ring.
         * If MT safety is requested, use RTS for ring enqueue as most
         * use cases involve dq-enqueue happening on the control plane.
         * Ring dequeue is always HTS due to the possibility of revert.
         */
-       flags = RING_F_MP_RTS_ENQ;
+      flags = RING_F_MP_HTS_ENQ;
        if (params->flags & RTE_RCU_QSBR_DQ_MT_UNSAFE)
                flags = RING_F_SP_ENQ;
        flags |= RING_F_MC_HTS_DEQ;
 
Note that we already using HTS mode for dequeue.

> So, I prefer alloca() option.
> 
> Thanks
> 
> --wathsala

      reply	other threads:[~2025-08-18 13:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-22 23:37 Andre Muezerie
2025-07-01  7:56 ` David Marchand
2025-07-01 14:17 ` Thomas Monjalon
2025-07-10 14:37   ` Andre Muezerie
2025-07-11 12:38     ` Thomas Monjalon
2025-07-14  9:01       ` Konstantin Ananyev
2025-07-21 17:47         ` Thomas Monjalon
2025-08-07 19:56         ` Wathsala Vithanage
2025-08-18 13:17           ` Konstantin Ananyev [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3d00b2eba2a0481dada17c7139c40d2f@huawei.com \
    --to=konstantin.ananyev@huawei.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=andremue@linux.microsoft.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=nd@arm.com \
    --cc=thomas@monjalon.net \
    --cc=wathsala.vithanage@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).