linux-ext4 - Re: [PATCH 2/2] ext4: Warn when discard request fails other than EOPNOTSUPP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.1211081208040.19842@dhcp-1-104.brq.redhat.com>
Date:	Thu, 8 Nov 2012 12:08:28 +0100 (CET)
From:	Lukáš Czerner <lczerner@...hat.com>
To:	Lukas Czerner <lczerner@...hat.com>
cc:	linux-ext4@...r.kernel.org, tytso@....edu
Subject: Re: [PATCH 2/2] ext4: Warn when discard request fails other than
 EOPNOTSUPP

On Fri, 19 Oct 2012, Lukas Czerner wrote:

> Date: Fri, 19 Oct 2012 14:12:38 +0200
> From: Lukas Czerner <lczerner@...hat.com>
> To: linux-ext4@...r.kernel.org
> Cc: tytso@....edu, Lukas Czerner <lczerner@...hat.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@...hat.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html