> > >> -----Original Message----- >> From: Ola Liljedahl [mailto:ola.liljed...@arm.com] >> Sent: Wednesday, June 21, 2017 7:31 PM >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com>; >> Brian Brooks <brian.bro...@arm.com>; lng-odp@lists.linaro.org >> Cc: nd <n...@arm.com> >> Subject: Re: [lng-odp] [API-NEXT PATCH v9 4/6] linux-gen: sched >>scalable: >> add a concurrent queue >> >> >> >> >> >> On 20/06/2017, 15:12, "Savolainen, Petri (Nokia - FI/Espoo)" >> <petri.savolai...@nokia.com> wrote: >> >> >> +++ b/platform/linux-generic/include/odp_llqueue.h >> >> @@ -0,0 +1,309 @@ >> >> +/* Copyright (c) 2017, ARM Limited. >> >> + * All rights reserved. >> >> + * >> >> + * SPDX-License-Identifier: BSD-3-Clause >> >> + */ >> >> + >> >> +#ifndef ODP_LLQUEUE_H_ >> >> +#define ODP_LLQUEUE_H_ >> >> + >> >> +#include <odp/api/cpu.h> >> >> +#include <odp/api/hints.h> >> >> +#include <odp/api/spinlock.h> >> >> + >> >> +#include <odp_config_internal.h> >> >> +#include <odp_debug_internal.h> >> >> +#include <odp_cpu.h> >> >> + >> >> +#include <stdint.h> >> >> +#include <stdlib.h> >> >> + >> >> >> >>>>+/********************************************************************* >>>>* >> * >> >>* >> >> ****** >> >> + * Linked list queues >> >> + >> >> >> >>>>*********************************************************************** >>>>* >> * >> >>* >> >> ***/ >> >> + >> >> +struct llqueue; >> >> +struct llnode; >> >> + >> >> +static struct llnode *llq_head(struct llqueue *llq); >> >> +static void llqueue_init(struct llqueue *llq); >> >> +static void llq_enqueue(struct llqueue *llq, struct llnode *node); >> >> +static struct llnode *llq_dequeue(struct llqueue *llq); >> >> +static odp_bool_t llq_dequeue_cond(struct llqueue *llq, struct >>llnode >> >> *exp); >> >> +static odp_bool_t llq_cond_rotate(struct llqueue *llq, struct llnode >> >> *node); >> >> +static odp_bool_t llq_on_queue(struct llnode *node); >> >> + >> >> >> >>>>+/********************************************************************* >>>>* >> * >> >>* >> >> ****** >> >> + * The implementation(s) >> >> + >> >> >> >>>>*********************************************************************** >>>>* >> * >> >>* >> >> ***/ >> >> + >> >> +#define SENTINEL ((void *)~(uintptr_t)0) >> >> + >> >> +#ifdef CONFIG_LLDSCD >> >> +/* Implement queue operations using double-word LL/SC */ >> > >> >> + >> >> +#else >> >> +/* Implement queue operations protected by a spin lock */ >> >> + >> > >> >There's a lot of ifdef'ed code in this file, basically two full >>parallel >> >implementations. >> This horse has been flogged before on the mailing list. > >Nothing has changed in our ifdef policy. The less ifdef'ed code, the >better. This patch set introduces about 60 new #ifdef/#if/#ifndefs (when >header file guards are not calculated). Yes but we already had agreement on how to handle this one. Now you are disagreeing with the maintainers?
Ideally, files/definitions (on an item-by-item basis) in the arch-specific subdirectories should override the default implementations in the arch/default directory. Then we wouldn¹t have to put duplicate implementations in every arch subdirectory (and ODP linux-generic would build for any architecture, also those not explicitly recognised). Arch/default would have an llqueue.c with the lock-based code and arch/arm would have an llqueue.c with the lld/scd implementation. > > >> >> > The first is built only for ARM and the second for the rest. Would >>there >> >be a way to build both always ? >> For ARMv7a and ARMv8a, you could build both versions. You really want to >> use the LL/SC version on these architectures. >> >> For architectures without double-word LL/SC, only the lock-based version >> can be built. > > >You could *compile* the lock version always. It's based on locks, not on >arch specific instructions. Yes we could always build the lock-based code but then we would need to refer to these functions from some other function in order to avoid the function-not-used error (but there would most likely be some code that would not actually be called even if it is compiled). It can be done. > >-Petri >
