| Message ID | 1350648758-3318-2-git-send-email-lczerner@redhat.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
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 <lczerner@redhat.com> > --- > fs/ext4/mballoc.c | 47 +++++++++++++++++++++++++++++++++++------------ > 1 files changed, 35 insertions(+), 12 deletions(-) Looks good Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
On Fri, 19 Oct 2012, Lukas Czerner wrote: > Date: Fri, 19 Oct 2012 14:12:38 +0200 > From: Lukas Czerner <lczerner@redhat.com> > To: linux-ext4@vger.kernel.org > Cc: tytso@mit.edu, Lukas Czerner <lczerner@redhat.com> > 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 <lczerner@redhat.com> > --- > 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; > } > > /** > -- 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
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 <lczerner@redhat.com> Thanks, applied. - 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
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; } /**
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 <lczerner@redhat.com> --- fs/ext4/mballoc.c | 47 +++++++++++++++++++++++++++++++++++------------ 1 files changed, 35 insertions(+), 12 deletions(-)