NTFS: - Fix bug in fs/ntfs/attrib.c::ntfs_find_vcn_nolock() where after

dropping the read lock and taking the write lock we were not checking
        whether someone else did not already do the work we wanted to do.
      - Rename ntfs_find_vcn_nolock() to ntfs_attr_find_vcn_nolock().
      - Tidy up some comments in fs/ntfs/runlist.c.
      - Add LCN_ENOMEM and LCN_EIO definitions to fs/ntfs/runlist.h.

Signed-off-by: Anton Altaparmakov <aia21@cantab.net>

fs/ntfs/ChangeLog

@@ -92,6 +92,11 @@ ToDo/Notes:
 	  non-resident in fs/ntfs/attrib.c::ntfs_attr_can_be_non_resident().
 	- Add fs/ntfs/attrib.c::ntfs_attr_vcn_to_lcn_nolock() used by the new
 	  write code.
+	- Fix bug in fs/ntfs/attrib.c::ntfs_find_vcn_nolock() where after
+	  dropping the read lock and taking the write lock we were not checking
+	  whether someone else did not already do the work we wanted to do.
+	- Rename fs/ntfs/attrib.c::ntfs_find_vcn_nolock() to
+	  ntfs_attr_find_vcn_nolock() and update all callers.
 
 2.1.22 - Many bug and race fixes and error handling improvements.
 

fs/ntfs/attrib.c

@@ -193,19 +193,19 @@ retry_remap:
 }
 
 /**
- * ntfs_find_vcn_nolock - find a vcn in the runlist described by an ntfs inode
+ * ntfs_attr_find_vcn_nolock - find a vcn in the runlist of an ntfs inode
  * @ni:			ntfs inode describing the runlist to search
  * @vcn:		vcn to find
  * @write_locked:	true if the runlist is locked for writing
  *
  * Find the virtual cluster number @vcn in the runlist described by the ntfs
  * inode @ni and return the address of the runlist element containing the @vcn.
- * The runlist is left locked and the caller has to unlock it.  In the error
- * case, the runlist is left in the same locking state as on entry.
  *
- * Note if @write_locked is FALSE the lock may be dropped inside the function
+ * If the @vcn is not mapped yet, the attempt is made to map the attribute
- * so you cannot rely on the runlist still being the same when this function
+ * extent containing the @vcn and the vcn to lcn conversion is retried.
- * returns.
+ *
+ * If @write_locked is true the caller has locked the runlist for writing and
+ * if false for reading.
  *
  * Note you need to distinguish between the lcn of the returned runlist element
  * being >= 0 and LCN_HOLE.  In the later case you have to return zeroes on
@@ -221,13 +221,12 @@ retry_remap:
  *	-ENOMEM - Not enough memory to map runlist.
  *	-EIO	- Critical error (runlist/file is corrupt, i/o error, etc).
  *
- * Locking: - The runlist must be unlocked on entry.
+ * Locking: - The runlist must be locked on entry and is left locked on return.
- *	    - On failing return, the runlist is unlocked.
+ *	    - If @write_locked is FALSE, i.e. the runlist is locked for reading,
- *	    - On successful return, the runlist is locked.  If @need_write us
+ *	      the lock may be dropped inside the function so you cannot rely on
- *	      true, it is locked for writing.  Otherwise is is locked for
+ *	      the runlist still being the same when this function returns.
- *	      reading.
  */
-runlist_element *ntfs_find_vcn_nolock(ntfs_inode *ni, const VCN vcn,
+runlist_element *ntfs_attr_find_vcn_nolock(ntfs_inode *ni, const VCN vcn,
 		const BOOL write_locked)
 {
 	runlist_element *rl;
@@ -268,6 +267,12 @@ retry_remap:
 		if (!write_locked) {
 			up_read(&ni->runlist.lock);
 			down_write(&ni->runlist.lock);
+			if (unlikely(ntfs_rl_vcn_to_lcn(ni->runlist.rl, vcn) !=
+					LCN_RL_NOT_MAPPED)) {
+				up_write(&ni->runlist.lock);
+				down_read(&ni->runlist.lock);
+				goto retry_remap;
+			}
 		}
 		err = ntfs_map_runlist_nolock(ni, vcn);
 		if (!write_locked) {

fs/ntfs/attrib.h

@@ -66,8 +66,8 @@ extern int ntfs_map_runlist(ntfs_inode *ni, VCN vcn);
 extern LCN ntfs_attr_vcn_to_lcn_nolock(ntfs_inode *ni, const VCN vcn,
 		const BOOL write_locked);
 
-extern runlist_element *ntfs_find_vcn_nolock(ntfs_inode *ni, const VCN vcn,
+extern runlist_element *ntfs_attr_find_vcn_nolock(ntfs_inode *ni,
-		const BOOL write_locked);
+		const VCN vcn, const BOOL write_locked);
 
 int ntfs_attr_lookup(const ATTR_TYPE type, const ntfschar *name,
 		const u32 name_len, const IGNORE_CASE_BOOL ic,

fs/ntfs/lcnalloc.c

@@ -850,7 +850,7 @@ s64 __ntfs_cluster_free(struct inode *vi, const VCN start_vcn, s64 count,
 
 	/* This returns with ni->runlist locked for reading on success. */
 	down_read(&ni->runlist.lock);
-	rl = ntfs_find_vcn_nolock(ni, start_vcn, FALSE);
+	rl = ntfs_attr_find_vcn_nolock(ni, start_vcn, FALSE);
 	if (IS_ERR(rl)) {
 		if (!is_rollback)
 			ntfs_error(vol->sb, "Failed to find first runlist "
@@ -904,7 +904,7 @@ s64 __ntfs_cluster_free(struct inode *vi, const VCN start_vcn, s64 count,
 
 			/* Attempt to map runlist. */
 			vcn = rl->vcn;
-			rl = ntfs_find_vcn_nolock(ni, vcn, FALSE);
+			rl = ntfs_attr_find_vcn_nolock(ni, vcn, FALSE);
 			if (IS_ERR(rl)) {
 				err = PTR_ERR(rl);
 				if (!is_rollback)

fs/ntfs/mft.c

@@ -1297,7 +1297,7 @@ static int ntfs_mft_bitmap_extend_allocation_nolock(ntfs_volume *vol)
 	read_lock_irqsave(&mftbmp_ni->size_lock, flags);
 	ll = mftbmp_ni->allocated_size;
 	read_unlock_irqrestore(&mftbmp_ni->size_lock, flags);
-	rl = ntfs_find_vcn_nolock(mftbmp_ni,
+	rl = ntfs_attr_find_vcn_nolock(mftbmp_ni,
 			(ll - 1) >> vol->cluster_size_bits, TRUE);
 	if (unlikely(IS_ERR(rl) || !rl->length || rl->lcn < 0)) {
 		up_write(&mftbmp_ni->runlist.lock);
@@ -1727,8 +1727,8 @@ static int ntfs_mft_data_extend_allocation_nolock(ntfs_volume *vol)
 	read_lock_irqsave(&mft_ni->size_lock, flags);
 	ll = mft_ni->allocated_size;
 	read_unlock_irqrestore(&mft_ni->size_lock, flags);
-	rl = ntfs_find_vcn_nolock(mft_ni, (ll - 1) >> vol->cluster_size_bits,
+	rl = ntfs_attr_find_vcn_nolock(mft_ni,
-			TRUE);
+			(ll - 1) >> vol->cluster_size_bits, TRUE);
 	if (unlikely(IS_ERR(rl) || !rl->length || rl->lcn < 0)) {
 		up_write(&mft_ni->runlist.lock);
 		ntfs_error(vol->sb, "Failed to determine last allocated "

fs/ntfs/runlist.c

@@ -933,17 +933,18 @@ err_out:
  *
  * It is up to the caller to serialize access to the runlist @rl.
  *
- * Since lcns must be >= 0, we use negative return values with special meaning:
+ * Since lcns must be >= 0, we use negative return codes with special meaning:
  *
- * Return value			Meaning / Description
+ * Return code		Meaning / Description
  * ==================================================
- *  -1 = LCN_HOLE		Hole / not allocated on disk.
+ *  LCN_HOLE		Hole / not allocated on disk.
- *  -2 = LCN_RL_NOT_MAPPED	This is part of the runlist which has not been
+ *  LCN_RL_NOT_MAPPED	This is part of the runlist which has not been
- *				inserted into the runlist yet.
+ *			inserted into the runlist yet.
- *  -3 = LCN_ENOENT		There is no such vcn in the attribute.
+ *  LCN_ENOENT		There is no such vcn in the attribute.
  *
  * Locking: - The caller must have locked the runlist (for reading or writing).
- *	    - This function does not touch the lock.
+ *	    - This function does not touch the lock, nor does it modify the
+ *	      runlist.
  */
 LCN ntfs_rl_vcn_to_lcn(const runlist_element *rl, const VCN vcn)
 {

fs/ntfs/runlist.h

@@ -66,6 +66,8 @@ typedef enum {
 	LCN_HOLE		= -1,	/* Keep this as highest value or die! */
 	LCN_RL_NOT_MAPPED	= -2,
 	LCN_ENOENT		= -3,
+	LCN_ENOMEM		= -4,
+	LCN_EIO			= -5,
 } LCN_SPECIAL_VALUES;
 
 extern runlist_element *ntfs_runlists_merge(runlist_element *drl,