linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Trond Myklebust <trond.myklebust@fys.uio.no>
To: Andrew Morton <akpm@osdl.org>
Cc: Matthew Wilcox <willy@debian.org>, linux-fsdevel@vger.kernel.org
Subject: Re: Fw: Spam: [Bugme-new] [Bug 2829] New: posix_locks_deadlock() loops infinitely
Date: Fri, 04 Jun 2004 23:36:48 -0400	[thread overview]
Message-ID: <1086406608.6959.32.camel@lade.trondhjem.org> (raw)
In-Reply-To: <1086403506.6959.29.camel@lade.trondhjem.org>

På fr , 04/06/2004 klokka 22:45, skreiv Trond Myklebust:
> Could we instead concentrate on fixing the fscking fl_owner? That would
> also get rid of crap like "steal_locks".
> The only process that needs to be setting fl_owner is lockd, since that
> takes locks on behalf of processes that are running on the client
> machines.

Something like the appended patch (untested).

Trond

diff -u --recursive --new-file --show-c-function linux-2.6.7-rc1/fs/locks.c linux-2.6.7-lock_nfsv4/fs/locks.c
--- linux-2.6.7-rc1/fs/locks.c	2004-05-29 09:06:52.000000000 -0400
+++ linux-2.6.7-lock_nfsv4/fs/locks.c	2004-06-04 23:13:00.000000000 -0400
@@ -317,7 +317,7 @@ static int flock_to_posix_lock(struct fi
 	if (l->l_len == 0)
 		fl->fl_end = OFFSET_MAX;
 	
-	fl->fl_owner = current->files;
+	fl->fl_owner = 0;
 	fl->fl_pid = current->tgid;
 	fl->fl_file = filp;
 	fl->fl_flags = FL_POSIX;
@@ -357,7 +357,7 @@ static int flock64_to_posix_lock(struct 
 	if (l->l_len == 0)
 		fl->fl_end = OFFSET_MAX;
 	
-	fl->fl_owner = current->files;
+	fl->fl_owner = 0;
 	fl->fl_pid = current->tgid;
 	fl->fl_file = filp;
 	fl->fl_flags = FL_POSIX;
@@ -920,7 +920,7 @@ int posix_lock_file(struct file *filp, s
  */
 int locks_mandatory_locked(struct inode *inode)
 {
-	fl_owner_t owner = current->files;
+	unsigned int pid = current->tgid;
 	struct file_lock *fl;
 
 	/*
@@ -930,7 +930,9 @@ int locks_mandatory_locked(struct inode 
 	for (fl = inode->i_flock; fl != NULL; fl = fl->fl_next) {
 		if (!IS_POSIX(fl))
 			continue;
-		if (fl->fl_owner != owner)
+		if (fl->fl_owner != 0)
+			break;
+		if (fl->fl_pid != pid)
 			break;
 	}
 	unlock_kernel();
@@ -958,7 +960,7 @@ int locks_mandatory_area(int read_write,
 	int error;
 
 	locks_init_lock(&fl);
-	fl.fl_owner = current->files;
+	fl.fl_owner = 0;
 	fl.fl_pid = current->tgid;
 	fl.fl_file = filp;
 	fl.fl_flags = FL_POSIX | FL_ACCESS;
@@ -1684,7 +1686,7 @@ void locks_remove_posix(struct file *fil
 	lock_kernel();
 	while (*before != NULL) {
 		struct file_lock *fl = *before;
-		if (IS_POSIX(fl) && (fl->fl_owner == owner)) {
+		if (IS_POSIX(fl) && posix_same_owner(fl, &lock)) {
 			locks_delete_lock(before);
 			continue;
 		}
@@ -1982,18 +1984,6 @@ int lock_may_write(struct inode *inode, 
 
 EXPORT_SYMBOL(lock_may_write);
 
-static inline void __steal_locks(struct file *file, fl_owner_t from)
-{
-	struct inode *inode = file->f_dentry->d_inode;
-	struct file_lock *fl = inode->i_flock;
-
-	while (fl) {
-		if (fl->fl_file == file && fl->fl_owner == from)
-			fl->fl_owner = current->files;
-		fl = fl->fl_next;
-	}
-}
-
 /* When getting ready for executing a binary, we make sure that current
  * has a files_struct on its own. Before dropping the old files_struct,
  * we take over ownership of all locks for all file descriptors we own.
@@ -2002,31 +1992,6 @@ static inline void __steal_locks(struct 
  */
 void steal_locks(fl_owner_t from)
 {
-	struct files_struct *files = current->files;
-	int i, j;
-
-	if (from == files)
-		return;
-
-	lock_kernel();
-	j = 0;
-	for (;;) {
-		unsigned long set;
-		i = j * __NFDBITS;
-		if (i >= files->max_fdset || i >= files->max_fds)
-			break;
-		set = files->open_fds->fds_bits[j++];
-		while (set) {
-			if (set & 1) {
-				struct file *file = files->fd[i];
-				if (file)
-					__steal_locks(file, from);
-			}
-			i++;
-			set >>= 1;
-		}
-	}
-	unlock_kernel();
 }
 EXPORT_SYMBOL(steal_locks);
 
diff -u --recursive --new-file --show-c-function linux-2.6.7-rc1/fs/nfs/file.c linux-2.6.7-lock_nfsv4/fs/nfs/file.c
--- linux-2.6.7-rc1/fs/nfs/file.c	2004-05-29 09:06:36.000000000 -0400
+++ linux-2.6.7-lock_nfsv4/fs/nfs/file.c	2004-06-04 22:58:49.000000000 -0400
@@ -340,7 +340,7 @@ nfs_lock(struct file *filp, int cmd, str
 	 * Not sure whether that would be unique, though, or whether
 	 * that would break in other places.
 	 */
-	if (!fl->fl_owner || !(fl->fl_flags & FL_POSIX))
+	if (!(fl->fl_flags & FL_POSIX))
 		return -ENOLCK;
 
 	/*
diff -u --recursive --new-file --show-c-function linux-2.6.7-rc1/fs/nfs/nfs4state.c linux-2.6.7-lock_nfsv4/fs/nfs/nfs4state.c
--- linux-2.6.7-rc1/fs/nfs/nfs4state.c	2004-05-29 09:06:20.000000000 -0400
+++ linux-2.6.7-lock_nfsv4/fs/nfs/nfs4state.c	2004-06-04 23:09:10.000000000 -0400
@@ -496,11 +496,11 @@ nfs4_close_state(struct nfs4_state *stat
  * that is compatible with current->files
  */
 static struct nfs4_lock_state *
-__nfs4_find_lock_state(struct nfs4_state *state, fl_owner_t fl_owner)
+__nfs4_find_lock_state(struct nfs4_state *state, unsigned int pid)
 {
 	struct nfs4_lock_state *pos;
 	list_for_each_entry(pos, &state->lock_states, ls_locks) {
-		if (pos->ls_owner != fl_owner)
+		if (pos->ls_pid != pid)
 			continue;
 		atomic_inc(&pos->ls_count);
 		return pos;
@@ -509,11 +509,11 @@ __nfs4_find_lock_state(struct nfs4_state
 }
 
 struct nfs4_lock_state *
-nfs4_find_lock_state(struct nfs4_state *state, fl_owner_t fl_owner)
+nfs4_find_lock_state(struct nfs4_state *state, unsigned int pid)
 {
 	struct nfs4_lock_state *lsp;
 	read_lock(&state->state_lock);
-	lsp = __nfs4_find_lock_state(state, fl_owner);
+	lsp = __nfs4_find_lock_state(state, pid);
 	read_unlock(&state->state_lock);
 	return lsp;
 }
@@ -525,7 +525,7 @@ nfs4_find_lock_state(struct nfs4_state *
  * The caller must be holding state->lock_sema
  */
 struct nfs4_lock_state *
-nfs4_alloc_lock_state(struct nfs4_state *state, fl_owner_t fl_owner)
+nfs4_alloc_lock_state(struct nfs4_state *state, unsigned int pid)
 {
 	struct nfs4_lock_state *lsp;
 	struct nfs4_client *clp = state->owner->so_client;
@@ -537,7 +537,7 @@ nfs4_alloc_lock_state(struct nfs4_state 
 	lsp->ls_id = -1; 
 	memset(lsp->ls_stateid.data, 0, sizeof(lsp->ls_stateid.data));
 	atomic_set(&lsp->ls_count, 1);
-	lsp->ls_owner = fl_owner;
+	lsp->ls_pid = pid;
 	lsp->ls_parent = state;
 	INIT_LIST_HEAD(&lsp->ls_locks);
 	spin_lock(&clp->cl_lock);
@@ -551,12 +551,12 @@ nfs4_alloc_lock_state(struct nfs4_state 
  * requests.
  */
 void
-nfs4_copy_stateid(nfs4_stateid *dst, struct nfs4_state *state, fl_owner_t fl_owner)
+nfs4_copy_stateid(nfs4_stateid *dst, struct nfs4_state *state, unsigned int pid)
 {
 	if (test_bit(LK_STATE_IN_USE, &state->flags)) {
 		struct nfs4_lock_state *lsp;
 
-		lsp = nfs4_find_lock_state(state, fl_owner);
+		lsp = nfs4_find_lock_state(state, pid);
 		if (lsp) {
 			memcpy(dst, &lsp->ls_stateid, sizeof(*dst));
 			nfs4_put_lock_state(lsp);
@@ -628,7 +628,7 @@ nfs4_notify_unlck(struct inode *inode, s
 	for (fl = inode->i_flock; fl != NULL; fl = fl->fl_next) {
 		if (!(fl->fl_flags & FL_POSIX))
 			continue;
-		if (fl->fl_owner != lsp->ls_owner)
+		if (fl->fl_pid != lsp->ls_pid)
 			continue;
 		/* Exit if we find at least one lock which is not consumed */
 		if (nfs4_check_unlock(fl,request) == 0)
diff -u --recursive --new-file --show-c-function linux-2.6.7-rc1/fs/open.c linux-2.6.7-lock_nfsv4/fs/open.c
--- linux-2.6.7-rc1/fs/open.c	2004-05-29 09:06:20.000000000 -0400
+++ linux-2.6.7-lock_nfsv4/fs/open.c	2004-06-04 22:54:32.000000000 -0400
@@ -1007,7 +1007,7 @@ int filp_close(struct file *filp, fl_own
 	}
 
 	dnotify_flush(filp, id);
-	locks_remove_posix(filp, id);
+	locks_remove_posix(filp, 0);
 	fput(filp);
 	return retval;
 }
diff -u --recursive --new-file --show-c-function linux-2.6.7-rc1/include/linux/fs.h linux-2.6.7-lock_nfsv4/include/linux/fs.h
--- linux-2.6.7-rc1/include/linux/fs.h	2004-05-29 09:06:23.000000000 -0400
+++ linux-2.6.7-lock_nfsv4/include/linux/fs.h	2004-06-04 23:03:52.000000000 -0400
@@ -625,7 +625,7 @@ struct file_lock {
 	struct file_lock *fl_next;	/* singly linked list for this inode  */
 	struct list_head fl_link;	/* doubly linked list of all locks */
 	struct list_head fl_block;	/* circular list of blocked processes */
-	fl_owner_t fl_owner;
+	fl_owner_t fl_owner;		/* 0 if lock owned by a local process */
 	unsigned int fl_pid;
 	wait_queue_head_t fl_wait;
 	struct file *fl_file;
diff -u --recursive --new-file --show-c-function linux-2.6.7-rc1/include/linux/nfs_fs.h linux-2.6.7-lock_nfsv4/include/linux/nfs_fs.h
--- linux-2.6.7-rc1/include/linux/nfs_fs.h	2004-05-29 09:06:25.000000000 -0400
+++ linux-2.6.7-lock_nfsv4/include/linux/nfs_fs.h	2004-06-04 23:09:26.000000000 -0400
@@ -595,7 +595,7 @@ struct nfs4_state_owner {
 
 struct nfs4_lock_state {
 	struct list_head	ls_locks;	/* Other lock stateids */
-	fl_owner_t		ls_owner;	/* POSIX lock owner */
+	unsigned int		ls_pid;		/* pid of owner process */
 	struct nfs4_state *	ls_parent;	/* Parent nfs4_state */
 	u32			ls_seqid;
 	u32			ls_id;
@@ -665,13 +665,13 @@ extern struct nfs4_state *nfs4_find_stat
 extern void nfs4_increment_seqid(int status, struct nfs4_state_owner *sp);
 extern int nfs4_handle_error(struct nfs_server *, int);
 extern void nfs4_schedule_state_recovery(struct nfs4_client *);
-extern struct nfs4_lock_state *nfs4_find_lock_state(struct nfs4_state *state, fl_owner_t);
-extern struct nfs4_lock_state *nfs4_alloc_lock_state(struct nfs4_state *state, fl_owner_t);
+extern struct nfs4_lock_state *nfs4_find_lock_state(struct nfs4_state *state, unsigned int pid);
+extern struct nfs4_lock_state *nfs4_alloc_lock_state(struct nfs4_state *state, unsigned int pid);
 extern void nfs4_put_lock_state(struct nfs4_lock_state *state);
 extern void nfs4_increment_lock_seqid(int status, struct nfs4_lock_state *ls);
 extern void nfs4_notify_setlk(struct inode *, struct file_lock *, struct nfs4_lock_state *);
 extern void nfs4_notify_unlck(struct inode *, struct file_lock *, struct nfs4_lock_state *);
-extern void nfs4_copy_stateid(nfs4_stateid *, struct nfs4_state *, fl_owner_t);
+extern void nfs4_copy_stateid(nfs4_stateid *, struct nfs4_state *, unsigned int pid);
 
 
 

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2004-06-05  3:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-06-04  5:50 Fw: Spam: [Bugme-new] [Bug 2829] New: posix_locks_deadlock() loops infinitely Andrew Morton
2004-06-04  5:53 ` Andrew Morton
2004-06-04  7:15   ` Ichiko Sakamoto
2004-06-04 11:20 ` Fw: Spam: [Bugme-new] " Matthew Wilcox
2004-06-05  2:18   ` Andrew Morton
2004-06-05  2:45     ` Trond Myklebust
2004-06-05  3:36       ` Trond Myklebust [this message]
  -- strict thread matches above, loose matches on Subject: below --
2004-06-05  3:29 Stephen Rothwell
2004-06-05  3:47 ` Trond Myklebust
2004-06-05  3:32 Stephen Rothwell
2004-06-05  7:25 Stephen Rothwell
2004-06-06  3:04 ` Stephen Rothwell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1086406608.6959.32.camel@lade.trondhjem.org \
    --to=trond.myklebust@fys.uio.no \
    --cc=akpm@osdl.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=willy@debian.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).