lkml.org 
[lkml]   [2021]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [GIT PULL] Btrfs updates for 5.16


On 2021/11/2 04:03, Linus Torvalds wrote:
> On Mon, Nov 1, 2021 at 9:46 AM David Sterba <dsterba@suse.com> wrote:
>>
>> There's a merge conflict due to the last minute 5.15 changes (kmap
>> reverts) and the conflict is not trivial.
>
> You don't say.
>
> I ended up just re-doing that resolution entirely, and as I did so, I
> think I found a bug in the original revert that caused the conflict in
> the first place.
>
> And since that revert made it into 5.15, I felt like I had to fix that
> bug first - and separately - so that the fix can be backported to
> stable.
>
> I then re-did my merge on top of that hopefully fixed state, and maybe
> it's correct.
>
> Or maybe I messed up entirely.
>
> I did end up comparing it to your other branch too, but that was
> equally as messy, apart from the "ok, I can mindlessly just take your
> side".
>
> And it was fairly different from what I had done in my merge
> resolution, so who knows.
>
> ANYWAY. What I'm trying to say is that you should look very very
> carefully at commits
>
> 2cf3f8133bda ("btrfs: fix lzo_decompress_bio() kmap leakage")

Since I'm doing the revert manually for lzo part, I double checked the code.

It turns out, your fix is the same as the original version I sent to
David (although not through the mail list).
Full patch attached.

@@ -345,8 +358,9 @@ int lzo_decompress_bio(struct list_head *ws, struct
compressed_bio *cb)
(cur_in + LZO_LEN - 1) / sectorsize);
cur_page = cb->compressed_pages[cur_in / PAGE_SIZE];
ASSERT(cur_page);
- seg_len = read_compress_length(page_address(cur_page) +
- offset_in_page(cur_in));
+ kaddr = kmap(cur_page);
+ seg_len = read_compress_length(kaddr + offset_in_page(cur_in));
+ kunmap(cur_page);
cur_in += LZO_LEN;

Thus it looks like by somehow my version is not applied?

Thanks,
Qu

> 037c50bfbeb3 ("Merge tag 'for-5.16-tag' of git://git.../linux") >
> because I marked that first one for stable, and the second is
> obviously my entirely untested merge.
>
> It makes sense to me, but apart from "it builds", I've not actually
> tested any of it. This is all purely from looking at the code and
> trying to figure out what the RightThing(tm) is.
>
> Obviously the kmap thing tends to only be noticeable on 32-bit
> platforms, and that lzo_decompress_bio() bug also needs just the
> proper filesystem settings to trigger in the first place.
>
> Again - please take a careful look. Both at my merge and at that
> alleged kmap fix.
>
> Linus
>
From 38c48395c1f3cb8aaea449dbaa81cf0efdc012d8 Mon Sep 17 00:00:00 2001
From: Qu Wenruo <wqu@suse.com>
Date: Wed, 27 Oct 2021 17:05:36 +0800
Subject: [PATCH] Revert "btrfs: compression: drop kmap/kunmap from lzo"

This reverts commit 8c945d32e60427cbc0859cf7045bbe6196bb03d8.
---
fs/btrfs/lzo.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index c25dfd1a8a54..295bbc13ace6 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -141,7 +141,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
*total_in = 0;

in_page = find_get_page(mapping, start >> PAGE_SHIFT);
- data_in = page_address(in_page);
+ data_in = kmap(in_page);

/*
* store the size of all chunks of compressed data in
@@ -152,7 +152,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -ENOMEM;
goto out;
}
- cpage_out = page_address(out_page);
+ cpage_out = kmap(out_page);
out_offset = LZO_LEN;
tot_out = LZO_LEN;
pages[0] = out_page;
@@ -210,6 +210,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
if (out_len == 0 && tot_in >= len)
break;

+ kunmap(out_page);
if (nr_pages == nr_dest_pages) {
out_page = NULL;
ret = -E2BIG;
@@ -221,7 +222,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -ENOMEM;
goto out;
}
- cpage_out = page_address(out_page);
+ cpage_out = kmap(out_page);
pages[nr_pages++] = out_page;

pg_bytes_left = PAGE_SIZE;
@@ -243,11 +244,12 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
break;

bytes_left = len - tot_in;
+ kunmap(in_page);
put_page(in_page);

start += PAGE_SIZE;
in_page = find_get_page(mapping, start >> PAGE_SHIFT);
- data_in = page_address(in_page);
+ data_in = kmap(in_page);
in_len = min(bytes_left, PAGE_SIZE);
}

@@ -257,17 +259,22 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
}

/* store the size of all chunks of compressed data */
- sizes_ptr = page_address(pages[0]);
+ sizes_ptr = kmap_local_page(pages[0]);
write_compress_length(sizes_ptr, tot_out);
+ kunmap_local(sizes_ptr);

ret = 0;
*total_out = tot_out;
*total_in = tot_in;
out:
*out_pages = nr_pages;
+ if (out_page)
+ kunmap(out_page);

- if (in_page)
+ if (in_page) {
+ kunmap(in_page);
put_page(in_page);
+ }

return ret;
}
@@ -283,6 +290,7 @@ static void copy_compressed_segment(struct compressed_bio *cb,
u32 orig_in = *cur_in;

while (*cur_in < orig_in + len) {
+ char *kaddr;
struct page *cur_page;
u32 copy_len = min_t(u32, PAGE_SIZE - offset_in_page(*cur_in),
orig_in + len - *cur_in);
@@ -290,9 +298,11 @@ static void copy_compressed_segment(struct compressed_bio *cb,
ASSERT(copy_len);
cur_page = cb->compressed_pages[*cur_in / PAGE_SIZE];

+ kaddr = kmap(cur_page);
memcpy(dest + *cur_in - orig_in,
- page_address(cur_page) + offset_in_page(*cur_in),
+ kaddr + offset_in_page(*cur_in),
copy_len);
+ kunmap(cur_page);

*cur_in += copy_len;
}
@@ -303,6 +313,7 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
struct workspace *workspace = list_entry(ws, struct workspace, list);
const struct btrfs_fs_info *fs_info = btrfs_sb(cb->inode->i_sb);
const u32 sectorsize = fs_info->sectorsize;
+ char *kaddr;
int ret;
/* Compressed data length, can be unaligned */
u32 len_in;
@@ -311,7 +322,9 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
/* Bytes decompressed so far */
u32 cur_out = 0;

- len_in = read_compress_length(page_address(cb->compressed_pages[0]));
+ kaddr = kmap(cb->compressed_pages[0]);
+ len_in = read_compress_length(kaddr);
+ kunmap(cb->compressed_pages[0]);
cur_in += LZO_LEN;

/*
@@ -345,8 +358,9 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
(cur_in + LZO_LEN - 1) / sectorsize);
cur_page = cb->compressed_pages[cur_in / PAGE_SIZE];
ASSERT(cur_page);
- seg_len = read_compress_length(page_address(cur_page) +
- offset_in_page(cur_in));
+ kaddr = kmap(cur_page);
+ seg_len = read_compress_length(kaddr + offset_in_page(cur_in));
+ kunmap(cur_page);
cur_in += LZO_LEN;

/* Copy the compressed segment payload into workspace */
@@ -431,7 +445,7 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
destlen = min_t(unsigned long, destlen, PAGE_SIZE);
bytes = min_t(unsigned long, destlen, out_len - start_byte);

- kaddr = page_address(dest_page);
+ kaddr = kmap_local_page(dest_page);
memcpy(kaddr, workspace->buf + start_byte, bytes);

/*
@@ -441,6 +455,7 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
*/
if (bytes < destlen)
memset(kaddr+bytes, 0, destlen-bytes);
+ kunmap_local(kaddr);
out:
return ret;
}
--
2.33.1
\
 
 \ /
  Last update: 2021-11-02 01:09    [W:0.096 / U:1.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site