-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Filelock #11724
Implement Filelock #11724
Conversation
merge form OpenGroup https://pubs.opengroup.org/onlinepubs/009696899/functions/hcreate.html Signed-off-by: guohao15 <[email protected]>
Signed-off-by: guohao15 <[email protected]>
Signed-off-by: chenrun1 <[email protected]>
Signed-off-by: chenrun1 <[email protected]>
When there is no conflicting lock, getlk should not tamper with the l_pid content from the upper layer Signed-off-by: chenrun1 <[email protected]>
When we close a socket fd, it will call get path on sockets. `close(socket_fd)` -> `file_closelk(filep)` -> `file_fcntl(F_GETPATH)` It causes a heavy stack load for each socket close operation. (We have `GETPATH` for sockets to be used for `fdinfo`) But the socket fds are not intended to be used for file locks. And so do some other file types, so we may just limit the usage of flock. Signed-off-by: Zhe Weng <[email protected]>
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 |
Please try this change #11729. I will submit the LFS support GET PATH implementation to another PR. |
Signed-off-by: chenrun1 <[email protected]>
Signed-off-by: chenrun1 <[email protected]>
using a path as a file id for locking seems broken by design. (consider rename.) |
but how to get id from the different file system? |
add it to file_operations. or it can be fcntl/ioctl as F_GETPATH. |
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. |
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. |
well, actually, i don't have a motivation to improve file locking. |
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
alternatively, you can probably try to convince the littlefs author to accept the get file path patch. 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. |
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