You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/01/04 08:37:22 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
anchao opened a new pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156
## Summary
net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
## Impact
N/A
## Testing
CI-check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778089628
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -94,23 +97,36 @@ static void _usrsock_semgive(FAR sem_t *sem)
FAR struct usrsock_conn_s *usrsock_alloc(void)
{
FAR struct usrsock_conn_s *conn;
+#ifdef CONFIG_NET_ALLOC_CONNS
+ int i;
+#endif
/* The free list is protected by a semaphore (that behaves like a mutex). */
_usrsock_semtake(&g_free_sem);
+#ifdef CONFIG_NET_ALLOC_CONNS
+ if (dq_peek(&g_free_usrsock_connections) == NULL)
+ {
+ conn = kmm_zalloc(sizeof(*conn) * CONFIG_NET_USRSOCK_CONNS);
+ if (conn != NULL)
+ {
+ for (i = 0; i < CONFIG_NET_USRSOCK_CONNS; i++)
+ {
+ dq_addlast(&conn[i].node, &g_free_usrsock_connections);
+ }
+ }
+ }
+#endif
Review comment:
Why not to do allocation in `usrsock_initialize`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778142296
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -94,23 +97,36 @@ static void _usrsock_semgive(FAR sem_t *sem)
FAR struct usrsock_conn_s *usrsock_alloc(void)
{
FAR struct usrsock_conn_s *conn;
+#ifdef CONFIG_NET_ALLOC_CONNS
+ int i;
+#endif
/* The free list is protected by a semaphore (that behaves like a mutex). */
_usrsock_semtake(&g_free_sem);
+#ifdef CONFIG_NET_ALLOC_CONNS
+ if (dq_peek(&g_free_usrsock_connections) == NULL)
+ {
+ conn = kmm_zalloc(sizeof(*conn) * CONFIG_NET_USRSOCK_CONNS);
+ if (conn != NULL)
+ {
+ for (i = 0; i < CONFIG_NET_USRSOCK_CONNS; i++)
+ {
+ dq_addlast(&conn[i].node, &g_free_usrsock_connections);
+ }
+ }
+ }
+#endif
Review comment:
allocate on use, all conn instances will be obtained through the allocator if g_free_usrsock_connections is NULL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778155218
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -94,23 +97,36 @@ static void _usrsock_semgive(FAR sem_t *sem)
FAR struct usrsock_conn_s *usrsock_alloc(void)
{
FAR struct usrsock_conn_s *conn;
+#ifdef CONFIG_NET_ALLOC_CONNS
+ int i;
+#endif
/* The free list is protected by a semaphore (that behaves like a mutex). */
_usrsock_semtake(&g_free_sem);
+#ifdef CONFIG_NET_ALLOC_CONNS
+ if (dq_peek(&g_free_usrsock_connections) == NULL)
+ {
+ conn = kmm_zalloc(sizeof(*conn) * CONFIG_NET_USRSOCK_CONNS);
+ if (conn != NULL)
+ {
+ for (i = 0; i < CONFIG_NET_USRSOCK_CONNS; i++)
+ {
+ dq_addlast(&conn[i].node, &g_free_usrsock_connections);
+ }
+ }
+ }
+#endif
Review comment:
In such case we do not limit the allocated connection number by `CONFIG_NET_ALLOC_CONNS`. The current condition seems to be true for the first time and when `CONFIG_NET_ALLOC_CONNS` number of connections are allocated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778164956
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -94,23 +97,36 @@ static void _usrsock_semgive(FAR sem_t *sem)
FAR struct usrsock_conn_s *usrsock_alloc(void)
{
FAR struct usrsock_conn_s *conn;
+#ifdef CONFIG_NET_ALLOC_CONNS
+ int i;
+#endif
/* The free list is protected by a semaphore (that behaves like a mutex). */
_usrsock_semtake(&g_free_sem);
+#ifdef CONFIG_NET_ALLOC_CONNS
+ if (dq_peek(&g_free_usrsock_connections) == NULL)
+ {
+ conn = kmm_zalloc(sizeof(*conn) * CONFIG_NET_USRSOCK_CONNS);
+ if (conn != NULL)
+ {
+ for (i = 0; i < CONFIG_NET_USRSOCK_CONNS; i++)
+ {
+ dq_addlast(&conn[i].node, &g_free_usrsock_connections);
+ }
+ }
+ }
+#endif
Review comment:
Reduce the memory fragmentation and access cycles of allocator.
BTW, your case can be achieved by set CONFIG_NET_ALLOC_CONNS to 1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778161335
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -94,23 +97,36 @@ static void _usrsock_semgive(FAR sem_t *sem)
FAR struct usrsock_conn_s *usrsock_alloc(void)
{
FAR struct usrsock_conn_s *conn;
+#ifdef CONFIG_NET_ALLOC_CONNS
+ int i;
+#endif
/* The free list is protected by a semaphore (that behaves like a mutex). */
_usrsock_semtake(&g_free_sem);
+#ifdef CONFIG_NET_ALLOC_CONNS
+ if (dq_peek(&g_free_usrsock_connections) == NULL)
+ {
+ conn = kmm_zalloc(sizeof(*conn) * CONFIG_NET_USRSOCK_CONNS);
+ if (conn != NULL)
+ {
+ for (i = 0; i < CONFIG_NET_USRSOCK_CONNS; i++)
+ {
+ dq_addlast(&conn[i].node, &g_free_usrsock_connections);
+ }
+ }
+ }
+#endif
Review comment:
So why we are not allocating one by one, but with `CONFIG_NET_ALLOC_CONNS` portions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778164956
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -94,23 +97,36 @@ static void _usrsock_semgive(FAR sem_t *sem)
FAR struct usrsock_conn_s *usrsock_alloc(void)
{
FAR struct usrsock_conn_s *conn;
+#ifdef CONFIG_NET_ALLOC_CONNS
+ int i;
+#endif
/* The free list is protected by a semaphore (that behaves like a mutex). */
_usrsock_semtake(&g_free_sem);
+#ifdef CONFIG_NET_ALLOC_CONNS
+ if (dq_peek(&g_free_usrsock_connections) == NULL)
+ {
+ conn = kmm_zalloc(sizeof(*conn) * CONFIG_NET_USRSOCK_CONNS);
+ if (conn != NULL)
+ {
+ for (i = 0; i < CONFIG_NET_USRSOCK_CONNS; i++)
+ {
+ dq_addlast(&conn[i].node, &g_free_usrsock_connections);
+ }
+ }
+ }
+#endif
Review comment:
Reduce the memory fragmentation and access cycles of allocator
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778140212
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -45,7 +46,9 @@
/* The array containing all usrsock connections. */
+#ifndef CONFIG_NET_ALLOC_CONNS
static struct usrsock_conn_s g_usrsock_connections[CONFIG_NET_USRSOCK_CONNS];
+#endif
Review comment:
this function will be removed from #5154
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778160048
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -94,23 +97,36 @@ static void _usrsock_semgive(FAR sem_t *sem)
FAR struct usrsock_conn_s *usrsock_alloc(void)
{
FAR struct usrsock_conn_s *conn;
+#ifdef CONFIG_NET_ALLOC_CONNS
+ int i;
+#endif
/* The free list is protected by a semaphore (that behaves like a mutex). */
_usrsock_semtake(&g_free_sem);
+#ifdef CONFIG_NET_ALLOC_CONNS
+ if (dq_peek(&g_free_usrsock_connections) == NULL)
+ {
+ conn = kmm_zalloc(sizeof(*conn) * CONFIG_NET_USRSOCK_CONNS);
+ if (conn != NULL)
+ {
+ for (i = 0; i < CONFIG_NET_USRSOCK_CONNS; i++)
+ {
+ dq_addlast(&conn[i].node, &g_free_usrsock_connections);
+ }
+ }
+ }
+#endif
Review comment:
Yes, we do not check the limit in the case of NET_ALLOC_CONNS enabled,
The limitation of connections will be implement from the VFS layer:
```
_POSIX_OPEN_MAX
RLIMIT_NOFILE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5156: net/usrsock: add support for CONFIG_NET_ALLOC_CONNS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5156:
URL: https://github.com/apache/incubator-nuttx/pull/5156#discussion_r778088951
##########
File path: net/usrsock/usrsock_conn.c
##########
@@ -45,7 +46,9 @@
/* The array containing all usrsock connections. */
+#ifndef CONFIG_NET_ALLOC_CONNS
static struct usrsock_conn_s g_usrsock_connections[CONFIG_NET_USRSOCK_CONNS];
+#endif
Review comment:
What about
```
int usrsock_connidx(FAR struct usrsock_conn_s *conn)
{
int idx = conn - g_usrsock_connections;
DEBUGASSERT(idx >= 0);
DEBUGASSERT(idx < ARRAY_SIZE(g_usrsock_connections));
return idx;
}
```
???
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org