From: Oleg Drokin Date: Sat, 23 Apr 2011 03:15:37 +0000 (-0400) Subject: LU-104 Properly address ownership of posix and flock locks X-Git-Tag: 2.0.60.0~18 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=3bb8d1b9656994b0313cdba6ad8eeb7b84f5ee9f LU-104 Properly address ownership of posix and flock locks This patch fixes the long standing problem where we only relied on pid to determine lock ownership. This patch uses kernel-passed fl_owner field for posix lock and fl_file for flock ownership like it should. This is a wire protocol change of sorts, but I have compat code in place. Also introduced a clear separation for on the wire and in-memory policy representations and ways to convert between them. This makes nfs4 locking work, also fixes original problem with flock locking of the same fd from different processes. Makes our own tests/flock_test.c to pass now which it never did before. connectathon on nfs4 also passes locally for me now. Added a flock test that is actually being run now. Signed-off-by: Oleg Drokin Change-Id: Ifb4f4a0246f34143497225f2b8974f5fd93817a1 Reviewed-on: http://review.whamcloud.com/459 Reviewed-by: Johann Lombardi Reviewed-by: Alex Zhuravlev Tested-by: Hudson --- diff --git a/lustre/include/lustre/lustre_idl.h b/lustre/include/lustre/lustre_idl.h index 826dcda..a139e998 100644 --- a/lustre/include/lustre/lustre_idl.h +++ b/lustre/include/lustre/lustre_idl.h @@ -2213,12 +2213,12 @@ struct ldlm_inodebits { __u64 bits; }; -struct ldlm_flock { - __u64 start; - __u64 end; - __u64 blocking_export; /* not actually used over the wire */ - __u32 blocking_pid; /* not actually used over the wire */ - __u32 pid; +struct ldlm_flock_wire { + __u64 lfw_start; + __u64 lfw_end; + __u64 lfw_owner; + __u32 lfw_padding; + __u32 lfw_pid; }; /* it's important that the fields of the ldlm_extent structure match @@ -2229,11 +2229,11 @@ struct ldlm_flock { typedef union { struct ldlm_extent l_extent; - struct ldlm_flock l_flock; + struct ldlm_flock_wire l_flock; struct ldlm_inodebits l_inodebits; -} ldlm_policy_data_t; +} ldlm_wire_policy_data_t; -extern void lustre_swab_ldlm_policy_data (ldlm_policy_data_t *d); +extern void lustre_swab_ldlm_policy_data (ldlm_wire_policy_data_t *d); struct ldlm_intent { __u64 opc; @@ -2253,7 +2253,7 @@ struct ldlm_lock_desc { struct ldlm_resource_desc l_resource; ldlm_mode_t l_req_mode; ldlm_mode_t l_granted_mode; - ldlm_policy_data_t l_policy_data; + ldlm_wire_policy_data_t l_policy_data; }; extern void lustre_swab_ldlm_lock_desc (struct ldlm_lock_desc *l); diff --git a/lustre/include/lustre_dlm.h b/lustre/include/lustre_dlm.h index 01ba8f8..23ebee7 100644 --- a/lustre/include/lustre_dlm.h +++ b/lustre/include/lustre_dlm.h @@ -583,6 +583,28 @@ typedef enum { * in the same RPC */ } ldlm_cancel_flags_t; +struct ldlm_flock { + __u64 start; + __u64 end; + __u64 owner; + __u64 blocking_owner; + void *blocking_export; + __u32 pid; +}; + +typedef union { + struct ldlm_extent l_extent; + struct ldlm_flock l_flock; + struct ldlm_inodebits l_inodebits; +} ldlm_policy_data_t; + +void ldlm_convert_policy_to_wire(ldlm_type_t type, + const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy); +void ldlm_convert_policy_to_local(ldlm_type_t type, + const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy); + struct ldlm_lock { /** * Must be first in the structure. diff --git a/lustre/ldlm/ldlm_extent.c b/lustre/ldlm/ldlm_extent.c index 8232d6f..bcbe0e2 100644 --- a/lustre/ldlm/ldlm_extent.c +++ b/lustre/ldlm/ldlm_extent.c @@ -916,3 +916,22 @@ void ldlm_extent_unlink_lock(struct ldlm_lock *lock) ldlm_interval_free(node); } } + +void ldlm_extent_policy_wire_to_local(const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy) +{ + memset(lpolicy, 0, sizeof(*lpolicy)); + lpolicy->l_extent.start = wpolicy->l_extent.start; + lpolicy->l_extent.end = wpolicy->l_extent.end; + lpolicy->l_extent.gid = wpolicy->l_extent.gid; +} + +void ldlm_extent_policy_local_to_wire(const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy) +{ + memset(wpolicy, 0, sizeof(*wpolicy)); + wpolicy->l_extent.start = lpolicy->l_extent.start; + wpolicy->l_extent.end = lpolicy->l_extent.end; + wpolicy->l_extent.gid = lpolicy->l_extent.gid; +} + diff --git a/lustre/ldlm/ldlm_flock.c b/lustre/ldlm/ldlm_flock.c index 32837d5..55117d1 100644 --- a/lustre/ldlm/ldlm_flock.c +++ b/lustre/ldlm/ldlm_flock.c @@ -82,8 +82,8 @@ int ldlm_flock_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc, static inline int ldlm_same_flock_owner(struct ldlm_lock *lock, struct ldlm_lock *new) { - return((new->l_policy_data.l_flock.pid == - lock->l_policy_data.l_flock.pid) && + return((new->l_policy_data.l_flock.owner == + lock->l_policy_data.l_flock.owner) && (new->l_export == lock->l_export)); } @@ -127,21 +127,22 @@ ldlm_flock_deadlock(struct ldlm_lock *req, struct ldlm_lock *blocking_lock) { struct obd_export *req_export = req->l_export; struct obd_export *blocking_export = blocking_lock->l_export; - pid_t req_pid = req->l_policy_data.l_flock.pid; - pid_t blocking_pid = blocking_lock->l_policy_data.l_flock.pid; + __u64 req_owner = req->l_policy_data.l_flock.owner; + __u64 blocking_owner = blocking_lock->l_policy_data.l_flock.owner; struct ldlm_lock *lock; cfs_spin_lock(&ldlm_flock_waitq_lock); restart: cfs_list_for_each_entry(lock, &ldlm_flock_waitq, l_flock_waitq) { - if ((lock->l_policy_data.l_flock.pid != blocking_pid) || + if ((lock->l_policy_data.l_flock.owner != blocking_owner) || (lock->l_export != blocking_export)) continue; - blocking_pid = lock->l_policy_data.l_flock.blocking_pid; - blocking_export = (struct obd_export *)(long) + blocking_owner = lock->l_policy_data.l_flock.blocking_owner; + blocking_export = (struct obd_export *) lock->l_policy_data.l_flock.blocking_export; - if (blocking_pid == req_pid && blocking_export == req_export) { + if (blocking_owner == req_owner && + blocking_export == req_export) { cfs_spin_unlock(&ldlm_flock_waitq_lock); return 1; } @@ -172,8 +173,9 @@ ldlm_process_flock_lock(struct ldlm_lock *req, int *flags, int first_enq, const struct ldlm_callback_suite null_cbs = { NULL }; ENTRY; - CDEBUG(D_DLMTRACE, "flags %#x pid %u mode %u start "LPU64" end "LPU64 - "\n", *flags, new->l_policy_data.l_flock.pid, mode, + CDEBUG(D_DLMTRACE, "flags %#x owner "LPU64" pid %u mode %u start "LPU64 + " end "LPU64"\n", *flags, new->l_policy_data.l_flock.owner, + new->l_policy_data.l_flock.pid, mode, req->l_policy_data.l_flock.start, req->l_policy_data.l_flock.end); @@ -250,10 +252,10 @@ reprocess: RETURN(LDLM_ITER_STOP); } - req->l_policy_data.l_flock.blocking_pid = - lock->l_policy_data.l_flock.pid; + req->l_policy_data.l_flock.blocking_owner = + lock->l_policy_data.l_flock.owner; req->l_policy_data.l_flock.blocking_export = - (long)(void *)lock->l_export; + lock->l_export; LASSERT(cfs_list_empty(&req->l_flock_waitq)); cfs_spin_lock(&ldlm_flock_waitq_lock); @@ -395,6 +397,8 @@ reprocess: new2->l_granted_mode = lock->l_granted_mode; new2->l_policy_data.l_flock.pid = new->l_policy_data.l_flock.pid; + new2->l_policy_data.l_flock.owner = + new->l_policy_data.l_flock.owner; new2->l_policy_data.l_flock.start = lock->l_policy_data.l_flock.start; new2->l_policy_data.l_flock.end = @@ -660,3 +664,27 @@ int ldlm_flock_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc, cfs_spin_unlock(&ldlm_flock_waitq_lock); RETURN(0); } + +void ldlm_flock_policy_wire_to_local(const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy) +{ + memset(lpolicy, 0, sizeof(*lpolicy)); + lpolicy->l_flock.start = wpolicy->l_flock.lfw_start; + lpolicy->l_flock.end = wpolicy->l_flock.lfw_end; + lpolicy->l_flock.pid = wpolicy->l_flock.lfw_pid; + lpolicy->l_flock.owner = wpolicy->l_flock.lfw_owner; + /* Compat code, old clients had no idea about owner field and + * relied solely on pid for ownership. Introduced in 2.1, April 2011 */ + if (!lpolicy->l_flock.owner) + lpolicy->l_flock.owner = wpolicy->l_flock.lfw_pid; +} + +void ldlm_flock_policy_local_to_wire(const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy) +{ + memset(wpolicy, 0, sizeof(*wpolicy)); + wpolicy->l_flock.lfw_start = lpolicy->l_flock.start; + wpolicy->l_flock.lfw_end = lpolicy->l_flock.end; + wpolicy->l_flock.lfw_pid = lpolicy->l_flock.pid; + wpolicy->l_flock.lfw_owner = lpolicy->l_flock.owner; +} diff --git a/lustre/ldlm/ldlm_inodebits.c b/lustre/ldlm/ldlm_inodebits.c index 187bb74..6b9bf12 100644 --- a/lustre/ldlm/ldlm_inodebits.c +++ b/lustre/ldlm/ldlm_inodebits.c @@ -191,3 +191,17 @@ int ldlm_process_inodebits_lock(struct ldlm_lock *lock, int *flags, } RETURN(0); } + +void ldlm_ibits_policy_wire_to_local(const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy) +{ + memset(lpolicy, 0, sizeof(*lpolicy)); + lpolicy->l_inodebits.bits = wpolicy->l_inodebits.bits; +} + +void ldlm_ibits_policy_local_to_wire(const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy) +{ + memset(wpolicy, 0, sizeof(*wpolicy)); + wpolicy->l_inodebits.bits = lpolicy->l_inodebits.bits; +} diff --git a/lustre/ldlm/ldlm_internal.h b/lustre/ldlm/ldlm_internal.h index c6345cb..c5b2139 100644 --- a/lustre/ldlm/ldlm_internal.h +++ b/lustre/ldlm/ldlm_internal.h @@ -258,3 +258,26 @@ static inline int is_granted_or_cancelled(struct ldlm_lock *lock) return ret; } + +typedef void (*ldlm_policy_wire_to_local_t)(const ldlm_wire_policy_data_t *, + ldlm_policy_data_t *); + +typedef void (*ldlm_policy_local_to_wire_t)(const ldlm_policy_data_t *, + ldlm_wire_policy_data_t *); + +void ldlm_plain_policy_wire_to_local(const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy); +void ldlm_plain_policy_local_to_wire(const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy); +void ldlm_ibits_policy_wire_to_local(const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy); +void ldlm_ibits_policy_local_to_wire(const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy); +void ldlm_extent_policy_wire_to_local(const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy); +void ldlm_extent_policy_local_to_wire(const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy); +void ldlm_flock_policy_wire_to_local(const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy); +void ldlm_flock_policy_local_to_wire(const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy); diff --git a/lustre/ldlm/ldlm_lock.c b/lustre/ldlm/ldlm_lock.c index 574b1ad..acb634b 100644 --- a/lustre/ldlm/ldlm_lock.c +++ b/lustre/ldlm/ldlm_lock.c @@ -71,6 +71,48 @@ char *ldlm_typename[] = { [LDLM_IBITS] "IBT", }; +static ldlm_policy_wire_to_local_t ldlm_policy_wire_to_local[] = { + [LDLM_PLAIN - LDLM_MIN_TYPE] ldlm_plain_policy_wire_to_local, + [LDLM_EXTENT - LDLM_MIN_TYPE] ldlm_extent_policy_wire_to_local, + [LDLM_FLOCK - LDLM_MIN_TYPE] ldlm_flock_policy_wire_to_local, + [LDLM_IBITS - LDLM_MIN_TYPE] ldlm_ibits_policy_wire_to_local, +}; + +static ldlm_policy_local_to_wire_t ldlm_policy_local_to_wire[] = { + [LDLM_PLAIN - LDLM_MIN_TYPE] ldlm_plain_policy_local_to_wire, + [LDLM_EXTENT - LDLM_MIN_TYPE] ldlm_extent_policy_local_to_wire, + [LDLM_FLOCK - LDLM_MIN_TYPE] ldlm_flock_policy_local_to_wire, + [LDLM_IBITS - LDLM_MIN_TYPE] ldlm_ibits_policy_local_to_wire, +}; + +/** + * Converts lock policy from local format to on the wire lock_desc format + */ +void ldlm_convert_policy_to_wire(ldlm_type_t type, + const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy) +{ + ldlm_policy_local_to_wire_t convert; + + convert = ldlm_policy_local_to_wire[type - LDLM_MIN_TYPE]; + + convert(lpolicy, wpolicy); +} + +/** + * Converts lock policy from on the wire lock_desc format to local format + */ +void ldlm_convert_policy_to_local(ldlm_type_t type, + const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy) +{ + ldlm_policy_wire_to_local_t convert; + + convert = ldlm_policy_wire_to_local[type - LDLM_MIN_TYPE]; + + convert(wpolicy, lpolicy); +} + char *ldlm_it2str(int it) { switch (it) { @@ -552,7 +594,9 @@ void ldlm_lock2desc(struct ldlm_lock *lock, struct ldlm_lock_desc *desc) ldlm_res2desc(lock->l_resource, &desc->l_resource); desc->l_req_mode = lock->l_req_mode; desc->l_granted_mode = lock->l_granted_mode; - desc->l_policy_data = lock->l_policy_data; + ldlm_convert_policy_to_wire(lock->l_resource->lr_type, + &lock->l_policy_data, + &desc->l_policy_data); } } diff --git a/lustre/ldlm/ldlm_lockd.c b/lustre/ldlm/ldlm_lockd.c index 6d2795f..101a848 100644 --- a/lustre/ldlm/ldlm_lockd.c +++ b/lustre/ldlm/ldlm_lockd.c @@ -1165,7 +1165,10 @@ existing_lock: } if (dlm_req->lock_desc.l_resource.lr_type != LDLM_PLAIN) - lock->l_policy_data = dlm_req->lock_desc.l_policy_data; + ldlm_convert_policy_to_local( + dlm_req->lock_desc.l_resource.lr_type, + &dlm_req->lock_desc.l_policy_data, + &lock->l_policy_data); if (dlm_req->lock_desc.l_resource.lr_type == LDLM_EXTENT) lock->l_req_extent = lock->l_policy_data.l_extent; @@ -1537,7 +1540,10 @@ static void ldlm_handle_cp_callback(struct ptlrpc_request *req, } if (lock->l_resource->lr_type != LDLM_PLAIN) { - lock->l_policy_data = dlm_req->lock_desc.l_policy_data; + ldlm_convert_policy_to_local( + dlm_req->lock_desc.l_resource.lr_type, + &dlm_req->lock_desc.l_policy_data, + &lock->l_policy_data); LDLM_DEBUG(lock, "completion AST, new policy data"); } diff --git a/lustre/ldlm/ldlm_plain.c b/lustre/ldlm/ldlm_plain.c index c8941a4..7cac918 100644 --- a/lustre/ldlm/ldlm_plain.c +++ b/lustre/ldlm/ldlm_plain.c @@ -158,3 +158,15 @@ int ldlm_process_plain_lock(struct ldlm_lock *lock, int *flags, int first_enq, } RETURN(0); } + +void ldlm_plain_policy_wire_to_local(const ldlm_wire_policy_data_t *wpolicy, + ldlm_policy_data_t *lpolicy) +{ + /* No policy for plain locks */ +} + +void ldlm_plain_policy_local_to_wire(const ldlm_policy_data_t *lpolicy, + ldlm_wire_policy_data_t *wpolicy) +{ + /* No policy for plain locks */ +} diff --git a/lustre/ldlm/ldlm_request.c b/lustre/ldlm/ldlm_request.c index 5d718c9..d03e0eb 100644 --- a/lustre/ldlm/ldlm_request.c +++ b/lustre/ldlm/ldlm_request.c @@ -587,8 +587,11 @@ int ldlm_cli_enqueue_fini(struct obd_export *exp, struct ptlrpc_request *req, if (with_policy) if (!(type == LDLM_IBITS && !(exp->exp_connect_flags & OBD_CONNECT_IBITS))) - lock->l_policy_data = - reply->lock_desc.l_policy_data; + /* We assume lock type cannot change on server*/ + ldlm_convert_policy_to_local( + lock->l_resource->lr_type, + &reply->lock_desc.l_policy_data, + &lock->l_policy_data); if (type != LDLM_PLAIN) LDLM_DEBUG(lock,"client-side enqueue, new policy data"); } diff --git a/lustre/llite/file.c b/lustre/llite/file.c index 5a54c94..8e5f240 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -1988,7 +1988,7 @@ int ll_file_flock(struct file *file, int cmd, struct file_lock *file_lock) .ei_cbdata = file_lock }; struct md_op_data *op_data; struct lustre_handle lockh = {0}; - ldlm_policy_data_t flock; + ldlm_policy_data_t flock = {{0}}; int flags = 0; int rc; ENTRY; @@ -2000,13 +2000,18 @@ int ll_file_flock(struct file *file, int cmd, struct file_lock *file_lock) if (file_lock->fl_flags & FL_FLOCK) { LASSERT((cmd == F_SETLKW) || (cmd == F_SETLK)); - /* set missing params for flock() calls */ - file_lock->fl_end = OFFSET_MAX; - file_lock->fl_pid = current->tgid; + /* flocks are whole-file locks */ + flock.l_flock.end = OFFSET_MAX; + /* For flocks owner is determined by the local file desctiptor*/ + flock.l_flock.owner = (unsigned long)file_lock->fl_file; + } else if (file_lock->fl_flags & FL_POSIX) { + flock.l_flock.owner = (unsigned long)file_lock->fl_owner; + flock.l_flock.start = file_lock->fl_start; + flock.l_flock.end = file_lock->fl_end; + } else { + RETURN(-EINVAL); } flock.l_flock.pid = file_lock->fl_pid; - flock.l_flock.start = file_lock->fl_start; - flock.l_flock.end = file_lock->fl_end; switch (file_lock->fl_type) { case F_RDLCK: diff --git a/lustre/ptlrpc/pack_generic.c b/lustre/ptlrpc/pack_generic.c index 03dbcca..659882c 100644 --- a/lustre/ptlrpc/pack_generic.c +++ b/lustre/ptlrpc/pack_generic.c @@ -1995,7 +1995,7 @@ void lustre_swab_ldlm_res_id (struct ldlm_res_id *id) __swab64s (&id->name[i]); } -void lustre_swab_ldlm_policy_data (ldlm_policy_data_t *d) +void lustre_swab_ldlm_policy_data (ldlm_wire_policy_data_t *d) { /* the lock data is a union and the first two fields are always an * extent so it's ok to process an LDLM_EXTENT and LDLM_FLOCK lock @@ -2003,7 +2003,8 @@ void lustre_swab_ldlm_policy_data (ldlm_policy_data_t *d) __swab64s(&d->l_extent.start); __swab64s(&d->l_extent.end); __swab64s(&d->l_extent.gid); - __swab32s(&d->l_flock.pid); + __swab64s(&d->l_flock.lfw_owner); + __swab32s(&d->l_flock.lfw_pid); } void lustre_swab_ldlm_intent (struct ldlm_intent *i) diff --git a/lustre/ptlrpc/wiretest.c b/lustre/ptlrpc/wiretest.c index 81fe0ff..b5da020 100644 --- a/lustre/ptlrpc/wiretest.c +++ b/lustre/ptlrpc/wiretest.c @@ -65,8 +65,8 @@ void lustre_assert_wire_constants(void) { /* Wire protocol assertions generated by 'wirecheck' * (make -C lustre/utils newwiretest) - * running on Linux vl1 2.6.18-prep #3 SMP Mon Apr 19 06:11:00 CDT 2010 x86_64 x86_64 x86_64 - * with gcc version 4.1.2 20070626 (Red Hat 4.1.2-14) */ + * running on Linux centos5.localhost 2.6.18-prep #3 SMP Mon Mar 22 08:28:01 EDT 2010 x86_64 + * with gcc version 4.1.2 20071124 (Red Hat 4.1.2-42) */ /* Constants... */ @@ -1735,25 +1735,25 @@ void lustre_assert_wire_constants(void) LASSERTF((int)sizeof(((struct ldlm_extent *)0)->gid) == 8, " found %lld\n", (long long)(int)sizeof(((struct ldlm_extent *)0)->gid)); - /* Checks for struct ldlm_flock */ - LASSERTF((int)sizeof(struct ldlm_flock) == 32, " found %lld\n", - (long long)(int)sizeof(struct ldlm_flock)); - LASSERTF((int)offsetof(struct ldlm_flock, start) == 0, " found %lld\n", - (long long)(int)offsetof(struct ldlm_flock, start)); - LASSERTF((int)sizeof(((struct ldlm_flock *)0)->start) == 8, " found %lld\n", - (long long)(int)sizeof(((struct ldlm_flock *)0)->start)); - LASSERTF((int)offsetof(struct ldlm_flock, end) == 8, " found %lld\n", - (long long)(int)offsetof(struct ldlm_flock, end)); - LASSERTF((int)sizeof(((struct ldlm_flock *)0)->end) == 8, " found %lld\n", - (long long)(int)sizeof(((struct ldlm_flock *)0)->end)); - LASSERTF((int)offsetof(struct ldlm_flock, blocking_pid) == 24, " found %lld\n", - (long long)(int)offsetof(struct ldlm_flock, blocking_pid)); - LASSERTF((int)sizeof(((struct ldlm_flock *)0)->blocking_pid) == 4, " found %lld\n", - (long long)(int)sizeof(((struct ldlm_flock *)0)->blocking_pid)); - LASSERTF((int)offsetof(struct ldlm_flock, pid) == 28, " found %lld\n", - (long long)(int)offsetof(struct ldlm_flock, pid)); - LASSERTF((int)sizeof(((struct ldlm_flock *)0)->pid) == 4, " found %lld\n", - (long long)(int)sizeof(((struct ldlm_flock *)0)->pid)); + /* Checks for struct ldlm_flock_wire */ + LASSERTF((int)sizeof(struct ldlm_flock_wire) == 32, " found %lld\n", + (long long)(int)sizeof(struct ldlm_flock_wire)); + LASSERTF((int)offsetof(struct ldlm_flock_wire, lfw_start) == 0, " found %lld\n", + (long long)(int)offsetof(struct ldlm_flock_wire, lfw_start)); + LASSERTF((int)sizeof(((struct ldlm_flock_wire *)0)->lfw_start) == 8, " found %lld\n", + (long long)(int)sizeof(((struct ldlm_flock_wire *)0)->lfw_start)); + LASSERTF((int)offsetof(struct ldlm_flock_wire, lfw_end) == 8, " found %lld\n", + (long long)(int)offsetof(struct ldlm_flock_wire, lfw_end)); + LASSERTF((int)sizeof(((struct ldlm_flock_wire *)0)->lfw_end) == 8, " found %lld\n", + (long long)(int)sizeof(((struct ldlm_flock_wire *)0)->lfw_end)); + LASSERTF((int)offsetof(struct ldlm_flock_wire, lfw_owner) == 16, " found %lld\n", + (long long)(int)offsetof(struct ldlm_flock_wire, lfw_owner)); + LASSERTF((int)sizeof(((struct ldlm_flock_wire *)0)->lfw_owner) == 8, " found %lld\n", + (long long)(int)sizeof(((struct ldlm_flock_wire *)0)->lfw_owner)); + LASSERTF((int)offsetof(struct ldlm_flock_wire, lfw_pid) == 28, " found %lld\n", + (long long)(int)offsetof(struct ldlm_flock_wire, lfw_pid)); + LASSERTF((int)sizeof(((struct ldlm_flock_wire *)0)->lfw_pid) == 4, " found %lld\n", + (long long)(int)sizeof(((struct ldlm_flock_wire *)0)->lfw_pid)); /* Checks for struct ldlm_inodebits */ LASSERTF((int)sizeof(struct ldlm_inodebits) == 8, " found %lld\n", diff --git a/lustre/tests/Makefile.am b/lustre/tests/Makefile.am index c2e791d..7a3fdd0 100644 --- a/lustre/tests/Makefile.am +++ b/lustre/tests/Makefile.am @@ -54,7 +54,7 @@ noinst_PROGRAMS += statone runas openfile rmdirmany noinst_PROGRAMS += small_write multiop ll_sparseness_verify noinst_PROGRAMS += ll_sparseness_write mrename ll_dirstripe_verify mkdirmany noinst_PROGRAMS += openfilleddirunlink rename_many memhog -noinst_PROGRAMS += mmap_sanity flock_test writemany reads flocks_test +noinst_PROGRAMS += mmap_sanity writemany reads flocks_test noinst_PROGRAMS += write_time_limit rwv copytool # noinst_PROGRAMS += copy_attr mkdirdeep bin_PROGRAMS = mcreate munlink diff --git a/lustre/tests/flock_test.c b/lustre/tests/flock_test.c deleted file mode 100644 index 089e23f..0000000 --- a/lustre/tests/flock_test.c +++ /dev/null @@ -1,122 +0,0 @@ -/* -*- mode: c; c-basic-offset: 8; indent-tabs-mode: nil; -*- - * vim:expandtab:shiftwidth=8:tabstop=8: - * - * GPL HEADER START - * - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 only, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License version 2 for more details (a copy is included - * in the LICENSE file that accompanied this code). - * - * You should have received a copy of the GNU General Public License - * version 2 along with this program; If not, see - * http://www.sun.com/software/products/lustre/docs/GPLv2.pdf - * - * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, - * CA 95054 USA or visit www.sun.com if you need additional information or - * have any questions. - * - * GPL HEADER END - */ -/* - * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved. - * Use is subject to license terms. - */ -/* - * This file is part of Lustre, http://www.lustre.org/ - * Lustre is a trademark of Sun Microsystems, Inc. - */ - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -void chd_lock_unlock(int); -char fname[1024]; - -int main(int argc, char **argv) -{ - pid_t pid; - int cfd, fd, rc; - - if (argc != 2) { - fprintf(stderr, "\nUSAGE: flock_test filepath\n"); - exit(2); - } - strncpy(fname, argv[1], 1023); - fname[1023] ='\0'; - fd = open(fname, O_RDWR|O_CREAT, (mode_t)0666); - if (fd == -1) { - fprintf(stderr, "flock_test: failed to open %s : ", fname); - perror(""); - exit(1); - } - if (flock(fd, LOCK_EX | LOCK_NB) == -1) { - fprintf(stderr, "flock_test: parent attempt to lock %s failed : ", \ - fname); - perror(""); - exit(1); - } - - pid = fork(); - if (pid == -1) { - fprintf(stderr, "flock_test: fork failed : "); - perror(""); - exit(1); - } - - if (pid == 0) { - pid = getpid(); - sleep(2); - if ((cfd = open(fname, O_RDWR)) == -1) { - fprintf(stderr, "flock_test child (%d) cannot open %s: ", \ - pid, fname); - perror(""); - exit(1); - } - if(flock(cfd, LOCK_EX | LOCK_NB) != -1) { - fprintf(stderr, "flock_test child (%d): %s not yet locked : ", \ - pid, fname); - exit(1); - } - if(flock(fd, LOCK_UN) == -1) { - fprintf(stderr, "flock_test child (%d): cannot unlock %s: ", \ - pid, fname); - perror(""); - exit(1); - } - if(flock(cfd, LOCK_EX | LOCK_NB) == -1 ) { - fprintf(stderr, \ - "flock_test: child (%d) cannot re-lock %s after unlocking : ", \ - pid, fname); - perror(""); - exit(1); - } - close(cfd); - exit(0); - } - - waitpid(pid, &rc, 0); - close(fd); - unlink(fname); - if (WIFEXITED(rc) && WEXITSTATUS(rc) != 0) { - fprintf(stderr, "flock_test: child (%d) exit code = %d\n", \ - pid, WEXITSTATUS(rc)); - exit(1); - } - exit(0); -} diff --git a/lustre/tests/flocks_test.c b/lustre/tests/flocks_test.c index fe2f045..85fc114 100644 --- a/lustre/tests/flocks_test.c +++ b/lustre/tests/flocks_test.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #define MAX_PATH_LENGTH 4096 @@ -268,6 +269,87 @@ out: return rc; } +/** ================================================================= + * test number 3 + * + * Bug 24040: Two conflicting flocks from same process different fds should fail + * two conflicting flocks from different processes but same fs + * should succeed. + */ +int t3(int argc, char *argv[]) +{ + int fd, fd2; + int pid; + int rc = EXIT_SUCCESS; + + if (argc != 3) { + fprintf(stderr, "Usage: ./flocks_test 3 filename\n"); + return EXIT_FAILURE; + } + + if ((fd = open(argv[2], O_RDWR)) < 0) { + fprintf(stderr, "Couldn't open file: %s\n", argv[1]); + return EXIT_FAILURE; + } + if (flock(fd, LOCK_EX | LOCK_NB) < 0) { + perror("first flock failed"); + rc = EXIT_FAILURE; + goto out; + } + if ((fd2 = open(argv[2], O_RDWR)) < 0) { + fprintf(stderr, "Couldn't open file: %s\n", argv[1]); + rc = EXIT_FAILURE; + goto out; + } + if (flock(fd2, LOCK_EX | LOCK_NB) >= 0) { + fprintf(stderr, "Second flock succeeded - FAIL\n"); + rc = EXIT_FAILURE; + close(fd2); + goto out; + } + + close(fd2); + + pid = fork(); + if (pid == -1) { + perror("fork"); + rc = EXIT_FAILURE; + goto out; + } + + if (pid == 0) { + if ((fd2 = open(argv[2], O_RDWR)) < 0) { + fprintf(stderr, "Couldn't open file: %s\n", argv[1]); + rc = EXIT_FAILURE; + exit(rc); + } + if (flock(fd2, LOCK_EX | LOCK_NB) >= 0) { + fprintf(stderr, "Second flock succeeded - FAIL\n"); + rc = EXIT_FAILURE; + goto out_child; + } + if (flock(fd, LOCK_UN) == -1) { + fprintf(stderr, "Child unlock on parent fd failed\n"); + rc = EXIT_FAILURE; + goto out_child; + } + if (flock(fd2, LOCK_EX | LOCK_NB) == -1) { + fprintf(stderr, "Relock after parent unlock failed!\n"); + rc = EXIT_FAILURE; + goto out_child; + } + out_child: + close(fd2); + exit(rc); + } + + waitpid(pid, &rc, 0); +out: + close(fd); + return rc; +} + + /** ============================================================== * program entry */ @@ -294,6 +376,9 @@ int main(int argc, char* argv[]) case 2: rc = t2(argc, argv); break; + case 3: + rc = t3(argc, argv); + break; default: fprintf(stderr, "unknow test number %s\n", argv[1]); break; diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 5d1f278..8ad845e 100644 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -5093,6 +5093,14 @@ test_105d() { # bug 15924 } run_test 105d "flock race (should not freeze) ========" +test_105e() { # bug 22660 && 22040 + [ -z "`mount | grep \"$DIR.*flock\" | grep -v noflock`" ] && \ + skip "mount w/o flock enabled" && return + touch $DIR/$tfile + flocks_test 3 $DIR/$tfile +} +run_test 105e "Two conflicting flocks from same process =======" + test_106() { #bug 10921 mkdir -p $DIR/$tdir $DIR/$tdir && error "exec $DIR/$tdir succeeded" diff --git a/lustre/utils/wirecheck.c b/lustre/utils/wirecheck.c index ab5b5a0..34a659b 100644 --- a/lustre/utils/wirecheck.c +++ b/lustre/utils/wirecheck.c @@ -745,11 +745,11 @@ static void check_ldlm_flock(void) { BLANK_LINE(); - CHECK_STRUCT(ldlm_flock); - CHECK_MEMBER(ldlm_flock, start); - CHECK_MEMBER(ldlm_flock, end); - CHECK_MEMBER(ldlm_flock, blocking_pid); - CHECK_MEMBER(ldlm_flock, pid); + CHECK_STRUCT(ldlm_flock_wire); + CHECK_MEMBER(ldlm_flock_wire, lfw_start); + CHECK_MEMBER(ldlm_flock_wire, lfw_end); + CHECK_MEMBER(ldlm_flock_wire, lfw_owner); + CHECK_MEMBER(ldlm_flock_wire, lfw_pid); } static void diff --git a/lustre/utils/wiretest.c b/lustre/utils/wiretest.c index cab0cc7..9679c69 100644 --- a/lustre/utils/wiretest.c +++ b/lustre/utils/wiretest.c @@ -62,8 +62,8 @@ void lustre_assert_wire_constants(void) { /* Wire protocol assertions generated by 'wirecheck' * (make -C lustre/utils newwiretest) - * running on Linux vl1 2.6.18-prep #3 SMP Mon Apr 19 06:11:00 CDT 2010 x86_64 x86_64 x86_64 - * with gcc version 4.1.2 20070626 (Red Hat 4.1.2-14) */ + * running on Linux centos5.localhost 2.6.18-prep #3 SMP Mon Mar 22 08:28:01 EDT 2010 x86_64 + * with gcc version 4.1.2 20071124 (Red Hat 4.1.2-42) */ /* Constants... */ @@ -1732,25 +1732,25 @@ void lustre_assert_wire_constants(void) LASSERTF((int)sizeof(((struct ldlm_extent *)0)->gid) == 8, " found %lld\n", (long long)(int)sizeof(((struct ldlm_extent *)0)->gid)); - /* Checks for struct ldlm_flock */ - LASSERTF((int)sizeof(struct ldlm_flock) == 32, " found %lld\n", - (long long)(int)sizeof(struct ldlm_flock)); - LASSERTF((int)offsetof(struct ldlm_flock, start) == 0, " found %lld\n", - (long long)(int)offsetof(struct ldlm_flock, start)); - LASSERTF((int)sizeof(((struct ldlm_flock *)0)->start) == 8, " found %lld\n", - (long long)(int)sizeof(((struct ldlm_flock *)0)->start)); - LASSERTF((int)offsetof(struct ldlm_flock, end) == 8, " found %lld\n", - (long long)(int)offsetof(struct ldlm_flock, end)); - LASSERTF((int)sizeof(((struct ldlm_flock *)0)->end) == 8, " found %lld\n", - (long long)(int)sizeof(((struct ldlm_flock *)0)->end)); - LASSERTF((int)offsetof(struct ldlm_flock, blocking_pid) == 24, " found %lld\n", - (long long)(int)offsetof(struct ldlm_flock, blocking_pid)); - LASSERTF((int)sizeof(((struct ldlm_flock *)0)->blocking_pid) == 4, " found %lld\n", - (long long)(int)sizeof(((struct ldlm_flock *)0)->blocking_pid)); - LASSERTF((int)offsetof(struct ldlm_flock, pid) == 28, " found %lld\n", - (long long)(int)offsetof(struct ldlm_flock, pid)); - LASSERTF((int)sizeof(((struct ldlm_flock *)0)->pid) == 4, " found %lld\n", - (long long)(int)sizeof(((struct ldlm_flock *)0)->pid)); + /* Checks for struct ldlm_flock_wire */ + LASSERTF((int)sizeof(struct ldlm_flock_wire) == 32, " found %lld\n", + (long long)(int)sizeof(struct ldlm_flock_wire)); + LASSERTF((int)offsetof(struct ldlm_flock_wire, lfw_start) == 0, " found %lld\n", + (long long)(int)offsetof(struct ldlm_flock_wire, lfw_start)); + LASSERTF((int)sizeof(((struct ldlm_flock_wire *)0)->lfw_start) == 8, " found %lld\n", + (long long)(int)sizeof(((struct ldlm_flock_wire *)0)->lfw_start)); + LASSERTF((int)offsetof(struct ldlm_flock_wire, lfw_end) == 8, " found %lld\n", + (long long)(int)offsetof(struct ldlm_flock_wire, lfw_end)); + LASSERTF((int)sizeof(((struct ldlm_flock_wire *)0)->lfw_end) == 8, " found %lld\n", + (long long)(int)sizeof(((struct ldlm_flock_wire *)0)->lfw_end)); + LASSERTF((int)offsetof(struct ldlm_flock_wire, lfw_owner) == 16, " found %lld\n", + (long long)(int)offsetof(struct ldlm_flock_wire, lfw_owner)); + LASSERTF((int)sizeof(((struct ldlm_flock_wire *)0)->lfw_owner) == 8, " found %lld\n", + (long long)(int)sizeof(((struct ldlm_flock_wire *)0)->lfw_owner)); + LASSERTF((int)offsetof(struct ldlm_flock_wire, lfw_pid) == 28, " found %lld\n", + (long long)(int)offsetof(struct ldlm_flock_wire, lfw_pid)); + LASSERTF((int)sizeof(((struct ldlm_flock_wire *)0)->lfw_pid) == 4, " found %lld\n", + (long long)(int)sizeof(((struct ldlm_flock_wire *)0)->lfw_pid)); /* Checks for struct ldlm_inodebits */ LASSERTF((int)sizeof(struct ldlm_inodebits) == 8, " found %lld\n",