dlm: core locking
PATCH 1/1 dlm: core locking
[Apologies, patch 1 was too large on its own.]
The core dlm functions. Processes dlm_lock() and dlm_unlock() requests. Creates lockspaces which give applications separate contexts/namespaces in which to do their locking. Manages locks on resources' grant/convert/wait queues. Sends and receives high level locking operations between nodes. Delivers completion and blocking callbacks (ast's) to lock holders. Manages the distributed directory that tracks the current master node for each resource.
---
drivers/dlm/ast.c | 166 +++++++++++++ drivers/dlm/ast.h | 26 ++ drivers/dlm/dir.c | 439 ++++++++++++++++++++++++++++++++++++ drivers/dlm/dir.h | 30 ++ drivers/dlm/dlm_internal.h | 472 +++++++++++++++++++++++++++++++++++++++ drivers/dlm/lock.h | 38 +++ drivers/dlm/lockspace.c | 535 +++++++++++++++++++++++++++++++++++++++++++++ drivers/dlm/lockspace.h | 26 ++ drivers/dlm/lvb_table.h | 37 +++ drivers/dlm/main.c | 93 +++++++ drivers/dlm/memory.c | 135 +++++++++++ drivers/dlm/memory.h | 30 ++ drivers/dlm/util.c | 190 +++++++++++++++ drivers/dlm/util.h | 23 + include/linux/dlm.h | 307 +++++++++++++++++++++++++ 15 files changed, 2547 insertions(+)
include/linux/dlm.h +307 -0
@@ -0,0 +1,307 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) Sistina Software, Inc. 1997-2003 All rights reserved. +** Copyright (C) 2004-2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __DLM_DOT_H__ +#define __DLM_DOT_H__ + +/* +* Interface to DLM - routines and structures to use DLM lockspaces. +*/ + +/* +* Lock Modes +*/ + +#define DLM_LOCK_IV (-1)/* invalid */ +#define DLM_LOCK_NL (0)/* null */ +#define DLM_LOCK_CR (1)/* concurrent read */ +#define DLM_LOCK_CW (2)/* concurrent write */ +#define DLM_LOCK_PR (3)/* protected read */ +#define DLM_LOCK_PW (4)/* protected write */ +#define DLM_LOCK_EX (5)/* exclusive */ + +/* +* Maximum size in bytes of a dlm_lock name +*/ + +#define DLM_RESNAME_MAXLEN (64) + +/* +* Size in bytes of Lock Value Block +*/ + +#define DLM_LVB_LEN (32) + +/* +* Flags to dlm_lock +* +* DLM_LKF_NOQUEUE +* +* Do not queue the lock request on the wait queue if it cannot be granted +* immediately. If the lock cannot be granted because of this flag, DLM will +* either return -EAGAIN from the dlm_lock call or will return 0 from +* dlm_lock and -EAGAIN in the lock status block when the AST is executed. +* +* DLM_LKF_CANCEL +* +* Used to cancel a pending lock request or conversion. A converting lock is +* returned to its previously granted mode. +* +* DLM_LKF_CONVERT +* +* Indicates a lock conversion request. For conversions the name and namelen +* are ignored and the lock ID in the LKSB is used to identify the lock. +* +* DLM_LKF_VALBLK +* +* Requests DLM to return the current contents of the lock value block in the +* lock status block. When this flag is set in a lock conversion from PW or EX +* modes, DLM assigns the value specified in the lock status block to the lock +* value block of the lock resource. The LVB is a DLM_LVB_LEN size array +* containing application-specific information. +* +* DLM_LKF_QUECVT +* +* Force a conversion request to be queued, even if it is compatible with +* the granted modes of other locks on the same resource. +* +* DLM_LKF_IVVALBLK +* +* Invalidate the lock value block. +* +* DLM_LKF_CONVDEADLK +* +* Allows the dlm to resolve conversion deadlocks internally by demoting the +* granted mode of a converting lock to NL. The DLM_SBF_DEMOTED flag is +* returned for a conversion that's been effected by this. +* +* DLM_LKF_PERSISTENT +* +* Only relevant to locks originating in userspace. A persistent lock will not +* be removed if the process holding the lock exits. +* +* DLM_LKF_NODLKWT +* DLM_LKF_NODLCKBLK +* +* net yet implemented +* +* DLM_LKF_EXPEDITE +* +* Used only with new requests for NL mode locks. Tells the lock manager +* to grant the lock, ignoring other locks in convert and wait queues. +* +* DLM_LKF_NOQUEUEBAST +* +* Send blocking AST's before returning -EAGAIN to the caller. It is only +* used along with the NOQUEUE flag. Blocking AST's are not sent for failed +* NOQUEUE requests otherwise. +* +* DLM_LKF_HEADQUE +* +* Add a lock to the head of the convert or wait queue rather than the tail. +* +* DLM_LKF_NOORDER +* +* Disregard the standard grant order rules and grant a lock as soon as it +* is compatible with other granted locks. +* +* DLM_LKF_ORPHAN +* +* not yet implemented +* +* DLM_LKF_ALTPR +* +* If the requested mode cannot be granted immediately, try to grant the lock +* in PR mode instead. If this alternate mode is granted instead of the +* requested mode, DLM_SBF_ALTMODE is returned in the lksb. +* +* DLM_LKF_ALTCW +* +* The same as ALTPR, but the alternate mode is CW. +*/ + +#define DLM_LKF_NOQUEUE (0x00000001) +#define DLM_LKF_CANCEL (0x00000002) +#define DLM_LKF_CONVERT (0x00000004) +#define DLM_LKF_VALBLK (0x00000008) +#define DLM_LKF_QUECVT (0x00000010) +#define DLM_LKF_IVVALBLK (0x00000020) +#define DLM_LKF_CONVDEADLK (0x00000040) +#define DLM_LKF_PERSISTENT (0x00000080) +#define DLM_LKF_NODLCKWT (0x00000100) +#define DLM_LKF_NODLCKBLK (0x00000200) +#define DLM_LKF_EXPEDITE (0x00000400) +#define DLM_LKF_NOQUEUEBAST (0x00000800) +#define DLM_LKF_HEADQUE (0x00001000) +#define DLM_LKF_NOORDER (0x00002000) +#define DLM_LKF_ORPHAN (0x00004000) +#define DLM_LKF_ALTPR (0x00008000) +#define DLM_LKF_ALTCW (0x00010000) + +/* +* Some return codes that are not in errno.h +*/ + +#define DLM_ECANCEL (0x10001) +#define DLM_EUNLOCK (0x10002) + +typedef void dlm_lockspace_t ; + +/* +* Lock range structure +*/ + +struct dlm_range { +uint64_t ra_start; +uint64_t ra_end; +}; + +/* +* Lock status block +* +* Use this structure to specify the contents of the lock value block. For a +* conversion request, this structure is used to specify the lock ID of the +* lock. DLM writes the status of the lock request and the lock ID assigned +* to the request in the lock status block. +* +* sb_lkid: the returned lock ID. It is set on new (non-conversion) requests. +* It is available when dlm_lock returns. +* +* sb_lvbptr: saves or returns the contents of the lock's LVB according to rules +* shown for the DLM_LKF_VALBLK flag. +* +* sb_flags: DLM_SBF_DEMOTED is returned if in the process of promoting a lock, +* it was first demoted to NL to avoid conversion deadlock. +* DLM_SBF_VALNOTVALID is returned if the resource's LVB is marked invalid. +* +* sb_status: the returned status of the lock request set prior to AST +* execution. Possible return values: +* +* 0 if lock request was successful +* -EAGAIN if request would block and is flagged DLM_LKF_NOQUEUE +* -ENOMEM if there is no memory to process request +* -EINVAL if there are invalid parameters +* -DLM_EUNLOCK if unlock request was successful +* -DLM_ECANCEL if a cancel completed successfully +*/ + +#define DLM_SBF_DEMOTED (0x01) +#define DLM_SBF_VALNOTVALID (0x02) +#define DLM_SBF_ALTMODE (0x04) + +struct dlm_lksb { +int sb_status; +uint32_t sb_lkid; +char sb_flags; +char * sb_lvbptr; +}; + + +#ifdef __KERNEL__ + +/* +* dlm_new_lockspace +* +* Starts a lockspace with the given name. If the named lockspace exists in +* the cluster, the calling node joins it. +*/ + +int dlm_new_lockspace (char *name,int namelen,dlm_lockspace_t **lockspace, +int flags); + +/* +* dlm_release_lockspace +* +* Stop a lockspace. +*/ + +int dlm_release_lockspace (dlm_lockspace_t *lockspace,int force); + +/* +* dlm_lock +* +* Make an asyncronous request to acquire or convert a lock on a named +* resource. +* +* lockspace: context for the request +* mode: the requested mode of the lock (DLM_LOCK_) +* lksb: lock status block for input and async return values +* flags: input flags (DLM_LKF_) +* name: name of the resource to lock, can be binary +* namelen: the length in bytes of the resource name (MAX_RESNAME_LEN) +* parent: the lock ID of a parent lock or 0 if none +* lockast: function DLM executes when it completes processing the request +* astarg: argument passed to lockast and bast functions +* bast: function DLM executes when this lock later blocks another request +* +* Returns: +* 0 if request is successfully queued for processing +* -EINVAL if any input parameters are invalid +* -EAGAIN if request would block and is flagged DLM_LKF_NOQUEUE +* -ENOMEM if there is no memory to process request +* -ENOTCONN if there is a communication error +* +* If the call to dlm_lock returns an error then the operation has failed and +* the AST routine will not be called. If dlm_lock returns 0 it is still +* possible that the lock operation will fail. The AST routine will be called +* when the locking is complete and the status is returned in the lksb. +* +* If the AST routines or parameter are passed to a conversion operation then +* they will overwrite those values that were passed to a previous dlm_lock +* call. +* +* AST routines should not block (at least not for long), but may make +* any locking calls they please. +*/ + +int dlm_lock (dlm_lockspace_t *lockspace, +int mode, +struct dlm_lksb *lksb, +uint32_t flags, +void *name, +unsigned int namelen, +uint32_t parent_lkid, +void (*lockast) (void *astarg), +void *astarg, +void (*bast) (void *astarg,int mode), +struct dlm_range *range); + +/* +* dlm_unlock +* +* Asynchronously release a lock on a resource. The AST routine is called +* when the resource is successfully unlocked. +* +* lockspace: context for the request +* lkid: the lock ID as returned in the lksb +* flags: input flags (DLM_LKF_) +* lksb: if NULL the lksb parameter passed to last lock request is used +* astarg: the arg used with the completion ast for the unlock +* +* Returns: +* 0 if request is successfully queued for processing +* -EINVAL if any input parameters are invalid +* -ENOTEMPTY if the lock still has sublocks +* -EBUSY if the lock is waiting for a remote lock operation +* -ENOTCONN if there is a communication error +*/ + +int dlm_unlock (dlm_lockspace_t *lockspace, +uint32_t lkid, +uint32_t flags, +struct dlm_lksb *lksb, +void *astarg); + +#endif /* __KERNEL__ */ + +#endif /* __DLM_DOT_H__ */
drivers/dlm/dlm_internal.h +472 -0
472 lines not on this page · raw message
drivers/dlm/lvb_table.h +37 -0
@@ -0,0 +1,37 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) 2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __LVB_TABLE_DOT_H__ +#define __LVB_TABLE_DOT_H__ + +/* +* This defines the direction of transfer of LVB data. +* Granted mode is the row; requested mode is the column. +* Usage: matrix[grmode+1][rqmode+1] +* 1 = LVB is returned to the caller +* 0 = LVB is written to the resource +* -1 = nothing happens to the LVB +*/ + +const int dlm_lvb_operations[8 ][8 ] = { +/* UN NL CR CW PR PW EX PD*/ + {-1 ,1 ,1 ,1 ,1 ,1 ,1 ,-1 },/* UN */ + {-1 ,1 ,1 ,1 ,1 ,1 ,1 ,0 },/* NL */ + {-1 ,-1 ,1 ,1 ,1 ,1 ,1 ,0 },/* CR */ + {-1 ,-1 ,-1 ,1 ,1 ,1 ,1 ,0 },/* CW */ + {-1 ,-1 ,-1 ,-1 ,1 ,1 ,1 ,0 },/* PR */ + {-1 ,0 ,0 ,0 ,0 ,0 ,1 ,0 },/* PW */ + {-1 ,0 ,0 ,0 ,0 ,0 ,0 ,0 },/* EX */ + {-1 ,0 ,0 ,0 ,0 ,0 ,0 ,0 }/* PD */ +}; + +#endif
drivers/dlm/ast.c +166 -0
166 lines not on this page · raw message
drivers/dlm/ast.h +26 -0
@@ -0,0 +1,26 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) 2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __ASTD_DOT_H__ +#define __ASTD_DOT_H__ + +void dlm_add_ast (struct dlm_lkb *lkb,int type); +void dlm_del_ast (struct dlm_lkb *lkb); + +void dlm_astd_wake (void ); +int dlm_astd_start (void ); +void dlm_astd_stop (void ); +void dlm_astd_suspend (void ); +void dlm_astd_resume (void ); + +#endif +
drivers/dlm/dir.c +439 -0
439 lines not on this page · raw message
drivers/dlm/dir.h +30 -0
@@ -0,0 +1,30 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) Sistina Software, Inc. 1997-2003 All rights reserved. +** Copyright (C) 2004-2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __DIR_DOT_H__ +#define __DIR_DOT_H__ + + +int dlm_dir_nodeid (struct dlm_rsb *rsb); +int dlm_dir_name2nodeid (struct dlm_ls *ls,char *name,int length); +void dlm_dir_remove_entry (struct dlm_ls *ls,int nodeid,char *name,int len); +void dlm_dir_clear (struct dlm_ls *ls); +void dlm_clear_free_entries (struct dlm_ls *ls); +int dlm_recover_directory (struct dlm_ls *ls); +int dlm_dir_lookup (struct dlm_ls *ls,int nodeid,char *name,int namelen, +int *r_nodeid); +int dlm_copy_master_names (struct dlm_ls *ls,char *inbuf,int inlen, +char *outbuf,int outlen,int nodeid); +int dlm_dir_rebuild_wait (struct dlm_ls *ls); + +#endif /* __DIR_DOT_H__ */
drivers/dlm/lock.h +38 -0
@@ -0,0 +1,38 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) 2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __LOCK_DOT_H__ +#define __LOCK_DOT_H__ + +void dlm_print_lkb (struct dlm_lkb *lkb); +void dlm_print_rsb (struct dlm_rsb *r); +int dlm_receive_message (struct dlm_header *hd,int nodeid,int recovery); +int dlm_modes_compat (int mode1,int mode2); +int dlm_find_rsb (struct dlm_ls *ls,char *name,int namelen, +unsigned int flags,struct dlm_rsb **r_ret); +int dlm_is_master (struct dlm_rsb *r); +void dlm_put_rsb (struct dlm_rsb *r); +void dlm_hold_rsb (struct dlm_rsb *r); +void dlm_lock_rsb (struct dlm_rsb *r); +void dlm_unlock_rsb (struct dlm_rsb *r); +int dlm_put_lkb (struct dlm_lkb *lkb); +int dlm_remove_from_waiters (struct dlm_lkb *lkb); +void dlm_scan_rsbs (struct dlm_ls *ls); + +int dlm_purge_locks (struct dlm_ls *ls); +int dlm_grant_after_purge (struct dlm_ls *ls); +int dlm_recover_waiters_post (struct dlm_ls *ls); +void dlm_recover_waiters_pre (struct dlm_ls *ls); +int dlm_recover_master_copy (struct dlm_ls *ls,struct dlm_rcom *rc); +int dlm_recover_process_copy (struct dlm_ls *ls,struct dlm_rcom *rc); + +#endif
drivers/dlm/lockspace.c +535 -0
535 lines not on this page · raw message
drivers/dlm/lockspace.h +26 -0
@@ -0,0 +1,26 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) Sistina Software, Inc. 1997-2003 All rights reserved. +** Copyright (C) 2004-2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __LOCKSPACE_DOT_H__ +#define __LOCKSPACE_DOT_H__ + +int dlm_lockspace_init (void ); +void dlm_lockspace_exit (void ); +int dlm_new_lockspace (char *name,int namelen,void **ls,int flags); +int dlm_release_lockspace (void *ls,int force); +struct dlm_ls *dlm_find_lockspace_global (uint32_t id); +struct dlm_ls *dlm_find_lockspace_local (void *id); +struct dlm_ls *dlm_find_lockspace_name (char *name,int namelen); +void dlm_put_lockspace (struct dlm_ls *ls); + +#endif /* __LOCKSPACE_DOT_H__ */
drivers/dlm/main.c +93 -0
93 lines not on this page · raw message
drivers/dlm/memory.c +135 -0
135 lines not on this page · raw message
drivers/dlm/memory.h +30 -0
@@ -0,0 +1,30 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) Sistina Software, Inc. 1997-2003 All rights reserved. +** Copyright (C) 2004-2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __MEMORY_DOT_H__ +#define __MEMORY_DOT_H__ + +int dlm_memory_init (void ); +void dlm_memory_exit (void ); +struct dlm_rsb *allocate_rsb (struct dlm_ls *ls,int namelen); +void free_rsb (struct dlm_rsb *r); +struct dlm_lkb *allocate_lkb (struct dlm_ls *ls); +void free_lkb (struct dlm_lkb *l); +struct dlm_direntry *allocate_direntry (struct dlm_ls *ls,int namelen); +void free_direntry (struct dlm_direntry *de); +char *allocate_lvb (struct dlm_ls *ls); +void free_lvb (char *l); +uint64_t *allocate_range (struct dlm_ls *ls); +void free_range (uint64_t *l); + +#endif /* __MEMORY_DOT_H__ */
drivers/dlm/util.c +190 -0
190 lines not on this page · raw message
drivers/dlm/util.h +23 -0
@@ -0,0 +1,23 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) 2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __UTIL_DOT_H__ +#define __UTIL_DOT_H__ + +uint32_t dlm_hash (const char *data,int len); + +void dlm_message_out (struct dlm_message *ms); +void dlm_message_in (struct dlm_message *ms); +void dlm_rcom_out (struct dlm_rcom *rc); +void dlm_rcom_in (struct dlm_rcom *rc); + +#endif
Quoted 12 lines · David Teigland
[Apologies, patch 1 was too large on its own.]
The core dlm functions. Processes
dlm_lock()anddlm_unlock()requests. Creates lockspaces which give applications separate contexts/namespaces in which to do their locking. Manages locks on resources' grant/convert/wait queues. Sends and receives high level locking operations between nodes. Delivers completion and blocking callbacks (ast's) to lock holders. Manages the distributed directory that tracks the current master node for each resource.Signed-off-by Dave Teigland <[email protected]>Signed-off-by Patrick Caulfield <[email protected]>
<snip>
Quoted 2 lines
drivers/dlm/lockspace.c 2005-04-25 22:52:03.956816760 +0800 renamed from drivers/dlm/lockspace.c 1970-01-01 07:30:00.000000000 +0730 +0
-0
<snip>
Quoted 12 lines
drivers/dlm/lockspace.c 2005-04-25 22:52:03.956816760 +0800 renamed from drivers/dlm/lockspace.c 1970-01-01 07:30:00.000000000 +0730 +12
-0+int dlm_scand(void *data) +{ + struct dlm_ls *ls; + + while (!kthread_should_stop()) { + list_for_each_entry(ls, &lslist, ls_list) + dlm_scan_rsbs(ls); + set_current_state(TASK_INTERRUPTIBLE); + schedule_timeout(DLM_SCAN_SECS * HZ); + } + return 0; +}
<snip>
Quoted 4 lines
drivers/dlm/lockspace.c 2005-04-25 22:52:03.956816760 +0800 renamed from drivers/dlm/lockspace.c 1970-01-01 07:30:00.000000000 +0730 +4
-0+static void remove_lockspace(struct dlm_ls *ls) +{ + for (;;) { + spin_lock(&lslist_lock);
drivers/dlm/lockspace.c 2005-04-25 22:52:03.956816760 +0800
Quoted 10 lines
drivers/dlm/lockspace.c 2005-04-25 22:52:03.956816760 +0800 renamed from drivers/dlm/lockspace.c 1970-01-01 07:30:00.000000000 +0730 +10
-0+ if (ls->ls_count == 0) { + list_del(&ls->ls_list); + spin_unlock(&lslist_lock); + return; + } + spin_unlock(&lslist_lock); + set_current_state(TASK_INTERRUPTIBLE); + schedule_timeout(HZ); + } +}
<snip>
These can both be msleep_interruptible() calls?
Thanks,
Nish
Quoted 13 lines · David Teigland
[Apologies, patch 1 was too large on its own.]
The core dlm functions. Processes
dlm_lock()anddlm_unlock()requests. Creates lockspaces which give applications separate contexts/namespaces in which to do their locking. Manages locks on resources' grant/convert/wait queues. Sends and receives high level locking operations between nodes. Delivers completion and blocking callbacks (ast's) to lock holders. Manages the distributed directory that tracks the current master node for each resource.Signed-off-by Dave Teigland <[email protected]>Signed-off-by Patrick Caulfield <[email protected]>
include/linux/dlm.h line 31
Quoted 7 lines
include/linux/dlm.h +7
-0+#define DLM_LOCK_IV (-1)/* invalid */ +#define DLM_LOCK_NL (0)/* null */ +#define DLM_LOCK_CR (1)/* concurrent read */ +#define DLM_LOCK_CW (2)/* concurrent write */ +#define DLM_LOCK_PR (3)/* protected read */ +#define DLM_LOCK_PW (4)/* protected write */ +#define DLM_LOCK_EX (5)/* exclusive */
^^^^^
|----- Why the parenthesis?[...]
Quoted 1 line
+#define
DLM_RESNAME_MAXLEN(64)
^^^^^--- more parens.
[...]
Quoted 1 line
+#define
DLM_LVB_LEN(32)
^^^^^--- yet more.
[...]
Quoted 7 lines
include/linux/dlm.h +7
-0+#define DLM_LKF_NOQUEUE (0x00000001) +#define DLM_LKF_CANCEL (0x00000002) +#define DLM_LKF_CONVERT (0x00000004) +#define DLM_LKF_VALBLK (0x00000008) +#define DLM_LKF_QUECVT (0x00000010) +#define DLM_LKF_IVVALBLK (0x00000020) +#define DLM_LKF_CONVDEADLK (0x00000040)
include/linux/dlm.h line 149
Quoted 10 lines
include/linux/dlm.h +10
-0+#define DLM_LKF_PERSISTENT (0x00000080) +#define DLM_LKF_NODLCKWT (0x00000100) +#define DLM_LKF_NODLCKBLK (0x00000200) +#define DLM_LKF_EXPEDITE (0x00000400) +#define DLM_LKF_NOQUEUEBAST (0x00000800) +#define DLM_LKF_HEADQUE (0x00001000) +#define DLM_LKF_NOORDER (0x00002000) +#define DLM_LKF_ORPHAN (0x00004000) +#define DLM_LKF_ALTPR (0x00008000) +#define DLM_LKF_ALTCW (0x00010000)
^^^^^^^^^^^^
what's your facination with parenthesis?
[...]
include/linux/dlm.h line 156
Quoted 2 lines
include/linux/dlm.h +2
-0+#define DLM_ECANCEL (0x10001) +#define DLM_EUNLOCK (0x10002)
^--- here we go again.
[...]
include/linux/dlm.h line 200
Quoted 3 lines
include/linux/dlm.h +3
-0+#define DLM_SBF_DEMOTED (0x01) +#define DLM_SBF_VALNOTVALID (0x02) +#define DLM_SBF_ALTMODE (0x04)
^--- and again.
[...]
include/linux/dlm.h line 206
Quoted 6 lines
include/linux/dlm.h +6
-0+ +struct dlm_lksb { +int sb_status; +uint32_t sb_lkid; +char sb_flags; +char * sb_lvbptr;
why not char *sb_lvbptr; ???
[...]
drivers/dlm/dlm_internal.h line 51
Quoted 11 lines
drivers/dlm/dlm_internal.h +11
-0+#define DLM_LOCKSPACE_LEN (64) +#define DLM_TOSS_SECS (10) +#define DLM_SCAN_SECS (5) + +#ifndef TRUE +#define TRUE (1) +#endif + +#ifndef FALSE +#define FALSE (0) +#endif
a few cases of pointless parenthesis around define values...
[...]
Quoted 1 line
+#define
MAX(a, b)(((a) > (b)) ? (a) : (b))
What's wrong with the standard min/max macros from linux/kernel.h ?
[...]
drivers/dlm/dlm_internal.h line 96
Quoted 5 lines
drivers/dlm/dlm_internal.h +5
-0+#define DLM_ASSERT (x, do) \ +{ \ + if (!(x)) \ + { \ + printk("\nDLM: Assertion failed on line %d of file %s\n" \
^^^--- a loglevel perhaps?
[...]
drivers/dlm/dlm_internal.h line 159
Quoted 3 lines
drivers/dlm/dlm_internal.h +3
-0+struct dlm_args { +uint32_t flags; +void * astaddr;
void *astaddr;
drivers/dlm/dlm_internal.h line 161
Quoted 2 lines
drivers/dlm/dlm_internal.h +2
-0+long astparam; +void * bastaddr;
void *bastaddr;
drivers/dlm/dlm_internal.h line 163
Quoted 2 lines
drivers/dlm/dlm_internal.h +2
-0+int mode; +struct dlm_lksb * lksb;
struct dlm_lksb *lksb;
drivers/dlm/dlm_internal.h line 164
Quoted 1 line
drivers/dlm/dlm_internal.h +1
-0+struct dlm_range * range;
struct dlm_range *range;
Quoted 1 line
+};
In other locations as well.
[...]
Quoted 11 lines
drivers/dlm/dlm_internal.h +11
-0+#define AST_COMP (1) +#define AST_BAST (2) + +/* lkb_range[] */ + +#define GR_RANGE_START (0) +#define GR_RANGE_END (1) +#define RQ_RANGE_START (2) +#define RQ_RANGE_END (3) + +/* lkb_status */
drivers/dlm/dlm_internal.h line 230
Quoted 10 lines
drivers/dlm/dlm_internal.h +10
-0+ +#define DLM_LKSTS_WAITING (1) +#define DLM_LKSTS_GRANTED (2) +#define DLM_LKSTS_CONVERT (3) + +/* lkb_flags */ + +#define DLM_IFL_MSTCPY (0x00010000) +#define DLM_IFL_RESEND (0x00020000) +#define DLM_IFL_RANGE (0x00000001)
Here, again, we have a lot of pointless parenthesis around the values. I'm not going to bother pointing out the remaining ones.
Quoted 5 lines
drivers/dlm/ast.c +5
-0+static int dlm_astd (void *data) +{ +while (!kthread_should_stop ()) { +set_current_state (TASK_INTERRUPTIBLE ); +if (!test_bit (WAKE_ASTS , &astd_wakeflags))
drivers/dlm/ast.c line 125
Quoted 10 lines
drivers/dlm/ast.c +10
-0+schedule (); +set_current_state (TASK_RUNNING ); + +down (&astd_running); +if (test_and_clear_bit (WAKE_ASTS , &astd_wakeflags)) +process_asts (); +up (&astd_running); + } +return 0 ; +}
Always returning 0 - why not a void function then?
[...]
drivers/dlm/dir.c line 202
Quoted 7 lines
drivers/dlm/dir.c +7
-0+ +int dlm_recover_directory (struct dlm_ls *ls) +{ +struct dlm_member *memb; +struct dlm_direntry *de; +char *b, *last_name; +int error = -ENOMEM , last_len, count =0 ;
Wouldn't
int error = -ENOMEM; int last_len; int count = 0;
be a bit more readable?
[...]
drivers/dlm/lockspace.c line 44
Quoted 3 lines
drivers/dlm/lockspace.c +3
-0+void dlm_lockspace_exit (void ) +{ +}
huh?
[...]
drivers/dlm/lockspace.c line 57
Quoted 12 lines
drivers/dlm/lockspace.c +12
-0+int dlm_scand (void *data) +{ +struct dlm_ls *ls; + +while (!kthread_should_stop ()) { +list_for_each_entry (ls, &lslist, ls_list) +dlm_scan_rsbs (ls); +set_current_state (TASK_INTERRUPTIBLE ); +schedule_timeout (DLM_SCAN_SECS *HZ ); + } +return 0 ; +}
void func?
[...]
Quoted 19 lines
drivers/dlm/main.c +19
-0+int __init init_dlm (void ) +{ +int error; + + error =dlm_memory_init (); +if (error) + goto out; + + error =dlm_lockspace_init (); +if (error) + goto out_mem; + + error =dlm_node_ioctl_init (); +if (error) + goto out_ls; + + error =dlm_member_sysfs_init (); +if (error) + goto out_node;
drivers/dlm/main.c line 55
Quoted 10 lines
drivers/dlm/main.c +10
-0+ + error =dlm_register_debugfs (); +if (error) + goto out_member; + + error =dlm_lowcomms_init (); +if (error) + goto out_debug; + +printk ("DLM (built %s %s) installed\n" , __DATE__, __TIME__);
^^^--- how about adding an explicit loglevel?
[...]
drivers/dlm/util.c line 29
Quoted 2 lines
drivers/dlm/util.c +2
-0+static inline uint32_t +hash_more_internal (const void *data,unsigned int len,uint32_t hash)
A lot of people prefer to have the return type on the same line as the function name - very nice when grep'ing for a function to be able to see the return type in the grep output.
[...]
Quoted 8 lines
drivers/dlm/util.c +8
-0+void dlm_message_out (struct dlm_message *ms) +{ +struct dlm_header *hd = (struct dlm_header *) ms; + +header_out (hd); + + ms->m_type =cpu_to_le32 (ms->m_type); + ms->m_nodeid =cpu_to_le32 (ms->m_nodeid);
drivers/dlm/util.c line 83
Quoted 10 lines
drivers/dlm/util.c +10
-0+ ms->m_pid =cpu_to_le32 (ms->m_pid); + ms->m_lkid =cpu_to_le32 (ms->m_lkid); + ms->m_remid =cpu_to_le32 (ms->m_remid); + ms->m_parent_lkid =cpu_to_le32 (ms->m_parent_lkid); + ms->m_parent_remid =cpu_to_le32 (ms->m_parent_remid); + ms->m_exflags =cpu_to_le32 (ms->m_exflags); + ms->m_sbflags =cpu_to_le32 (ms->m_sbflags); + ms->m_flags =cpu_to_le32 (ms->m_flags); + ms->m_lvbseq =cpu_to_le32 (ms->m_lvbseq); +
why this blank line.?
drivers/dlm/util.c line 90
Quoted 7 lines
drivers/dlm/util.c +7
-0+ ms->m_status =cpu_to_le32 (ms->m_status); + ms->m_grmode =cpu_to_le32 (ms->m_grmode); + ms->m_rqmode =cpu_to_le32 (ms->m_rqmode); + ms->m_bastmode =cpu_to_le32 (ms->m_bastmode); + ms->m_asts =cpu_to_le32 (ms->m_asts); + ms->m_result =cpu_to_le32 (ms->m_result); +
one more blank line..
Quoted 12 lines
drivers/dlm/util.c +12
-0+ ms->m_range[0 ] =cpu_to_le64 (ms->m_range[0 ]); + ms->m_range[1 ] =cpu_to_le64 (ms->m_range[1 ]); +} + +void dlm_message_in (struct dlm_message *ms) +{ +struct dlm_header *hd = (struct dlm_header *) ms; + +header_in (hd); + + ms->m_type =le32_to_cpu (ms->m_type); + ms->m_nodeid =le32_to_cpu (ms->m_nodeid);
drivers/dlm/util.c line 112
Quoted 10 lines
drivers/dlm/util.c +10
-0+ ms->m_pid =le32_to_cpu (ms->m_pid); + ms->m_lkid =le32_to_cpu (ms->m_lkid); + ms->m_remid =le32_to_cpu (ms->m_remid); + ms->m_parent_lkid =le32_to_cpu (ms->m_parent_lkid); + ms->m_parent_remid =le32_to_cpu (ms->m_parent_remid); + ms->m_exflags =le32_to_cpu (ms->m_exflags); + ms->m_sbflags =le32_to_cpu (ms->m_sbflags); + ms->m_flags =le32_to_cpu (ms->m_flags); + ms->m_lvbseq =le32_to_cpu (ms->m_lvbseq); +
again a blank line...
drivers/dlm/util.c line 119
Quoted 7 lines
drivers/dlm/util.c +7
-0+ ms->m_status =le32_to_cpu (ms->m_status); + ms->m_grmode =le32_to_cpu (ms->m_grmode); + ms->m_rqmode =le32_to_cpu (ms->m_rqmode); + ms->m_bastmode =le32_to_cpu (ms->m_bastmode); + ms->m_asts =le32_to_cpu (ms->m_asts); + ms->m_result =le32_to_cpu (ms->m_result); +
and one more blank line....
Quoted 3 lines
drivers/dlm/util.c +3
-0+ ms->m_range[0 ] =le64_to_cpu (ms->m_range[0 ]); + ms->m_range[1 ] =le64_to_cpu (ms->m_range[1 ]); +}
[...]
Quoted 16 lines
drivers/dlm/util.h +13
-0@@ -0,0 +1,23 @@ +/****************************************************************************** +******************************************************************************* +** +** Copyright (C) 2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/ + +#ifndef __UTIL_DOT_H__
drivers/dlm/util.h line 23
Quoted 10 lines
drivers/dlm/util.h +10
-0+#define __UTIL_DOT_H__ + +uint32_t dlm_hash (const char *data,int len); + +void dlm_message_out (struct dlm_message *ms); +void dlm_message_in (struct dlm_message *ms); +void dlm_rcom_out (struct dlm_rcom *rc); +void dlm_rcom_in (struct dlm_rcom *rc); + +#endif
No newline at end of file?
Signature · 1 line
Jesper Juhl
Quoted 7 lines · Jesper Juhl
Quoted 1 line
+ int error =
-ENOMEM,last_len, count = 0;Wouldn't
int error =-ENOMEM;
intlast_len;
int count = 0;
be a bit more readable?
The rest of your nits are fine, upstanding nits, but this one seems a little extreme :-)
Regards,
Daniel
Quoted 9 lines · Jesper Juhl
|----- Why the parenthesis? ^^^^^--- more parens. ^^^^^--- yet more. what's your facination with parenthesis? ^--- here we go again. ^--- and again.a few cases of pointless parenthesis around define values... Here, again, we have a lot of pointless parenthesis around the values. I'm not going to bother pointing out the remaining ones.
Hm, you might have removed some remaining doubt about my paren usage. Anyway, they're all gone now.
Quoted 6 lines
include/linux/dlm.h line 206
Quoted 4 lines
include/linux/dlm.h +4
-0+int sb_status; +uint32_t sb_lkid; +char sb_flags; +char * sb_lvbptr;why not char
*sb_lvbptr; ???
I personally think the right column looks nicer when it's lined up, but a quick survey shows I'm in the minority, so I'd better get with the program...
Quoted 5 lines
Quoted 1 line
+static int
dlm_astd(void *data)Always returning 0 - why not a void function then?
Quoted 1 line
+int
dlm_scand(void *data)void func?
I think kthread_run() demands this.
Dave
Hi!
Quoted 7 lines
The core dlm functions. Processes
dlm_lock()anddlm_unlock()requests. Creates lockspaces which give applications separate contexts/namespaces in which to do their locking. Manages locks on resources' grant/convert/wait queues. Sends and receives high level locking operations between nodes. Delivers completion and blocking callbacks (ast's) to lock holders. Manages the distributed directory that tracks the current master node for each resource.
dlm stands for "distributed lock manager"? Is this component of lustre or what
include/linux/dlm.h line 12
Quoted 12 lines
include/linux/dlm.h +12
-0+/****************************************************************************** +******************************************************************************* +** +** Copyright (C) Sistina Software, Inc. 1997-2003 All rights reserved. +** Copyright (C) 2004-2005 Red Hat, Inc. All rights reserved. +** +** This copyrighted material is made available to anyone wishing to use, +** modify, copy, or redistribute it subject to the terms and conditions +** of the GNU General Public License v.2. +** +******************************************************************************* +******************************************************************************/
I'd read this as GPL v0.2. You have one dot too many... Also it would be nice to use less stars, as other kernel sources do...
Pavel
Signature · 1 line
Boycott Kodak -- for their patent abuse against Java.
Could you spell out what "DLM" stands for at the top of the header file, like you do in the configuration blurb, something like the following:
< * Interface to DLM - routines and structures to use DLM lockspaces.
Quoted 1 line
* Interface to Distributed Lock Manager (DLM) lockspaces.
To some outsiders, DLM is just another unrecognized three letter acronym.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401Quoted 9 lines · David Teigland
[Apologies, patch 1 was too large on its own.]
The core dlm functions. Processes
dlm_lock()anddlm_unlock()requests. Creates lockspaces which give applications separate contexts/namespaces in which to do their locking. Manages locks on resources' grant/convert/wait queues. Sends and receives high level locking operations between nodes. Delivers completion and blocking callbacks (ast's) to lock holders. Manages the distributed directory that tracks the current master node for each resource.
Just a couple comments here, more will come as time permits. I know you consider cluster file systems to be "obscure" apps, so be warned as that is my current bias ;)
<snip>
include/linux/dlm.h line 43
Quoted 5 lines
include/linux/dlm.h +5
-0+/* +* Size in bytes of Lock Value Block +*/ + +#define DLM_LVB_LEN (32)
Why so small? In OCFS2 land, a nice healthy 64 bytes helps us fit most of our important inode bits, thus ensuring that we don't have to go to disk to update our metadata in some cases :)
<snip>
Quoted 4 lines
+ *
DLM_LKF_EXPEDITE
+ *
+ * Used only with new requests for NL mode locks. Tells the lock manager
+ * to grant the lock, ignoring other locks in convert and wait queues.
What's happens to non DLM_LKF_EXPEDITE NL mode requests? It seems that an new NL mode lock should always be immediately be put on the grant queue...
<snip>
Quoted 7 lines
include/linux/dlm.h +7
-0+#define DLM_LKF_NOQUEUE (0x00000001) +#define DLM_LKF_CANCEL (0x00000002) +#define DLM_LKF_CONVERT (0x00000004) +#define DLM_LKF_VALBLK (0x00000008) +#define DLM_LKF_QUECVT (0x00000010) +#define DLM_LKF_IVVALBLK (0x00000020) +#define DLM_LKF_CONVDEADLK (0x00000040)
include/linux/dlm.h line 149
Quoted 10 lines
include/linux/dlm.h +10
-0+#define DLM_LKF_PERSISTENT (0x00000080) +#define DLM_LKF_NODLCKWT (0x00000100) +#define DLM_LKF_NODLCKBLK (0x00000200) +#define DLM_LKF_EXPEDITE (0x00000400) +#define DLM_LKF_NOQUEUEBAST (0x00000800) +#define DLM_LKF_HEADQUE (0x00001000) +#define DLM_LKF_NOORDER (0x00002000) +#define DLM_LKF_ORPHAN (0x00004000) +#define DLM_LKF_ALTPR (0x00008000) +#define DLM_LKF_ALTCW (0x00010000)
Where's the LKM_LOCAL equivalent? What happens a dlm user wants to create a lock on a resource it knows to be unique in the cluster (think file creation for a cfs)? Does it have to hit the network for a resource lookup on those locks?
Perhaps I should explain how this is interpreted in the OCFS2 dlm: When LKM_LOCAL is provided with a request for a new lock, the normal master lookup process is skipped and the resource is immediately created and mastered to the local node. Obviously this requires that users be careful not to create duplicate resources in the cluster. Any future requests for the lock from other nodes go through the master discovery process and will find it on the originating node.
We explicitly do not support LKM_FINDLOCAL - the notion of "local only" lookups does not apply as the resource is only considered to have been created locally and explicitly *not* hidden from the rest of the cluster.
Quoted 1 line
From a light skimming of
dir.c(specificallydlm_dir_name2nodeid), I have a
hunch that our methods for determing a resource master are fundamentally different, which would make implementation of LKM_LOCAL (at least as I have described it) on your side, difficult.
<snip>
drivers/dlm/ast.c line 20
Quoted 2 lines
drivers/dlm/ast.c +2
-0+static struct list_head ast_queue; +static struct semaphore ast_queue_lock;
Why a semaphore here? On quick inspection I'm not seeing much more than list operations being protected by ast_queue_lock... A spinlock might be more appropriate.
--Mark
--
Mark Fasheh
Senior Software Developer, Oracle
[email protected]
Quoted 2 lines · Mark Fasheh
Just a couple comments here, more will come as time permits. I know you consider cluster file systems to be "obscure" apps...
Oh the contrary, cluster filesystems are the main focus of and reason for the current submission.
Regards,
Daniel
Quoted 4 lines · Daniel Phillips
Quoted 2 lines
Just a couple comments here, more will come as time permits. I know you consider cluster file systems to be "obscure" apps...
Oh the contrary, cluster filesystems are the main focus of and reason for the current submission.
He was actually quoting David. And indeed it is very important that the DLM interfaces be generally useful, not just for a specific cluster filesystem; if that was the goal, it would be an internal component only and no need to expose it.
Signature · 3 lines
High Availability & Clustering SUSE Labs, Research and Development SUSE LINUX Products GmbH - A Novell Business
Quoted 11 lines · Lars Marowsky-Bree
Quoted 5 lines · Daniel Phillips
Quoted 2 lines
Just a couple comments here, more will come as time permits. I know you consider cluster file systems to be "obscure" apps...
Oh the contrary, cluster filesystems are the main focus of and reason for the current submission.
He was actually quoting David. And indeed it is very important that the DLM interfaces be generally useful, not just for a specific cluster filesystem; if that was the goal, it would be an internal component only and no need to expose it.
True, sort of. Remember, the _only_ argument for (g)dlm being in-kernel is to tighten up the interface for filesystems. If (g)dlm could be trimmed down by supporting _only_ in-kernel filesystems, with a different, userspace lock manager for user space apps, well, that is a strategy that has to be considered.
Taking part of (g)dlm out of kernel is also something that needs to be looked it. It is a little on the biggish side.
Don't forget, (g)dlm is just a cluster service, and at present, a service that has exactly one user in the whole world: gfs. We'd like to attract more, but not at the cost of kernel bloat.
Regards,
Daniel
Quoted 4 lines · Mark Fasheh
Quoted 1 line
+#define
DLM_LVB_LEN(32)Why so small? In OCFS2 land, a nice healthy 64 bytes helps us fit most of our important inode bits, thus ensuring that we don't have to go to disk to update our metadata in some cases :)
We were questioned for 32 being unnecessarily large when we started, which seems to make a case for it being configurable.
Quoted 8 lines
Quoted 4 lines
+ *
DLM_LKF_EXPEDITE
+ *
+ * Used only with new requests for NL mode locks. Tells the lock manager
+ * to grant the lock, ignoring other locks in convert and wait queues.What's happens to non
DLM_LKF_EXPEDITENL mode requests? It seems that an new NL mode lock should always be immediately be put on the grant queue...
A comment in _can_be_granted() quotes the VMS rule:
"By default, a new request is immediately granted only if all three of the following conditions are satisfied when the request is issued:
- The queue of ungranted conversion requests for the resoure is empty.
- The queue of ungranted new requests for the resource is empty.
- The mode of the new request is compatible with the most restrictive mode of all granted locks on the resource."
Which means without EXPEDITE it could go on the waiting queue. I suspect EXPEDITE was invented because most people want NL requests to work as you suggest, despite the rules.
Quoted 13 lines
Where's the
LKM_LOCALequivalent? What happens a dlm user wants to create a lock on a resource it knows to be unique in the cluster (think file creation for a cfs)? Does it have to hit the network for a resource lookup on those locks?Perhaps I should explain how this is interpreted in the OCFS2 dlm: When
LKM_LOCALis provided with a request for a new lock, the normal master lookup process is skipped and the resource is immediately created and mastered to the local node. Obviously this requires that users be careful not to create duplicate resources in the cluster. Any future requests for the lock from other nodes go through the master discovery process and will find it on the originating node.
Quoted 8 lines
We explicitly do not support
LKM_FINDLOCAL- the notion of "local only" lookups does not apply as the resource is only considered to have been created locally and explicitly *not* hidden from the rest of the cluster.Quoted 1 line
From a light skimming of
dir.c(specificallydlm_dir_name2nodeid), I have ahunch that our methods for determing a resource master are fundamentally different, which would make implementation of
LKM_LOCAL(at least as I have described it) on your side, difficult.
Interesting, I was reading about this recently and wondered if people really used it. I figured parent/child locks were probably a more common way to get similar benefits.
Just to clarify, though: when the LOCAL resource is immediately created and mastered locally, there must be a resource directory entry added for it, right? For us, the resource directory entry is added as part of a new master lookup (which is being skipped). If you don't add a directory entry, how does another node that later wants to lock the same resource (without LOCAL) discover who the master is?
If I understand LOCAL correctly, it should be simple for us to do. We'd still have a LOCAL request _send_ the lookup to create the directory entry, but we'd simply not wait for the reply. We'd assume, based on LOCAL, that the lookup result indicates we're the master.
Some people don't use a resource directory, and maybe that's why dlm_dir_name2nodeid() doesn't look familiar? That function determines the directory node for a resource, not the master node. The nodeid returned from that function is where we send the master lookup, and the lookup reply says where we send the lock request.
[We'll be adding a simple config option to change this so you can operate without a resource directory in which case there's never a master lookup to do. The downside is that the first node to request a lock on a resource would no longer always be the master of it.]
Quoted 6 lines
drivers/dlm/ast.c line 20
Quoted 2 lines
drivers/dlm/ast.c +2
-0+static struct list_head ast_queue; +static struct semaphore ast_queue_lock;Why a semaphore here? On quick inspection I'm not seeing much more than list operations being protected by
ast_queue_lock... A spinlock might be more appropriate.
You're right, thanks.
Dave
Hi,
Quoted 9 lines · David Teigland
A comment in
_can_be_granted()quotes the VMS rule:"By default, a new request is immediately granted only if all three of the following conditions are satisfied when the request is issued:
- The queue of ungranted conversion requests for the resoure is empty.
- The queue of ungranted new requests for the resource is empty.
- The mode of the new request is compatible with the most restrictive mode of all granted locks on the resource."
Right. It's a fairness issue. If you've got a read lock, then a new read lock request *can* be granted immediately; but if there are write lock requests on the queue (and remember, those may be on other nodes, you can't tell just from the local node state), then granting new read locks immediately can lead to writer starvation.
By default NL requests just follow the same rule, but because it's always theoretically possible to grant NL immediately, you can expedite it. And NL is usually used for one of two reasons: either to pin the lock value block for the resource in the DLM, or to keep the resource around while it's unlocked for performance reasons (resources are destroyed when there are no more locks against them; a NL lock keeps the resource present, so new lock requests can be granted much more quickly.) In both of those cases, observing the normal lock ordering rules is irrelevant.
Quoted 4 lines
Quoted 4 lines
Where's the
LKM_LOCALequivalent? What happens a dlm user wants to create a lock on a resource it knows to be unique in the cluster (think file creation for a cfs)? Does it have to hit the network for a resource lookup on those locks?
That is always needed if you've got a lock directory model --- you have to have, at minimum, a communication with the lock directory node for the given resource (which might be the local node if you're lucky). Even if you know the resource did not previously exist, you still need to tell the directory node that it *does* exist now so that future users can find it.
I suppose you could short-cut the wait for the response, though, to reduce the latency for this case. My gut feeling, though, is that I'd still prefer to see the DLM doing its work properly, cluster-wide in this case, as precaution against accidents if we get inconsistent states on disk leading to two nodes trying to create the same lock at once. Experience suggests that such things *do* go wrong, and it's as well to plan for them --- early detection is good!
--Stephen
Quoted 6 lines · Stephen C. Tweedie
reduce the latency for this case. My gut feeling, though, is that I'd still prefer to see the DLM doing its work properly, cluster-wide in this case, as precaution against accidents if we get inconsistent states on disk leading to two nodes trying to create the same lock at once. Experience suggests that such things *do* go wrong, and it's as well to plan for them --- early detection is good!
And unacceptably slow. With LKM_LOCAL, OCFS2 approaches ext3
speed untarring a kernel tree, because everything under the toplevel directory is a candidate for LKM_LOCAL. Network communication may be fast, but pagecache operations are even faster. I don't know by how much, but I bet if we turned off LKM_LOCAL in the OCFS2 DLM, we'd lose a lot of speed.
Joel
Signature · 10 lines
One look at the From: understanding has blossomed .procmailrc grows - Alexander Viro Joel Becker Senior Member of Technical Staff Oracle E-mail: [email protected] Phone: (650) 506-8127
Quoted 8 lines · Joel Becker
Quoted 6 lines · Stephen C. Tweedie
reduce the latency for this case. My gut feeling, though, is that I'd still prefer to see the DLM doing its work properly, cluster-wide in this case, as precaution against accidents if we get inconsistent states on disk leading to two nodes trying to create the same lock at once. Experience suggests that such things *do* go wrong, and it's as well to plan for them --- early detection is good!
Quoted 6 lines
And unacceptably slow. With
LKM_LOCAL, OCFS2 approaches ext3
speed untarring a kernel tree, because everything under the toplevel directory is a candidate forLKM_LOCAL. Network communication may be fast, but pagecache operations are even faster. I don't know by how much, but I bet if we turned offLKM_LOCALin the OCFS2 DLM, we'd lose a lot of speed.
Why don't you try it, and post the numbers?
Regards,
Daniel
Quoted 1 line · Daniel Phillips
Why don't you try it, and post the numbers?
*Sigh*
Done from a test machine I have over here on a two node cluster. I rebooted and re-formatted between tests.
* Without LKM_LOCAL:
[root@ca-test7 ocfs2]# time tar -zxf /tmp/linux-2.6.11.7.tar.gz
real 0m39.699s user 0m3.644s sys 0m8.076s
* With LKM_LOCAL
[root@ca-test7 ocfs2]# time tar -zxf /tmp/linux-2.6.11.7.tar.gz
real 0m22.076s user 0m3.869s sys 0m7.234s
So yes, I'd say it's worth a significant amount of performance to us :)
--Mark
--
Mark Fasheh
Senior Software Developer, Oracle
[email protected]
Quoted 7 lines · Mark Fasheh
* Without
LKM_LOCAL:
[root@ca-test7 ocfs2]# time tar -zxf/tmp/linux-2.6.11.7.tar.gzreal 0m39.699s user 0m3.644s sys 0m8.076s
Quoted 8 lines
* With
LKM_LOCAL
[root@ca-test7 ocfs2]# time tar -zxf/tmp/linux-2.6.11.7.tar.gzreal 0m22.076s user 0m3.869s sys 0m7.234sSo yes, I'd say it's worth a significant amount of performance to us :)
To be precise, LKM_LOCAL saves you 44%, and even without LKM_LOCAL you turn in a respectable number. Could you please provide your node, shared disk and network specs?
Because I am greedy, I would like to have seen the Ext3 number, too. And oh yes, the Ext2 number!
Regards,
Daniel
Quoted 2 lines · David Teigland
We were questioned for 32 being unnecessarily large when we started, which seems to make a case for it being configurable.
Agreed.
Quoted 3 lines
Which means without EXPEDITE it could go on the waiting queue. I suspect EXPEDITE was invented because most people want NL requests to work as you suggest, despite the rules.
Ok, fair enough -- as long as there's a way to immediately grant NL requests, I'm happy :)
Quoted 10 lines
Interesting, I was reading about this recently and wondered if people really used it. I figured parent/child locks were probably a more common way to get similar benefits.
Just to clarify, though: when the LOCAL resource is immediately created and mastered locally, there must be a resource directory entry added for it, right? For us, the resource directory entry is added as part of a new master lookup (which is being skipped). If you don't add a directory entry, how does another node that later wants to lock the same resource (without LOCAL) discover who the master is?
Yes, I believe LOCAL would always have to at least add a directory entry. For the OCFS2 dlm which does not use a resource directory, the entry would just exist on the creating node and other nodes would discover it later via query.
Quoted 4 lines
If I understand LOCAL correctly, it should be simple for us to do. We'd still have a LOCAL request _send_ the lookup to create the directory entry, but we'd simply not wait for the reply. We'd assume, based on LOCAL, that the lookup result indicates we're the master.
I assume then that you can do that without racing the node who sent the LOCAL request and another node who comes in (just afterwards) for a master lookup? I bet the answer to that question would come to me if I read more of the code :)
--Mark
--
Mark Fasheh
Senior Software Developer, Oracle
[email protected]
Quoted 11 lines · Mark Fasheh
Quoted 6 lines
Just to clarify, though: when the LOCAL resource is immediately created and mastered locally, there must be a resource directory entry added for it, right? For us, the resource directory entry is added as part of a new master lookup (which is being skipped). If you don't add a directory entry, how does another node that later wants to lock the same resource (without LOCAL) discover who the master is?
Yes, I believe LOCAL would always have to at least add a directory entry. For the OCFS2 dlm which does not use a resource directory, the entry would just exist on the creating node and other nodes would discover it later via query.
OK, sounds like an interesting tradeoff. If any node can be master of a resource, and there's no resource directory, then any new non-LOCAL request must query every other node to check if it's the master. A look at your dlm shows this is the case:
while ((nodenum = dlm_node_iter_next(&iter)) >= 0) {
ret = dlm_do_master_request(mle, nodenum);Of course with a directory you just query one node to find if/who the master is. Which is better would depend on the usage.
If you wanted to do a similar optimization as LOCAL with our dlm, you'd use parent/child locks where the parent at the top of the tree would require a lookup but all the child locks would only require local processing.
Quoted 9 lines
Quoted 4 lines
If I understand LOCAL correctly, it should be simple for us to do. We'd still have a LOCAL request _send_ the lookup to create the directory entry, but we'd simply not wait for the reply. We'd assume, based on LOCAL, that the lookup result indicates we're the master.
I assume then that you can do that without racing the node who sent the LOCAL request and another node who comes in (just afterwards) for a master lookup? I bet the answer to that question would come to me if I read more of the code :)
Nope, my speculation on how we might do LOCAL above wouldn't work. It now seems clear that LOCAL can't really be done with a directory.
Dave