[PATCH 0/5] fs/ntfs3: Some fixes and refactoring

[PATCH 0/5] fs/ntfs3: Some fixes and refactoring

[Konstantin Komarov]

5 patches with various fixes and refactoring:
- fix for fragmented case
- remove old code
- fix double locking
- hide internal function
- refactor function

Konstantin Komarov (5):
   fs/ntfs3: Fix very fragmented case in attr_punch_hole
   fs/ntfs3: Remove unused mi_mark_free
   fs/ntfs3: Add new argument is_mft to ntfs_mark_rec_free
   fs/ntfs3: Make static function attr_load_runs
   fs/ntfs3: Fill duplicate info in ni_add_name

  fs/ntfs3/attrib.c  | 17 ++++++++++++++---
  fs/ntfs3/frecord.c | 32 ++++++++++++++++++++------------
  fs/ntfs3/fsntfs.c  |  9 ++++++---
  fs/ntfs3/inode.c   | 12 +-----------
  fs/ntfs3/namei.c   |  2 +-
  fs/ntfs3/ntfs_fs.h |  5 +----
  fs/ntfs3/record.c  | 22 ----------------------
  fs/ntfs3/super.c   |  2 +-
  8 files changed, 44 insertions(+), 57 deletions(-)

-- 
2.37.0

[PATCH 1/5] fs/ntfs3: Fix very fragmented case in attr_punch_hole

[Konstantin Komarov]

In some cases we need to ni_find_attr attr_b

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrib.c | 13 ++++++++++++-
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index 59d8f482ef0a..43b9482f9830 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -2054,6 +2054,7 @@ int attr_punch_hole(struct ntfs_inode *ni, u64 vbo, u64 bytes, u32 *frame_size)
  				if (err)
  					goto out;
  				/* Layout of records maybe changed. */
+				attr_b = NULL;
  			}
  		}
  		/* Free all allocated memory. */
@@ -2073,6 +2074,14 @@ int attr_punch_hole(struct ntfs_inode *ni, u64 vbo, u64 bytes, u32 *frame_size)
  	}
  
  	total_size -= (u64)dealloc << sbi->cluster_bits;
+	if (!attr_b) {
+		attr_b = ni_find_attr(ni, NULL, NULL, ATTR_DATA, NULL, 0, NULL,
+				      &mi_b);
+		if (!attr_b) {
+			err = -EINVAL;
+			goto out;
+		}
+	}
  	attr_b->nres.total_size = cpu_to_le64(total_size);
  	mi_b->dirty = true;
  
@@ -2083,8 +2092,10 @@ int attr_punch_hole(struct ntfs_inode *ni, u64 vbo, u64 bytes, u32 *frame_size)
  
  out:
  	up_write(&ni->file.run_lock);
-	if (err)
+	if (err) {
+		ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
  		make_bad_inode(&ni->vfs_inode);
+	}
  
  	return err;
  }
-- 
2.37.0

[PATCH 2/5] fs/ntfs3: Remove unused mi_mark_free

[Konstantin Komarov]

Cleaning up dead code
Fix wrong comments

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/namei.c   |  2 +-
  fs/ntfs3/ntfs_fs.h |  1 -
  fs/ntfs3/record.c  | 22 ----------------------
  fs/ntfs3/super.c   |  2 +-
  4 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index bc741213ad84..1cc700760c7e 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -208,7 +208,7 @@ static int ntfs_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
  }
  
  /*
- * ntfs_rmdir - inode_operations::rm_dir
+ * ntfs_rmdir - inode_operations::rmdir
   */
  static int ntfs_rmdir(struct inode *dir, struct dentry *dentry)
  {
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 2bc6563601d7..ebe4a8ecc20d 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -734,7 +734,6 @@ static inline struct ATTRIB *rec_find_attr_le(struct mft_inode *rec,
  int mi_write(struct mft_inode *mi, int wait);
  int mi_format_new(struct mft_inode *mi, struct ntfs_sb_info *sbi, CLST rno,
  		  __le16 flags, bool is_mft);
-void mi_mark_free(struct mft_inode *mi);
  struct ATTRIB *mi_insert_attr(struct mft_inode *mi, enum ATTR_TYPE type,
  			      const __le16 *name, u8 name_len, u32 asize,
  			      u16 name_off);
diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 8fe0a876400a..7d2fac5ee215 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -394,28 +394,6 @@ int mi_format_new(struct mft_inode *mi, struct ntfs_sb_info *sbi, CLST rno,
  	return err;
  }
  
-/*
- * mi_mark_free - Mark record as unused and marks it as free in bitmap.
- */
-void mi_mark_free(struct mft_inode *mi)
-{
-	CLST rno = mi->rno;
-	struct ntfs_sb_info *sbi = mi->sbi;
-
-	if (rno >= MFT_REC_RESERVED && rno < MFT_REC_FREE) {
-		ntfs_clear_mft_tail(sbi, rno, rno + 1);
-		mi->dirty = false;
-		return;
-	}
-
-	if (mi->mrec) {
-		clear_rec_inuse(mi->mrec);
-		mi->dirty = true;
-		mi_write(mi, 0);
-	}
-	ntfs_mark_rec_free(sbi, rno);
-}
-
  /*
   * mi_insert_attr - Reserve space for new attribute.
   *
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 4b0dad2ac598..eacea72ff92f 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1377,7 +1377,7 @@ static const struct fs_context_operations ntfs_context_ops = {
  /*
   * ntfs_init_fs_context - Initialize spi and opts
   *
- * This will called when mount/remount. We will first initiliaze
+ * This will called when mount/remount. We will first initialize
   * options so that if remount we can use just that.
   */
  static int ntfs_init_fs_context(struct fs_context *fc)
-- 
2.37.0

[PATCH 3/5] fs/ntfs3: Add new argument is_mft to ntfs_mark_rec_free

[Konstantin Komarov]

This argument helps in avoiding double locking

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/frecord.c | 12 ++++++------
  fs/ntfs3/fsntfs.c  |  9 ++++++---
  fs/ntfs3/inode.c   |  2 +-
  fs/ntfs3/ntfs_fs.h |  2 +-
  4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 64041152fd98..756d9a18fa00 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -1048,7 +1048,7 @@ static int ni_ins_attr_ext(struct ntfs_inode *ni, struct ATTR_LIST_ENTRY *le,
  	err = -EINVAL;
  
  out1:
-	ntfs_mark_rec_free(sbi, rno);
+	ntfs_mark_rec_free(sbi, rno, is_mft);
  
  out:
  	return err;
@@ -1243,7 +1243,7 @@ static int ni_expand_mft_list(struct ntfs_inode *ni)
  		mft_min = mft_new;
  		mi_min = mi_new;
  	} else {
-		ntfs_mark_rec_free(sbi, mft_new);
+		ntfs_mark_rec_free(sbi, mft_new, true);
  		mft_new = 0;
  		ni_remove_mi(ni, mi_new);
  	}
@@ -1326,7 +1326,7 @@ static int ni_expand_mft_list(struct ntfs_inode *ni)
  
  out:
  	if (mft_new) {
-		ntfs_mark_rec_free(sbi, mft_new);
+		ntfs_mark_rec_free(sbi, mft_new, true);
  		ni_remove_mi(ni, mi_new);
  	}
  
@@ -1585,7 +1585,7 @@ int ni_delete_all(struct ntfs_inode *ni)
  		mi->dirty = true;
  		mi_write(mi, 0);
  
-		ntfs_mark_rec_free(sbi, mi->rno);
+		ntfs_mark_rec_free(sbi, mi->rno, false);
  		ni_remove_mi(ni, mi);
  		mi_put(mi);
  		node = next;
@@ -1596,7 +1596,7 @@ int ni_delete_all(struct ntfs_inode *ni)
  	ni->mi.dirty = true;
  	err = mi_write(&ni->mi, 0);
  
-	ntfs_mark_rec_free(sbi, ni->mi.rno);
+	ntfs_mark_rec_free(sbi, ni->mi.rno, false);
  
  	return err;
  }
@@ -3286,7 +3286,7 @@ int ni_write_inode(struct inode *inode, int sync, const char *hint)
  			err = err2;
  
  		if (is_empty) {
-			ntfs_mark_rec_free(sbi, mi->rno);
+			ntfs_mark_rec_free(sbi, mi->rno, false);
  			rb_erase(node, &ni->mi_tree);
  			mi_put(mi);
  		}
diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 3de5700a9b83..c53dd4c9e47b 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -703,12 +703,14 @@ int ntfs_look_free_mft(struct ntfs_sb_info *sbi, CLST *rno, bool mft,
  
  /*
   * ntfs_mark_rec_free - Mark record as free.
+ * is_mft - true if we are changing MFT
   */
-void ntfs_mark_rec_free(struct ntfs_sb_info *sbi, CLST rno)
+void ntfs_mark_rec_free(struct ntfs_sb_info *sbi, CLST rno, bool is_mft)
  {
  	struct wnd_bitmap *wnd = &sbi->mft.bitmap;
  
-	down_write_nested(&wnd->rw_lock, BITMAP_MUTEX_MFT);
+	if (!is_mft)
+		down_write_nested(&wnd->rw_lock, BITMAP_MUTEX_MFT);
  	if (rno >= wnd->nbits)
  		goto out;
  
@@ -727,7 +729,8 @@ void ntfs_mark_rec_free(struct ntfs_sb_info *sbi, CLST rno)
  		sbi->mft.next_free = rno;
  
  out:
-	up_write(&wnd->rw_lock);
+	if (!is_mft)
+		up_write(&wnd->rw_lock);
  }
  
  /*
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 6c78930be035..a49da4ec6dc3 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1636,7 +1636,7 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
  	ni->mi.dirty = false;
  	discard_new_inode(inode);
  out3:
-	ntfs_mark_rec_free(sbi, ino);
+	ntfs_mark_rec_free(sbi, ino, false);
  
  out2:
  	__putname(new_de);
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index ebe4a8ecc20d..5472cde2aa5f 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -590,7 +590,7 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
  			     enum ALLOCATE_OPT opt);
  int ntfs_look_free_mft(struct ntfs_sb_info *sbi, CLST *rno, bool mft,
  		       struct ntfs_inode *ni, struct mft_inode **mi);
-void ntfs_mark_rec_free(struct ntfs_sb_info *sbi, CLST rno);
+void ntfs_mark_rec_free(struct ntfs_sb_info *sbi, CLST rno, bool is_mft);
  int ntfs_clear_mft_tail(struct ntfs_sb_info *sbi, size_t from, size_t to);
  int ntfs_refresh_zone(struct ntfs_sb_info *sbi);
  int ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait);
-- 
2.37.0

[PATCH 4/5] fs/ntfs3: Make static function attr_load_runs

[Konstantin Komarov]

attr_load_runs is an internal function

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrib.c  | 4 ++--
  fs/ntfs3/ntfs_fs.h | 2 --
  2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index 43b9482f9830..68a210bb54fe 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -84,8 +84,8 @@ static inline bool attr_must_be_resident(struct ntfs_sb_info *sbi,
  /*
   * attr_load_runs - Load all runs stored in @attr.
   */
-int attr_load_runs(struct ATTRIB *attr, struct ntfs_inode *ni,
-		   struct runs_tree *run, const CLST *vcn)
+static int attr_load_runs(struct ATTRIB *attr, struct ntfs_inode *ni,
+			  struct runs_tree *run, const CLST *vcn)
  {
  	int err;
  	CLST svcn = le64_to_cpu(attr->nres.svcn);
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 5472cde2aa5f..b88721e48458 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -408,8 +408,6 @@ enum REPARSE_SIGN {
  };
  
  /* Functions from attrib.c */
-int attr_load_runs(struct ATTRIB *attr, struct ntfs_inode *ni,
-		   struct runs_tree *run, const CLST *vcn);
  int attr_allocate_clusters(struct ntfs_sb_info *sbi, struct runs_tree *run,
  			   CLST vcn, CLST lcn, CLST len, CLST *pre_alloc,
  			   enum ALLOCATE_OPT opt, CLST *alen, const size_t fr,
-- 
2.37.0

[PATCH 5/5] fs/ntfs3: Fill duplicate info in ni_add_name

[Konstantin Komarov]

Work with names must be completed in ni_add_name

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/frecord.c | 20 ++++++++++++++------
  fs/ntfs3/inode.c   | 10 ----------
  2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 756d9a18fa00..acd9f444bd64 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -1614,7 +1614,8 @@ struct ATTR_FILE_NAME *ni_fname_name(struct ntfs_inode *ni,
  	struct ATTRIB *attr = NULL;
  	struct ATTR_FILE_NAME *fname;
  
-	*le = NULL;
+	if (le)
+		*le = NULL;
  
  	/* Enumerate all names. */
  next:
@@ -1630,7 +1631,7 @@ struct ATTR_FILE_NAME *ni_fname_name(struct ntfs_inode *ni,
  		goto next;
  
  	if (!uni)
-		goto next;
+		return fname;
  
  	if (uni->len != fname->name_len)
  		goto next;
@@ -2969,7 +2970,7 @@ bool ni_remove_name_undo(struct ntfs_inode *dir_ni, struct ntfs_inode *ni,
  }
  
  /*
- * ni_add_name - Add new name in MFT and in directory.
+ * ni_add_name - Add new name into MFT and into directory.
   */
  int ni_add_name(struct ntfs_inode *dir_ni, struct ntfs_inode *ni,
  		struct NTFS_DE *de)
@@ -2978,13 +2979,20 @@ int ni_add_name(struct ntfs_inode *dir_ni, struct ntfs_inode *ni,
  	struct ATTRIB *attr;
  	struct ATTR_LIST_ENTRY *le;
  	struct mft_inode *mi;
+	struct ATTR_FILE_NAME *fname;
  	struct ATTR_FILE_NAME *de_name = (struct ATTR_FILE_NAME *)(de + 1);
  	u16 de_key_size = le16_to_cpu(de->key_size);
  
  	mi_get_ref(&ni->mi, &de->ref);
  	mi_get_ref(&dir_ni->mi, &de_name->home);
  
-	/* Insert new name in MFT. */
+	/* Fill duplicate from any ATTR_NAME. */
+	fname = ni_fname_name(ni, NULL, NULL, NULL, NULL);
+	if (fname)
+		memcpy(&de_name->dup, &fname->dup, sizeof(fname->dup));
+	de_name->dup.fa = ni->std_fa;
+
+	/* Insert new name into MFT. */
  	err = ni_insert_resident(ni, de_key_size, ATTR_NAME, NULL, 0, &attr,
  				 &mi, &le);
  	if (err)
@@ -2992,7 +3000,7 @@ int ni_add_name(struct ntfs_inode *dir_ni, struct ntfs_inode *ni,
  
  	memcpy(Add2Ptr(attr, SIZEOF_RESIDENT), de_name, de_key_size);
  
-	/* Insert new name in directory. */
+	/* Insert new name into directory. */
  	err = indx_insert_entry(&dir_ni->dir, dir_ni, de, ni->mi.sbi, NULL, 0);
  	if (err)
  		ni_remove_attr_le(ni, attr, mi, le);
@@ -3016,7 +3024,7 @@ int ni_rename(struct ntfs_inode *dir_ni, struct ntfs_inode *new_dir_ni,
  	 * 1) Add new name and remove old name.
  	 * 2) Remove old name and add new name.
  	 *
-	 * In most cases (not all!) adding new name in MFT and in directory can
+	 * In most cases (not all!) adding new name into MFT and into directory can
  	 * allocate additional cluster(s).
  	 * Second way may result to bad inode if we can't add new name
  	 * and then can't restore (add) old name.
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index a49da4ec6dc3..3ed319663747 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1659,7 +1659,6 @@ int ntfs_link_inode(struct inode *inode, struct dentry *dentry)
  	struct ntfs_inode *ni = ntfs_i(inode);
  	struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info;
  	struct NTFS_DE *de;
-	struct ATTR_FILE_NAME *de_name;
  
  	/* Allocate PATH_MAX bytes. */
  	de = __getname();
@@ -1674,15 +1673,6 @@ int ntfs_link_inode(struct inode *inode, struct dentry *dentry)
  	if (err)
  		goto out;
  
-	de_name = (struct ATTR_FILE_NAME *)(de + 1);
-	/* Fill duplicate info. */
-	de_name->dup.cr_time = de_name->dup.m_time = de_name->dup.c_time =
-		de_name->dup.a_time = kernel2nt(&inode->i_ctime);
-	de_name->dup.alloc_size = de_name->dup.data_size =
-		cpu_to_le64(inode->i_size);
-	de_name->dup.fa = ni->std_fa;
-	de_name->dup.ea_size = de_name->dup.reparse = 0;
-
  	err = ni_add_name(ntfs_i(d_inode(dentry->d_parent)), ni, de);
  out:
  	__putname(de);
-- 
2.37.0