[PATCH 1/3] ext4: remove unused variable in ext4_free_blocks()

[PATCH 1/3] ext4: remove unused variable in ext4_free_blocks()

[Lukas Czerner]

Remove unused variable 'freed' in ext4_free_blocks().

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/mballoc.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 6540ebe..0a493a1 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4464,7 +4464,6 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
 	struct buffer_head *bitmap_bh = NULL;
 	struct super_block *sb = inode->i_sb;
 	struct ext4_group_desc *gdp;
-	unsigned long freed = 0;
 	unsigned int overflow;
 	ext4_grpblk_t bit;
 	struct buffer_head *gd_bh;
@@ -4672,8 +4671,6 @@ do_more:
 
 	ext4_mb_unload_buddy(&e4b);
 
-	freed += count;

[PATCH 2/3] ext4: Use kstrtoul() instead of deprecated simple_strtoul()

[Lukas Czerner]

In parse_strtoul() we're still using deprecated simple_strtoul(). Change
that in favour of kstrtoul().

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/super.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 620cf56..7bcdef1 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2359,14 +2359,12 @@ struct ext4_attr {
 static int parse_strtoul(const char *buf,
 		unsigned long max, unsigned long *value)
 {
-	char *endp;

Re: [PATCH 2/3] ext4: Use kstrtoul() instead of deprecated simple_strtoul()

[Theodore Ts'o]

On Fri, Mar 08, 2013 at 09:23:17AM +0100, Lukas Czerner wrote:
> In parse_strtoul() we're still using deprecated simple_strtoul(). Change
> that in favour of kstrtoul().
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Once you make this change, wouldn't it be better and even more
simplifying to replace the two places where we call parse_strtoul()
with kstrtoul()?

					- Ted

Re: [PATCH 2/3] ext4: Use kstrtoul() instead of deprecated simple_strtoul()

[Lukáš Czerner]

On Sun, 10 Mar 2013, Theodore Ts'o wrote:

> Date: Sun, 10 Mar 2013 22:26:35 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org
> Subject: Re: [PATCH 2/3] ext4: Use kstrtoul() instead of deprecated
>     simple_strtoul()
> 
> On Fri, Mar 08, 2013 at 09:23:17AM +0100, Lukas Czerner wrote:
> > In parse_strtoul() we're still using deprecated simple_strtoul(). Change
> > that in favour of kstrtoul().
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> 
> Once you make this change, wouldn't it be better and even more
> simplifying to replace the two places where we call parse_strtoul()
> with kstrtoul()?
> 
> 					- Ted

I think that parse_strtoul() is still useful to have because we
check the "max" value as well and return -EINVAL if it is exceeded.
Removing it we would have to add the check to the callers
where we're using is now, which seems unnecessary, especially since
we might expect more users of the helper.

-Lukas

Re: [PATCH 2/3] ext4: Use kstrtoul() instead of deprecated simple_strtoul()

[Theodore Ts'o]

On Mon, Mar 11, 2013 at 08:36:16AM +0100, Lukáš Czerner wrote:
> I think that parse_strtoul() is still useful to have because we
> check the "max" value as well and return -EINVAL if it is exceeded.
> Removing it we would have to add the check to the callers
> where we're using is now, which seems unnecessary, especially since
> we might expect more users of the helper.

Well, at the moment only one of the two users of parse_strtoul() is
using the "max" value check feature.  I see one other potential user
of parse_strtoul (there's one use of simple_strtoul in get_sb_block()
which wasn't converted in your patch), but it wouldn't need the max
value check feature either.

So I don't have a super strong feeling about this, since this isn't
performance critical code, and it's probably not going to cost a large
amount of object code or stack space, but sometimes extra levels of
abstraction end up hurting more than they help.

Regards,

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] ext4: Do no use yield() in ext4 code

[Lukas Czerner]

Using yield() is strongly discouraged (see sched/core.c) especially
since we can just use cond_resched().

Replace all use of yield() with cond_resched().

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/inode.c   |    2 +-
 fs/ext4/mballoc.c |    8 ++------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9ea0cde..ab28090 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1263,7 +1263,7 @@ repeat:
 		ei->i_da_metadata_calc_last_lblock = save_last_lblock;
 		spin_unlock(&ei->i_block_reservation_lock);
 		if (ext4_should_retry_alloc(inode->i_sb, &retries)) {
-			yield();
+			cond_resched();
 			goto repeat;
 		}
 		dquot_release_reservation_block(inode, EXT4_C2B(sbi, 1));
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 0a493a1..d2ec0d8 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3692,11 +3692,7 @@ repeat:
 	if (free < needed && busy) {
 		busy = 0;
 		ext4_unlock_group(sb, group);
-		/*
-		 * Yield the CPU here so that we don't get soft lockup
-		 * in non preempt case.
-		 */
-		yield();
+		cond_resched();
 		goto repeat;
 	}
 
@@ -4246,7 +4242,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
 			ext4_claim_free_clusters(sbi, ar->len, ar->flags)) {
 
 			/* let others to free the space */
-			yield();
+			cond_resched();
 			ar->len = ar->len >> 1;
 		}
 		if (!ar->len) {
-- 
1.7.7.6

Re: [PATCH 3/3] ext4: Do no use yield() in ext4 code

[Theodore Ts'o]

On Fri, Mar 08, 2013 at 09:23:18AM +0100, Lukas Czerner wrote:
> Using yield() is strongly discouraged (see sched/core.c) especially
> since we can just use cond_resched().
> 
> Replace all use of yield() with cond_resched().
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Thanks, applied.

						- Ted

Re: [PATCH 1/3] ext4: remove unused variable in ext4_free_blocks()

[Theodore Ts'o]

On Fri, Mar 08, 2013 at 09:23:16AM +0100, Lukas Czerner wrote:
> Remove unused variable 'freed' in ext4_free_blocks().
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Thanks, applied.

					- Ted