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/03/28 14:38:58 UTC
[GitHub] [incubator-nuttx] GUIDINGLI opened a new pull request #5892: rpmsgfs related update
GUIDINGLI opened a new pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892
## Summary
rpmsgfs related update
1. add wait ready to rpmsgfs
2. add timeout when mount in rpmsgfs
3. fix blocked when connecting
4. use rptun_wait/post to resolve deadlock with usrsock rpmsg
This depends on:
https://github.com/apache/incubator-nuttx/pull/5847/
## Impact
rpmsgfs
## Testing
VELA
--
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 merged pull request #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892
--
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 commented on a change in pull request #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r838806490
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -1101,6 +1129,7 @@ static int rpmsgfs_bind(FAR struct inode *blkdriver, FAR const void *data,
*/
fs->fs_head = NULL;
+ fs->timeout = INT32_MAX;
Review comment:
INT32_MAX->INT_MAX
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -265,7 +265,7 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
struct stat buf;
int ret;
- ret = rpmsgfs_client_stat(fs->handle, path, &buf);
+ ret = rpmsgfs_client_stat(fs->handle, fs->fs_root, &buf);
Review comment:
merge to previous patch
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -1103,7 +1103,6 @@ static int rpmsgfs_bind(FAR struct inode *blkdriver, FAR const void *data,
fs->timeout = atoi(&ptr[8]);
}
-
Review comment:
move to previous patch
--
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] GUIDINGLI commented on a change in pull request #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r839722112
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
+ {
+ struct stat buf;
+ int ret;
+
+ ret = rpmsgfs_client_stat(fs->handle, fs->fs_root, &buf);
+ if (ret == 0)
+ {
+ break;
+ }
+
+ usleep(RPMSGFS_RETRY_DELAY_MS * USEC_PER_MSEC);
+ fs->timeout -= RPMSGFS_RETRY_DELAY_MS;
Review comment:
We don't need so accurate time, just a timeout
--
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 #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r839930373
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
Review comment:
If this is by design the changes LGTM
--
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 commented on a change in pull request #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r839887614
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
Review comment:
Almost, 0x7fffffff/1000/60/60/24=24.5days.
--
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 #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r839929905
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
Review comment:
The current code handles any positive value as a timeout, so no support of infinite wait. I just want to make sure that it is by design
--
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 #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r839928817
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
Review comment:
Haha... The question is do we support infinite wait somehow or any value is considered a timeout?
--
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 #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r836729465
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
+ {
+ struct stat buf;
+ int ret;
+
+ ret = rpmsgfs_client_stat(fs->handle, fs->fs_root, &buf);
+ if (ret == 0)
+ {
+ break;
+ }
+
+ usleep(RPMSGFS_RETRY_DELAY_MS * 1000);
Review comment:
```suggestion
usleep(RPMSGFS_RETRY_DELAY_MS * USEC_PER_MSEC);
```
--
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 #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r839501747
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
+ {
+ struct stat buf;
+ int ret;
+
+ ret = rpmsgfs_client_stat(fs->handle, fs->fs_root, &buf);
+ if (ret == 0)
+ {
+ break;
+ }
+
+ usleep(RPMSGFS_RETRY_DELAY_MS * USEC_PER_MSEC);
+ fs->timeout -= RPMSGFS_RETRY_DELAY_MS;
Review comment:
This is not very good for delay calculation. I mean that the error is very big due to dependency on systick granularity. The better way is to use `clock_systime_timespec()`.
--
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] GUIDINGLI commented on a change in pull request #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r840204319
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
Review comment:
Yes.
That I found the default value setting in wrong place, so update it.
--
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 #5892: rpmsgfs related update
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5892:
URL: https://github.com/apache/incubator-nuttx/pull/5892#discussion_r839829640
##########
File path: fs/rpmsgfs/rpmsgfs.c
##########
@@ -252,6 +259,21 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
{
strncat(path, &relpath[first], pathlen - strlen(path) - 1);
}
+
+ while (fs->timeout > 0)
Review comment:
Does `INT_MAX` timeout means "wait forever"?
--
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