| Message ID | 1424716517-17175-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | New |
| Headers | show |
This looks good, however the documentation of what ODP_[CONFIG]_SHM_NUM_BLOCKS means could use improvement. This the the number of separate SHM areas that can be reserved concurrently. Referring to this as "blocks" is somewhat confusing as that gives the impression it's somehow referring to the size of these areas when it's their count. Since each pool requires a corresponding SHM, this number always needs to be >= ODP_CONFIG_NUM_POOLS. I suspect the original value of 32 was sized at ODP_CONFIG_NUM_POOLS*2 since that's set to 16. Perhaps this should be a derived value from ODP_CONFIG_NUM_POOLS? Bill On Mon, Feb 23, 2015 at 12:35 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > ODP_SHM_NUM_BLOCKS was defined down in the implementation, move it out > to the config.h > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > include/odp/api/config.h | 6 ++++++ > platform/linux-generic/odp_shared_memory.c | 5 +---- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/include/odp/api/config.h b/include/odp/api/config.h > index 8f1139d..f767021 100644 > --- a/include/odp/api/config.h > +++ b/include/odp/api/config.h > @@ -121,6 +121,12 @@ extern "C" { > */ > #define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) > > +/** Maximum number of shared memory blocks. > + * > + * Limits how many blocks are available for calls to odp_shm_reserve() > + */ > +#define ODP_SHM_NUM_BLOCKS 32 > + > /** > * @} > */ > diff --git a/platform/linux-generic/odp_shared_memory.c > b/platform/linux-generic/odp_shared_memory.c > index dbaec22..9b6e92b 100644 > --- a/platform/linux-generic/odp_shared_memory.c > +++ b/platform/linux-generic/odp_shared_memory.c > @@ -15,6 +15,7 @@ > #include <odp/debug.h> > #include <odp_debug_internal.h> > #include <odp_align_internal.h> > +#include <odp/config.h> > > #include <unistd.h> > #include <sys/mman.h> > @@ -26,10 +27,6 @@ > #include <string.h> > #include <errno.h> > > - > -#define ODP_SHM_NUM_BLOCKS 32 > - > - > typedef struct { > char name[ODP_SHM_NAME_LEN]; > uint64_t size; > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
Will add your description and the following #define ODP_CONFIG_SHM_BLOCKS (ODP_CONFIG_POOLS * 2) On 23 February 2015 at 17:08, Bill Fischofer <bill.fischofer@linaro.org> wrote: > This looks good, however the documentation of what > ODP_[CONFIG]_SHM_NUM_BLOCKS means could use improvement. This the the > number of separate SHM areas that can be reserved concurrently. Referring > to this as "blocks" is somewhat confusing as that gives the impression it's > somehow referring to the size of these areas when it's their count. > > Since each pool requires a corresponding SHM, this number always needs to > be >= ODP_CONFIG_NUM_POOLS. I suspect the original value of 32 was sized > at ODP_CONFIG_NUM_POOLS*2 since that's set to 16. Perhaps this should be a > derived value from ODP_CONFIG_NUM_POOLS? > > Bill > > On Mon, Feb 23, 2015 at 12:35 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> ODP_SHM_NUM_BLOCKS was defined down in the implementation, move it out >> to the config.h >> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> --- >> include/odp/api/config.h | 6 ++++++ >> platform/linux-generic/odp_shared_memory.c | 5 +---- >> 2 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/include/odp/api/config.h b/include/odp/api/config.h >> index 8f1139d..f767021 100644 >> --- a/include/odp/api/config.h >> +++ b/include/odp/api/config.h >> @@ -121,6 +121,12 @@ extern "C" { >> */ >> #define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) >> >> +/** Maximum number of shared memory blocks. >> + * >> + * Limits how many blocks are available for calls to odp_shm_reserve() >> + */ >> +#define ODP_SHM_NUM_BLOCKS 32 >> + >> /** >> * @} >> */ >> diff --git a/platform/linux-generic/odp_shared_memory.c >> b/platform/linux-generic/odp_shared_memory.c >> index dbaec22..9b6e92b 100644 >> --- a/platform/linux-generic/odp_shared_memory.c >> +++ b/platform/linux-generic/odp_shared_memory.c >> @@ -15,6 +15,7 @@ >> #include <odp/debug.h> >> #include <odp_debug_internal.h> >> #include <odp_align_internal.h> >> +#include <odp/config.h> >> >> #include <unistd.h> >> #include <sys/mman.h> >> @@ -26,10 +27,6 @@ >> #include <string.h> >> #include <errno.h> >> >> - >> -#define ODP_SHM_NUM_BLOCKS 32 >> - >> - >> typedef struct { >> char name[ODP_SHM_NAME_LEN]; >> uint64_t size; >> -- >> 2.1.0 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > >
Except that I thought the original motivation for the change was that someone discovered that 32 was insufficient. On Tue, Feb 24, 2015 at 9:46 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > Will add your description > and the following > #define ODP_CONFIG_SHM_BLOCKS (ODP_CONFIG_POOLS * 2) > > > On 23 February 2015 at 17:08, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> This looks good, however the documentation of what >> ODP_[CONFIG]_SHM_NUM_BLOCKS means could use improvement. This the the >> number of separate SHM areas that can be reserved concurrently. Referring >> to this as "blocks" is somewhat confusing as that gives the impression it's >> somehow referring to the size of these areas when it's their count. >> >> Since each pool requires a corresponding SHM, this number always needs to >> be >= ODP_CONFIG_NUM_POOLS. I suspect the original value of 32 was sized >> at ODP_CONFIG_NUM_POOLS*2 since that's set to 16. Perhaps this should be a >> derived value from ODP_CONFIG_NUM_POOLS? >> >> Bill >> >> On Mon, Feb 23, 2015 at 12:35 PM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> >>> ODP_SHM_NUM_BLOCKS was defined down in the implementation, move it out >>> to the config.h >>> >>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>> --- >>> include/odp/api/config.h | 6 ++++++ >>> platform/linux-generic/odp_shared_memory.c | 5 +---- >>> 2 files changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/include/odp/api/config.h b/include/odp/api/config.h >>> index 8f1139d..f767021 100644 >>> --- a/include/odp/api/config.h >>> +++ b/include/odp/api/config.h >>> @@ -121,6 +121,12 @@ extern "C" { >>> */ >>> #define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) >>> >>> +/** Maximum number of shared memory blocks. >>> + * >>> + * Limits how many blocks are available for calls to odp_shm_reserve() >>> + */ >>> +#define ODP_SHM_NUM_BLOCKS 32 >>> + >>> /** >>> * @} >>> */ >>> diff --git a/platform/linux-generic/odp_shared_memory.c >>> b/platform/linux-generic/odp_shared_memory.c >>> index dbaec22..9b6e92b 100644 >>> --- a/platform/linux-generic/odp_shared_memory.c >>> +++ b/platform/linux-generic/odp_shared_memory.c >>> @@ -15,6 +15,7 @@ >>> #include <odp/debug.h> >>> #include <odp_debug_internal.h> >>> #include <odp_align_internal.h> >>> +#include <odp/config.h> >>> >>> #include <unistd.h> >>> #include <sys/mman.h> >>> @@ -26,10 +27,6 @@ >>> #include <string.h> >>> #include <errno.h> >>> >>> - >>> -#define ODP_SHM_NUM_BLOCKS 32 >>> - >>> - >>> typedef struct { >>> char name[ODP_SHM_NAME_LEN]; >>> uint64_t size; >>> -- >>> 2.1.0 >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> > > > -- > *Mike Holmes* > Linaro Sr Technical Manager > LNG - ODP >
Ok I had thought we would bump the POOLS, I will use this instead then #define ODP_CONFIG_SHM_BLOCKS (ODP_CONFIG_POOLS * 4) On 24 February 2015 at 11:01, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Except that I thought the original motivation for the change was that > someone discovered that 32 was insufficient. > > On Tue, Feb 24, 2015 at 9:46 AM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> Will add your description >> and the following >> #define ODP_CONFIG_SHM_BLOCKS (ODP_CONFIG_POOLS * 2) >> >> >> On 23 February 2015 at 17:08, Bill Fischofer <bill.fischofer@linaro.org> >> wrote: >> >>> This looks good, however the documentation of what >>> ODP_[CONFIG]_SHM_NUM_BLOCKS means could use improvement. This the the >>> number of separate SHM areas that can be reserved concurrently. Referring >>> to this as "blocks" is somewhat confusing as that gives the impression it's >>> somehow referring to the size of these areas when it's their count. >>> >>> Since each pool requires a corresponding SHM, this number always needs >>> to be >= ODP_CONFIG_NUM_POOLS. I suspect the original value of 32 was >>> sized at ODP_CONFIG_NUM_POOLS*2 since that's set to 16. Perhaps this >>> should be a derived value from ODP_CONFIG_NUM_POOLS? >>> >>> Bill >>> >>> On Mon, Feb 23, 2015 at 12:35 PM, Mike Holmes <mike.holmes@linaro.org> >>> wrote: >>> >>>> ODP_SHM_NUM_BLOCKS was defined down in the implementation, move it out >>>> to the config.h >>>> >>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>>> --- >>>> include/odp/api/config.h | 6 ++++++ >>>> platform/linux-generic/odp_shared_memory.c | 5 +---- >>>> 2 files changed, 7 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/include/odp/api/config.h b/include/odp/api/config.h >>>> index 8f1139d..f767021 100644 >>>> --- a/include/odp/api/config.h >>>> +++ b/include/odp/api/config.h >>>> @@ -121,6 +121,12 @@ extern "C" { >>>> */ >>>> #define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) >>>> >>>> +/** Maximum number of shared memory blocks. >>>> + * >>>> + * Limits how many blocks are available for calls to odp_shm_reserve() >>>> + */ >>>> +#define ODP_SHM_NUM_BLOCKS 32 >>>> + >>>> /** >>>> * @} >>>> */ >>>> diff --git a/platform/linux-generic/odp_shared_memory.c >>>> b/platform/linux-generic/odp_shared_memory.c >>>> index dbaec22..9b6e92b 100644 >>>> --- a/platform/linux-generic/odp_shared_memory.c >>>> +++ b/platform/linux-generic/odp_shared_memory.c >>>> @@ -15,6 +15,7 @@ >>>> #include <odp/debug.h> >>>> #include <odp_debug_internal.h> >>>> #include <odp_align_internal.h> >>>> +#include <odp/config.h> >>>> >>>> #include <unistd.h> >>>> #include <sys/mman.h> >>>> @@ -26,10 +27,6 @@ >>>> #include <string.h> >>>> #include <errno.h> >>>> >>>> - >>>> -#define ODP_SHM_NUM_BLOCKS 32 >>>> - >>>> - >>>> typedef struct { >>>> char name[ODP_SHM_NAME_LEN]; >>>> uint64_t size; >>>> -- >>>> 2.1.0 >>>> >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>> >>> >> >> >> -- >> *Mike Holmes* >> Linaro Sr Technical Manager >> LNG - ODP >> > >
diff --git a/include/odp/api/config.h b/include/odp/api/config.h index 8f1139d..f767021 100644 --- a/include/odp/api/config.h +++ b/include/odp/api/config.h @@ -121,6 +121,12 @@ extern "C" { */ #define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) +/** Maximum number of shared memory blocks. + * + * Limits how many blocks are available for calls to odp_shm_reserve() + */ +#define ODP_SHM_NUM_BLOCKS 32 + /** * @} */ diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-generic/odp_shared_memory.c index dbaec22..9b6e92b 100644 --- a/platform/linux-generic/odp_shared_memory.c +++ b/platform/linux-generic/odp_shared_memory.c @@ -15,6 +15,7 @@ #include <odp/debug.h> #include <odp_debug_internal.h> #include <odp_align_internal.h> +#include <odp/config.h> #include <unistd.h> #include <sys/mman.h> @@ -26,10 +27,6 @@ #include <string.h> #include <errno.h> - -#define ODP_SHM_NUM_BLOCKS 32 - - typedef struct { char name[ODP_SHM_NAME_LEN]; uint64_t size;
ODP_SHM_NUM_BLOCKS was defined down in the implementation, move it out to the config.h Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- include/odp/api/config.h | 6 ++++++ platform/linux-generic/odp_shared_memory.c | 5 +---- 2 files changed, 7 insertions(+), 4 deletions(-)