在 2020/6/2 23:56, Jeff Layton 写道:
On Tue, 2020-06-02 at 21:49 +0800, yangerkun wrote:
在 2020/6/2 7:10, NeilBrown 写道:
On Mon, Jun 01 2020, yangerkun wrote:
We forget to call locks_move_blocks in posix_lock_inode when try to
process same owner and different types.
This patch is not necessary.
The caller of posix_lock_inode() must calls locks_delete_block() on
'request', and that will remove all blocked request and retry them.
So calling locks_move_blocks() here is at most an optimization. Maybe
it is a useful one.
What led you to suggesting this patch? Were you just examining the
code, or was there some problem that you were trying to solve?
Actually, case of this means just replace a exists file_lock. And once
we forget to call locks_move_blocks, the function call of
posix_lock_inode will also call locks_delete_block, and will wakeup all
blocked requests and retry them. But we should do this until we UNLOCK
the file_lock! So, it's really a bug here.
Waking up waiters to re-poll a lock that's still blocked seems wrong. I
agree with Neil that this is mainly an optimization, but it does look
useful.
Agree. Logic of this seems wrong, but it won't trigger any problem since
the waiters will conflict and try wait again.
Unfortunately this is the type of thing that's quite difficult to test
for in a userland testcase. Is this something you noticed due to the
extra wakeups or did you find it by inspection? It'd be great to have a
better way to test for this in xfstests or something.
Notice this after reading the patch 5946c4319ebb ("fs/locks: allow a
lock request to block other requests."), and find that we have do the
same thing exist in flock_lock_inode and another place exists in
posix_lock_inode.
I'll plan to add this to linux-next. It should make v5.9, but let me
know if this is causing real-world problems and maybe we can make a case
for v5.8.
Actually, I have not try to find will this lead to some real-world
problems... Sorry for this.:(
Thanks,
Kun.
Thanks,
Jeff
Signed-off-by: yangerkun <yangerkun@xxxxxxxxxx>
---
fs/locks.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/locks.c b/fs/locks.c
index b8a31c1c4fff..36bd2c221786 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1282,6 +1282,7 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request,
if (!new_fl)
goto out;
locks_copy_lock(new_fl, request);
+ locks_move_blocks(new_fl, request);
request = new_fl;
new_fl = NULL;
locks_insert_lock_ctx(request, &fl->fl_list);
--
2.21.3
