| Message ID | 1433347561-5761-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 03, 2015 at 07:06:01PM +0300, Maxim Uvarov wrote: > Use the right order in test: first lookup for pktio, > if it's not exist - create it. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > test/validation/odp_pktio.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c > index e1025d6..227ae83 100644 > --- a/test/validation/odp_pktio.c > +++ b/test/validation/odp_pktio.c > @@ -225,9 +225,9 @@ static odp_pktio_t create_pktio(const char *iface, int num) > { > odp_pktio_t pktio; > > - pktio = odp_pktio_open(iface, pool[num]); > + pktio = odp_pktio_lookup(iface); > if (pktio == ODP_PKTIO_INVALID) > - pktio = odp_pktio_lookup(iface); > + pktio = odp_pktio_open(iface, pool[num]); > CU_ASSERT(pktio != ODP_PKTIO_INVALID); > CU_ASSERT(odp_pktio_to_u64(pktio) != > odp_pktio_to_u64(ODP_PKTIO_INVALID)); > -- > 1.9.1 > Is how it is actually causing a problem? It was intentionally that way around to avoid a race condition, with the lookup->open sequence two threads may both fail on the lookup and then both call open and only one thread ends up with a valid handle. If they call open first, one will succeed and the other fails then calls lookup which should succeed so both threads get a valid handle. Granted this particular code is currently single threaded, but I prefer the open->lookup sequence generally.
On 06/03/15 19:53, Stuart Haslam wrote: > On Wed, Jun 03, 2015 at 07:06:01PM +0300, Maxim Uvarov wrote: >> Use the right order in test: first lookup for pktio, >> if it's not exist - create it. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> test/validation/odp_pktio.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c >> index e1025d6..227ae83 100644 >> --- a/test/validation/odp_pktio.c >> +++ b/test/validation/odp_pktio.c >> @@ -225,9 +225,9 @@ static odp_pktio_t create_pktio(const char *iface, int num) >> { >> odp_pktio_t pktio; >> >> - pktio = odp_pktio_open(iface, pool[num]); >> + pktio = odp_pktio_lookup(iface); >> if (pktio == ODP_PKTIO_INVALID) >> - pktio = odp_pktio_lookup(iface); >> + pktio = odp_pktio_open(iface, pool[num]); >> CU_ASSERT(pktio != ODP_PKTIO_INVALID); >> CU_ASSERT(odp_pktio_to_u64(pktio) != >> odp_pktio_to_u64(ODP_PKTIO_INVALID)); >> -- >> 1.9.1 >> > Is how it is actually causing a problem? It was intentionally that way > around to avoid a race condition, with the lookup->open sequence two > threads may both fail on the lookup and then both call open and only one > thread ends up with a valid handle. If they call open first, one will > succeed and the other fails then calls lookup which should succeed so > both threads get a valid handle. > > Granted this particular code is currently single threaded, but I prefer > the open->lookup sequence generally. > No, there was no any problem. I just thought that it will be logical to do lookup first then open in single thread app. But I'm also ok to drop this patch in case if we will add more threads in future. Maxim.
diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c index e1025d6..227ae83 100644 --- a/test/validation/odp_pktio.c +++ b/test/validation/odp_pktio.c @@ -225,9 +225,9 @@ static odp_pktio_t create_pktio(const char *iface, int num) { odp_pktio_t pktio; - pktio = odp_pktio_open(iface, pool[num]); + pktio = odp_pktio_lookup(iface); if (pktio == ODP_PKTIO_INVALID) - pktio = odp_pktio_lookup(iface); + pktio = odp_pktio_open(iface, pool[num]); CU_ASSERT(pktio != ODP_PKTIO_INVALID); CU_ASSERT(odp_pktio_to_u64(pktio) != odp_pktio_to_u64(ODP_PKTIO_INVALID));
Use the right order in test: first lookup for pktio, if it's not exist - create it. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- test/validation/odp_pktio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)