You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "anchao (via GitHub)" <gi...@apache.org> on 2026/10/21 05:26:29 UTC
[PR] fs/rpmsgfs: fix open cleanup and root wait locking [nuttx]
anchao opened a new pull request, #20858:
URL: https://github.com/apache/nuttx/pull/20858
## Summary
Fix two rpmsgfs error/wait path issues.
First, `rpmsgfs_mkpath()` waits for the remote root to become available
before continuing. That wait may issue remote stat requests and sleep
between retries. The previous code kept the mount lock held across this
slow path, which could unnecessarily block other local rpmsgfs operations
while waiting for the remote endpoint.
This change releases the mount lock while performing the remote root
stat and while sleeping between retries, then reacquires it before
accessing local mount state again.
Second, `rpmsgfs_open()` opens the remote file before seeking to the end
for `O_APPEND` mode. If the append seek failed, the previous error path
freed the local open-file structure without closing the already-opened
remote file descriptor.
This change routes that failure through a cleanup path that closes the
remote fd before freeing the local structure.
## Impact
- Avoids holding the rpmsgfs mount lock across remote stat/sleep waits.
- Prevents leaking a remote file descriptor when `O_APPEND` seek setup
fails.
- Keeps successful open behavior unchanged.
- Only affects remote-root wait and append-open error paths.
## Testing
sim/rpproxy, sim/rpserver
--
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
Re: [PR] fs/rpmsgfs: fix open cleanup and root wait locking [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #20858:
URL: https://github.com/apache/nuttx/pull/20858#discussion_r4336311697
##########
fs/rpmsgfs/rpmsgfs.c:
##########
@@ -241,18 +241,28 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
strlcpy(path, fs->fs_root, pathlen);
strlcat(path, relpath, pathlen);
+ /* The remote root check can block and sleep while waiting for the remote
+ * endpoint to become ready. Do not hold the mount lock across that slow
+ * path; callers only need the lock for local state updates.
+ */
+
while (fs->timeout > 0)
{
struct stat buf;
int ret;
+ nxmutex_unlock(&fs->fs_lock);
Review Comment:
Maybe the better fix is to move rpmsgfs_mkpath() out of the fs_lock protected region, instead of unlocking/relocking inside 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
Re: [PR] fs/rpmsgfs: fix open cleanup and root wait locking [nuttx]
Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on code in PR #20858:
URL: https://github.com/apache/nuttx/pull/20858#discussion_r4334054618
##########
fs/rpmsgfs/rpmsgfs.c:
##########
@@ -241,18 +241,28 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
strlcpy(path, fs->fs_root, pathlen);
strlcat(path, relpath, pathlen);
+ /* The remote root check can block and sleep while waiting for the remote
+ * endpoint to become ready. Do not hold the mount lock across that slow
+ * path; callers only need the lock for local state updates.
+ */
+
while (fs->timeout > 0)
{
struct stat buf;
int ret;
+ nxmutex_unlock(&fs->fs_lock);
+
ret = rpmsgfs_client_stat(fs->handle, fs->fs_root, &buf);
+ nxmutex_lock(&fs->fs_lock);
Review Comment:
Should we handle lock fail here?
##########
fs/rpmsgfs/rpmsgfs.c:
##########
@@ -241,18 +241,28 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
strlcpy(path, fs->fs_root, pathlen);
strlcat(path, relpath, pathlen);
+ /* The remote root check can block and sleep while waiting for the remote
+ * endpoint to become ready. Do not hold the mount lock across that slow
+ * path; callers only need the lock for local state updates.
+ */
+
while (fs->timeout > 0)
{
struct stat buf;
int ret;
+ nxmutex_unlock(&fs->fs_lock);
+
ret = rpmsgfs_client_stat(fs->handle, fs->fs_root, &buf);
+ nxmutex_lock(&fs->fs_lock);
if (ret == 0)
{
break;
}
+ nxmutex_unlock(&fs->fs_lock);
nxsched_usleep(RPMSGFS_RETRY_DELAY_MS * USEC_PER_MSEC);
+ nxmutex_lock(&fs->fs_lock);
Review Comment:
ditto
--
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
Re: [PR] fs/rpmsgfs: fix open cleanup and root wait locking [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao closed pull request #20858: fs/rpmsgfs: fix open cleanup and root wait locking
URL: https://github.com/apache/nuttx/pull/20858
--
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
Re: [PR] fs/rpmsgfs: fix open cleanup and root wait locking [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20858:
URL: https://github.com/apache/nuttx/pull/20858#discussion_r4336406471
##########
fs/rpmsgfs/rpmsgfs.c:
##########
@@ -241,18 +241,28 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
strlcpy(path, fs->fs_root, pathlen);
strlcat(path, relpath, pathlen);
+ /* The remote root check can block and sleep while waiting for the remote
+ * endpoint to become ready. Do not hold the mount lock across that slow
+ * path; callers only need the lock for local state updates.
+ */
+
while (fs->timeout > 0)
{
struct stat buf;
int ret;
+ nxmutex_unlock(&fs->fs_lock);
Review Comment:
yes, but we can drop mutex, the lock isn't nesssary
--
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
Re: [PR] fs/rpmsgfs: fix open cleanup and root wait locking [nuttx]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20858:
URL: https://github.com/apache/nuttx/pull/20858#issuecomment-6309849841
<!-- membrowse-pr-comment -->
## <img src="https://membrowse.com/membrowse-logo.svg" height="24" align="top"> MemBrowse Memory Report · [Explore what changed →](https://membrowse.com/public/apache/nuttx?commit_sha=c1ce6a6a289f82159f721bd44cd164cca9fad34f)
*No memory changes detected for:*
- [arduino-mega2560](https://membrowse.com/public/apache/nuttx/target/arduino-mega2560/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
- [esp32-devkitc](https://membrowse.com/public/apache/nuttx/target/esp32-devkitc/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
- [hifive1-revb](https://membrowse.com/public/apache/nuttx/target/hifive1-revb/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
- [mirtoo](https://membrowse.com/public/apache/nuttx/target/mirtoo/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
- [qemu-armv8a](https://membrowse.com/public/apache/nuttx/target/qemu-armv8a/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
- [qemu-intel64](https://membrowse.com/public/apache/nuttx/target/qemu-intel64/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
- [rx65n-rsk2mb](https://membrowse.com/public/apache/nuttx/target/rx65n-rsk2mb/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
- [s698pm-dkit](https://membrowse.com/public/apache/nuttx/target/s698pm-dkit/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
- [stm32-nucleo-f103rb](https://membrowse.com/public/apache/nuttx/target/stm32-nucleo-f103rb/compare?head=c1ce6a6a289f82159f721bd44cd164cca9fad34f&base=a16dbf2100e6a1484a59cb41826034d01834b36d)
---
<sub>Catch memory regressions in CI with [MemBrowse](https://membrowse.com). Free for open source.</sub>
--
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
Re: [PR] fs/rpmsgfs: fix open cleanup and root wait locking [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20858:
URL: https://github.com/apache/nuttx/pull/20858#discussion_r4334464564
##########
fs/rpmsgfs/rpmsgfs.c:
##########
@@ -241,18 +241,28 @@ static void rpmsgfs_mkpath(FAR struct rpmsgfs_mountpt_s *fs,
strlcpy(path, fs->fs_root, pathlen);
strlcat(path, relpath, pathlen);
+ /* The remote root check can block and sleep while waiting for the remote
+ * endpoint to become ready. Do not hold the mount lock across that slow
+ * path; callers only need the lock for local state updates.
+ */
+
while (fs->timeout > 0)
{
struct stat buf;
int ret;
+ nxmutex_unlock(&fs->fs_lock);
Review Comment:
why not remove fs_lock directly? the lock is unnecessary?
--
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