You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "crafcat7 (via GitHub)" <gi...@apache.org> on 2024/02/20 08:25:27 UTC
[PR] Implement Filelock [nuttx]
crafcat7 opened a new pull request, #11724:
URL: https://github.com/apache/nuttx/pull/11724
## Summary
I made functional implementations of F_SETLK, F_SETLKW, F_GETLK in fcntl to extend the capabilities of fcntl.
The way to implement this is to use a simple Hashtable implementation, using the path as the key to store the lock information context of a file, from which you can go to the lock, unlock, merge locks and other operations.
## Impact
Implemented file locking functionality, extending the F_SETLK, F_SETLKW, F_GETLK implementations in fcntl.
## Testing
Through a series of tests on file locks, its functionality aligns with Linux file locks
--
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] Implement Filelock [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#discussion_r1496160999
##########
fs/vfs/fs_lock.c:
##########
@@ -181,7 +184,6 @@ static int file_lock_normalize(FAR struct file *filep,
}
out->l_whence = SEEK_SET;
- out->l_pid = getpid();
Review Comment:
Why was PID removed?
--
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] Implement Filelock [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-1954244010
> If underlying fs is on a MTD device, function call file_fcntl(filep, F_GETPATH, path) returns with ENOTTY in my case where MX25L device is used to serve littlefs fileystem.
our littlefs fs wrapper implement F_GETPATH to return the file name
--
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] Implement Filelock [nuttx]
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-1954198904
If underlying fs is on a MTD device, function call file_fcntl(filep, F_GETPATH, path) returns with ENOTTY in my case where MX25L device is used to serve littlefs fileystem.
--
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] Implement Filelock [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2109310508
I don't object to utilize some unique id, but this approach need improve all file system implementation, which isn't a minor work. The current implementation of filelock work well in most case. If your case require handle rename correctly, feel free to improve the current implementation, thanks.
--
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] Implement Filelock [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #11724:
URL: https://github.com/apache/nuttx/pull/11724
--
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] Implement Filelock [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#discussion_r1497777792
##########
fs/vfs/fs_lock.c:
##########
@@ -181,7 +184,6 @@ static int file_lock_normalize(FAR struct file *filep,
}
out->l_whence = SEEK_SET;
- out->l_pid = getpid();
Review Comment:
@acassis do you have more comment?
--
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] Implement Filelock [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2108169224
but how to get id from the different file system?
--
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] Implement Filelock [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2109306366
The problem is that many fs designed for RTOS doesn't define the unique id in file metadata, adding file_operations can't resolve this problem.
--
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] Implement Filelock [nuttx]
Posted by "yamt (via GitHub)" <gi...@apache.org>.
yamt commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2107246022
using a path as a file id for locking seems broken by design. (consider rename.)
can you consider to use something more appropriate?
eg. a dedicated id for file locking, which is not changed by a rename.
--
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] Implement Filelock [nuttx]
Posted by "yamt (via GitHub)" <gi...@apache.org>.
yamt commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2109054353
add it to file_operations. or it can be fcntl/ioctl as F_GETPATH.
--
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] Implement Filelock [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2109366017
If you don't use file lock, could skip the local patch safely.
--
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] Implement Filelock [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2112594135
> > If you don't use file lock, could skip the local patch safely.
>
> it doesn't solve the maintenance problem.
>
> if no one wants to work on a solution, i'd suggest to revert it.
This feature is used other community member not us. It work fine, why do you want to revert 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] Implement Filelock [nuttx]
Posted by "yamt (via GitHub)" <gi...@apache.org>.
yamt commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2116644633
> > > If you don't use file lock, could skip the local patch safely.
> >
> >
> > it doesn't solve the maintenance problem.
> > if no one wants to work on a solution, i'd suggest to revert it.
>
> This feature is used by other community member not us. It work fine, why do you want to revert it?
because it's responsibility for the person adding a feature to find a maintainable implementation.
my suggestion is
* introduce an optional api to query filesystems file id usable for this purpose. say, F_GETIDFORLOCK
* adapt this PR to use F_GETIDFORLOCK. when a filesystem doesn't support it, probably fall back to F_GETPATH. or just bail out with ENOTSUP or such.
* revert F_GETPATH patch in littlefs because it seems unacceptable for the upstream.
* add necessary api to the upstream littlefs, probably something along the line with: https://github.com/littlefs-project/littlefs/pull/976#issuecomment-2106900479. importantly we should provide something the upstream littlefs can accept.
* implement F_GETIDFORLOCK in our littlefs driver, based on the new api. i guess you can iterate all open files and find aliases, and assign/reuse the in-core file id accordingly.
alternatively, you can probably try to convince the littlefs author to accept [the get file path patch](https://github.com/littlefs-project/littlefs/pull/976). if it gets merged in the upstream, i have no big objection against having the patch in our tree for older littlefs versions in the meantime. i personally agree with his rationale of the rejection though.
--
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] Implement Filelock [nuttx]
Posted by "anjiahao1 (via GitHub)" <gi...@apache.org>.
anjiahao1 commented on code in PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#discussion_r1496802038
##########
fs/vfs/Make.defs:
##########
@@ -29,6 +29,10 @@ CSRCS += fs_syncfs.c fs_truncate.c
# Certain interfaces are not available if there is no mountpoint support
+ifneq ($(CONFIG_FS_LOCK_BUCKET_SIZE),0)
+CSRCS += fs_lock.c
Review Comment:
add CMakeLists.txt too?
--
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] Implement Filelock [nuttx]
Posted by "crafcat7 (via GitHub)" <gi...@apache.org>.
crafcat7 commented on code in PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#discussion_r1496991406
##########
fs/vfs/Make.defs:
##########
@@ -29,6 +29,10 @@ CSRCS += fs_syncfs.c fs_truncate.c
# Certain interfaces are not available if there is no mountpoint support
+ifneq ($(CONFIG_FS_LOCK_BUCKET_SIZE),0)
+CSRCS += fs_lock.c
Review Comment:
Done.
--
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] Implement Filelock [nuttx]
Posted by "crafcat7 (via GitHub)" <gi...@apache.org>.
crafcat7 commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-1954294113
> If underlying fs is on a MTD device, function call file_fcntl(filep, F_GETPATH, path) returns with ENOTTY in my case where MX25L device is used to serve littlefs fileystem.
Please try this change https://github.com/apache/nuttx/pull/11729. I will submit the LFS support GET PATH implementation to another PR.
--
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] Implement Filelock [nuttx]
Posted by "crafcat7 (via GitHub)" <gi...@apache.org>.
crafcat7 commented on code in PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#discussion_r1496753262
##########
fs/vfs/fs_lock.c:
##########
@@ -181,7 +184,6 @@ static int file_lock_normalize(FAR struct file *filep,
}
out->l_whence = SEEK_SET;
- out->l_pid = getpid();
Review Comment:
The reason for removing the PID from normalize is that both setlk and getlk reuse this interface to convert the flock once.
For getlk, in some test scenarios it is not expected to change the contents of a user-inputted flock when it cannot find a conflicting lock, so getpid is taken out separately and placed in the setlk process.
--
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] Implement Filelock [nuttx]
Posted by "yamt (via GitHub)" <gi...@apache.org>.
yamt commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2111493542
> If you don't use file lock, could skip the local patch safely.
it doesn't solve the maintenance problem.
if no one wants to work on a solution, i'd suggest to revert 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] Implement Filelock [nuttx]
Posted by "yamt (via GitHub)" <gi...@apache.org>.
yamt commented on PR #11724:
URL: https://github.com/apache/nuttx/pull/11724#issuecomment-2109313913
well, actually, i don't have a motivation to improve file locking.
i'm just unhappy with having the local littlefs 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