All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Make orphan functions no-op in no-journal mode
@ 2012-09-17 17:47 Anatol Pomozov
  2012-09-17 17:47 ` [PATCH 2/2] Extract 'ext4_sb_info' variable Anatol Pomozov
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Anatol Pomozov @ 2012-09-17 17:47 UTC (permalink / raw)
  To: tytso, linux-ext4; +Cc: dmonakhov, Anatol Pomozov

This avoids using shared mutext and thus improves scalability of
no-journal mode. The goal of this change is similar to 3d287de3b828
but it avoids mutex usage for all ext4_orphan_del(NULL,..) calls.

Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
---
 fs/ext4/namei.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 2a42cc0..6863cdf 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2362,6 +2362,8 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
 	struct ext4_iloc iloc;
 	int err = 0, rc;
 
+	if (!EXT4_SB(sb)->s_journal)
+		return 0;
 	if (!ext4_handle_valid(handle))
 		return 0;
 
@@ -2436,6 +2438,8 @@ int ext4_orphan_del(handle_t *handle, struct inode *inode)
 	struct ext4_iloc iloc;
 	int err = 0;
 
+	if (!EXT4_SB(inode->i_sb)->s_journal)
+		return 0;
 	/* ext4_handle_valid() assumes a valid handle_t pointer */
 	if (handle && !ext4_handle_valid(handle))
 		return 0;
@@ -2456,7 +2460,7 @@ int ext4_orphan_del(handle_t *handle, struct inode *inode)
 	 * transaction handle with which to update the orphan list on
 	 * disk, but we still need to remove the inode from the linked
 	 * list in memory. */
-	if (sbi->s_journal && !handle)
+	if (!handle)
 		goto out;
 
 	err = ext4_reserve_inode_write(handle, inode, &iloc);
-- 
1.7.12


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] Extract 'ext4_sb_info' variable
  2012-09-17 17:47 [PATCH 1/2] Make orphan functions no-op in no-journal mode Anatol Pomozov
@ 2012-09-17 17:47 ` Anatol Pomozov
  2012-09-17 17:55 ` [PATCH 1/2] Make orphan functions no-op in no-journal mode Anatol Pomozov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Anatol Pomozov @ 2012-09-17 17:47 UTC (permalink / raw)
  To: tytso, linux-ext4; +Cc: dmonakhov, Anatol Pomozov

It makes the code more compact and consistent. This change is
a refactoring that does not change any functionality.

Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
---
 fs/ext4/namei.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 6863cdf..f9846e4 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2359,15 +2359,16 @@ static int empty_dir(struct inode *inode)
 int ext4_orphan_add(handle_t *handle, struct inode *inode)
 {
 	struct super_block *sb = inode->i_sb;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct ext4_iloc iloc;
 	int err = 0, rc;
 
-	if (!EXT4_SB(sb)->s_journal)
+	if (!sbi->s_journal)
 		return 0;
 	if (!ext4_handle_valid(handle))
 		return 0;
 
-	mutex_lock(&EXT4_SB(sb)->s_orphan_lock);
+	mutex_lock(&sbi->s_orphan_lock);
 	if (!list_empty(&EXT4_I(inode)->i_orphan))
 		goto out_unlock;
 
@@ -2380,8 +2381,8 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
 	J_ASSERT((S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 		  S_ISLNK(inode->i_mode)) || inode->i_nlink == 0);
 
-	BUFFER_TRACE(EXT4_SB(sb)->s_sbh, "get_write_access");
-	err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
+	BUFFER_TRACE(sbi->s_sbh, "get_write_access");
+	err = ext4_journal_get_write_access(handle, sbi->s_sbh);
 	if (err)
 		goto out_unlock;
 
@@ -2393,12 +2394,12 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
 	 * orphan list. If so skip on-disk list modification.
 	 */
 	if (NEXT_ORPHAN(inode) && NEXT_ORPHAN(inode) <=
-		(le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count)))
+		(le32_to_cpu(sbi->s_es->s_inodes_count)))
 			goto mem_insert;
 
 	/* Insert this inode at the head of the on-disk orphan list... */
-	NEXT_ORPHAN(inode) = le32_to_cpu(EXT4_SB(sb)->s_es->s_last_orphan);
-	EXT4_SB(sb)->s_es->s_last_orphan = cpu_to_le32(inode->i_ino);
+	NEXT_ORPHAN(inode) = le32_to_cpu(sbi->s_es->s_last_orphan);
+	sbi->s_es->s_last_orphan = cpu_to_le32(inode->i_ino);
 	err = ext4_handle_dirty_super(handle, sb);
 	rc = ext4_mark_iloc_dirty(handle, inode, &iloc);
 	if (!err)
@@ -2414,13 +2415,13 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
 	 * anyway on the next recovery. */
 mem_insert:
 	if (!err)
-		list_add(&EXT4_I(inode)->i_orphan, &EXT4_SB(sb)->s_orphan);
+		list_add(&EXT4_I(inode)->i_orphan, &sbi->s_orphan);
 
 	jbd_debug(4, "superblock will point to %lu\n", inode->i_ino);
 	jbd_debug(4, "orphan inode %lu will point to %d\n",
 			inode->i_ino, NEXT_ORPHAN(inode));
 out_unlock:
-	mutex_unlock(&EXT4_SB(sb)->s_orphan_lock);
+	mutex_unlock(&sbi->s_orphan_lock);
 	ext4_std_error(inode->i_sb, err);
 	return err;
 }
@@ -2433,24 +2434,23 @@ int ext4_orphan_del(handle_t *handle, struct inode *inode)
 {
 	struct list_head *prev;
 	struct ext4_inode_info *ei = EXT4_I(inode);
-	struct ext4_sb_info *sbi;
+	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
 	__u32 ino_next;
 	struct ext4_iloc iloc;
 	int err = 0;
 
-	if (!EXT4_SB(inode->i_sb)->s_journal)
+	if (!sbi->s_journal)
 		return 0;
 	/* ext4_handle_valid() assumes a valid handle_t pointer */
 	if (handle && !ext4_handle_valid(handle))
 		return 0;
 
-	mutex_lock(&EXT4_SB(inode->i_sb)->s_orphan_lock);
+	mutex_lock(&sbi->s_orphan_lock);
 	if (list_empty(&ei->i_orphan))
 		goto out;
 
 	ino_next = NEXT_ORPHAN(inode);
 	prev = ei->i_orphan.prev;
-	sbi = EXT4_SB(inode->i_sb);
 
 	jbd_debug(4, "remove inode %lu from orphan list\n", inode->i_ino);
 
@@ -2496,7 +2496,7 @@ int ext4_orphan_del(handle_t *handle, struct inode *inode)
 out_err:
 	ext4_std_error(inode->i_sb, err);
 out:
-	mutex_unlock(&EXT4_SB(inode->i_sb)->s_orphan_lock);
+	mutex_unlock(&sbi->s_orphan_lock);
 	return err;
 
 out_brelse:
-- 
1.7.12


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Make orphan functions no-op in no-journal mode
  2012-09-17 17:47 [PATCH 1/2] Make orphan functions no-op in no-journal mode Anatol Pomozov
  2012-09-17 17:47 ` [PATCH 2/2] Extract 'ext4_sb_info' variable Anatol Pomozov
@ 2012-09-17 17:55 ` Anatol Pomozov
  2012-09-18  3:32 ` Theodore Ts'o
  2012-09-18 15:53 ` [PATCH] " Anatol Pomozov
  3 siblings, 0 replies; 6+ messages in thread
From: Anatol Pomozov @ 2012-09-17 17:55 UTC (permalink / raw)
  To: tytso, linux-ext4; +Cc: dmonakhov, Anatol Pomozov

Hi

On Mon, Sep 17, 2012 at 10:47 AM, Anatol Pomozov
<anatol.pomozov@gmail.com> wrote:
> This avoids using shared mutext and thus improves scalability of
> no-journal mode. The goal of this change is similar to 3d287de3b828
> but it avoids mutex usage for all ext4_orphan_del(NULL,..) calls.

This change does the same as 3d287de3b828 but for all
ext4_orphan_del() usages. So 3d287de3b828 can be reverted.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Make orphan functions no-op in no-journal mode
  2012-09-17 17:47 [PATCH 1/2] Make orphan functions no-op in no-journal mode Anatol Pomozov
  2012-09-17 17:47 ` [PATCH 2/2] Extract 'ext4_sb_info' variable Anatol Pomozov
  2012-09-17 17:55 ` [PATCH 1/2] Make orphan functions no-op in no-journal mode Anatol Pomozov
@ 2012-09-18  3:32 ` Theodore Ts'o
  2012-09-18 15:53 ` [PATCH] " Anatol Pomozov
  3 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2012-09-18  3:32 UTC (permalink / raw)
  To: Anatol Pomozov; +Cc: linux-ext4, dmonakhov

On Mon, Sep 17, 2012 at 10:47:16AM -0700, Anatol Pomozov wrote:
> This avoids using shared mutext and thus improves scalability of
> no-journal mode. The goal of this change is similar to 3d287de3b828
> but it avoids mutex usage for all ext4_orphan_del(NULL,..) calls.

It doesn't really improve scalability in no-journal except in the
error case.  This is because....

> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 2a42cc0..6863cdf 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2362,6 +2362,8 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
>  	struct ext4_iloc iloc;
>  	int err = 0, rc;
>  
> +	if (!EXT4_SB(sb)->s_journal)
> +		return 0;
>  	if (!ext4_handle_valid(handle))
>  		return 0;


The two checks above are equivalent.  If (!EXT4_SB(sb)->s_journal),
then ext4_journal_start() will return return an "invalid" handle.  So
this change is purely cosmetic.  I don't object to making the change
for consistency with the change to be made in ext4_orphan_del() below,
but we should remove the !ext4_handle_valid(handle) call in that case.


> @@ -2436,6 +2438,8 @@ int ext4_orphan_del(handle_t *handle, struct inode *inode)
>  	struct ext4_iloc iloc;
>  	int err = 0;
>  
> +	if (!EXT4_SB(inode->i_sb)->s_journal)
> +		return 0;
>  	/* ext4_handle_valid() assumes a valid handle_t pointer */
>  	if (handle && !ext4_handle_valid(handle))
>  		return 0;

We can remove the (handle && !ext4_handle_valid(handle)) conditional
here too.

						- Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] Make orphan functions no-op in no-journal mode
  2012-09-17 17:47 [PATCH 1/2] Make orphan functions no-op in no-journal mode Anatol Pomozov
                   ` (2 preceding siblings ...)
  2012-09-18  3:32 ` Theodore Ts'o
@ 2012-09-18 15:53 ` Anatol Pomozov
  2012-09-18 17:48   ` Theodore Ts'o
  3 siblings, 1 reply; 6+ messages in thread
From: Anatol Pomozov @ 2012-09-18 15:53 UTC (permalink / raw)
  To: tytso, linux-ext4; +Cc: Anatol Pomozov

Instead of checking whether the handle is valid, we check if journal is
enabled. This avoids using shared mutext usage in all cases when
ext4_orphan_del(NULL,..) is called in no-journal mode (i.e. in error
path).

Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
---
 fs/ext4/namei.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 2a42cc0..341ab7e 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2362,7 +2362,7 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
 	struct ext4_iloc iloc;
 	int err = 0, rc;
 
-	if (!ext4_handle_valid(handle))
+	if (!EXT4_SB(sb)->s_journal)
 		return 0;
 
 	mutex_lock(&EXT4_SB(sb)->s_orphan_lock);
@@ -2436,8 +2436,7 @@ int ext4_orphan_del(handle_t *handle, struct inode *inode)
 	struct ext4_iloc iloc;
 	int err = 0;
 
-	/* ext4_handle_valid() assumes a valid handle_t pointer */
-	if (handle && !ext4_handle_valid(handle))
+	if (!EXT4_SB(inode->i_sb)->s_journal)
 		return 0;
 
 	mutex_lock(&EXT4_SB(inode->i_sb)->s_orphan_lock);
@@ -2456,7 +2455,7 @@ int ext4_orphan_del(handle_t *handle, struct inode *inode)
 	 * transaction handle with which to update the orphan list on
 	 * disk, but we still need to remove the inode from the linked
 	 * list in memory. */
-	if (sbi->s_journal && !handle)
+	if (!handle)
 		goto out;
 
 	err = ext4_reserve_inode_write(handle, inode, &iloc);
-- 
1.7.12


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] Make orphan functions no-op in no-journal mode
  2012-09-18 15:53 ` [PATCH] " Anatol Pomozov
@ 2012-09-18 17:48   ` Theodore Ts'o
  0 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2012-09-18 17:48 UTC (permalink / raw)
  To: Anatol Pomozov; +Cc: linux-ext4

On Tue, Sep 18, 2012 at 08:53:05AM -0700, Anatol Pomozov wrote:
> Instead of checking whether the handle is valid, we check if journal is
> enabled. This avoids using shared mutext usage in all cases when
> ext4_orphan_del(NULL,..) is called in no-journal mode (i.e. in error
> path).
> 
> Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>

Thanks, applied.

						- Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-18 17:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-17 17:47 [PATCH 1/2] Make orphan functions no-op in no-journal mode Anatol Pomozov
2012-09-17 17:47 ` [PATCH 2/2] Extract 'ext4_sb_info' variable Anatol Pomozov
2012-09-17 17:55 ` [PATCH 1/2] Make orphan functions no-op in no-journal mode Anatol Pomozov
2012-09-18  3:32 ` Theodore Ts'o
2012-09-18 15:53 ` [PATCH] " Anatol Pomozov
2012-09-18 17:48   ` Theodore Ts'o

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.