Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

What I'm thinking is that, besides the TLS based solution, adding a 
non-standard getopt() seems to be a good option anyway, since it is a 
lightweight solution to this particular function.


Why do you think TLS is not lightweight.  It is very lightweight. The 
non-standard, non-portable approach is essentially the same 
computationally.


TLS is simply a little chunk of memory that lies at the "bottom" of 
the stack ("bottom" meaning the lowest address when the push-down 
stack memory was allocated).  Get the bottom of the stack and you have 
the TLS data.


TLS is more efficient if you can align stacks.  Then the TLS pointer 
can be obtained by just ANDing the current stack pointer. That is 
trivial.


If the stack is not aligned, then we have to ask the OS where the 
stack allocation begins.


Since each thread has its own stack, this provides a nearly 
instantaneous way to get thread-specific data.


So I would say that the complexity is higher only because this is not 
standard, familiar C programming, but in terms of light- vs 
heavy-weight, I do not see a real difference.  No decisions should be 
made based on that weighty-ness dimension.  The significant, important 
criteria are standard vs. non-standard and portable vs. non-portable.  
Those matter.


And given that there are a dozen or more cases where thread-safety of 
globals needed, TLS is a more reasonable general solution than trying to 
re=write re-entrant versions of all of the functions that rely on 
globals and damaging the OS.

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

What I'm thinking is that, besides the TLS based solution, adding a 
non-standard getopt() seems to be a good option anyway, since it is a 
lightweight solution to this particular function.


Why do you think TLS is not lightweight.  It is very lightweight. The 
non-standard, non-portable approach is essentially the same 
computationally.


TLS is simply a little chunk of memory that lies at the "bottom" of the 
stack ("bottom" meaning the lowest address when the push-down stack 
memory was allocated).  Get the bottom of the stack and you have the TLS 
data.


TLS is more efficient if you can align stacks.  Then the TLS pointer can 
be obtained by just ANDing the current stack pointer. That is trivial.


If the stack is not aligned, then we have to ask the OS where the stack 
allocation begins.


Since each thread has its own stack, this provides a nearly 
instantaneous way to get thread-specific data.


So I would say that the complexity is higher only because this is not 
standard, familiar C programming, but in terms of light- vs 
heavy-weight, I do not see a real difference.  No decisions should be 
made based on that weighty-ness dimension.  The significant, important 
criteria are standard vs. non-standard and portable vs. non-portable.  
Those matter.

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

What I'm thinking is that, besides the TLS based solution, adding a 
non-standard getopt() seems to be a good option anyway, since it is a 
lightweight solution to this particular function.


Why do you think TLS is not lightweight.  It is very lightweight.  The 
non-standard, non-portable approach is essentially the same computationally.

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

Thanks for all answers. I don't entirely understand most of them though as I'm 
not really familiar with the implications of TLS or how to use it correctly. 
Also, do we need per-thread or per-task data here?
You would expect getopt() to be used only on the many thread since that
is the only thread that receives argc and argv.

So if it is only used in one thread there would only be a copy of the data? 
What if I spawn multiple threads and call getopt only on one?


Yes there is only one copy of the data for each thread that uses 
getopt().  In the normal case, only one thread, the main thread, uses 
getopt().


I had considered a single allocation for the main thread.  If other 
threads in the same task call getopt(), it could share that single 
per-task allocation (rather than creating another copy), i.e., each 
thread's TLS data would refer to the same memory.  That is how other 
Unix systems work.

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

On Wed, Mar 24, 2021, at 19:27, Byron Ellacott wrote:
> Here's what I found in libc that would need task (thread) specific data:
> 
>   - libs/libc/misc/lib_umask.c has g_mask
>   - libs/libc/libgen/lib_dirname.c and libs/libc/libgen/lib_basename each
> have a g_retchar
>   - libs/libc/syslog/lib_setlogmask.c has g_syslog_mask (and a comment
> describing this issue)
>   - libs/libc/pwd/* uses either g_passwd and g_passwd_buffer or g_pwd and
> g_buf
>   - libs/libc/grp/* uses a similar pair for group data
>   - libs/libc/unistd/lib_getopt.c we know of, it has four words of global
> data
>   - libs/libc/time/lib_localtime.c uses g_tm and may need per-task timezone
> settings
>   - libs/libc/netdb/lib_netdb.c specifies h_errno as a global
>   - libs/libc/netdb/lib_gethostbyname2.c  and lib_gethostbyaddr.c use
> g_hostent and g_hostbuffer
>   - libs/libc/stdlib/lib_srand.c uses a variety of globals depending on
> build options
>   - libs/libc/string/lib_strtok.c uses g_saveptr

Thanks for this list, I will update the issue and make it into a task list.

> 
> Statically allocating a TLS key for each module would consume around 11
> keys in each task. Dynamically allocated TLS keys cannot ever be released,
> because these are globals handed over to user code with no indication when
> they're no longer needed. It may be better to have an additional static
> element in tls_info_s pointing to a heap-allocated structure containing the
> libc globals. Functionally this is the same as a statically reserved TLS
> key, but it's clearer what it's for.

I think that is more or less the idea discussed in the PR. It will be done 
later on.

Best,
Matias

> -- 
> Byron
> 
> On Thu, Mar 25, 2021 at 12:51 AM Gregory Nutt  > wrote:
> 
> > On 3/24/2021 8:38 AM, Matias N. wrote:
> > > So, if I follow correctly, we could maybe have one TLS pointer pointing
> > to a struct of pointers, one per each group of globals (one of this groups,
> > would be the set of variables used by getopt()), like:
> > >
> > > struct task_globals_s
> > > {
> > >struct getopt_globals_s *getopt_globals;
> > >/* ...others */
> > > };
> > >
> > > Then getopt globals would only be allocated once for each task, and only
> > when getopt() is called.
> > >
> > > Something like that?
> >
> > Yes, that is a possibility.  But that is already implemented just as you
> > describe as POSIX thread-specific data.
> >
> > The TLS data structure is defined in include/nuttx/tls.h as following.
> > it is just an array of pointer size things and the errno variable.
> >
> > struct tls_info_s
> > {
> > #if CONFIG_TLS_NELEM > 0
> >uintptr_t tl_elem[CONFIG_TLS_NELEM]; /* TLS elements */
> > #endif
> >int tl_errno;/* Per-thread error number */
> > };
> >
> > This structure lies at the "bottom" of stack of every thread in user space.
> >
> > The standard pthread_getspecific() is then implemented as:
> >
> > FAR void *pthread_getspecific(pthread_key_t key)
> > {
> >return (FAR void *)tls_get_value((int)key);
> > }
> >
> > Where
> >
> > uintptr_t tls_get_value(int tlsindex)
> > {
> >FAR struct tls_info_s *info;
> >uintptr_t ret = 0;
> >
> >DEBUGASSERT(tlsindex >= 0 && tlsindex < CONFIG_TLS_NELEM);
> >if (tlsindex >= 0 && tlsindex < CONFIG_TLS_NELEM)
> >  {
> >/* Get the TLS info structure from the current threads stack */
> >
> >info = up_tls_info();
> >DEBUGASSERT(info != NULL);
> >
> >/* Get the element value from the TLS info. */
> >
> >ret = info->tl_elem[tlsindex];
> >  }
> >
> >return ret;
> > }
> >
> > The POSIX interface supports a pthread_key_create() to manage the indexing.
> >
> >
> >
> >
> 

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Byron Ellacott]

On Thu, Mar 25, 2021 at 8:27 AM Byron Ellacott 
wrote:

> Hi,
>
> Since the basic problem is that `getopt` doesn't have a per-task value it
> can use, how would it keep track of which TLS key it's been allocated?
>

This question, at least, I understand the answer to having looked at the PR
- the TLS key is shared across all threads (of course it would need to be)
so can be stored in a single global.

-- 
Byron

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Byron Ellacott]

Hi,

Since the basic problem is that `getopt` doesn't have a per-task value it
can use, how would it keep track of which TLS key it's been allocated?

Here's what I found in libc that would need task (thread) specific data:

  - libs/libc/misc/lib_umask.c has g_mask
  - libs/libc/libgen/lib_dirname.c and libs/libc/libgen/lib_basename each
have a g_retchar
  - libs/libc/syslog/lib_setlogmask.c has g_syslog_mask (and a comment
describing this issue)
  - libs/libc/pwd/* uses either g_passwd and g_passwd_buffer or g_pwd and
g_buf
  - libs/libc/grp/* uses a similar pair for group data
  - libs/libc/unistd/lib_getopt.c we know of, it has four words of global
data
  - libs/libc/time/lib_localtime.c uses g_tm and may need per-task timezone
settings
  - libs/libc/netdb/lib_netdb.c specifies h_errno as a global
  - libs/libc/netdb/lib_gethostbyname2.c  and lib_gethostbyaddr.c use
g_hostent and g_hostbuffer
  - libs/libc/stdlib/lib_srand.c uses a variety of globals depending on
build options
  - libs/libc/string/lib_strtok.c uses g_saveptr

Statically allocating a TLS key for each module would consume around 11
keys in each task. Dynamically allocated TLS keys cannot ever be released,
because these are globals handed over to user code with no indication when
they're no longer needed. It may be better to have an additional static
element in tls_info_s pointing to a heap-allocated structure containing the
libc globals. Functionally this is the same as a statically reserved TLS
key, but it's clearer what it's for.

-- 
Byron

On Thu, Mar 25, 2021 at 12:51 AM Gregory Nutt  wrote:

> On 3/24/2021 8:38 AM, Matias N. wrote:
> > So, if I follow correctly, we could maybe have one TLS pointer pointing
> to a struct of pointers, one per each group of globals (one of this groups,
> would be the set of variables used by getopt()), like:
> >
> > struct task_globals_s
> > {
> >struct getopt_globals_s *getopt_globals;
> >/* ...others */
> > };
> >
> > Then getopt globals would only be allocated once for each task, and only
> when getopt() is called.
> >
> > Something like that?
>
> Yes, that is a possibility.  But that is already implemented just as you
> describe as POSIX thread-specific data.
>
> The TLS data structure is defined in include/nuttx/tls.h as following.
> it is just an array of pointer size things and the errno variable.
>
> struct tls_info_s
> {
> #if CONFIG_TLS_NELEM > 0
>uintptr_t tl_elem[CONFIG_TLS_NELEM]; /* TLS elements */
> #endif
>int tl_errno;/* Per-thread error number */
> };
>
> This structure lies at the "bottom" of stack of every thread in user space.
>
> The standard pthread_getspecific() is then implemented as:
>
> FAR void *pthread_getspecific(pthread_key_t key)
> {
>return (FAR void *)tls_get_value((int)key);
> }
>
> Where
>
> uintptr_t tls_get_value(int tlsindex)
> {
>FAR struct tls_info_s *info;
>uintptr_t ret = 0;
>
>DEBUGASSERT(tlsindex >= 0 && tlsindex < CONFIG_TLS_NELEM);
>if (tlsindex >= 0 && tlsindex < CONFIG_TLS_NELEM)
>  {
>/* Get the TLS info structure from the current threads stack */
>
>info = up_tls_info();
>DEBUGASSERT(info != NULL);
>
>/* Get the element value from the TLS info. */
>
>ret = info->tl_elem[tlsindex];
>  }
>
>return ret;
> }
>
> The POSIX interface supports a pthread_key_create() to manage the indexing.
>
>
>
>

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

Great, thanks!
I was just writing an issue to have this noted somewhere.

Best,
Matias

On Wed, Mar 24, 2021, at 13:23, Gregory Nutt wrote:
> I think it is not very much work to implement.  Perhaps I will submit a 
> draft PR for your review.
> 
> 
> On 3/24/2021 9:34 AM, Matias N. wrote:
> > Yes, you're right, TLS is the way to go.
> > I only wonder how to minimize the impact. Could this array inside the TLS 
> > struct be grown as needed during runtime? That way if no application calls 
> > to getopt() (or any other function requiring similar solution), no extra 
> > space on TLS is used.
> >
> > On Wed, Mar 24, 2021, at 12:32, Gregory Nutt wrote:
>  Se we can either add something special just as for errno or use
>  entries in that array (which would require establishing a minimum
>  number of entries to satisfy the case of getopt en potentially
>  others). I think it is better to somehow "reserve" space for the
>  known required cases.
> 
>  What i'm worried about is: how many other cases like this there could
>  be? Maybe there will be a considerable number of this entries added
>  to TLS structure (yes, four bytes, but they can add up quickly). I
>  would personally prefer to use reentrant versions when they are
>  available, instead of increasing memory use of every thread. Not sure
>  what is really best here...
> >>> Standardization is certainly the highest value of the OS and the thing
> >>> that makes NuttX what it is.  Sacrificing standardization sacrifices
> >>> the core value of the OS.
> >> Standardization supports portability.  If we bring in code from Linux,
> >> it will not use getopt_r(), it will use getopt() or getopt_long() and
> >> may not work as expected without the TLS-based change.  Similarly, if we
> >> write applications that depend on the non-standard getop_r(), that code
> >> will not compile or build under Linux.  We will have lost portability.
> >>
> >> Many people support code components that operate under either Linux or
> >> NuttX and they depend on having this compatibility.  Why break it?  It
> >> is not consistent with the principles set out in INVIOLABLES.md
> >>
> >>
> >>
> >>
> 
> 

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

I think it is not very much work to implement.  Perhaps I will submit a 
draft PR for your review.



On 3/24/2021 9:34 AM, Matias N. wrote:

Yes, you're right, TLS is the way to go.
I only wonder how to minimize the impact. Could this array inside the TLS 
struct be grown as needed during runtime? That way if no application calls to 
getopt() (or any other function requiring similar solution), no extra space on 
TLS is used.

On Wed, Mar 24, 2021, at 12:32, Gregory Nutt wrote:

Se we can either add something special just as for errno or use
entries in that array (which would require establishing a minimum
number of entries to satisfy the case of getopt en potentially
others). I think it is better to somehow "reserve" space for the
known required cases.

What i'm worried about is: how many other cases like this there could
be? Maybe there will be a considerable number of this entries added
to TLS structure (yes, four bytes, but they can add up quickly). I
would personally prefer to use reentrant versions when they are
available, instead of increasing memory use of every thread. Not sure
what is really best here...

Standardization is certainly the highest value of the OS and the thing
that makes NuttX what it is.  Sacrificing standardization sacrifices
the core value of the OS.

Standardization supports portability.  If we bring in code from Linux,
it will not use getopt_r(), it will use getopt() or getopt_long() and
may not work as expected without the TLS-based change.  Similarly, if we
write applications that depend on the non-standard getop_r(), that code
will not compile or build under Linux.  We will have lost portability.

Many people support code components that operate under either Linux or
NuttX and they depend on having this compatibility.  Why break it?  It
is not consistent with the principles set out in INVIOLABLES.md

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

Yes, you're right, TLS is the way to go.
I only wonder how to minimize the impact. Could this array inside the TLS 
struct be grown as needed during runtime? That way if no application calls to 
getopt() (or any other function requiring similar solution), no extra space on 
TLS is used.

On Wed, Mar 24, 2021, at 12:32, Gregory Nutt wrote:
> 
> >
> >> Se we can either add something special just as for errno or use 
> >> entries in that array (which would require establishing a minimum 
> >> number of entries to satisfy the case of getopt en potentially 
> >> others). I think it is better to somehow "reserve" space for the 
> >> known required cases.
> >>
> >> What i'm worried about is: how many other cases like this there could 
> >> be? Maybe there will be a considerable number of this entries added 
> >> to TLS structure (yes, four bytes, but they can add up quickly). I 
> >> would personally prefer to use reentrant versions when they are 
> >> available, instead of increasing memory use of every thread. Not sure 
> >> what is really best here...
> > Standardization is certainly the highest value of the OS and the thing 
> > that makes NuttX what it is.  Sacrificing standardization sacrifices 
> > the core value of the OS.
> 
> Standardization supports portability.  If we bring in code from Linux, 
> it will not use getopt_r(), it will use getopt() or getopt_long() and 
> may not work as expected without the TLS-based change.  Similarly, if we 
> write applications that depend on the non-standard getop_r(), that code 
> will not compile or build under Linux.  We will have lost portability.
> 
> Many people support code components that operate under either Linux or 
> NuttX and they depend on having this compatibility.  Why break it?  It 
> is not consistent with the principles set out in INVIOLABLES.md
> 
> 
> 
> 

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

Se we can either add something special just as for errno or use 
entries in that array (which would require establishing a minimum 
number of entries to satisfy the case of getopt en potentially 
others). I think it is better to somehow "reserve" space for the 
known required cases.


What i'm worried about is: how many other cases like this there could 
be? Maybe there will be a considerable number of this entries added 
to TLS structure (yes, four bytes, but they can add up quickly). I 
would personally prefer to use reentrant versions when they are 
available, instead of increasing memory use of every thread. Not sure 
what is really best here...
Standardization is certainly the highest value of the OS and the thing 
that makes NuttX what it is.  Sacrificing standardization sacrifices 
the core value of the OS.


Standardization supports portability.  If we bring in code from Linux, 
it will not use getopt_r(), it will use getopt() or getopt_long() and 
may not work as expected without the TLS-based change.  Similarly, if we 
write applications that depend on the non-standard getop_r(), that code 
will not compile or build under Linux.  We will have lost portability.


Many people support code components that operate under either Linux or 
NuttX and they depend on having this compatibility.  Why break it?  It 
is not consistent with the principles set out in INVIOLABLES.md

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

Se we can either add something special just as for errno or use entries in that array 
(which would require establishing a minimum number of entries to satisfy the case of 
getopt en potentially others). I think it is better to somehow "reserve" space 
for the known required cases.

What i'm worried about is: how many other cases like this there could be? Maybe 
there will be a considerable number of this entries added to TLS structure 
(yes, four bytes, but they can add up quickly). I would personally prefer to 
use reentrant versions when they are available, instead of increasing memory 
use of every thread. Not sure what is really best here...
Standardization is certainly the highest value of the OS and the thing 
that makes NuttX what it is.  Sacrificing standardization sacrifices the 
core value of the OS.

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

Se we can either add something special just as for errno or use entries in that 
array (which would require establishing a minimum number of entries to satisfy 
the case of getopt en potentially others). I think it is better to somehow 
"reserve" space for the known required cases.

What i'm worried about is: how many other cases like this there could be? Maybe 
there will be a considerable number of this entries added to TLS structure 
(yes, four bytes, but they can add up quickly). I would personally prefer to 
use reentrant versions when they are available, instead of increasing memory 
use of every thread. Not sure what is really best here...

On Wed, Mar 24, 2021, at 11:51, Gregory Nutt wrote:
> On 3/24/2021 8:38 AM, Matias N. wrote:
> > So, if I follow correctly, we could maybe have one TLS pointer pointing to 
> > a struct of pointers, one per each group of globals (one of this groups, 
> > would be the set of variables used by getopt()), like:
> >
> > struct task_globals_s
> > {
> >struct getopt_globals_s *getopt_globals;
> >/* ...others */
> > };
> >
> > Then getopt globals would only be allocated once for each task, and only 
> > when getopt() is called.
> >
> > Something like that?
> 
> Yes, that is a possibility.  But that is already implemented just as you 
> describe as POSIX thread-specific data.
> 
> The TLS data structure is defined in include/nuttx/tls.h as following.  
> it is just an array of pointer size things and the errno variable.
> 
> struct tls_info_s
> {
> #if CONFIG_TLS_NELEM > 0
>uintptr_t tl_elem[CONFIG_TLS_NELEM]; /* TLS elements */
> #endif
>int tl_errno;/* Per-thread error number */
> };
> 
> This structure lies at the "bottom" of stack of every thread in user space.
> 
> The standard pthread_getspecific() is then implemented as:
> 
> FAR void *pthread_getspecific(pthread_key_t key)
> {
>return (FAR void *)tls_get_value((int)key);
> }
> 
> Where
> 
> uintptr_t tls_get_value(int tlsindex)
> {
>FAR struct tls_info_s *info;
>uintptr_t ret = 0;
> 
>DEBUGASSERT(tlsindex >= 0 && tlsindex < CONFIG_TLS_NELEM);
>if (tlsindex >= 0 && tlsindex < CONFIG_TLS_NELEM)
>  {
>/* Get the TLS info structure from the current threads stack */
> 
>info = up_tls_info();
>DEBUGASSERT(info != NULL);
> 
>/* Get the element value from the TLS info. */
> 
>ret = info->tl_elem[tlsindex];
>  }
> 
>return ret;
> }
> 
> The POSIX interface supports a pthread_key_create() to manage the indexing.
> 
> 
> 
> 

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

On 3/24/2021 8:38 AM, Matias N. wrote:

So, if I follow correctly, we could maybe have one TLS pointer pointing to a 
struct of pointers, one per each group of globals (one of this groups, would be 
the set of variables used by getopt()), like:

struct task_globals_s
{
   struct getopt_globals_s *getopt_globals;
   /* ...others */
};

Then getopt globals would only be allocated once for each task, and only when 
getopt() is called.

Something like that?


Yes, that is a possibility.  But that is already implemented just as you 
describe as POSIX thread-specific data.


The TLS data structure is defined in include/nuttx/tls.h as following.  
it is just an array of pointer size things and the errno variable.


   struct tls_info_s
   {
   #if CONFIG_TLS_NELEM > 0
  uintptr_t tl_elem[CONFIG_TLS_NELEM]; /* TLS elements */
   #endif
  int tl_errno;    /* Per-thread error number */
   };

This structure lies at the "bottom" of stack of every thread in user space.

The standard pthread_getspecific() is then implemented as:

   FAR void *pthread_getspecific(pthread_key_t key)
   {
  return (FAR void *)tls_get_value((int)key);
   }

Where

   uintptr_t tls_get_value(int tlsindex)
   {
  FAR struct tls_info_s *info;
  uintptr_t ret = 0;

  DEBUGASSERT(tlsindex >= 0 && tlsindex < CONFIG_TLS_NELEM);
  if (tlsindex >= 0 && tlsindex < CONFIG_TLS_NELEM)
    {
  /* Get the TLS info structure from the current threads stack */

  info = up_tls_info();
  DEBUGASSERT(info != NULL);

  /* Get the element value from the TLS info. */

  ret = info->tl_elem[tlsindex];
    }

  return ret;
   }

The POSIX interface supports a pthread_key_create() to manage the indexing.

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

So, if I follow correctly, we could maybe have one TLS pointer pointing to a 
struct of pointers, one per each group of globals (one of this groups, would be 
the set of variables used by getopt()), like:

struct task_globals_s
{
  struct getopt_globals_s *getopt_globals;
  /* ...others */
};

Then getopt globals would only be allocated once for each task, and only when 
getopt() is called.

Something like that?

On Wed, Mar 24, 2021, at 11:24, Gregory Nutt wrote:
> 
> > Of course, I would only call getopt() once. My question was if we use TLS, 
> > would the memory use scale with the number of threads? Or would this memory 
> > for getopt() only be allocated on getopt() calls?
> 
> Yes and yes, but the memory use might be as small as a single pointer.  
> Per task data would be better and exists now in the OS, but we would 
> have to implement some internal OS api's that libc could use to access it.
> 
> Another thing to consider is that the current per-task-data is protected 
> and can only be accessed in supervisor mode.  That is not a problem for 
> the FLAT build but might add complication to the PROTECTED build (or at 
> least more system call overhead).
> 
> KERNEL build mode does not need per-task data at all.  It would be 
> better if the data could just be kept in globals for KERNEL build, just 
> as with Linux.
> 
> 
> 

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

Of course, I would only call getopt() once. My question was if we use TLS, 
would the memory use scale with the number of threads? Or would this memory for 
getopt() only be allocated on getopt() calls?


Yes and yes, but the memory use might be as small as a single pointer.  
Per task data would be better and exists now in the OS, but we would 
have to implement some internal OS api's that libc could use to access it.


Another thing to consider is that the current per-task-data is protected 
and can only be accessed in supervisor mode.  That is not a problem for 
the FLAT build but might add complication to the PROTECTED build (or at 
least more system call overhead).


KERNEL build mode does not need per-task data at all.  It would be 
better if the data could just be kept in globals for KERNEL build, just 
as with Linux.

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

Of course, I would only call getopt() once. My question was if we use TLS, 
would the memory use scale with the number of threads? Or would this memory for 
getopt() only be allocated on getopt() calls?

On Wed, Mar 24, 2021, at 10:56, Gregory Nutt wrote:
> 
> >> You would expect getopt() to be used only on the many thread since that
> >> is the only thread that receives argc and argv.
> > So if it is only used in one thread there would only be a copy of the data? 
> > What if I spawn multiple threads and call getopt only on one?
> 
> It is hard to imagine how you could could call getopt() on any pthread 
> created with pthread_create():  The thread has no argc and argv inputs 
> so how could a pthread use getopt() unless you contrive something very 
> artificial situation.  pthreads do not receive argument lists and, 
> hence, don't need to parse argument lists.  The single thread that 
> starts with main() is the only thread that has argc and argv and the 
> only thread that can normally call getopt().
> 
> 
> 

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

You would expect getopt() to be used only on the many thread since that
is the only thread that receives argc and argv.

So if it is only used in one thread there would only be a copy of the data? 
What if I spawn multiple threads and call getopt only on one?


It is hard to imagine how you could could call getopt() on any pthread 
created with pthread_create():  The thread has no argc and argv inputs 
so how could a pthread use getopt() unless you contrive something very 
artificial situation.  pthreads do not receive argument lists and, 
hence, don't need to parse argument lists.  The single thread that 
starts with main() is the only thread that has argc and argv and the 
only thread that can normally call getopt().

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

On Wed, Mar 24, 2021, at 10:37, Gregory Nutt wrote:
> 
> > Thanks for all answers. I don't entirely understand most of them though as 
> > I'm not really familiar with the implications of TLS or how to use it 
> > correctly. Also, do we need per-thread or per-task data here?
> 
> You would expect getopt() to be used only on the many thread since that 
> is the only thread that receives argc and argv.

So if it is only used in one thread there would only be a copy of the data? 
What if I spawn multiple threads and call getopt only on one?

> 
> A faithful, bug-for-bug, implementation would require a per task, but 
> AFAIK there would be no real problem with per thread either.

Yes, my thinking is that getopt() does not provide thread safety guarantees but 
it is not wrong to provide them.
I think that some obscure case of changing getopt() globals from different 
threads is worst to support than just not
doing anything in our case.

> 
> >
> > What I'm thinking is that, besides the TLS based solution, adding a 
> > non-standard getopt() seems to be a good option anyway, since it is a 
> > lightweight solution to this particular function.
> 
> Except that NuttX is a standards based OS and we avoid non-standard 
> interfaces like the plague.  Using TLS is 100% transparent and 100% 
> compatible.  Why would you adopt a non-standard solution when a better, 
> fully compliant implementation is readily available?

My thinking is that this could be a case of something that in FLAT mode would 
give the wrong results if just used as is and the non-standard function allows 
to overcome this. But of course if the TLS solution is the right approach and 
does not incur in extra resource usage, the extra function would of course not 
be needed. My concern was that "the right approach" would incur in too much 
resource usage. But again, I don't understand how TLS works yet.

Best,
Matias

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

Thanks for all answers. I don't entirely understand most of them though as I'm 
not really familiar with the implications of TLS or how to use it correctly. 
Also, do we need per-thread or per-task data here?


You would expect getopt() to be used only on the many thread since that 
is the only thread that receives argc and argv.


A faithful, bug-for-bug, implementation would require a per task, but 
AFAIK there would be no real problem with per thread either.




What I'm thinking is that, besides the TLS based solution, adding a 
non-standard getopt() seems to be a good option anyway, since it is a 
lightweight solution to this particular function.


Except that NuttX is a standards based OS and we avoid non-standard 
interfaces like the plague.  Using TLS is 100% transparent and 100% 
compatible.  Why would you adopt a non-standard solution when a better, 
fully compliant implementation is readily available?

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

Thanks for all answers. I don't entirely understand most of them though as I'm 
not really familiar with the implications of TLS or how to use it correctly. 
Also, do we need per-thread or per-task data here? 

What I'm thinking is that, besides the TLS based solution, adding a 
non-standard getopt() seems to be a good option anyway, since it is a 
lightweight solution to this particular function.

So, how should we proceed to address this somehow? 

Best,
Matias

On Wed, Mar 24, 2021, at 10:22, Gregory Nutt wrote:
> 
> >> The custom handler isn't enough here, because the real problem is we need
> >> the global variables per task/process.
> >> As Greg suggests, we need something like TLS but per task/process not per
> >> thread(e.g. task_getspecific/task_setspecific).
> >> Once the mechanism is done, getopt can be converted to confirm the standard
> >> trivally.
> >>
> > I was looking at this exact issue last week (see comment in
> > https://github.com/apache/incubator-nuttx/pull/3054).
> >
> > The basis for this mechanism exists in the way errno is handled. Perhaps a
> > structure defined for all libc globals added to TLS and a call from the
> > task creation code to initialise it?
> 
> That would, of course, make the stack usage much larger.  Perhaps an 
> allocate-on-demand approach would make that doable.
> 
> POSIX thread specific data has a good, interface for such 
> allocate-on-demand usage.  POSIX thread specific data is built on top of 
> TLS.
> 
> 
> 

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

The custom handler isn't enough here, because the real problem is we need
the global variables per task/process.
As Greg suggests, we need something like TLS but per task/process not per
thread(e.g. task_getspecific/task_setspecific).
Once the mechanism is done, getopt can be converted to confirm the standard
trivally.


I was looking at this exact issue last week (see comment in
https://github.com/apache/incubator-nuttx/pull/3054).

The basis for this mechanism exists in the way errno is handled. Perhaps a
structure defined for all libc globals added to TLS and a call from the
task creation code to initialise it?


That would, of course, make the stack usage much larger.  Perhaps an 
allocate-on-demand approach would make that doable.


POSIX thread specific data has a good, interface for such 
allocate-on-demand usage.  POSIX thread specific data is built on top of 
TLS.

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Gregory Nutt]

The custom handler isn't enough here, because the real problem is we need
the global variables per task/process.
As Greg suggests, we need something like TLS but per task/process not per
thread(e.g. task_getspecific/task_setspecific).
Once the mechanism is done, getopt can be converted to confirm the standard
trivally.
The mechanism there to emulate per process global variables.  It is the 
task group.  Data added to the task group structure is per 
task/process.  Examples are file descriptors and signal handlers which 
must follow that scoping

The transparent/standard solution is switched to the ELF binary(note: it
doesn't depend on KERNEL mode), and then loses the XIP benefit(huge memory
penalty). But, it's doable to XIP again by combining ELF loader and ROMFS.


ELF can never be XIP from ROMFS because it requires relocations inside 
of the ELF image to link to the base system.


In Linux, ELF is separated into a text region that holds the shared code 
and a RAM region (the Global Offset Table "GOT") that is positioned 
right after the text region using the MMU.  The GOT is not useful in the 
NuttX model because we cannot force the GOT to a known position with the 
MMU.  So the GOT is not included in the link and relocations must be 
performed directly into the ELF text region


NxFLAT will run XIP from ROMFS because it does all relocations in a 
"thunk" layer outside of the text region.  But NxFLAT has some other 
limitations with regard to points.



When using globals, best practice is to make it really clear that the
variables are global. Many programmers do this by prefixing global
variable names with g_*.


This prefix is required by the coding standard.



Yes, my concern is about functions such as getopt(). If you just follow the
description of the API and use it as normal you reach this pitfall. I was
looking
for some approach to avoid this as much as possible. For getopt() I see
there's
even no standard getopt_r(), so we would have to provide our own, which
may not
be a bad idea.
Still, this issue will probably present in many other places.



Seldom people will call getopt_r in Linux, because the different process
gets the new and clean copy, but it is crucial for NuttX to work correctly.
Yes, getopt_r isn't standardized by committee, but it follows the
convention used by other similar functions(e.g. strtok_r) and implemented
by glibc.
getopt_r is unnecessary in Linux since it naturally has per process 
global variables.  Using getopt_r would serve no purpose in a true Unix 
environment and, hence, it not included in any standard.

RE: avoiding pitfal of reuse of globals in FLAT mode?

[David Sidrane]

> For getopt() I see there's
even no standard getopt_r(), so we would have to provide our own, which
may not
be a bad idea.

Here is the one we have been using.

https://github.com/PX4/PX4-Autopilot/commit/eab32572f42f8e3e715b952512b6f5
df9041f848

https://github.com/PX4/PX4-Autopilot/blob/master/platforms/common/px4_geto
pt.c


David

-Original Message-
From: Matias N. [mailto:mat...@imap.cc]
Sent: Tuesday, March 23, 2021 6:18 PM
To: dev@nuttx.apache.org
Subject: Re: avoiding pitfal of reuse of globals in FLAT mode?



On Tue, Mar 23, 2021, at 22:09, Nathan Hartman wrote:
> On Tue, Mar 23, 2021 at 8:39 PM Matias N. mailto:matias%40imap.cc>> wrote:
>
> > Hi,
> > while using getopt() from a task started from NSH I realized
subsequent
> > calls reused the global optind and similar variables resulting in
different
> > results each time. I'm aware this is expected in FLAT mode and is
related
> > to the issue of static C++ constructors (they would only be called
once,
> > not every time the task is started).
> >
> > What I wonder is what could we do to avoid this common pitfall:
> > - document it somewhere (a common issues/troubleshooting section in
the
> > docs would be good to have anyways) and just accept the issue
> > - religiously initialize globals myself before being used (a pain,
error
> > prone, and a bit adhoc, working only for FLAT mode)
>
>
> When using globals, best practice is to make it really clear that the
> variables are global. Many programmers do this by prefixing global
variable
> names with g_*.
>
> I take a different approach: A long time ago, I started grouping all
> globals in a struct, which has one global instance called Global. It
makes
> it easy to find all globals, and furthermore at the start of the program
as
> a matter of policy the first thing I do is memset() the Global struct to
0.
> Yes, I know that is often redundant to the startup code, but in some
> situations the startup code doesn't initialize globals. The FLAT model
is
> one example of this (from the 2nd invocation onwards). I've seen other
> examples of this over the years. By memset()ing your globals at the
start
> of main() you can rest assured that the globals are in fact zeroed,
> regardless of whatever else happened before main(). It has another side
> benefit: with globals grouped this way, it becomes trivial to take a
> standalone program and turn it into a component of a larger program.
> tl;dr, this
> approach has worked great for me for a long time.

That sounds like a good approach.

>
> Caveat: It won't help if your program (or any API called by it) uses
> globals that are outside your control, and therefore, not initialized by
> you. :-/

Yes, my concern is about functions such as getopt(). If you just follow
the
description of the API and use it as normal you reach this pitfall. I was
looking
for some approach to avoid this as much as possible. For getopt() I see
there's
even no standard getopt_r(), so we would have to provide our own, which
may not
be a bad idea.
Still, this issue will probably present in many other places.

>
> Nathan
>

Best,
Matias

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Byron Ellacott]

On Wed, Mar 24, 2021 at 2:08 PM Xiang Xiao 
wrote:

> On Wed, Mar 24, 2021 at 9:18 AM Matias N.  wrote:
>
> >
> > > > - devise a mechanism to mimic what would be done by OS in KERNEL mode
> > (add
> >
> > > some custom handler to APIs internally using globals, such as getopt,
> > that can be
> >
> > > called either manually by user or by the OS itself when the task is
> > started?)
>
>
> The custom handler isn't enough here, because the real problem is we need
> the global variables per task/process.
> As Greg suggests, we need something like TLS but per task/process not per
> thread(e.g. task_getspecific/task_setspecific).
> Once the mechanism is done, getopt can be converted to confirm the standard
> trivally.
>

I was looking at this exact issue last week (see comment in
https://github.com/apache/incubator-nuttx/pull/3054).

The basis for this mechanism exists in the way errno is handled. Perhaps a
structure defined for all libc globals added to TLS and a call from the
task creation code to initialise it?


> >
> > > - other?
>
> The transparent/standard solution is switched to the ELF binary(note: it
> doesn't depend on KERNEL mode), and then loses the XIP benefit(huge memory
> penalty). But, it's doable to XIP again by combining ELF loader and ROMFS.
>

Switching to ELF doesn't necessarily help - I encountered the problem
loading nsh as an ELF binary in a FLAT built. The globals optarg and optind
aren't in libc.csv. They'll be picked up and included in symtab_apps, but
those symbols will simply point to the (system) global variables, not a
per-task variable.

  Byron

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Xiang Xiao]

On Wed, Mar 24, 2021 at 9:18 AM Matias N.  wrote:

>
>
> On Tue, Mar 23, 2021, at 22:09, Nathan Hartman wrote:
> > On Tue, Mar 23, 2021 at 8:39 PM Matias N.  matias%40imap.cc>> wrote:
> >
> > > Hi,
> > > while using getopt() from a task started from NSH I realized subsequent
> > > calls reused the global optind and similar variables resulting in
> different
> > > results each time. I'm aware this is expected in FLAT mode and is
> related
> > > to the issue of static C++ constructors (they would only be called
> once,
> > > not every time the task is started).
> > >
> > > What I wonder is what could we do to avoid this common pitfall:
> > > - document it somewhere (a common issues/troubleshooting section in the
> > > docs would be good to have anyways) and just accept the issue
>

Wiki already mention this issue here:
https://cwiki.apache.org/confluence/display/NUTTX/Linux+Processes+vs+NuttX+Tasks
Greg shares many knowledge in wiki. To improve the visibility, it's better
to migrate Confluence to Documentation.

> > - religiously initialize globals myself before being used (a pain, error
> > > prone, and a bit adhoc, working only for FLAT mode)
>
> > - support reentrant versions of all possible APIs and always use these
> in FLAT mode


Yes, it's the right direction to promote the reentrant API.


> > > - devise a mechanism to mimic what would be done by OS in KERNEL mode
> (add
>
> > some custom handler to APIs internally using globals, such as getopt,
> that can be
>
> > called either manually by user or by the OS itself when the task is
> started?)


The custom handler isn't enough here, because the real problem is we need
the global variables per task/process.
As Greg suggests, we need something like TLS but per task/process not per
thread(e.g. task_getspecific/task_setspecific).
Once the mechanism is done, getopt can be converted to confirm the standard
trivally.


>
> > - other?


The transparent/standard solution is switched to the ELF binary(note: it
doesn't depend on KERNEL mode), and then loses the XIP benefit(huge memory
penalty). But, it's doable to XIP again by combining ELF loader and ROMFS.


> >
> >
> > When using globals, best practice is to make it really clear that the
> > variables are global. Many programmers do this by prefixing global
> variable
> > names with g_*.
> >
> > I take a different approach: A long time ago, I started grouping all
> > globals in a struct, which has one global instance called Global. It
> makes
> > it easy to find all globals, and furthermore at the start of the program
> as
> > a matter of policy the first thing I do is memset() the Global struct to
> 0.
> > Yes, I know that is often redundant to the startup code, but in some
> > situations the startup code doesn't initialize globals. The FLAT model is
> > one example of this (from the 2nd invocation onwards). I've seen other
> > examples of this over the years. By memset()ing your globals at the start
> > of main() you can rest assured that the globals are in fact zeroed,
> > regardless of whatever else happened before main(). It has another side
> > benefit: with globals grouped this way, it becomes trivial to take a
> > standalone program and turn it into a component of a larger program.
> > tl;dr, this
> > approach has worked great for me for a long time.
>
> That sounds like a good approach.
>
> >
> > Caveat: It won't help if your program (or any API called by it) uses
> > globals that are outside your control, and therefore, not initialized by
> > you. :-/
>
> Yes, my concern is about functions such as getopt(). If you just follow the
> description of the API and use it as normal you reach this pitfall. I was
> looking
> for some approach to avoid this as much as possible. For getopt() I see
> there's
> even no standard getopt_r(), so we would have to provide our own, which
> may not
> be a bad idea.
> Still, this issue will probably present in many other places.
>
>
Seldom people will call getopt_r in Linux, because the different process
gets the new and clean copy, but it is crucial for NuttX to work correctly.
Yes, getopt_r isn't standardized by committee, but it follows the
convention used by other similar functions(e.g. strtok_r) and implemented
by glibc.

>
> > Nathan
> >
>
> Best,
> Matias

Re: avoiding pitfal of reuse of globals in FLAT mode?

[spudaneco]

One option is to replace the globals with accessor functions and use TLS to 
hold the actua. data.Sent from my Galaxy
 Original message From: "Matias N."  Date: 
3/23/21  7:18 PM  (GMT-06:00) To: dev@nuttx.apache.org Subject: Re: avoiding 
pitfal of reuse of globals in FLAT mode? On Tue, Mar 23, 2021, at 22:09, Nathan 
Hartman wrote:> On Tue, Mar 23, 2021 at 8:39 PM Matias N. mailto:matias%40imap.cc>> wrote:> > > Hi,> > while using getopt() from a task 
started from NSH I realized subsequent> > calls reused the global optind and 
similar variables resulting in different> > results each time. I'm aware this 
is expected in FLAT mode and is related> > to the issue of static C++ 
constructors (they would only be called once,> > not every time the task is 
started).> >> > What I wonder is what could we do to avoid this common 
pitfall:> > - document it somewhere (a common issues/troubleshooting section in 
the> > docs would be good to have anyways) and just accept the issue> > - 
religiously initialize globals myself before being used (a pain, error> > 
prone, and a bit adhoc, working only for FLAT mode)> > > When using globals, 
best practice is to make it really clear that the> variables are global. Many 
programmers do this by prefixing global variable> names with g_*.> > I take a 
different approach: A long time ago, I started grouping all> globals in a 
struct, which has one global instance called Global. It makes> it easy to find 
all globals, and furthermore at the start of the program as> a matter of policy 
the first thing I do is memset() the Global struct to 0.> Yes, I know that is 
often redundant to the startup code, but in some> situations the startup code 
doesn't initialize globals. The FLAT model is> one example of this (from the 
2nd invocation onwards). I've seen other> examples of this over the years. By 
memset()ing your globals at the start> of main() you can rest assured that the 
globals are in fact zeroed,> regardless of whatever else happened before 
main(). It has another side> benefit: with globals grouped this way, it becomes 
trivial to take a> standalone program and turn it into a component of a larger 
program.> tl;dr, this> approach has worked great for me for a long time.That 
sounds like a good approach.> > Caveat: It won't help if your program (or any 
API called by it) uses> globals that are outside your control, and therefore, 
not initialized by> you. :-/Yes, my concern is about functions such as 
getopt(). If you just follow thedescription of the API and use it as normal you 
reach this pitfall. I was lookingfor some approach to avoid this as much as 
possible. For getopt() I see there'seven no standard getopt_r(), so we would 
have to provide our own, which may notbe a bad idea.Still, this issue will 
probably present in many other places.> > Nathan> Best,Matias

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

On Tue, Mar 23, 2021, at 22:09, Nathan Hartman wrote:
> On Tue, Mar 23, 2021 at 8:39 PM Matias N.  > wrote:
> 
> > Hi,
> > while using getopt() from a task started from NSH I realized subsequent
> > calls reused the global optind and similar variables resulting in different
> > results each time. I'm aware this is expected in FLAT mode and is related
> > to the issue of static C++ constructors (they would only be called once,
> > not every time the task is started).
> >
> > What I wonder is what could we do to avoid this common pitfall:
> > - document it somewhere (a common issues/troubleshooting section in the
> > docs would be good to have anyways) and just accept the issue
> > - religiously initialize globals myself before being used (a pain, error
> > prone, and a bit adhoc, working only for FLAT mode)
> 
> 
> When using globals, best practice is to make it really clear that the
> variables are global. Many programmers do this by prefixing global variable
> names with g_*.
> 
> I take a different approach: A long time ago, I started grouping all
> globals in a struct, which has one global instance called Global. It makes
> it easy to find all globals, and furthermore at the start of the program as
> a matter of policy the first thing I do is memset() the Global struct to 0.
> Yes, I know that is often redundant to the startup code, but in some
> situations the startup code doesn't initialize globals. The FLAT model is
> one example of this (from the 2nd invocation onwards). I've seen other
> examples of this over the years. By memset()ing your globals at the start
> of main() you can rest assured that the globals are in fact zeroed,
> regardless of whatever else happened before main(). It has another side
> benefit: with globals grouped this way, it becomes trivial to take a
> standalone program and turn it into a component of a larger program.
> tl;dr, this
> approach has worked great for me for a long time.

That sounds like a good approach.

> 
> Caveat: It won't help if your program (or any API called by it) uses
> globals that are outside your control, and therefore, not initialized by
> you. :-/

Yes, my concern is about functions such as getopt(). If you just follow the
description of the API and use it as normal you reach this pitfall. I was 
looking
for some approach to avoid this as much as possible. For getopt() I see there's
even no standard getopt_r(), so we would have to provide our own, which may not
be a bad idea.
Still, this issue will probably present in many other places.

> 
> Nathan
> 

Best,
Matias

Re: avoiding pitfal of reuse of globals in FLAT mode?

[Nathan Hartman]

On Tue, Mar 23, 2021 at 8:39 PM Matias N.  wrote:

> Hi,
> while using getopt() from a task started from NSH I realized subsequent
> calls reused the global optind and similar variables resulting in different
> results each time. I'm aware this is expected in FLAT mode and is related
> to the issue of static C++ constructors (they would only be called once,
> not every time the task is started).
>
> What I wonder is what could we do to avoid this common pitfall:
> - document it somewhere (a common issues/troubleshooting section in the
> docs would be good to have anyways) and just accept the issue
> - religiously initialize globals myself before being used (a pain, error
> prone, and a bit adhoc, working only for FLAT mode)


When using globals, best practice is to make it really clear that the
variables are global. Many programmers do this by prefixing global variable
names with g_*.

I take a different approach: A long time ago, I started grouping all
globals in a struct, which has one global instance called Global. It makes
it easy to find all globals, and furthermore at the start of the program as
a matter of policy the first thing I do is memset() the Global struct to 0.
Yes, I know that is often redundant to the startup code, but in some
situations the startup code doesn't initialize globals. The FLAT model is
one example of this (from the 2nd invocation onwards). I've seen other
examples of this over the years. By memset()ing your globals at the start
of main() you can rest assured that the globals are in fact zeroed,
regardless of whatever else happened before main(). It has another side
benefit: with globals grouped this way, it becomes trivial to take a
standalone program and turn it into a component of a larger program.
tl;dr, this
approach has worked great for me for a long time.

Caveat: It won't help if your program (or any API called by it) uses
globals that are outside your control, and therefore, not initialized by
you. :-/

Nathan

avoiding pitfal of reuse of globals in FLAT mode?

[Matias N.]

Hi,
while using getopt() from a task started from NSH I realized subsequent calls 
reused the global optind and similar variables resulting in different results 
each time. I'm aware this is expected in FLAT mode and is related to the issue 
of static C++ constructors (they would only be called once, not every time the 
task is started).

What I wonder is what could we do to avoid this common pitfall:
- document it somewhere (a common issues/troubleshooting section in the docs 
would be good to have anyways) and just accept the issue
- religiously initialize globals myself before being used (a pain, error prone, 
and a bit adhoc, working only for FLAT mode)
- support reentrant versions of all possible APIs and always use these in FLAT 
mode
- devise a mechanism to mimic what would be done by OS in KERNEL mode (add some 
custom handler to APIs internally using globals, such as getopt, that can be 
called either manually by user or by the OS itself when the task is started?)
- other?

Just trying to avoid stepping n-times on the same stone. I think this can also 
happen to unsuspecting users since
a lot of these POSIX interfaces kind of promote these interfaces which use 
globals internally.

Best,
Matias