[PATCH] (5/5) more BKL shifting

old_inode is locked by vfs_link().

Documentation/filesystems/Locking

@@ -55,7 +55,7 @@ locking rules:
 		BKL	i_sem(inode)
 lookup:		no	yes
 create:		no	yes
-link:		no	yes
+link:		no	yes (both)
 mknod:		no	yes
 symlink:	no	yes
 mkdir:		no	yes

Documentation/filesystems/directory-locking

@@ -14,7 +14,13 @@ locks victim and calls the method.
 the parent, finds source and target, if target already exists - locks it
 and then calls the method.
 
-5) cross-directory rename.  The trickiest in the whole bunch.  Locking
+5) link creation.  Locking rules:
+	* lock parent
+	* check that source is not a directory
+	* lock source
+	* call the method.
+
+6) cross-directory rename.  The trickiest in the whole bunch.  Locking
 rules:
 	* lock the filesystem
 	* lock parents in "ancestors first" order.
@@ -39,7 +45,7 @@ objects - A < B iff A is an ancestor of B.
 
 	That ordering can change.  However, the following is true:
 
-(1) if operation different from cross-directory rename holds lock on A and
+(1) if object removal or non-cross-directory rename holds lock on A and
     attempts to acquire lock on B, A will remain the parent of B until we
     acquire the lock on B.  (Proof: only cross-directory rename can change
     the parent of object and it would have to lock the parent).
@@ -49,12 +55,20 @@ objects - A < B iff A is an ancestor of B.
     renames will be blocked on filesystem lock and we don't start changing
     the order until we had acquired all locks).
 
+(3) any operation holds at most one lock on non-directory object and
+    that lock is acquired after all other locks.  (Proof: see descriptions
+    of operations).
+
 	Now consider the minimal deadlock.  Each process is blocked on
 attempt to acquire some lock and already holds at least one lock.  Let's
 consider the set of contended locks.  First of all, filesystem lock is
 not contended, since any process blocked on it is not holding any locks.
 Thus all processes are blocked on ->i_sem.
 
+	Non-directory objects are not contended due to (3).  Thus link
+creation can't be a part of deadlock - it can't be blocked on source
+and it means that it doesn't hold any locks.
+
 	Any contended object is either held by cross-directory rename or
 has a child that is also contended.  Indeed, suppose that it is held by
 operation other than cross-directory rename.  Then the lock this operation

Documentation/filesystems/porting

@@ -92,3 +92,9 @@ exactly what needs to be protected.
 
 check for ->link() target not being a directory is done by callers.  Feel
 free to drop it...
+
+---
+[informational]
+
+->link() callers hold ->i_sem on the object we are linking to.  Some of your
+problems might be over...

drivers/hotplug/pci_hotplug_core.c

@@ -159,9 +159,7 @@ static int pcihpfs_link (struct dentry *old_dentry, struct inode *dir,
 {
 	struct inode *inode = old_dentry->d_inode;
 
-	lock_kernel();
 	inode->i_nlink++;
-	unlock_kernel();
 	atomic_inc(&inode->i_count);
  	dget(dentry);
 	d_instantiate(dentry, inode);
@@ -198,9 +196,7 @@ static int pcihpfs_unlink (struct inode *dir, struct dentry *dentry)
 	if (pcihpfs_empty(dentry)) {
 		struct inode *inode = dentry->d_inode;
 
-		lock_kernel();
 		inode->i_nlink--;
-		unlock_kernel();
 		dput(dentry);
 		error = 0;
 	}

drivers/usb/inode.c

@@ -216,9 +216,7 @@ static int usbfs_link (struct dentry *old_dentry, struct inode *dir,
 {
 	struct inode *inode = old_dentry->d_inode;
 
-	lock_kernel();
 	inode->i_nlink++;
-	unlock_kernel();
 	atomic_inc(&inode->i_count);
  	dget(dentry);
 	d_instantiate(dentry, inode);
@@ -255,9 +253,7 @@ static int usbfs_unlink (struct inode *dir, struct dentry *dentry)
 	if (usbfs_empty(dentry)) {
 		struct inode *inode = dentry->d_inode;
 
-		lock_kernel();
 		inode->i_nlink--;
-		unlock_kernel();
 		dput(dentry);
 		error = 0;
 	}

fs/ext2/namei.c

@@ -39,17 +39,13 @@
 
 static inline void ext2_inc_count(struct inode *inode)
 {
-	lock_kernel();
 	inode->i_nlink++;
-	unlock_kernel();
 	mark_inode_dirty(inode);
 }
 
 static inline void ext2_dec_count(struct inode *inode)
 {
-	lock_kernel();
 	inode->i_nlink--;
-	unlock_kernel();
 	mark_inode_dirty(inode);
 }
 
@@ -168,15 +164,11 @@ static int ext2_link (struct dentry * old_dentry, struct inode * dir,
 {
 	struct inode *inode = old_dentry->d_inode;
 
-	lock_kernel();
-	if (inode->i_nlink >= EXT2_LINK_MAX) {
-		unlock_kernel();
+	if (inode->i_nlink >= EXT2_LINK_MAX)
 		return -EMLINK;
-	}
 
 	inode->i_ctime = CURRENT_TIME;
 	ext2_inc_count(inode);
-	unlock_kernel();
 	atomic_inc(&inode->i_count);
 
 	return ext2_add_nondir(dentry, inode);
@@ -299,11 +291,8 @@ static int ext2_rename (struct inode * old_dir, struct dentry * old_dentry,
 		ext2_inc_count(old_inode);
 		ext2_set_link(new_dir, new_de, new_page, old_inode);
 		new_inode->i_ctime = CURRENT_TIME;
-		if (dir_de) {
-			lock_kernel();
+		if (dir_de)
 			new_inode->i_nlink--;
-			unlock_kernel();
-		}
 		ext2_dec_count(new_inode);
 	} else {
 		if (dir_de) {

fs/minix/namei.c

@@ -11,17 +11,13 @@
 
 static inline void inc_count(struct inode *inode)
 {
-	lock_kernel();
 	inode->i_nlink++;
-	unlock_kernel();
 	mark_inode_dirty(inode);
 }
 
 static inline void dec_count(struct inode *inode)
 {
-	lock_kernel();
 	inode->i_nlink--;
-	unlock_kernel();
 	mark_inode_dirty(inode);
 }
 
@@ -136,15 +132,11 @@ static int minix_link(struct dentry * old_dentry, struct inode * dir,
 {
 	struct inode *inode = old_dentry->d_inode;
 
-	lock_kernel();
-	if (inode->i_nlink >= inode->i_sb->u.minix_sb.s_link_max) {
-		unlock_kernel();
+	if (inode->i_nlink >= inode->i_sb->u.minix_sb.s_link_max)
 		return -EMLINK;
-	}
 
 	inode->i_ctime = CURRENT_TIME;
 	inc_count(inode);
-	unlock_kernel();
 	atomic_inc(&inode->i_count);
 	return add_nondir(dentry, inode);
 }
@@ -265,11 +257,8 @@ static int minix_rename(struct inode * old_dir, struct dentry *old_dentry,
 		inc_count(old_inode);
 		minix_set_link(new_de, new_page, old_inode);
 		new_inode->i_ctime = CURRENT_TIME;
-		if (dir_de) {
-			lock_kernel();
+		if (dir_de)
 			new_inode->i_nlink--;
-			unlock_kernel();
-		}
 		dec_count(new_inode);
 	} else {
 		if (dir_de) {

fs/namei.c

@@ -1614,8 +1614,10 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
 	if (S_ISDIR(old_dentry->d_inode->i_mode))
 		return -EPERM;
 
+	down(&old_dentry->d_inode->i_sem);
 	DQUOT_INIT(dir);
 	error = dir->i_op->link(old_dentry, dir, new_dentry);
+	up(&old_dentry->d_inode->i_sem);
 	if (!error)
 		inode_dir_notify(dir, DN_CREATE);
 	return error;

fs/ramfs/inode.c

@@ -166,9 +166,7 @@ static int ramfs_link(struct dentry *old_dentry, struct inode * dir, struct dent
 {
 	struct inode *inode = old_dentry->d_inode;
 
-	lock_kernel();
 	inode->i_nlink++;
-	unlock_kernel();
 	atomic_inc(&inode->i_count);	/* New dentry reference */
 	dget(dentry);		/* Extra pinning count for the created dentry */
 	d_instantiate(dentry, inode);
@@ -219,9 +217,7 @@ static int ramfs_unlink(struct inode * dir, struct dentry *dentry)
 	if (ramfs_empty(dentry)) {
 		struct inode *inode = dentry->d_inode;
 
-		lock_kernel();
 		inode->i_nlink--;
-		unlock_kernel();
 		dput(dentry);			/* Undo the count from "create" - this does all the work */
 		retval = 0;
 	}

fs/sysv/namei.c

@@ -19,17 +19,13 @@
 
 static inline void inc_count(struct inode *inode)
 {
-	lock_kernel();
 	inode->i_nlink++;
-	unlock_kernel();
 	mark_inode_dirty(inode);
 }
 
 static inline void dec_count(struct inode *inode)
 {
-	lock_kernel();
 	inode->i_nlink--;
-	unlock_kernel();
 	mark_inode_dirty(inode);
 }
 
@@ -142,15 +138,11 @@ static int sysv_link(struct dentry * old_dentry, struct inode * dir,
 {
 	struct inode *inode = old_dentry->d_inode;
 
-	lock_kernel();
-	if (inode->i_nlink >= inode->i_sb->sv_link_max) {
-		unlock_kernel();
+	if (inode->i_nlink >= inode->i_sb->sv_link_max)
 		return -EMLINK;
-	}
 
 	inode->i_ctime = CURRENT_TIME;
 	inc_count(inode);
-	unlock_kernel();
 	atomic_inc(&inode->i_count);
 
 	return add_nondir(dentry, inode);
@@ -273,11 +265,8 @@ static int sysv_rename(struct inode * old_dir, struct dentry * old_dentry,
 		inc_count(old_inode);
 		sysv_set_link(new_de, new_page, old_inode);
 		new_inode->i_ctime = CURRENT_TIME;
-		if (dir_de) {
-			lock_kernel();
+		if (dir_de)
 			new_inode->i_nlink--;
-			unlock_kernel();
-		}
 		dec_count(new_inode);
 	} else {
 		if (dir_de) {

mm/shmem.c

@@ -1022,9 +1022,7 @@ static int shmem_link(struct dentry *old_dentry, struct inode * dir, struct dent
 	struct inode *inode = old_dentry->d_inode;
 
 	inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
-	lock_kernel();
 	inode->i_nlink++;
-	unlock_kernel();
 	atomic_inc(&inode->i_count);	/* New dentry reference */
 	dget(dentry);		/* Extra pinning count for the created dentry */
 	d_instantiate(dentry, inode);
@@ -1068,9 +1066,7 @@ static int shmem_unlink(struct inode * dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
 	inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
-	lock_kernel();
 	inode->i_nlink--;
-	unlock_kernel();
 	dput(dentry);	/* Undo the count from "create" - this does all the work */
 	return 0;
 }