[LU-104] Lustre grants flock exclusive locks on two file descriptors for the same file

• Type: Bug
• Resolution: Fixed
• Priority: Blocker
• Fix Version/s: Lustre 2.1.0
• Affects Version/s: Lustre 2.1.0
• Severity: 3
• Bugzilla ID: 22,660
• Assignee: Oleg Drokin
• Reporter: Christopher Morrone (Inactive)

Description

Lustre grants flock exclusive locks on two file descriptors for the same
file in the same process. However, the second flock SHOULD fail.

Fortunately, the VFS already has the correct behavior. And since we are
already calling the flock_lock_file_wait() or posix_lock_file_wait() VFS
calls, we just need to record that return code and return it in
ll_file_lock().

This patch was already landed on 1.8, needs to land on master.

Comments

[Build Master (Inactive)]

Integrated in lustre-master » i686,client,el5,inkernel #101
LU-104 Properly address ownership of posix and flock locks

Oleg Drokin : 3bb8d1b9656994b0313cdba6ad8eeb7b84f5ee9f
Files :

• lustre/ldlm/ldlm_flock.c
• lustre/tests/flocks_test.c
• lustre/utils/wirecheck.c
• lustre/include/lustre_dlm.h
• lustre/ldlm/ldlm_extent.c
• lustre/tests/sanity.sh
• lustre/ldlm/ldlm_lockd.c
• lustre/tests/Makefile.am
• lustre/include/lustre/lustre_idl.h
• lustre/ldlm/ldlm_internal.h
• lustre/ldlm/ldlm_lock.c
• lustre/ldlm/ldlm_plain.c
• lustre/utils/wiretest.c
• lustre/ldlm/ldlm_request.c
• lustre/tests/flock_test.c
• lustre/llite/file.c
• lustre/ptlrpc/wiretest.c
• lustre/ldlm/ldlm_inodebits.c
• lustre/ptlrpc/pack_generic.c

[Peter Jones]

Landed for 2.1. Please reopen if any further instances of this issue seen

[Peter Jones]

LLNL confirmed on the weekly 2.1 call that the patch attached to 22040 fixes this issue in their tests

[Christopher Morrone (Inactive)]

Ah, I see. Well, flocks_test.c already does one flock() test (in t1), so it isn't so obvious that it shouldn't have more. Additionally, flocks_test.c has a crude framework for adding multiple tests while flock_test.c does not. Perhaps flock_test.c should just be folded into flocks_test.c and removed.

Or we should clean them up and give them sane names...it would probably be best of all if we had a good way in test suite to easily compile and run individual C regression tests (sort of like autoconf does), so we don't need to piggy-back multiple tests into one C file.

In any event, I'd like our test to appear somewhere that actually gets run.

[Oleg Drokin]

Ok.
What I was trying to convey was that your test went into flocks_test.c and that one deal with posix locks.
There is also flock_test.c that deals with bsd locks and your test belongs there.
Of course the problem is flock_test.c is never run because bsd locks just don't work correctly in Lustre anyway.

[Christopher Morrone (Inactive)]

I could not really understand what was being said on the phone, so I'll start the conversation here again.

Regardless of how the bug is fixed, I think the test from our patch should be added (or something equivalent). There was no existing test that did what our test added as far as I can see.

[Peter Jones]

ok, I have marked it as a blocker and assigned it to you while you ponder. We can reassign to another engineer once you have decided the approach if you prefer

[Oleg Drokin]

Yes. I think we need in in 2.1. I am still debating with myself if I need to include it verbatim or if I should add compat flag + its check for extra safety.

[Peter Jones]

Oleg

So, is the patch from bz 22040 something that we could include in Lustre 2.1?

Peter

[Oleg Drokin]

the flock (not posix locks you get with fcntl) in lustre is totally broken.
The proper fix for tihs problem that actually addresses the problem at the lustre level + fixes nfs4 locking is located in bug 22040.

[Christopher Morrone (Inactive)]

Patch for head pushed to gerrit:

http://review.whamcloud.com/279