diff mbox

[2/7] btrfs: btrfs_issue_discard ensure offset/length are aligned to sector boundaries

Message ID 1434375680-4115-2-git-send-email-jeffm@suse.com (mailing list archive)
State Accepted
Headers show

Commit Message

Jeff Mahoney June 15, 2015, 1:41 p.m. UTC
From: Jeff Mahoney <jeffm@suse.com>

It's possible, though unexpected, to pass unaligned offsets and lengths
to btrfs_issue_discard.  We then shift the offset/length values to sector
units.  If an unaligned offset has been passed, it will result in the
entire sector being discarded, possibly losing data.  An unaligned
length is safe but we'll end up returning an inaccurate number of
discarded bytes.

This patch aligns the offset to the 512B boundary, adjusts the length,
and warns, since we shouldn't be discarding on an offset that isn't
aligned with our sector size.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 fs/btrfs/extent-tree.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Comments

Filipe Manana June 17, 2015, 9:55 a.m. UTC | #1
On Mon, Jun 15, 2015 at 2:41 PM,  <jeffm@suse.com> wrote:
> From: Jeff Mahoney <jeffm@suse.com>
>
> It's possible, though unexpected, to pass unaligned offsets and lengths
> to btrfs_issue_discard.  We then shift the offset/length values to sector
> units.  If an unaligned offset has been passed, it will result in the
> entire sector being discarded, possibly losing data.  An unaligned
> length is safe but we'll end up returning an inaccurate number of
> discarded bytes.
>
> This patch aligns the offset to the 512B boundary, adjusts the length,
> and warns, since we shouldn't be discarding on an offset that isn't
> aligned with our sector size.
>
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Tested-by: Filipe Manana <fdmanana@suse.com>

> ---
>  fs/btrfs/extent-tree.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index da1145d..cf9cefd 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -1888,12 +1888,21 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
>                                u64 *discarded_bytes)
>  {
>         int ret = 0;
> +       u64 aligned_start = ALIGN(start, 1 << 9);
>
> -       *discarded_bytes = 0;
> -       ret = blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_NOFS, 0);
> -       if (!ret)
> -               *discarded_bytes = len;
> +       if (WARN_ON(start != aligned_start)) {
> +               len -= aligned_start - start;
> +               len = round_down(len, 1 << 9);
> +               start = aligned_start;
> +       }
>
> +       *discarded_bytes = 0;
> +       if (len) {
> +               ret = blkdev_issue_discard(bdev, start >> 9, len >> 9,
> +                                          GFP_NOFS, 0);
> +               if (!ret)
> +                       *discarded_bytes = len;
> +       }
>         return ret;
>  }
>
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index da1145d..cf9cefd 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1888,12 +1888,21 @@  static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
 			       u64 *discarded_bytes)
 {
 	int ret = 0;
+	u64 aligned_start = ALIGN(start, 1 << 9);
 
-	*discarded_bytes = 0;
-	ret = blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_NOFS, 0);
-	if (!ret)
-		*discarded_bytes = len;
+	if (WARN_ON(start != aligned_start)) {
+		len -= aligned_start - start;
+		len = round_down(len, 1 << 9);
+		start = aligned_start;
+	}
 
+	*discarded_bytes = 0;
+	if (len) {
+		ret = blkdev_issue_discard(bdev, start >> 9, len >> 9,
+					   GFP_NOFS, 0);
+		if (!ret)
+			*discarded_bytes = len;
+	}
 	return ret;
 }