| Message ID | 1473770446-54915-1-git-send-email-christophe.milard@linaro.org |
|---|---|
| State | New |
| Headers | show |
This looks good now. Only question is whether you need to worry about cache-aligning the table, but that's a tuning option that can be considered later if needed. On Tue, Sep 13, 2016 at 7:40 AM, Christophe Milard < christophe.milard@linaro.org> wrote: > The table containing the saved file-descriptors<->{context, key} couples is > now dynamically malloc'd in the fd server process, hence avoiding > the memory waste which happened in other process when the table was > staticaly reserved in all processes. > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > platform/linux-generic/_fdserver.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/_fdserver.c b/platform/linux-generic/_ > fdserver.c > index bf36eb2..5cef806 100644 > --- a/platform/linux-generic/_fdserver.c > +++ b/platform/linux-generic/_fdserver.c > @@ -73,7 +73,7 @@ typedef struct fdentry_s { > uint64_t key; > int fd; > } fdentry_t; > -static fdentry_t fd_table[FDSERVER_MAX_ENTRIES]; > +static fdentry_t *fd_table; > static int fd_table_nb_entries; > > /* > @@ -622,8 +622,20 @@ int _odp_fdserver_init_global(void) > /* TODO: pin the server on appropriate service cpu mask */ > /* when (if) we can agree on the usage of service mask */ > > + /* allocate the space for the file descriptor<->key table: > */ > + fd_table = malloc(FDSERVER_MAX_ENTRIES * > sizeof(fdentry_t)); > + if (!fd_table) { > + ODP_ERR("maloc failed!\n"); > + exit(1); > + } > + > + /* wait for clients requests */ > wait_requests(sock); /* Returns when server is stopped */ > close(sock); > + > + /* release the file descriptor table: */ > + free(fd_table); > + > exit(0); > } > > -- > 2.7.4 > >
Hi Bill, and thanks for your review/comments: I don't think that there any reason (at this time, at least) to worry about table alignment: The server is -hopefully- used only at non critical times (such as init...). It is based on socket communications which will be dead slow compared to the alignment gain anyway. So I don't think there is much to gain with the alignment. Christophe. On 13 September 2016 at 14:02, Bill Fischofer <bill.fischofer@linaro.org> wrote: > This looks good now. Only question is whether you need to worry about > cache-aligning the table, but that's a tuning option that can be considered > later if needed. > > On Tue, Sep 13, 2016 at 7:40 AM, Christophe Milard < > christophe.milard@linaro.org> wrote: > >> The table containing the saved file-descriptors<->{context, key} couples >> is >> now dynamically malloc'd in the fd server process, hence avoiding >> the memory waste which happened in other process when the table was >> staticaly reserved in all processes. >> >> Signed-off-by: Christophe Milard <christophe.milard@linaro.org> >> > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > > >> --- >> platform/linux-generic/_fdserver.c | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/platform/linux-generic/_fdserver.c >> b/platform/linux-generic/_fdserver.c >> index bf36eb2..5cef806 100644 >> --- a/platform/linux-generic/_fdserver.c >> +++ b/platform/linux-generic/_fdserver.c >> @@ -73,7 +73,7 @@ typedef struct fdentry_s { >> uint64_t key; >> int fd; >> } fdentry_t; >> -static fdentry_t fd_table[FDSERVER_MAX_ENTRIES]; >> +static fdentry_t *fd_table; >> static int fd_table_nb_entries; >> >> /* >> @@ -622,8 +622,20 @@ int _odp_fdserver_init_global(void) >> /* TODO: pin the server on appropriate service cpu mask */ >> /* when (if) we can agree on the usage of service mask */ >> >> + /* allocate the space for the file descriptor<->key >> table: */ >> + fd_table = malloc(FDSERVER_MAX_ENTRIES * >> sizeof(fdentry_t)); >> + if (!fd_table) { >> + ODP_ERR("maloc failed!\n"); >> + exit(1); >> + } >> + >> + /* wait for clients requests */ >> wait_requests(sock); /* Returns when server is stopped */ >> close(sock); >> + >> + /* release the file descriptor table: */ >> + free(fd_table); >> + >> exit(0); >> } >> >> -- >> 2.7.4 >> >> >
Merged, Maxim. On 09/13/16 15:46, Christophe Milard wrote: > Hi Bill, and thanks for your review/comments: > I don't think that there any reason (at this time, at least) to worry about > table alignment: The server is -hopefully- used only at non critical times > (such as init...). It is based on socket communications which will be dead > slow compared to the alignment gain anyway. > So I don't think there is much to gain with the alignment. > > Christophe. > > On 13 September 2016 at 14:02, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> This looks good now. Only question is whether you need to worry about >> cache-aligning the table, but that's a tuning option that can be considered >> later if needed. >> >> On Tue, Sep 13, 2016 at 7:40 AM, Christophe Milard < >> christophe.milard@linaro.org> wrote: >> >>> The table containing the saved file-descriptors<->{context, key} couples >>> is >>> now dynamically malloc'd in the fd server process, hence avoiding >>> the memory waste which happened in other process when the table was >>> staticaly reserved in all processes. >>> >>> Signed-off-by: Christophe Milard <christophe.milard@linaro.org> >>> >> Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> >> >> >>> --- >>> platform/linux-generic/_fdserver.c | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/platform/linux-generic/_fdserver.c >>> b/platform/linux-generic/_fdserver.c >>> index bf36eb2..5cef806 100644 >>> --- a/platform/linux-generic/_fdserver.c >>> +++ b/platform/linux-generic/_fdserver.c >>> @@ -73,7 +73,7 @@ typedef struct fdentry_s { >>> uint64_t key; >>> int fd; >>> } fdentry_t; >>> -static fdentry_t fd_table[FDSERVER_MAX_ENTRIES]; >>> +static fdentry_t *fd_table; >>> static int fd_table_nb_entries; >>> >>> /* >>> @@ -622,8 +622,20 @@ int _odp_fdserver_init_global(void) >>> /* TODO: pin the server on appropriate service cpu mask */ >>> /* when (if) we can agree on the usage of service mask */ >>> >>> + /* allocate the space for the file descriptor<->key >>> table: */ >>> + fd_table = malloc(FDSERVER_MAX_ENTRIES * >>> sizeof(fdentry_t)); >>> + if (!fd_table) { >>> + ODP_ERR("maloc failed!\n"); >>> + exit(1); >>> + } >>> + >>> + /* wait for clients requests */ >>> wait_requests(sock); /* Returns when server is stopped */ >>> close(sock); >>> + >>> + /* release the file descriptor table: */ >>> + free(fd_table); >>> + >>> exit(0); >>> } >>> >>> -- >>> 2.7.4 >>> >>>
diff --git a/platform/linux-generic/_fdserver.c b/platform/linux-generic/_fdserver.c index bf36eb2..5cef806 100644 --- a/platform/linux-generic/_fdserver.c +++ b/platform/linux-generic/_fdserver.c @@ -73,7 +73,7 @@ typedef struct fdentry_s { uint64_t key; int fd; } fdentry_t; -static fdentry_t fd_table[FDSERVER_MAX_ENTRIES]; +static fdentry_t *fd_table; static int fd_table_nb_entries; /* @@ -622,8 +622,20 @@ int _odp_fdserver_init_global(void) /* TODO: pin the server on appropriate service cpu mask */ /* when (if) we can agree on the usage of service mask */ + /* allocate the space for the file descriptor<->key table: */ + fd_table = malloc(FDSERVER_MAX_ENTRIES * sizeof(fdentry_t)); + if (!fd_table) { + ODP_ERR("maloc failed!\n"); + exit(1); + } + + /* wait for clients requests */ wait_requests(sock); /* Returns when server is stopped */ close(sock); + + /* release the file descriptor table: */ + free(fd_table); + exit(0); }
The table containing the saved file-descriptors<->{context, key} couples is now dynamically malloc'd in the fd server process, hence avoiding the memory waste which happened in other process when the table was staticaly reserved in all processes. Signed-off-by: Christophe Milard <christophe.milard@linaro.org> --- platform/linux-generic/_fdserver.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) -- 2.7.4