Notify user when mounting the file system with -o discard option, but
the device does not support discard. Obviously we do not want to fail
the mount or disable the options, because the underlying device might
change in future even without file system remount.
Signed-off-by: Lukas Czerner <[email protected]>
---
fs/ext4/super.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 7265a03..fd3ff41 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4017,6 +4017,14 @@ no_journal:
}
#endif /* CONFIG_QUOTA */
+ if (test_opt(sb, DISCARD)) {
+ struct request_queue *q = bdev_get_queue(sb->s_bdev);
+ if (!blk_queue_discard(q))
+ ext4_msg(sb, KERN_WARNING,
+ "mounting with \"discard\" option, but "
+ "the device does not support discard");
+ }
+
ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "
"Opts: %s%s%s", descr, sbi->s_es->s_mount_opts,
*sbi->s_es->s_mount_opts ? "; " : "", orig_data);
--
1.7.7.6
We should warn user then the discard request fails. However we need to
exclude -EOPNOTSUPP case since parts of the device might not support it
while other parts can. So print the kernel warning when the error !=
-EOPNOTSUPP is returned from ext4_issue_discard().
We should also handle error cases in batched discard, again excluding
EOPNOTSUPP.
Signed-off-by: Lukas Czerner <[email protected]>
---
fs/ext4/mballoc.c | 47 +++++++++++++++++++++++++++++++++++------------
1 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index f8b27bf..cbab0c8 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2607,9 +2607,17 @@ static void ext4_free_data_callback(struct super_block *sb,
mb_debug(1, "gonna free %u blocks in group %u (0x%p):",
entry->efd_count, entry->efd_group, entry);
- if (test_opt(sb, DISCARD))
- ext4_issue_discard(sb, entry->efd_group,
- entry->efd_start_cluster, entry->efd_count);
+ if (test_opt(sb, DISCARD)) {
+ err = ext4_issue_discard(sb, entry->efd_group,
+ entry->efd_start_cluster,
+ entry->efd_count);
+ if (err && err != -EOPNOTSUPP)
+ ext4_msg(sb, KERN_WARNING, "discard request in"
+ " group:%d block:%d count:%d failed"
+ " with %d", entry->efd_group,
+ entry->efd_start_cluster,
+ entry->efd_count, err);
+ }
err = ext4_mb_load_buddy(sb, entry->efd_group, &e4b);
/* we expect to find existing buddy because it's pinned */
@@ -4657,8 +4665,16 @@ do_more:
* with group lock held. generate_buddy look at
* them with group lock_held
*/
- if (test_opt(sb, DISCARD))
- ext4_issue_discard(sb, block_group, bit, count);
+ if (test_opt(sb, DISCARD)) {
+ err = ext4_issue_discard(sb, block_group, bit, count);
+ if (err && err != -EOPNOTSUPP)
+ ext4_msg(sb, KERN_WARNING, "discard request in"
+ " group:%d block:%d count:%lu failed"
+ " with %d", block_group, bit, count,
+ err);
+ }
+
+
ext4_lock_group(sb, block_group);
mb_clear_bits(bitmap_bh->b_data, bit, count_clusters);
mb_free_blocks(inode, &e4b, bit, count_clusters);
@@ -4854,10 +4870,11 @@ error_return:
* one will allocate those blocks, mark it as used in buddy bitmap. This must
* be called with under the group lock.
*/
-static void ext4_trim_extent(struct super_block *sb, int start, int count,
+static int ext4_trim_extent(struct super_block *sb, int start, int count,
ext4_group_t group, struct ext4_buddy *e4b)
{
struct ext4_free_extent ex;
+ int ret = 0;
trace_ext4_trim_extent(sb, group, start, count);
@@ -4873,9 +4890,10 @@ static void ext4_trim_extent(struct super_block *sb, int start, int count,
*/
mb_mark_used(e4b, &ex);
ext4_unlock_group(sb, group);
- ext4_issue_discard(sb, group, start, count);
+ ret = ext4_issue_discard(sb, group, start, count);
ext4_lock_group(sb, group);
mb_free_blocks(NULL, e4b, start, ex.fe_len);
+ return ret;
}
/**
@@ -4904,7 +4922,7 @@ ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
void *bitmap;
ext4_grpblk_t next, count = 0, free_count = 0;
struct ext4_buddy e4b;
- int ret;
+ int ret = 0;
trace_ext4_trim_all_free(sb, group, start, max);
@@ -4931,8 +4949,11 @@ ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
next = mb_find_next_bit(bitmap, max + 1, start);
if ((next - start) >= minblocks) {
- ext4_trim_extent(sb, start,
- next - start, group, &e4b);
+ ret = ext4_trim_extent(sb, start,
+ next - start, group, &e4b);
+ if (ret && ret != -EOPNOTSUPP)
+ break;
+ ret = 0;
count += next - start;
}
free_count += next - start;
@@ -4953,8 +4974,10 @@ ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
break;
}
- if (!ret)
+ if (!ret) {
+ ret = count;
EXT4_MB_GRP_SET_TRIMMED(e4b.bd_info);
+ }
out:
ext4_unlock_group(sb, group);
ext4_mb_unload_buddy(&e4b);
@@ -4962,7 +4985,7 @@ out:
ext4_debug("trimmed %d blocks in the group %d\n",
count, group);
- return count;
+ return ret;
}
/**
--
1.7.7.6
On Fri, Oct 19, 2012 at 02:12:37PM +0200, Lukas Czerner wrote:
> Notify user when mounting the file system with -o discard option, but
> the device does not support discard. Obviously we do not want to fail
> the mount or disable the options, because the underlying device might
> change in future even without file system remount.
>
> Signed-off-by: Lukas Czerner <[email protected]>
> ---
> fs/ext4/super.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 7265a03..fd3ff41 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4017,6 +4017,14 @@ no_journal:
> }
> #endif /* CONFIG_QUOTA */
>
> + if (test_opt(sb, DISCARD)) {
> + struct request_queue *q = bdev_get_queue(sb->s_bdev);
> + if (!blk_queue_discard(q))
> + ext4_msg(sb, KERN_WARNING,
> + "mounting with \"discard\" option, but "
> + "the device does not support discard");
> + }
> +
> ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "
> "Opts: %s%s%s", descr, sbi->s_es->s_mount_opts,
> *sbi->s_es->s_mount_opts ? "; " : "", orig_data);
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks good to me
Reviewed-by: Carlos Maiolino <[email protected]>
--
--Carlos
On Fri, Oct 19, 2012 at 02:12:38PM +0200, Lukas Czerner wrote:
> We should warn user then the discard request fails. However we need to
> exclude -EOPNOTSUPP case since parts of the device might not support it
> while other parts can. So print the kernel warning when the error !=
> -EOPNOTSUPP is returned from ext4_issue_discard().
>
> We should also handle error cases in batched discard, again excluding
> EOPNOTSUPP.
>
> Signed-off-by: Lukas Czerner <[email protected]>
> ---
> fs/ext4/mballoc.c | 47 +++++++++++++++++++++++++++++++++++------------
> 1 files changed, 35 insertions(+), 12 deletions(-)
Looks good
Reviewed-by: Carlos Maiolino <[email protected]>
--
--Carlos
On Fri, 19 Oct 2012, Lukas Czerner wrote:
> Date: Fri, 19 Oct 2012 14:12:38 +0200
> From: Lukas Czerner <[email protected]>
> To: [email protected]
> Cc: [email protected], Lukas Czerner <[email protected]>
> Subject: [PATCH 2/2] ext4: Warn when discard request fails other than
> EOPNOTSUPP
>
> We should warn user then the discard request fails. However we need to
> exclude -EOPNOTSUPP case since parts of the device might not support it
> while other parts can. So print the kernel warning when the error !=
> -EOPNOTSUPP is returned from ext4_issue_discard().
>
> We should also handle error cases in batched discard, again excluding
> EOPNOTSUPP.
ping to the series. Any comments here ?
Thanks!
-Lukas
>
> Signed-off-by: Lukas Czerner <[email protected]>
> ---
> fs/ext4/mballoc.c | 47 +++++++++++++++++++++++++++++++++++------------
> 1 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index f8b27bf..cbab0c8 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2607,9 +2607,17 @@ static void ext4_free_data_callback(struct super_block *sb,
> mb_debug(1, "gonna free %u blocks in group %u (0x%p):",
> entry->efd_count, entry->efd_group, entry);
>
> - if (test_opt(sb, DISCARD))
> - ext4_issue_discard(sb, entry->efd_group,
> - entry->efd_start_cluster, entry->efd_count);
> + if (test_opt(sb, DISCARD)) {
> + err = ext4_issue_discard(sb, entry->efd_group,
> + entry->efd_start_cluster,
> + entry->efd_count);
> + if (err && err != -EOPNOTSUPP)
> + ext4_msg(sb, KERN_WARNING, "discard request in"
> + " group:%d block:%d count:%d failed"
> + " with %d", entry->efd_group,
> + entry->efd_start_cluster,
> + entry->efd_count, err);
> + }
>
> err = ext4_mb_load_buddy(sb, entry->efd_group, &e4b);
> /* we expect to find existing buddy because it's pinned */
> @@ -4657,8 +4665,16 @@ do_more:
> * with group lock held. generate_buddy look at
> * them with group lock_held
> */
> - if (test_opt(sb, DISCARD))
> - ext4_issue_discard(sb, block_group, bit, count);
> + if (test_opt(sb, DISCARD)) {
> + err = ext4_issue_discard(sb, block_group, bit, count);
> + if (err && err != -EOPNOTSUPP)
> + ext4_msg(sb, KERN_WARNING, "discard request in"
> + " group:%d block:%d count:%lu failed"
> + " with %d", block_group, bit, count,
> + err);
> + }
> +
> +
> ext4_lock_group(sb, block_group);
> mb_clear_bits(bitmap_bh->b_data, bit, count_clusters);
> mb_free_blocks(inode, &e4b, bit, count_clusters);
> @@ -4854,10 +4870,11 @@ error_return:
> * one will allocate those blocks, mark it as used in buddy bitmap. This must
> * be called with under the group lock.
> */
> -static void ext4_trim_extent(struct super_block *sb, int start, int count,
> +static int ext4_trim_extent(struct super_block *sb, int start, int count,
> ext4_group_t group, struct ext4_buddy *e4b)
> {
> struct ext4_free_extent ex;
> + int ret = 0;
>
> trace_ext4_trim_extent(sb, group, start, count);
>
> @@ -4873,9 +4890,10 @@ static void ext4_trim_extent(struct super_block *sb, int start, int count,
> */
> mb_mark_used(e4b, &ex);
> ext4_unlock_group(sb, group);
> - ext4_issue_discard(sb, group, start, count);
> + ret = ext4_issue_discard(sb, group, start, count);
> ext4_lock_group(sb, group);
> mb_free_blocks(NULL, e4b, start, ex.fe_len);
> + return ret;
> }
>
> /**
> @@ -4904,7 +4922,7 @@ ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
> void *bitmap;
> ext4_grpblk_t next, count = 0, free_count = 0;
> struct ext4_buddy e4b;
> - int ret;
> + int ret = 0;
>
> trace_ext4_trim_all_free(sb, group, start, max);
>
> @@ -4931,8 +4949,11 @@ ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
> next = mb_find_next_bit(bitmap, max + 1, start);
>
> if ((next - start) >= minblocks) {
> - ext4_trim_extent(sb, start,
> - next - start, group, &e4b);
> + ret = ext4_trim_extent(sb, start,
> + next - start, group, &e4b);
> + if (ret && ret != -EOPNOTSUPP)
> + break;
> + ret = 0;
> count += next - start;
> }
> free_count += next - start;
> @@ -4953,8 +4974,10 @@ ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
> break;
> }
>
> - if (!ret)
> + if (!ret) {
> + ret = count;
> EXT4_MB_GRP_SET_TRIMMED(e4b.bd_info);
> + }
> out:
> ext4_unlock_group(sb, group);
> ext4_mb_unload_buddy(&e4b);
> @@ -4962,7 +4985,7 @@ out:
> ext4_debug("trimmed %d blocks in the group %d\n",
> count, group);
>
> - return count;
> + return ret;
> }
>
> /**
>
On Fri, Oct 19, 2012 at 02:12:37PM +0200, Lukas Czerner wrote:
> Notify user when mounting the file system with -o discard option, but
> the device does not support discard. Obviously we do not want to fail
> the mount or disable the options, because the underlying device might
> change in future even without file system remount.
>
> Signed-off-by: Lukas Czerner <[email protected]>
Thanks, applied.
- Ted
On Fri, Oct 19, 2012 at 02:12:38PM +0200, Lukas Czerner wrote:
> We should warn user then the discard request fails. However we need to
> exclude -EOPNOTSUPP case since parts of the device might not support it
> while other parts can. So print the kernel warning when the error !=
> -EOPNOTSUPP is returned from ext4_issue_discard().
>
> We should also handle error cases in batched discard, again excluding
> EOPNOTSUPP.
>
> Signed-off-by: Lukas Czerner <[email protected]>
Thanks, applied.
- Ted