| Message ID | 1414007408-37435-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
Default is 0. It includes all current and future flags. There's no point to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not include ODP_SHM_SW_ONLY. This is how flag parameters work in general. -Petri > -----Original Message----- > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > Sent: Wednesday, October 22, 2014 10:50 PM > To: lng-odp@lists.linaro.org > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > Although adding the default description to the list of legal #defined > values > helps the reader understand what the default means, it looks like this > could be more clearly coded as an enum. > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > b/platform/linux-generic/include/api/odp_shared_memory.h > index d8d40dd..d2965af 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -34,6 +34,7 @@ extern "C" { > */ > > /* Share level */ > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > * @param size Block size in bytes > * @param align Block alignment in bytes > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > 0. > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > ODP_SHM_SW_AND_HW > * > * @return Pointer to the reserved block, or NULL > */ > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
We need to document what 0 means to the user of the API, a document reader gets no information what behaviour to expect. Describing that as code feels more accurate than writing a comment because an application wanting to make it explicit can use that flag and their code is then documented clearly which behaviour they expect. If the default behaviour changes in future, having explicitly passed a flag describing the behaviour the app wants allows that app to continue working after the change. Would it be better for the default to be the most restrictive and subsequent flags relax that, is wide open access the most prevalent case ? On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > > Default is 0. It includes all current and future flags. There's no point > to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not > include ODP_SHM_SW_ONLY. This is how flag parameters work in general. > > -Petri > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Wednesday, October 22, 2014 10:50 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > Although adding the default description to the list of legal #defined > > values > > helps the reader understand what the default means, it looks like this > > could be more clearly coded as an enum. > > > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > > b/platform/linux-generic/include/api/odp_shared_memory.h > > index d8d40dd..d2965af 100644 > > --- a/platform/linux-generic/include/api/odp_shared_memory.h > > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > > @@ -34,6 +34,7 @@ extern "C" { > > */ > > > > /* Share level */ > > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > > * @param size Block size in bytes > > * @param align Block alignment in bytes > > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value > is > > 0. > > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value > is > > ODP_SHM_SW_AND_HW > > * > > * @return Pointer to the reserved block, or NULL > > */ > > -- > > 1.9.1 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp >
0 means that application does not have any special requirements. Each special requirement is indicated by a flag. If you use shm with any ODP API and within the local ODP instance, it’s default behavior. Later on we may add more limitation/access right flags. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org] Sent: Thursday, October 23, 2014 3:39 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value We need to document what 0 means to the user of the API, a document reader gets no information what behaviour to expect. Describing that as code feels more accurate than writing a comment because an application wanting to make it explicit can use that flag and their code is then documented clearly which behaviour they expect. If the default behaviour changes in future, having explicitly passed a flag describing the behaviour the app wants allows that app to continue working after the change. Would it be better for the default to be the most restrictive and subsequent flags relax that, is wide open access the most prevalent case ? On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: Default is 0. It includes all current and future flags. There's no point to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not include ODP_SHM_SW_ONLY. This is how flag parameters work in general. -Petri > -----Original Message----- > From: lng-odp-bounces@lists.linaro.org<mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-<mailto:lng-odp-> > bounces@lists.linaro.org<mailto:bounces@lists.linaro.org>] On Behalf Of ext Mike Holmes > Sent: Wednesday, October 22, 2014 10:50 PM > To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> > --- > Although adding the default description to the list of legal #defined > values > helps the reader understand what the default means, it looks like this > could be more clearly coded as an enum. > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > b/platform/linux-generic/include/api/odp_shared_memory.h > index d8d40dd..d2965af 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -34,6 +34,7 @@ extern "C" { > */ > > /* Share level */ > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > * @param size Block size in bytes > * @param align Block alignment in bytes > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > 0. > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > ODP_SHM_SW_AND_HW > * > * @return Pointer to the reserved block, or NULL > */ > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Linaro Sr Technical Manager LNG - ODP
On 10/23/2014 05:07 PM, Savolainen, Petri (NSN - FI/Espoo) wrote: > > 0 means that application does not have any special requirements. > Can we add that words to the function description? I think it's clear. > Each special requirement is indicated by a flag. If you use shm with > any ODP API and within the local ODP instance, it’s default behavior. > Later on we may add more limitation/access right flags. > > -Petri > > *From:*ext Mike Holmes [mailto:mike.holmes@linaro.org] > *Sent:* Thursday, October 23, 2014 3:39 PM > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default > value > > We need to document what 0 means to the user of the API, a document > reader gets no information what behaviour to expect. > > Describing that as code feels more accurate than writing a comment > because an application wanting to make it explicit can use that flag > and their code is then documented clearly which behaviour they expect. > > If the default behaviour changes in future, having explicitly passed a > flag describing the behaviour the app wants allows that app to > continue working after the change. > > Would it be better for the default to be the most restrictive and > subsequent flags relax that, is wide open access the most prevalent case ? > > On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolainen@nsn.com <mailto:petri.savolainen@nsn.com>> wrote: > > > Default is 0. It includes all current and future flags. There's no > point to define ODP_SHM_SW_AND_HW, that's defined when the flag value > does not include ODP_SHM_SW_ONLY. This is how flag parameters work in > general. > > -Petri > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org > <mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp- > <mailto:lng-odp-> > > bounces@lists.linaro.org <mailto:bounces@lists.linaro.org>] On > Behalf Of ext Mike Holmes > > Sent: Wednesday, October 22, 2014 10:50 PM > > To: lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> > > --- > > Although adding the default description to the list of legal #defined > > values > > helps the reader understand what the default means, it looks like this > > could be more clearly coded as an enum. > > > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > > b/platform/linux-generic/include/api/odp_shared_memory.h > > index d8d40dd..d2965af 100644 > > --- a/platform/linux-generic/include/api/odp_shared_memory.h > > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > > @@ -34,6 +34,7 @@ extern "C" { > > */ > > > > /* Share level */ > > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > > * @param size Block size in bytes > > * @param align Block alignment in bytes > > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default > value is > > 0. > > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default > value is > > ODP_SHM_SW_AND_HW > > * > > * @return Pointer to the reserved block, or NULL > > */ > > -- > > 1.9.1 > > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > -- > > *Mike Holmes* > > Linaro Sr Technical Manager > > LNG - ODP > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
So shouldn't we define ODP_SHM_ANY 0, then an application could call to get that behaviour and if the default ever changes they will be covered. On 23 October 2014 09:07, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > 0 means that application does not have any special requirements. Each > special requirement is indicated by a flag. If you use shm with any ODP API > and within the local ODP instance, it’s default behavior. Later on we may > add more limitation/access right flags. > > > > -Petri > > > > > > *From:* ext Mike Holmes [mailto:mike.holmes@linaro.org] > *Sent:* Thursday, October 23, 2014 3:39 PM > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default > value > > > > We need to document what 0 means to the user of the API, a document reader > gets no information what behaviour to expect. > > Describing that as code feels more accurate than writing a comment because > an application wanting to make it explicit can use that flag and their code > is then documented clearly which behaviour they expect. > > If the default behaviour changes in future, having explicitly passed a > flag describing the behaviour the app wants allows that app to continue > working after the change. > > > > Would it be better for the default to be the most restrictive and > subsequent flags relax that, is wide open access the most prevalent case ? > > > > > > On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > > > Default is 0. It includes all current and future flags. There's no point > to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not > include ODP_SHM_SW_ONLY. This is how flag parameters work in general. > > -Petri > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Wednesday, October 22, 2014 10:50 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > Although adding the default description to the list of legal #defined > > values > > helps the reader understand what the default means, it looks like this > > could be more clearly coded as an enum. > > > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > > b/platform/linux-generic/include/api/odp_shared_memory.h > > index d8d40dd..d2965af 100644 > > --- a/platform/linux-generic/include/api/odp_shared_memory.h > > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > > @@ -34,6 +34,7 @@ extern "C" { > > */ > > > > /* Share level */ > > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > > * @param size Block size in bytes > > * @param align Block alignment in bytes > > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value > is > > 0. > > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value > is > > ODP_SHM_SW_AND_HW > > * > > * @return Pointer to the reserved block, or NULL > > */ > > -- > > 1.9.1 > > > > > > > _______________________________________________ > > 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 >
Default should stay as zero also in the future. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org] Sent: Thursday, October 23, 2014 4:49 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value So shouldn't we define ODP_SHM_ANY 0, then an application could call to get that behaviour and if the default ever changes they will be covered. On 23 October 2014 09:07, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: 0 means that application does not have any special requirements. Each special requirement is indicated by a flag. If you use shm with any ODP API and within the local ODP instance, it’s default behavior. Later on we may add more limitation/access right flags. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>] Sent: Thursday, October 23, 2014 3:39 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value We need to document what 0 means to the user of the API, a document reader gets no information what behaviour to expect. Describing that as code feels more accurate than writing a comment because an application wanting to make it explicit can use that flag and their code is then documented clearly which behaviour they expect. If the default behaviour changes in future, having explicitly passed a flag describing the behaviour the app wants allows that app to continue working after the change. Would it be better for the default to be the most restrictive and subsequent flags relax that, is wide open access the most prevalent case ? On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: Default is 0. It includes all current and future flags. There's no point to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not include ODP_SHM_SW_ONLY. This is how flag parameters work in general. -Petri > -----Original Message----- > From: lng-odp-bounces@lists.linaro.org<mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-<mailto:lng-odp-> > bounces@lists.linaro.org<mailto:bounces@lists.linaro.org>] On Behalf Of ext Mike Holmes > Sent: Wednesday, October 22, 2014 10:50 PM > To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> > --- > Although adding the default description to the list of legal #defined > values > helps the reader understand what the default means, it looks like this > could be more clearly coded as an enum. > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > b/platform/linux-generic/include/api/odp_shared_memory.h > index d8d40dd..d2965af 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -34,6 +34,7 @@ extern "C" { > */ > > /* Share level */ > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > * @param size Block size in bytes > * @param align Block alignment in bytes > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > 0. > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > ODP_SHM_SW_AND_HW > * > * @return Pointer to the reserved block, or NULL > */ > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Linaro Sr Technical Manager LNG - ODP -- Mike Holmes Linaro Sr Technical Manager LNG - ODP
Thanks, I could be satisfied if the doc clearly states "The default indicates that application does not have any special requirements, this default is guaranteed never to change" But I still think a define is better - compare these for readability in the application foo(0) or slightly better foo(0); // take the default and foo(NO_ACCESS_RESTRICTOINS) On 23 October 2014 09:56, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > Default should stay as zero also in the future. > > > > -Petri > > > > *From:* ext Mike Holmes [mailto:mike.holmes@linaro.org] > *Sent:* Thursday, October 23, 2014 4:49 PM > > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default > value > > > > So shouldn't we define ODP_SHM_ANY 0, then an application could call to > get that behaviour and if the default ever changes they will be covered. > > > > On 23 October 2014 09:07, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > > 0 means that application does not have any special requirements. Each > special requirement is indicated by a flag. If you use shm with any ODP API > and within the local ODP instance, it’s default behavior. Later on we may > add more limitation/access right flags. > > > > -Petri > > > > > > *From:* ext Mike Holmes [mailto:mike.holmes@linaro.org] > *Sent:* Thursday, October 23, 2014 3:39 PM > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default > value > > > > We need to document what 0 means to the user of the API, a document reader > gets no information what behaviour to expect. > > Describing that as code feels more accurate than writing a comment because > an application wanting to make it explicit can use that flag and their code > is then documented clearly which behaviour they expect. > > If the default behaviour changes in future, having explicitly passed a > flag describing the behaviour the app wants allows that app to continue > working after the change. > > > > Would it be better for the default to be the most restrictive and > subsequent flags relax that, is wide open access the most prevalent case ? > > > > > > On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > > > Default is 0. It includes all current and future flags. There's no point > to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not > include ODP_SHM_SW_ONLY. This is how flag parameters work in general. > > -Petri > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Wednesday, October 22, 2014 10:50 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > Although adding the default description to the list of legal #defined > > values > > helps the reader understand what the default means, it looks like this > > could be more clearly coded as an enum. > > > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > > b/platform/linux-generic/include/api/odp_shared_memory.h > > index d8d40dd..d2965af 100644 > > --- a/platform/linux-generic/include/api/odp_shared_memory.h > > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > > @@ -34,6 +34,7 @@ extern "C" { > > */ > > > > /* Share level */ > > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > > * @param size Block size in bytes > > * @param align Block alignment in bytes > > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value > is > > 0. > > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value > is > > ODP_SHM_SW_AND_HW > > * > > * @return Pointer to the reserved block, or NULL > > */ > > -- > > 1.9.1 > > > > > > > _______________________________________________ > > 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 > > > > > > -- > > *Mike Holmes* > > Linaro Sr Technical Manager > > LNG - ODP >
Restrictions will be highlighted with flags. So you’d compare … foo(param1, param2, 0); vs. foo(param1, param2, ODP_SHM_FLAG1 | ODP_SHM_FLAG2 | ODP_SHM_FLAG3 ); .. and spot easily that the second call has defined some special requirements. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org] Sent: Thursday, October 23, 2014 6:08 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value Thanks, I could be satisfied if the doc clearly states "The default indicates that application does not have any special requirements, this default is guaranteed never to change" But I still think a define is better - compare these for readability in the application foo(0) or slightly better foo(0); // take the default and foo(NO_ACCESS_RESTRICTOINS) On 23 October 2014 09:56, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: Default should stay as zero also in the future. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>] Sent: Thursday, October 23, 2014 4:49 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value So shouldn't we define ODP_SHM_ANY 0, then an application could call to get that behaviour and if the default ever changes they will be covered. On 23 October 2014 09:07, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: 0 means that application does not have any special requirements. Each special requirement is indicated by a flag. If you use shm with any ODP API and within the local ODP instance, it’s default behavior. Later on we may add more limitation/access right flags. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>] Sent: Thursday, October 23, 2014 3:39 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value We need to document what 0 means to the user of the API, a document reader gets no information what behaviour to expect. Describing that as code feels more accurate than writing a comment because an application wanting to make it explicit can use that flag and their code is then documented clearly which behaviour they expect. If the default behaviour changes in future, having explicitly passed a flag describing the behaviour the app wants allows that app to continue working after the change. Would it be better for the default to be the most restrictive and subsequent flags relax that, is wide open access the most prevalent case ? On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: Default is 0. It includes all current and future flags. There's no point to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not include ODP_SHM_SW_ONLY. This is how flag parameters work in general. -Petri > -----Original Message----- > From: lng-odp-bounces@lists.linaro.org<mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-<mailto:lng-odp-> > bounces@lists.linaro.org<mailto:bounces@lists.linaro.org>] On Behalf Of ext Mike Holmes > Sent: Wednesday, October 22, 2014 10:50 PM > To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> > --- > Although adding the default description to the list of legal #defined > values > helps the reader understand what the default means, it looks like this > could be more clearly coded as an enum. > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > b/platform/linux-generic/include/api/odp_shared_memory.h > index d8d40dd..d2965af 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -34,6 +34,7 @@ extern "C" { > */ > > /* Share level */ > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > * @param size Block size in bytes > * @param align Block alignment in bytes > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > 0. > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > ODP_SHM_SW_AND_HW > * > * @return Pointer to the reserved block, or NULL > */ > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Linaro Sr Technical Manager LNG - ODP -- Mike Holmes Linaro Sr Technical Manager LNG - ODP -- Mike Holmes Linaro Sr Technical Manager LNG - ODP
Hi Petri, I remember default (0x0) for shared mem means accessible by SW and HW. I think accessible by HW is rather a special requirement. Alex On 24 October 2014 10:05, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > Restrictions will be highlighted with flags. So you’d compare … > > > > foo(param1, param2, 0); > > > > vs. > > > > foo(param1, param2, ODP_SHM_FLAG1 | ODP_SHM_FLAG2 | ODP_SHM_FLAG3 ); > > > > .. and spot easily that the second call has defined some special > requirements. > > > > > > -Petri > > > > > > *From:* ext Mike Holmes [mailto:mike.holmes@linaro.org] > *Sent:* Thursday, October 23, 2014 6:08 PM > > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default > value > > > > Thanks, I could be satisfied if the doc clearly states > > "The default indicates that application does not have any special > requirements, this default is guaranteed never to change" > > > > But I still think a define is better - compare these for readability in > the application > > foo(0) > > or slightly better > > foo(0); // take the default > > and > > foo(NO_ACCESS_RESTRICTOINS) > > > > On 23 October 2014 09:56, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > > Default should stay as zero also in the future. > > > > -Petri > > > > *From:* ext Mike Holmes [mailto:mike.holmes@linaro.org] > *Sent:* Thursday, October 23, 2014 4:49 PM > > > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default > value > > > > So shouldn't we define ODP_SHM_ANY 0, then an application could call to > get that behaviour and if the default ever changes they will be covered. > > > > On 23 October 2014 09:07, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > > 0 means that application does not have any special requirements. Each > special requirement is indicated by a flag. If you use shm with any ODP API > and within the local ODP instance, it’s default behavior. Later on we may > add more limitation/access right flags. > > > > -Petri > > > > > > *From:* ext Mike Holmes [mailto:mike.holmes@linaro.org] > *Sent:* Thursday, October 23, 2014 3:39 PM > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default > value > > > > We need to document what 0 means to the user of the API, a document reader > gets no information what behaviour to expect. > > Describing that as code feels more accurate than writing a comment because > an application wanting to make it explicit can use that flag and their code > is then documented clearly which behaviour they expect. > > If the default behaviour changes in future, having explicitly passed a > flag describing the behaviour the app wants allows that app to continue > working after the change. > > > > Would it be better for the default to be the most restrictive and > subsequent flags relax that, is wide open access the most prevalent case ? > > > > > > On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > > > Default is 0. It includes all current and future flags. There's no point > to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not > include ODP_SHM_SW_ONLY. This is how flag parameters work in general. > > -Petri > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Wednesday, October 22, 2014 10:50 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > Although adding the default description to the list of legal #defined > > values > > helps the reader understand what the default means, it looks like this > > could be more clearly coded as an enum. > > > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > > b/platform/linux-generic/include/api/odp_shared_memory.h > > index d8d40dd..d2965af 100644 > > --- a/platform/linux-generic/include/api/odp_shared_memory.h > > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > > @@ -34,6 +34,7 @@ extern "C" { > > */ > > > > /* Share level */ > > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > > * @param size Block size in bytes > > * @param align Block alignment in bytes > > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value > is > > 0. > > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value > is > > ODP_SHM_SW_AND_HW > > * > > * @return Pointer to the reserved block, or NULL > > */ > > -- > > 1.9.1 > > > > > > > _______________________________________________ > > 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 > > > > > > -- > > *Mike Holmes* > > Linaro Sr Technical Manager > > LNG - ODP > > > > > > -- > > *Mike Holmes* > > Linaro Sr Technical Manager > > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
Accessible by HW is default behavior, since ODP APIs may be (and usually are) HW accelerated. By default application wants to share memory with all HW accelerators used by the ODP implementation. -Petri From: ext Alexandru Badicioiu [mailto:alexandru.badicioiu@linaro.org] Sent: Friday, October 24, 2014 12:04 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: ext Mike Holmes; lng-odp@lists.linaro.org Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value Hi Petri, I remember default (0x0) for shared mem means accessible by SW and HW. I think accessible by HW is rather a special requirement. Alex On 24 October 2014 10:05, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: Restrictions will be highlighted with flags. So you’d compare … foo(param1, param2, 0); vs. foo(param1, param2, ODP_SHM_FLAG1 | ODP_SHM_FLAG2 | ODP_SHM_FLAG3 ); .. and spot easily that the second call has defined some special requirements. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>] Sent: Thursday, October 23, 2014 6:08 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value Thanks, I could be satisfied if the doc clearly states "The default indicates that application does not have any special requirements, this default is guaranteed never to change" But I still think a define is better - compare these for readability in the application foo(0) or slightly better foo(0); // take the default and foo(NO_ACCESS_RESTRICTOINS) On 23 October 2014 09:56, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: Default should stay as zero also in the future. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>] Sent: Thursday, October 23, 2014 4:49 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value So shouldn't we define ODP_SHM_ANY 0, then an application could call to get that behaviour and if the default ever changes they will be covered. On 23 October 2014 09:07, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: 0 means that application does not have any special requirements. Each special requirement is indicated by a flag. If you use shm with any ODP API and within the local ODP instance, it’s default behavior. Later on we may add more limitation/access right flags. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>] Sent: Thursday, October 23, 2014 3:39 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document default value We need to document what 0 means to the user of the API, a document reader gets no information what behaviour to expect. Describing that as code feels more accurate than writing a comment because an application wanting to make it explicit can use that flag and their code is then documented clearly which behaviour they expect. If the default behaviour changes in future, having explicitly passed a flag describing the behaviour the app wants allows that app to continue working after the change. Would it be better for the default to be the most restrictive and subsequent flags relax that, is wide open access the most prevalent case ? On 23 October 2014 02:56, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: Default is 0. It includes all current and future flags. There's no point to define ODP_SHM_SW_AND_HW, that's defined when the flag value does not include ODP_SHM_SW_ONLY. This is how flag parameters work in general. -Petri > -----Original Message----- > From: lng-odp-bounces@lists.linaro.org<mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-<mailto:lng-odp-> > bounces@lists.linaro.org<mailto:bounces@lists.linaro.org>] On Behalf Of ext Mike Holmes > Sent: Wednesday, October 22, 2014 10:50 PM > To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document default value > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> > --- > Although adding the default description to the list of legal #defined > values > helps the reader understand what the default means, it looks like this > could be more clearly coded as an enum. > > platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > b/platform/linux-generic/include/api/odp_shared_memory.h > index d8d40dd..d2965af 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -34,6 +34,7 @@ extern "C" { > */ > > /* Share level */ > +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ > #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > #define ODP_SHM_PROC 0x2 /**< Share with external processes */ > > @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { > * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) > * @param size Block size in bytes > * @param align Block alignment in bytes > - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > 0. > + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is > ODP_SHM_SW_AND_HW > * > * @return Pointer to the reserved block, or NULL > */ > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Linaro Sr Technical Manager LNG - ODP -- Mike Holmes Linaro Sr Technical Manager LNG - ODP -- Mike Holmes Linaro Sr Technical Manager LNG - ODP _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> http://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/include/api/odp_shared_memory.h b/platform/linux-generic/include/api/odp_shared_memory.h index d8d40dd..d2965af 100644 --- a/platform/linux-generic/include/api/odp_shared_memory.h +++ b/platform/linux-generic/include/api/odp_shared_memory.h @@ -34,6 +34,7 @@ extern "C" { */ /* Share level */ +#define ODP_SHM_SW_AND_HW 0x0 /**< Both SW and HW acess */ #define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ #define ODP_SHM_PROC 0x2 /**< Share with external processes */ @@ -64,7 +65,7 @@ typedef struct odp_shm_info_t { * @param name Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars) * @param size Block size in bytes * @param align Block alignment in bytes - * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is 0. + * @param flags Shared mem parameter flags (ODP_SHM_*). Default value is ODP_SHM_SW_AND_HW * * @return Pointer to the reserved block, or NULL */
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- Although adding the default description to the list of legal #defined values helps the reader understand what the default means, it looks like this could be more clearly coded as an enum. platform/linux-generic/include/api/odp_shared_memory.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)