On Fri, Feb 02, 2024 at 10:45:34AM +0100, Mattias Rönnblom wrote:
> On 2024-02-01 18:02, Bruce Richardson wrote:
> > On Wed, Jan 24, 2024 at 12:34:50PM +0100, Mattias Rönnblom wrote:
> > > On 2024-01-19 18:43, Bruce Richardson wrote:
> > > > Clarify the meaning of the NEW, FORWARD and RELEASE event types.
> > > > For the fields in "rte_event" struct, enhance the comments on each to
> > > > clarify the field's use, and whether it is preserved between enqueue and
> > > > dequeue, and it's role, if any, in scheduling.
> > > > 
> > > > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
> > > > ---
> > > > 
> > > > As with the previous patch, please review this patch to ensure that the
> > > > expected semantics of the various event types and event fields have not
> > > > changed in an unexpected way.
> > > > ---
> > > >    lib/eventdev/rte_eventdev.h | 105 
> > > > ++++++++++++++++++++++++++----------
> > > >    1 file changed, 77 insertions(+), 28 deletions(-)
> > > > 
> > > > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > > > index cb13602ffb..4eff1c4958 100644
> > > > --- a/lib/eventdev/rte_eventdev.h
> > > > +++ b/lib/eventdev/rte_eventdev.h
> > <snip>
> > 
> > > >    /**
> > > > @@ -1473,53 +1475,100 @@ struct rte_event {
> > > >                         /**< Targeted flow identifier for the enqueue 
> > > > and
> > > >                          * dequeue operation.
> > > >                          * The value must be in the range of
> > > > -                        * [0, nb_event_queue_flows - 1] which
> > > > +                        * [0, @ref 
> > > > rte_event_dev_config.nb_event_queue_flows - 1] which
> > > 
> > > The same comment as I had before about ranges for unsigned types.
> > > 
> > Actually, is this correct, does a range actually apply here?
> > 
> > I thought that the number of queue flows supported was a guide as to how
> > internal HW resources were to be allocated, and that the flow_id was always
> > a 20-bit value, where it was up to the scheduler to work out how to map
> > that to internal atomic locks (when combined with queue ids etc.). It
> > should not be up to the app to have to do the range limiting itself!
> > 
> 
> Indeed, I also operated under this belief, which is reflected in DSW, which
> just takes the flow_id and (pseudo-)hash+mask it into the appropriate range.
> 
> Leaving it to the app allows app-level attempts to avoid collisions between
> large flows, I guess. Not sure I think apps will (or even should) really do
> this.

I'm just going to drop this restriction from v3.

Reply via email to