linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] fs/ntfs3: Refactor fill_super
@ 2021-09-09 18:09 Kari Argillander
  2021-09-09 18:09 ` [PATCH 01/11] fs/ntfs3: Fix wrong error message $Logfile -> $UpCase Kari Argillander
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Two first ones are only ones that fix something. Everything else
just take off dead code or make it little bit cleaner. They should
be pretty trivial.

As promise to Christian, 10/11 makes one section cleaner as he
wanted [1].

[1]: lore.kernel.org/ntfs3/20210824113217.ncuwc3zs452mdgec@wittgenstein

Kari Argillander (11):
  fs/ntfs3: Fix wrong error message $Logfile -> $UpCase
  fs/ntfs3: Change EINVAL to ENOMEM when d_make_root fails
  fs/ntfs3: Remove impossible fault condition in fill_super
  fs/ntfs3: Return straight without goto in fill_super
  fs/ntfs3: Remove unnecessary variable loading in fill_super
  fs/ntfs3: Use sb instead of sbi->sb in fill_super
  fs/ntfs3: Remove tmp var is_ro in ntfs_fill_super
  fs/ntfs3: Remove tmp pointer bd_inode in fill_super
  fs/ntfs3: Remove tmp pointer upcase in fill_super
  fs/ntfs3: Initialize pointer before use place in fill_super
  fs/ntfs3: Initiliaze sb blocksize only in one place + refactor

 fs/ntfs3/super.c | 121 +++++++++++++----------------------------------
 1 file changed, 33 insertions(+), 88 deletions(-)


base-commit: 15b2ae776044ac52cddda8a3e6c9fecd15226b8c
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/11] fs/ntfs3: Fix wrong error message $Logfile -> $UpCase
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 02/11] fs/ntfs3: Change EINVAL to ENOMEM when d_make_root fails Kari Argillander
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Fix wrong error message $Logfile -> $UpCase. Probably copy paste.

Fixes: 203c2b3a406a ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 3cba0b5e7ac7..c3df29bb21eb 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1203,7 +1203,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	inode = ntfs_iget5(sb, &ref, &NAME_UPCASE);
 	if (IS_ERR(inode)) {
 		err = PTR_ERR(inode);
-		ntfs_err(sb, "Failed to load \x24LogFile.");
+		ntfs_err(sb, "Failed to load $UpCase.");
 		inode = NULL;
 		goto out;
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/11] fs/ntfs3: Change EINVAL to ENOMEM when d_make_root fails
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
  2021-09-09 18:09 ` [PATCH 01/11] fs/ntfs3: Fix wrong error message $Logfile -> $UpCase Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 03/11] fs/ntfs3: Remove impossible fault condition in fill_super Kari Argillander
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Change EINVAL to ENOMEM when d_make_root fails because that is right
errno.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index c3df29bb21eb..6ddc0051fe73 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1286,7 +1286,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_root = d_make_root(inode);
 
 	if (!sb->s_root) {
-		err = -EINVAL;
+		err = -ENOMEM;
 		goto out;
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/11] fs/ntfs3: Remove impossible fault condition in fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
  2021-09-09 18:09 ` [PATCH 01/11] fs/ntfs3: Fix wrong error message $Logfile -> $UpCase Kari Argillander
  2021-09-09 18:09 ` [PATCH 02/11] fs/ntfs3: Change EINVAL to ENOMEM when d_make_root fails Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 04/11] fs/ntfs3: Return straight without goto " Kari Argillander
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Remove root drop when we fault out. This can never happened because
when we allocate root we eather fault when no root or success.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 6ddc0051fe73..793c064833c2 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1297,12 +1297,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 out:
 	iput(inode);
-
-	if (sb->s_root) {
-		d_drop(sb->s_root);
-		sb->s_root = NULL;
-	}
-
 	return err;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/11] fs/ntfs3: Return straight without goto in fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (2 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 03/11] fs/ntfs3: Remove impossible fault condition in fill_super Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 05/11] fs/ntfs3: Remove unnecessary variable loading " Kari Argillander
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

In many places it is not needed to use goto out. We can just return
right away. This will make code little bit more cleaner as we won't
need to check error path.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 56 ++++++++++++++----------------------------------
 1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 793c064833c2..f3c3c2bea6ca 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -923,7 +923,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	err = ntfs_init_from_boot(sb, rq ? queue_logical_block_size(rq) : 512,
 				  bd_inode->i_size);
 	if (err)
-		goto out;
+		return err;
 
 #ifdef CONFIG_NTFS3_64BIT_CLUSTER
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
@@ -939,10 +939,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	ref.seq = cpu_to_le16(MFT_REC_VOL);
 	inode = ntfs_iget5(sb, &ref, &NAME_VOLUME);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load $Volume.");
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	ni = ntfs_i(inode);
@@ -990,10 +988,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	ref.seq = cpu_to_le16(MFT_REC_MIRR);
 	inode = ntfs_iget5(sb, &ref, &NAME_MIRROR);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load $MFTMirr.");
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	sbi->mft.recs_mirr =
@@ -1006,10 +1002,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	ref.seq = cpu_to_le16(MFT_REC_LOG);
 	inode = ntfs_iget5(sb, &ref, &NAME_LOGFILE);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load \x24LogFile.");
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	ni = ntfs_i(inode);
@@ -1027,16 +1021,14 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		if (!is_ro) {
 			ntfs_warn(sb,
 				  "failed to replay log file. Can't mount rw!");
-			err = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
 	} else if (sbi->volume.flags & VOLUME_FLAG_DIRTY) {
 		if (!is_ro && !sbi->options->force) {
 			ntfs_warn(
 				sb,
 				"volume is dirty and \"force\" flag is not set!");
-			err = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
 	}
 
@@ -1046,10 +1038,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	inode = ntfs_iget5(sb, &ref, &NAME_MFT);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load $MFT.");
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	ni = ntfs_i(inode);
@@ -1073,10 +1063,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	ref.seq = cpu_to_le16(MFT_REC_BADCLUST);
 	inode = ntfs_iget5(sb, &ref, &NAME_BADCLUS);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load $BadClus.");
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	ni = ntfs_i(inode);
@@ -1098,10 +1086,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	ref.seq = cpu_to_le16(MFT_REC_BITMAP);
 	inode = ntfs_iget5(sb, &ref, &NAME_BITMAP);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load $Bitmap.");
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	ni = ntfs_i(inode);
@@ -1131,17 +1117,15 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	/* Compute the MFT zone. */
 	err = ntfs_refresh_zone(sbi);
 	if (err)
-		goto out;
+		return err;
 
 	/* Load $AttrDef. */
 	ref.low = cpu_to_le32(MFT_REC_ATTR);
 	ref.seq = cpu_to_le16(MFT_REC_ATTR);
 	inode = ntfs_iget5(sbi->sb, &ref, &NAME_ATTRDEF);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load $AttrDef -> %d", err);
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	if (inode->i_size < sizeof(struct ATTR_DEF_ENTRY)) {
@@ -1202,10 +1186,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	ref.seq = cpu_to_le16(MFT_REC_UPCASE);
 	inode = ntfs_iget5(sb, &ref, &NAME_UPCASE);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load $UpCase.");
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	ni = ntfs_i(inode);
@@ -1251,7 +1233,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		/* Load $Secure. */
 		err = ntfs_security_init(sbi);
 		if (err)
-			goto out;
+			return err;
 
 		/* Load $Extend. */
 		err = ntfs_extend_init(sbi);
@@ -1275,26 +1257,20 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	ref.seq = cpu_to_le16(MFT_REC_ROOT);
 	inode = ntfs_iget5(sb, &ref, &NAME_ROOT);
 	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
 		ntfs_err(sb, "Failed to load root.");
-		inode = NULL;
-		goto out;
+		return PTR_ERR(inode);
 	}
 
 	ni = ntfs_i(inode);
 
 	sb->s_root = d_make_root(inode);
-
-	if (!sb->s_root) {
-		err = -ENOMEM;
-		goto out;
-	}
+	if (!sb->s_root)
+		return -ENOMEM;
 
 	fc->fs_private = NULL;
 	fc->s_fs_info = NULL;
 
 	return 0;
-
 out:
 	iput(inode);
 	return err;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/11] fs/ntfs3: Remove unnecessary variable loading in fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (3 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 04/11] fs/ntfs3: Return straight without goto " Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 06/11] fs/ntfs3: Use sb instead of sbi->sb " Kari Argillander
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Remove some unnecessary variable loading. These look like copy paste
work and they are not used to anything.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index f3c3c2bea6ca..2eb1227bbf5a 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -879,7 +879,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	struct block_device *bdev = sb->s_bdev;
 	struct inode *bd_inode = bdev->bd_inode;
 	struct request_queue *rq = bdev_get_queue(bdev);
-	struct inode *inode = NULL;
+	struct inode *inode;
 	struct ntfs_inode *ni;
 	size_t i, tt;
 	CLST vcn, lcn, len;
@@ -979,9 +979,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sbi->volume.major_ver = info->major_ver;
 	sbi->volume.minor_ver = info->minor_ver;
 	sbi->volume.flags = info->flags;
-
 	sbi->volume.ni = ni;
-	inode = NULL;
 
 	/* Load $MFTMirr to estimate recs_mirr. */
 	ref.low = cpu_to_le32(MFT_REC_MIRR);
@@ -1013,7 +1011,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		goto out;
 
 	iput(inode);
-	inode = NULL;
 
 	is_ro = sb_rdonly(sbi->sb);
 
@@ -1090,8 +1087,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		return PTR_ERR(inode);
 	}
 
-	ni = ntfs_i(inode);
-
 #ifndef CONFIG_NTFS3_64BIT_CLUSTER
 	if (inode->i_size >> 32) {
 		err = -EINVAL;
@@ -1190,8 +1185,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		return PTR_ERR(inode);
 	}
 
-	ni = ntfs_i(inode);
-
 	if (inode->i_size != 0x10000 * sizeof(short)) {
 		err = -EINVAL;
 		goto out;
@@ -1227,7 +1220,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	}
 
 	iput(inode);
-	inode = NULL;
 
 	if (is_ntfs3(sbi)) {
 		/* Load $Secure. */
@@ -1261,8 +1253,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		return PTR_ERR(inode);
 	}
 
-	ni = ntfs_i(inode);
-
 	sb->s_root = d_make_root(inode);
 	if (!sb->s_root)
 		return -ENOMEM;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/11] fs/ntfs3: Use sb instead of sbi->sb in fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (4 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 05/11] fs/ntfs3: Remove unnecessary variable loading " Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 07/11] fs/ntfs3: Remove tmp var is_ro in ntfs_fill_super Kari Argillander
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Use sb instead of sbi->sb in fill_super. We have sb so why not use
it. Also makes code more readable.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 2eb1227bbf5a..efe12c45e421 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1012,7 +1012,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	iput(inode);
 
-	is_ro = sb_rdonly(sbi->sb);
+	is_ro = sb_rdonly(sb);
 
 	if (sbi->flags & NTFS_FLAGS_NEED_REPLAY) {
 		if (!is_ro) {
@@ -1103,7 +1103,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* Not necessary. */
 	sbi->used.bitmap.set_tail = true;
-	err = wnd_init(&sbi->used.bitmap, sbi->sb, tt);
+	err = wnd_init(&sbi->used.bitmap, sb, tt);
 	if (err)
 		goto out;
 
@@ -1117,7 +1117,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	/* Load $AttrDef. */
 	ref.low = cpu_to_le32(MFT_REC_ATTR);
 	ref.seq = cpu_to_le16(MFT_REC_ATTR);
-	inode = ntfs_iget5(sbi->sb, &ref, &NAME_ATTRDEF);
+	inode = ntfs_iget5(sb, &ref, &NAME_ATTRDEF);
 	if (IS_ERR(inode)) {
 		ntfs_err(sb, "Failed to load $AttrDef -> %d", err);
 		return PTR_ERR(inode);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/11] fs/ntfs3: Remove tmp var is_ro in ntfs_fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (5 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 06/11] fs/ntfs3: Use sb instead of sbi->sb " Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 08/11] fs/ntfs3: Remove tmp pointer bd_inode in fill_super Kari Argillander
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

We only use this in two places so we do not really need it. Also
wrapper sb_rdonly() is pretty self explanatory. This will make little
bit easier to read this super long variable list in the beginning of
ntfs_fill_super().

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index efe12c45e421..8022149f6b88 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -889,7 +889,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	struct ATTR_DEF_ENTRY *t;
 	u16 *upcase;
 	u16 *shared;
-	bool is_ro;
 	struct MFT_REF ref;
 
 	ref.high = 0;
@@ -1012,16 +1011,14 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	iput(inode);
 
-	is_ro = sb_rdonly(sb);
-
 	if (sbi->flags & NTFS_FLAGS_NEED_REPLAY) {
-		if (!is_ro) {
+		if (!sb_rdonly(sb)) {
 			ntfs_warn(sb,
 				  "failed to replay log file. Can't mount rw!");
 			return -EINVAL;
 		}
 	} else if (sbi->volume.flags & VOLUME_FLAG_DIRTY) {
-		if (!is_ro && !sbi->options->force) {
+		if (!sb_rdonly(sb) && !sbi->options->force) {
 			ntfs_warn(
 				sb,
 				"volume is dirty and \"force\" flag is not set!");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/11] fs/ntfs3: Remove tmp pointer bd_inode in fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (6 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 07/11] fs/ntfs3: Remove tmp var is_ro in ntfs_fill_super Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 09/11] fs/ntfs3: Remove tmp pointer upcase " Kari Argillander
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Drop tmp pointer bd_inode because this is only used ones in fill_super.
Also we have so many initializing happening at the beginning that it is
already way too much to follow.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 8022149f6b88..14cb90a4c133 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -877,7 +877,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	int err;
 	struct ntfs_sb_info *sbi = sb->s_fs_info;
 	struct block_device *bdev = sb->s_bdev;
-	struct inode *bd_inode = bdev->bd_inode;
 	struct request_queue *rq = bdev_get_queue(bdev);
 	struct inode *inode;
 	struct ntfs_inode *ni;
@@ -920,7 +919,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* Parse boot. */
 	err = ntfs_init_from_boot(sb, rq ? queue_logical_block_size(rq) : 512,
-				  bd_inode->i_size);
+				  bdev->bd_inode->i_size);
 	if (err)
 		return err;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/11] fs/ntfs3: Remove tmp pointer upcase in fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (7 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 08/11] fs/ntfs3: Remove tmp pointer bd_inode in fill_super Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 10/11] fs/ntfs3: Initialize pointer before use place " Kari Argillander
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

We can survive without this tmp point upcase. So remove it we don't have
so many tmp pointer in this function.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 14cb90a4c133..9a096be32fb2 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -886,7 +886,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	const struct VOLUME_INFO *info;
 	u32 idx, done, bytes;
 	struct ATTR_DEF_ENTRY *t;
-	u16 *upcase;
 	u16 *shared;
 	struct MFT_REF ref;
 
@@ -1186,11 +1185,9 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		goto out;
 	}
 
-	upcase = sbi->upcase;
-
 	for (idx = 0; idx < (0x10000 * sizeof(short) >> PAGE_SHIFT); idx++) {
 		const __le16 *src;
-		u16 *dst = Add2Ptr(upcase, idx << PAGE_SHIFT);
+		u16 *dst = Add2Ptr(sbi->upcase, idx << PAGE_SHIFT);
 		struct page *page = ntfs_map_page(inode->i_mapping, idx);
 
 		if (IS_ERR(page)) {
@@ -1209,10 +1206,10 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		ntfs_unmap_page(page);
 	}
 
-	shared = ntfs_set_shared(upcase, 0x10000 * sizeof(short));
-	if (shared && upcase != shared) {
+	shared = ntfs_set_shared(sbi->upcase, 0x10000 * sizeof(short));
+	if (shared && sbi->upcase != shared) {
+		kvfree(sbi->upcase);
 		sbi->upcase = shared;
-		kvfree(upcase);
 	}
 
 	iput(inode);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/11] fs/ntfs3: Initialize pointer before use place in fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (8 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 09/11] fs/ntfs3: Remove tmp pointer upcase " Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-09 18:09 ` [PATCH 11/11] fs/ntfs3: Initiliaze sb blocksize only in one place + refactor Kari Argillander
  2021-09-20 16:07 ` [PATCH 00/11] fs/ntfs3: Refactor fill_super Konstantin Komarov
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Initializing should be as close as possible when we use it so that
we do not need to scroll up to see what is happening.

Also bdev_get_queue() can never return NULL so we do not need to check
for !rq.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 9a096be32fb2..321fcfce64e1 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -877,7 +877,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	int err;
 	struct ntfs_sb_info *sbi = sb->s_fs_info;
 	struct block_device *bdev = sb->s_bdev;
-	struct request_queue *rq = bdev_get_queue(bdev);
+	struct request_queue *rq;
 	struct inode *inode;
 	struct ntfs_inode *ni;
 	size_t i, tt;
@@ -906,9 +906,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		return -EINVAL;
 	}
 
-	if (!rq || !blk_queue_discard(rq) || !rq->limits.discard_granularity) {
-		;
-	} else {
+	rq = bdev_get_queue(bdev);
+	if (blk_queue_discard(rq) && rq->limits.discard_granularity) {
 		sbi->discard_granularity = rq->limits.discard_granularity;
 		sbi->discard_granularity_mask_inv =
 			~(u64)(sbi->discard_granularity - 1);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 11/11] fs/ntfs3: Initiliaze sb blocksize only in one place + refactor
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (9 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 10/11] fs/ntfs3: Initialize pointer before use place " Kari Argillander
@ 2021-09-09 18:09 ` Kari Argillander
  2021-09-20 16:07 ` [PATCH 00/11] fs/ntfs3: Refactor fill_super Konstantin Komarov
  11 siblings, 0 replies; 13+ messages in thread
From: Kari Argillander @ 2021-09-09 18:09 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3
  Cc: Kari Argillander, linux-kernel, Christian Brauner

Right now sb blocksize first get initiliazed in fill_super but in can be
changed in helper function. It makes more sense to that this happened
only in one place.

Because we move this to helper function it makes more sense that
s_maxbytes will also be there. I rather have every sb releted thing in
fill_super, but because there is already sb releted stuff in this
helper. This will have to do for now.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
I really would like to initialize all sb stuff in fill_super and I try
it but it did not work out so well. Maybe if we do more refactoring
then maybe, but this should still be better that these can be found in
one place.

---
 fs/ntfs3/super.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 321fcfce64e1..9ad04fcf535a 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -842,8 +842,7 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size,
 	rec->total = cpu_to_le32(sbi->record_size);
 	((struct ATTRIB *)Add2Ptr(rec, ao))->type = ATTR_END;
 
-	if (sbi->cluster_size < PAGE_SIZE)
-		sb_set_blocksize(sb, sbi->cluster_size);
+	sb_set_blocksize(sb, min_t(u32, sbi->cluster_size, PAGE_SIZE));
 
 	sbi->block_mask = sb->s_blocksize - 1;
 	sbi->blocks_per_cluster = sbi->cluster_size >> sb->s_blocksize_bits;
@@ -856,9 +855,11 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size,
 	if (clusters >= (1ull << (64 - sbi->cluster_bits)))
 		sbi->maxbytes = -1;
 	sbi->maxbytes_sparse = -1;
+	sb->s_maxbytes = MAX_LFS_FILESIZE;
 #else
 	/* Maximum size for sparse file. */
 	sbi->maxbytes_sparse = (1ull << (sbi->cluster_bits + 32)) - 1;
+	sb->s_maxbytes = 0xFFFFFFFFull << sbi->cluster_bits;
 #endif
 
 	err = 0;
@@ -913,20 +914,12 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 			~(u64)(sbi->discard_granularity - 1);
 	}
 
-	sb_set_blocksize(sb, PAGE_SIZE);
-
 	/* Parse boot. */
 	err = ntfs_init_from_boot(sb, rq ? queue_logical_block_size(rq) : 512,
 				  bdev->bd_inode->i_size);
 	if (err)
 		return err;
 
-#ifdef CONFIG_NTFS3_64BIT_CLUSTER
-	sb->s_maxbytes = MAX_LFS_FILESIZE;
-#else
-	sb->s_maxbytes = 0xFFFFFFFFull << sbi->cluster_bits;
-#endif
-
 	/*
 	 * Load $Volume. This should be done before $LogFile
 	 * 'cause 'sbi->volume.ni' is used 'ntfs_set_state'.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 00/11] fs/ntfs3: Refactor fill_super
  2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
                   ` (10 preceding siblings ...)
  2021-09-09 18:09 ` [PATCH 11/11] fs/ntfs3: Initiliaze sb blocksize only in one place + refactor Kari Argillander
@ 2021-09-20 16:07 ` Konstantin Komarov
  11 siblings, 0 replies; 13+ messages in thread
From: Konstantin Komarov @ 2021-09-20 16:07 UTC (permalink / raw)
  To: Kari Argillander, ntfs3; +Cc: linux-kernel, Christian Brauner



On 09.09.2021 21:09, Kari Argillander wrote:
> Two first ones are only ones that fix something. Everything else
> just take off dead code or make it little bit cleaner. They should
> be pretty trivial.
> 
> As promise to Christian, 10/11 makes one section cleaner as he
> wanted [1].
> 
> [1]: lore.kernel.org/ntfs3/20210824113217.ncuwc3zs452mdgec@wittgenstein
> 
> Kari Argillander (11):
>   fs/ntfs3: Fix wrong error message $Logfile -> $UpCase
>   fs/ntfs3: Change EINVAL to ENOMEM when d_make_root fails
>   fs/ntfs3: Remove impossible fault condition in fill_super
>   fs/ntfs3: Return straight without goto in fill_super
>   fs/ntfs3: Remove unnecessary variable loading in fill_super
>   fs/ntfs3: Use sb instead of sbi->sb in fill_super
>   fs/ntfs3: Remove tmp var is_ro in ntfs_fill_super
>   fs/ntfs3: Remove tmp pointer bd_inode in fill_super
>   fs/ntfs3: Remove tmp pointer upcase in fill_super
>   fs/ntfs3: Initialize pointer before use place in fill_super
>   fs/ntfs3: Initiliaze sb blocksize only in one place + refactor
> 
>  fs/ntfs3/super.c | 121 +++++++++++++----------------------------------
>  1 file changed, 33 insertions(+), 88 deletions(-)
> 
> 
> base-commit: 15b2ae776044ac52cddda8a3e6c9fecd15226b8c
> 

Thanks for work - applied!

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-09-20 16:07 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09 18:09 [PATCH 00/11] fs/ntfs3: Refactor fill_super Kari Argillander
2021-09-09 18:09 ` [PATCH 01/11] fs/ntfs3: Fix wrong error message $Logfile -> $UpCase Kari Argillander
2021-09-09 18:09 ` [PATCH 02/11] fs/ntfs3: Change EINVAL to ENOMEM when d_make_root fails Kari Argillander
2021-09-09 18:09 ` [PATCH 03/11] fs/ntfs3: Remove impossible fault condition in fill_super Kari Argillander
2021-09-09 18:09 ` [PATCH 04/11] fs/ntfs3: Return straight without goto " Kari Argillander
2021-09-09 18:09 ` [PATCH 05/11] fs/ntfs3: Remove unnecessary variable loading " Kari Argillander
2021-09-09 18:09 ` [PATCH 06/11] fs/ntfs3: Use sb instead of sbi->sb " Kari Argillander
2021-09-09 18:09 ` [PATCH 07/11] fs/ntfs3: Remove tmp var is_ro in ntfs_fill_super Kari Argillander
2021-09-09 18:09 ` [PATCH 08/11] fs/ntfs3: Remove tmp pointer bd_inode in fill_super Kari Argillander
2021-09-09 18:09 ` [PATCH 09/11] fs/ntfs3: Remove tmp pointer upcase " Kari Argillander
2021-09-09 18:09 ` [PATCH 10/11] fs/ntfs3: Initialize pointer before use place " Kari Argillander
2021-09-09 18:09 ` [PATCH 11/11] fs/ntfs3: Initiliaze sb blocksize only in one place + refactor Kari Argillander
2021-09-20 16:07 ` [PATCH 00/11] fs/ntfs3: Refactor fill_super Konstantin Komarov

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).