From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: [PATCH v4 16/17] ring: add sched_yield to avoid spin forever Date: Tue, 10 Feb 2015 17:53:01 +0100 Message-ID: <54DA376D.2070709@6wind.com> References: <1422491072-5114-1-git-send-email-cunming.liang@intel.com> <1422842559-13617-1-git-send-email-cunming.liang@intel.com> <1422842559-13617-17-git-send-email-cunming.liang@intel.com> <54D4DB9F.6080601@6wind.com> <2601191342CEEE43887BDE71AB977258213E461C@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit To: "Ananyev, Konstantin" , "Liang, Cunming" , "dev-VfR2kkLFssw@public.gmane.org" Return-path: In-Reply-To: <2601191342CEEE43887BDE71AB977258213E461C-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" Hi Konstantin, On 02/09/2015 04:43 PM, Ananyev, Konstantin wrote: >> The ring library was designed with the assumption that the code is not >> preemptable. The code is lock-less but not wait-less. Actually, if the >> code is preempted at a bad moment, it can spin forever until it's >> unscheduled. >> >> I wonder if adding a sched_yield() may not penalize the current >> implementations that only use one pthread per core? Even if there >> is only one pthread in the scheduler queue for this CPU, calling >> the scheduler code may cost thousands of cycles. >> >> Also, where does this value "RTE_RING_PAUSE_REP 0x100" comes from? >> Why 0x100 is better than 42 or than 10000? > > The idea was to have something few times bigger than actual number > active cores in the system, to minimise chance of a sched_yield() being called > for the case when we have one thread per physical core. > My thought was that having that many repeats would make such chance neglectable. > Though, right now, I don't have any data to back it up. > >> I think it could be good to check if there is a performance impact >> with this change, especially where there is a lot of contention on >> the ring. If it has an impact, what about adding a compile or runtime >> option? > > Good idea, probably we should make RTE_RING_PAUSE_REP configuration option > and let say avoid emitting ' sched_yield();' at all, if RTE_RING_PAUSE_REP == 0. Yes, it looks like a good compromise. Olivier