From: Bill Fischofer [mailto:[email protected]] Sent: Monday, June 27, 2016 5:20 PM To: Savolainen, Petri (Nokia - FI/Espoo) <[email protected]> Cc: [email protected] Subject: Re: [lng-odp] [PATCH 2/2] validation: queue: use malloc to avoid artificial limits on max_queues
On Mon, Jun 27, 2016 at 2:39 AM, Savolainen, Petri (Nokia - FI/Espoo) <[email protected]> wrote: > -----Original Message----- > From: lng-odp [mailto:[email protected]] On Behalf Of Bill > Fischofer > Sent: Monday, June 27, 2016 5:09 AM > To: [email protected] > Subject: [lng-odp] [PATCH 2/2] validation: queue: use malloc to avoid > artificial limits on max_queues > > odp_queue_capability() returns max_queues which may be more than 64K. > Use malloc to allocate an array of queue handles to test the ability to > create max_queues to avoid limiting the test to 64K queues. > > Signed-off-by: Bill Fischofer <[email protected]> > --- > test/validation/queue/queue.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c > index c21897b..9af8c9c 100644 > --- a/test/validation/queue/queue.c > +++ b/test/validation/queue/queue.c > @@ -11,7 +11,6 @@ > #define MAX_BUFFER_QUEUE (8) > #define MSG_POOL_SIZE (4 * 1024 * 1024) > #define CONFIG_MAX_ITERATION (100) > -#define MAX_QUEUES (64 * 1024) > > static int queue_context = 0xff; > static odp_pool_t pool; > @@ -55,7 +54,7 @@ void queue_test_capa(void) > odp_queue_capability_t capa; > odp_queue_param_t qparams; > char name[ODP_QUEUE_NAME_LEN]; > - odp_queue_t queue[MAX_QUEUES]; > + odp_queue_t *queue; > int num_queues, i; > > memset(&capa, 0, sizeof(odp_queue_capability_t)); > @@ -71,10 +70,9 @@ void queue_test_capa(void) > > name[ODP_QUEUE_NAME_LEN - 1] = 0; > > - if (capa.max_queues > MAX_QUEUES) > - num_queues = MAX_QUEUES; > - else > - num_queues = capa.max_queues; > + num_queues = capa.max_queues; > + queue = malloc(num_queues * sizeof(odp_queue_t)); Num_queues may be large. E.g. a malloc of 100M * 8 bytes may jam a system which is light on DRAM but has a hard disk, etc. I think it's better to limit the test to some number which will run sanely on any system. If the purpose of the test is to verify that an application can allocate the max_queues number of queues, then this is the way to do that. You're suggesting that a platform would support a number of queues that cannot actually be allocated because is would have insufficient memory to support the maximum? That doesn't sound like a reasonable design. If there is to be a "cap" it should be in the implementation of odp_queue_capability(). <<< Reply to a HTML mail ... >>> Purpose of the test is to first of all check that .max_queues field is defined and filled. Malloc may not be usable always for storing maximum number of resource handles. May be the platform under test provides 16 GB shm memory, but only 1GB general system memory backed up by a hard disk. A malloc of 800MB could stuck the system totally (generally fast path applications avoid using malloc), but a 800MB shm allocation would work just fine. I think it would be better idea to add another test case which really tries to create and use the max number of queues, groups, priorities, etc. So that it's easier contain failure cases between the capability call not supported vs. failure to use maximum (very large) number of resource X. <<< ... reply ends >>> Isn't there a less intrusive workaround for the coverity issue. The Coverity issue is legitimate and changing from uint32_t to int is the simplest (and most correct) solution. In what way is that intrusive? <<< Reply to a HTML mail >>> Did realize later on that patch 1/2 was fixing that already. But also there int32_t, or uint32_t with check against i == 0, would be more appropriate fix than using int. C spec guarantees that int is at least 16 bits (not 32 bits) which in theory could cause problems when int is actually 16 bits and number of queues is >32k. That's why the capa.max_queues is uint32_t and not int. -Petri _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
