On 2023/8/19 19:57, Konstantin Ananyev wrote: > 18/08/2023 10:38, Jack Min пишет: >> On 2023/8/18 17:05, Konstantin Ananyev wrote: >>>> On 2023/8/17 22:06, Stephen Hemminger wrote: >>>>> On Thu, 17 Aug 2023 05:06:20 +0000 >>>>> Honnappa Nagarahalli wrote: >>>>> >>>>>> Hi Matan, Viacheslav, >>>>>>     Tyler pointed out that the function >>>>>> __mlx5_hws_cnt_pool_enqueue_revert is accessing the ring private >>>>>> structure members >>>> (prod.head and prod.tail) directly. Even though ' struct rte_ring' >>>> is a public structure (mainly because the library provides inline >>>> functions), the structure members are considered private to the >>>> ring library. So, this needs to be corrected. >>>>>> It looks like the function __mlx5_hws_cnt_pool_enqueue_revert is >>>>>> trying to revert things that were enqueued. It is not clear to >>>> me why this functionality is required. Can you provide the use case >>>> for this? We can discuss possible solutions. >>>>> How can reverting be thread safe? Consumer could have already >>>>> looked at them? >>>> Hey, >>>> >>>> In our case, this ring is SC/SP, only accessed by one thread >>>> (enqueue/dequeue/revert). >>>> >>>> The scenario we have "revert" is: >>>> >>>>    We use ring to manager our HW objects (counter in this case) and >>>> for >>>> each core (thread) has "cache" (a SC/SP ring) for sake of performance. >>>> >>>> 1. Get objects from "cache" firstly, if cache is empty, we fetch a >>>> bulk >>>> of free objects from global ring into cache. >>>> >>>> 2. Put (free) objects also into "cache" firstly, if cache is full, we >>>> flush a bulk of objects into global ring in order to make some >>>> rooms in >>>> cache. >>>> >>>> However, this HW object cannot be immediately reused after free. It >>>> needs time to be reset and then can be used again. >>>> >>>> So when we flush cache, we want to keep the first enqueued objects >>>> still >>>> stay there because they have more chance already be reset than the >>>> latest enqueued objects. >>>> >>>> Only flush recently enqueued objects back into global ring, act as >>>> "LIFO" behavior. >>>> >>>> This is why we require "revert" enqueued objects. >>>> >>> Wouldn't then simple stack fit you better? >>> Something like lib/stack/rte_stack_std.h, but even without spinlock >>> around? >> >> No, stack is always a "LIFO" struct, right? > > Yep. > >> >> Here first we need this cache works as "FIFO" in most cases (get/put) >> because the first enqueued objects have more chance that are already >> reset so can reuse them. >> >> We only require "LIFO" behavior when "flush" cache in order to make >> some room, so next free will be quick because it happens in our local >> cache, needn't access global ring. >> >> In short, we require a struct supports "FIFO" and "LIFO". > > Ok, thanks fro explanation. > So you need a ring, but with an ability to revert prod N elements > back, right? Yes, exactly!