* [PATCH v7 1/8] btrfs: Export btrfs_release_disk_super
2020-02-12 7:16 [PATCH v7 0/8] btrfs: remove buffer heads form superblock handling Johannes Thumshirn
@ 2020-02-12 7:16 ` Johannes Thumshirn
2020-02-12 7:16 ` [PATCH v7 2/8] btrfs: don't kmap() pages from block devices Johannes Thumshirn
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:16 UTC (permalink / raw)
To: David Sterba
Cc: linux-btrfs @ vger . kernel . org, Nikolay Borisov, Johannes Thumshirn
From: Nikolay Borisov <nborisov@suse.com>
Preparatory patch for removal of buffer_head usage in btrfs.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
fs/btrfs/volumes.c | 2 +-
fs/btrfs/volumes.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b092021e41e9..6ebdd95b798d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1247,7 +1247,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
return ret;
}
-static void btrfs_release_disk_super(struct page *page)
+void btrfs_release_disk_super(struct page *page)
{
kunmap(page);
put_page(page);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 690d4f5a0653..b7f2edbc6581 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -483,6 +483,7 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans,
int btrfs_remove_chunk(struct btrfs_trans_handle *trans, u64 chunk_offset);
struct extent_map *btrfs_get_chunk_map(struct btrfs_fs_info *fs_info,
u64 logical, u64 length);
+void btrfs_release_disk_super(struct page *page);
static inline void btrfs_dev_stat_inc(struct btrfs_device *dev,
int index)
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 2/8] btrfs: don't kmap() pages from block devices
2020-02-12 7:16 [PATCH v7 0/8] btrfs: remove buffer heads form superblock handling Johannes Thumshirn
2020-02-12 7:16 ` [PATCH v7 1/8] btrfs: Export btrfs_release_disk_super Johannes Thumshirn
@ 2020-02-12 7:16 ` Johannes Thumshirn
2020-02-12 7:25 ` Christoph Hellwig
2020-02-12 7:16 ` [PATCH v7 3/8] btrfs: unexport btrfs_scratch_superblocks Johannes Thumshirn
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:16 UTC (permalink / raw)
To: David Sterba
Cc: linux-btrfs @ vger . kernel . org, Johannes Thumshirn, Christoph Hellwig
Block device mappings are never in highmem so kmap() / kunmap() calls for
pages from block devices are unneeded. Use page_address() instead of
kmap() to get to the virtual addreses.
While we're at it, read_cache_page_gfp() doesn't return NULL on error,
only an ERR_PTR, so use IS_ERR() to check for errors.
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
fs/btrfs/volumes.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6ebdd95b798d..620794f1ea64 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1249,7 +1249,6 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
void btrfs_release_disk_super(struct page *page)
{
- kunmap(page);
put_page(page);
}
@@ -1277,10 +1276,10 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
*page = read_cache_page_gfp(bdev->bd_inode->i_mapping,
index, GFP_KERNEL);
- if (IS_ERR_OR_NULL(*page))
+ if (IS_ERR(*page))
return 1;
- p = kmap(*page);
+ p = page_address(*page);
/* align our pointer to the offset of the super block */
*disk_super = p + offset_in_page(bytenr);
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 2/8] btrfs: don't kmap() pages from block devices
2020-02-12 7:16 ` [PATCH v7 2/8] btrfs: don't kmap() pages from block devices Johannes Thumshirn
@ 2020-02-12 7:25 ` Christoph Hellwig
0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-02-12 7:25 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: David Sterba, linux-btrfs @ vger . kernel . org, Christoph Hellwig
On Wed, Feb 12, 2020 at 04:16:58PM +0900, Johannes Thumshirn wrote:
> Block device mappings are never in highmem so kmap() / kunmap() calls for
> pages from block devices are unneeded. Use page_address() instead of
> kmap() to get to the virtual addreses.
>
> While we're at it, read_cache_page_gfp() doesn't return NULL on error,
> only an ERR_PTR, so use IS_ERR() to check for errors.
>
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 3/8] btrfs: unexport btrfs_scratch_superblocks
2020-02-12 7:16 [PATCH v7 0/8] btrfs: remove buffer heads form superblock handling Johannes Thumshirn
2020-02-12 7:16 ` [PATCH v7 1/8] btrfs: Export btrfs_release_disk_super Johannes Thumshirn
2020-02-12 7:16 ` [PATCH v7 2/8] btrfs: don't kmap() pages from block devices Johannes Thumshirn
@ 2020-02-12 7:16 ` Johannes Thumshirn
2020-02-12 7:27 ` Christoph Hellwig
2020-02-12 7:17 ` [PATCH v7 4/8] btrfs: use the page-cache for super block reading Johannes Thumshirn
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:16 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs @ vger . kernel . org, Johannes Thumshirn
btrfs_scratch_superblocks() isn't used anywhere outside volumes.c so
remove it from the header file and mark it as static.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
fs/btrfs/volumes.c | 6 +++++-
fs/btrfs/volumes.h | 1 -
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 620794f1ea64..e17d4d7a6eb4 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -32,6 +32,9 @@
#include "block-group.h"
#include "discard.h"
+static void btrfs_scratch_superblocks(struct block_device *bdev,
+ const char *device_path);
+
const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
[BTRFS_RAID_RAID10] = {
.sub_stripes = 2,
@@ -7316,7 +7319,8 @@ int btrfs_get_dev_stats(struct btrfs_fs_info *fs_info,
return 0;
}
-void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_path)
+static void btrfs_scratch_superblocks(struct block_device *bdev,
+ const char *device_path)
{
struct buffer_head *bh;
struct btrfs_super_block *disk_super;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index b7f2edbc6581..a3d86ee6a883 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -473,7 +473,6 @@ int btrfs_run_dev_stats(struct btrfs_trans_handle *trans);
void btrfs_rm_dev_replace_remove_srcdev(struct btrfs_device *srcdev);
void btrfs_rm_dev_replace_free_srcdev(struct btrfs_device *srcdev);
void btrfs_destroy_dev_replace_tgtdev(struct btrfs_device *tgtdev);
-void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_path);
int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info,
u64 logical, u64 len);
unsigned long btrfs_full_stripe_len(struct btrfs_fs_info *fs_info,
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 3/8] btrfs: unexport btrfs_scratch_superblocks
2020-02-12 7:16 ` [PATCH v7 3/8] btrfs: unexport btrfs_scratch_superblocks Johannes Thumshirn
@ 2020-02-12 7:27 ` Christoph Hellwig
2020-02-12 7:51 ` Johannes Thumshirn
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2020-02-12 7:27 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: David Sterba, linux-btrfs @ vger . kernel . org
On Wed, Feb 12, 2020 at 04:16:59PM +0900, Johannes Thumshirn wrote:
> btrfs_scratch_superblocks() isn't used anywhere outside volumes.c so
> remove it from the header file and mark it as static.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
It wasn't exported to start with, just global scope while you
mark it static now.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 3/8] btrfs: unexport btrfs_scratch_superblocks
2020-02-12 7:27 ` Christoph Hellwig
@ 2020-02-12 7:51 ` Johannes Thumshirn
0 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:51 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: David Sterba, linux-btrfs @ vger . kernel . org
On 12/02/2020 08:27, Christoph Hellwig wrote:
> On Wed, Feb 12, 2020 at 04:16:59PM +0900, Johannes Thumshirn wrote:
>> btrfs_scratch_superblocks() isn't used anywhere outside volumes.c so
>> remove it from the header file and mark it as static.
>>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>
> It wasn't exported to start with, just global scope while you
> mark it static now.
>
I think David can tweak the commit message when applying.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 4/8] btrfs: use the page-cache for super block reading
2020-02-12 7:16 [PATCH v7 0/8] btrfs: remove buffer heads form superblock handling Johannes Thumshirn
` (2 preceding siblings ...)
2020-02-12 7:16 ` [PATCH v7 3/8] btrfs: unexport btrfs_scratch_superblocks Johannes Thumshirn
@ 2020-02-12 7:17 ` Johannes Thumshirn
2020-02-12 7:28 ` Christoph Hellwig
2020-02-12 7:17 ` [PATCH v7 5/8] btrfs: use BIOs instead of buffer_heads from superblock writeout Johannes Thumshirn
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:17 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs @ vger . kernel . org, Johannes Thumshirn
Super-block reading in BTRFS is done using buffer_heads. Buffer_heads have
some drawbacks, like not being able to propagate errors from the lower
layers.
Directly use the page cache for reading the super-blocks from disk or
invalidating an on-disk super-block. We have to use the page-cache so to
avoid races between mkfs and udev. See also 6f60cbd3ae44 ("btrfs: access
superblock via pagecache in scan_one_device").
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
Changes to v6:
- Warn if we can't write out a page for a superblock (David)
Changes to v5:
- Removed kmap()/kunmap() calls (hch/David)
Changes to v4:
- Remove mapping_gfp_constraint() and GFP_NOFAIL (hch)
Changes to v3:
- Use read_cache_pages() and write_one_page() for IO (hch)
- Changed subject (David)
- Dropped Josef's R-b due to change
Changes to v2:
- open-code kunmap() + put_page() (David)
- fix double kunmap() (David)
- don't use bi_set_op_attrs() (David)
Changes to v1:
- move 'super_page' into for-loop in btrfs_scratch_superblocks() (Nikolay)
- switch to using pagecahce instead of alloc_pages() (Nikolay, David)
---
fs/btrfs/disk-io.c | 74 ++++++++++++++++++------------------------
fs/btrfs/disk-io.h | 6 ++--
fs/btrfs/volumes.c | 80 ++++++++++++++++++++++++++--------------------
fs/btrfs/volumes.h | 4 +--
4 files changed, 80 insertions(+), 84 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 018681ec159b..9f422edd6cce 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2850,7 +2850,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
u64 features;
u16 csum_type;
struct btrfs_key location;
- struct buffer_head *bh;
struct btrfs_super_block *disk_super;
struct btrfs_fs_info *fs_info = btrfs_sb(sb);
struct btrfs_root *tree_root;
@@ -2891,9 +2890,9 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
/*
* Read super block and check the signature bytes only
*/
- bh = btrfs_read_dev_super(fs_devices->latest_bdev);
- if (IS_ERR(bh)) {
- err = PTR_ERR(bh);
+ disk_super = btrfs_read_dev_super(fs_devices->latest_bdev);
+ if (IS_ERR(disk_super)) {
+ err = PTR_ERR(disk_super);
goto fail_alloc;
}
@@ -2901,18 +2900,19 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
* Verify the type first, if that or the the checksum value are
* corrupted, we'll find out
*/
- csum_type = btrfs_super_csum_type((struct btrfs_super_block *)bh->b_data);
+ csum_type = btrfs_super_csum_type(disk_super);
if (!btrfs_supported_super_csum(csum_type)) {
btrfs_err(fs_info, "unsupported checksum algorithm: %u",
csum_type);
err = -EINVAL;
- brelse(bh);
+ btrfs_release_disk_super(disk_super);
goto fail_alloc;
}
ret = btrfs_init_csum_hash(fs_info, csum_type);
if (ret) {
err = ret;
+ btrfs_release_disk_super(disk_super);
goto fail_alloc;
}
@@ -2920,10 +2920,10 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
* We want to check superblock checksum, the type is stored inside.
* Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
*/
- if (btrfs_check_super_csum(fs_info, bh->b_data)) {
+ if (btrfs_check_super_csum(fs_info, (u8 *) disk_super)) {
btrfs_err(fs_info, "superblock checksum mismatch");
err = -EINVAL;
- brelse(bh);
+ btrfs_release_disk_super(disk_super);
goto fail_alloc;
}
@@ -2932,8 +2932,8 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
* following bytes up to INFO_SIZE, the checksum is calculated from
* the whole block of INFO_SIZE
*/
- memcpy(fs_info->super_copy, bh->b_data, sizeof(*fs_info->super_copy));
- brelse(bh);
+ memcpy(fs_info->super_copy, disk_super, sizeof(*fs_info->super_copy));
+ btrfs_release_disk_super(disk_super);
disk_super = fs_info->super_copy;
@@ -3419,45 +3419,38 @@ static void btrfs_end_buffer_write_sync(struct buffer_head *bh, int uptodate)
put_bh(bh);
}
-int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
- struct buffer_head **bh_ret)
+struct btrfs_super_block *btrfs_read_dev_one_super(struct block_device *bdev,
+ int copy_num)
{
- struct buffer_head *bh;
struct btrfs_super_block *super;
+ struct page *page;
u64 bytenr;
+ struct address_space *mapping = bdev->bd_inode->i_mapping;
bytenr = btrfs_sb_offset(copy_num);
if (bytenr + BTRFS_SUPER_INFO_SIZE >= i_size_read(bdev->bd_inode))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
- bh = __bread(bdev, bytenr / BTRFS_BDEV_BLOCKSIZE, BTRFS_SUPER_INFO_SIZE);
- /*
- * If we fail to read from the underlying devices, as of now
- * the best option we have is to mark it EIO.
- */
- if (!bh)
- return -EIO;
+ page = read_cache_page_gfp(mapping, bytenr >> PAGE_SHIFT, GFP_NOFS);
+ if (IS_ERR(page))
+ return ERR_PTR(-ENOMEM);
- super = (struct btrfs_super_block *)bh->b_data;
+ super = page_address(page);
if (btrfs_super_bytenr(super) != bytenr ||
btrfs_super_magic(super) != BTRFS_MAGIC) {
- brelse(bh);
- return -EINVAL;
+ btrfs_release_disk_super(super);
+ return ERR_PTR(-EINVAL);
}
- *bh_ret = bh;
- return 0;
+ return super;
}
-struct buffer_head *btrfs_read_dev_super(struct block_device *bdev)
+struct btrfs_super_block *btrfs_read_dev_super(struct block_device *bdev)
{
- struct buffer_head *bh;
- struct buffer_head *latest = NULL;
- struct btrfs_super_block *super;
+ struct btrfs_super_block *super, *latest = NULL;
int i;
u64 transid = 0;
- int ret = -EINVAL;
/* we would like to check all the supers, but that would make
* a btrfs mount succeed after a mkfs from a different FS.
@@ -3465,25 +3458,20 @@ struct buffer_head *btrfs_read_dev_super(struct block_device *bdev)
* later supers, using BTRFS_SUPER_MIRROR_MAX instead
*/
for (i = 0; i < 1; i++) {
- ret = btrfs_read_dev_one_super(bdev, i, &bh);
- if (ret)
+ super = btrfs_read_dev_one_super(bdev, i);
+ if (IS_ERR(super))
continue;
- super = (struct btrfs_super_block *)bh->b_data;
-
if (!latest || btrfs_super_generation(super) > transid) {
- brelse(latest);
- latest = bh;
+ if (latest)
+ btrfs_release_disk_super(super);
+
+ latest = super;
transid = btrfs_super_generation(super);
- } else {
- brelse(bh);
}
}
- if (!latest)
- return ERR_PTR(ret);
-
- return latest;
+ return super;
}
/*
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index db21ab614357..59c885860bf8 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -56,9 +56,9 @@ int __cold open_ctree(struct super_block *sb,
char *options);
void __cold close_ctree(struct btrfs_fs_info *fs_info);
int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors);
-struct buffer_head *btrfs_read_dev_super(struct block_device *bdev);
-int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
- struct buffer_head **bh_ret);
+struct btrfs_super_block *btrfs_read_dev_super(struct block_device *bdev);
+struct btrfs_super_block *btrfs_read_dev_one_super(struct block_device *bdev,
+ int copy_num);
int btrfs_commit_super(struct btrfs_fs_info *fs_info);
struct btrfs_root *btrfs_read_tree_root(struct btrfs_root *tree_root,
struct btrfs_key *key);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index e17d4d7a6eb4..daf9c10db337 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6,7 +6,6 @@
#include <linux/sched.h>
#include <linux/bio.h>
#include <linux/slab.h>
-#include <linux/buffer_head.h>
#include <linux/blkdev.h>
#include <linux/ratelimit.h>
#include <linux/kthread.h>
@@ -32,7 +31,8 @@
#include "block-group.h"
#include "discard.h"
-static void btrfs_scratch_superblocks(struct block_device *bdev,
+static void btrfs_scratch_superblocks(struct btrfs_fs_info *fs_info,
+ struct block_device *bdev,
const char *device_path);
const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
@@ -503,7 +503,7 @@ static struct btrfs_fs_devices *find_fsid_with_metadata_uuid(
static int
btrfs_get_bdev_and_sb(const char *device_path, fmode_t flags, void *holder,
int flush, struct block_device **bdev,
- struct buffer_head **bh)
+ struct btrfs_super_block **disk_super)
{
int ret;
@@ -522,9 +522,9 @@ btrfs_get_bdev_and_sb(const char *device_path, fmode_t flags, void *holder,
goto error;
}
invalidate_bdev(*bdev);
- *bh = btrfs_read_dev_super(*bdev);
- if (IS_ERR(*bh)) {
- ret = PTR_ERR(*bh);
+ *disk_super = btrfs_read_dev_super(*bdev);
+ if (IS_ERR(*disk_super)) {
+ ret = PTR_ERR(*disk_super);
blkdev_put(*bdev, flags);
goto error;
}
@@ -533,7 +533,6 @@ btrfs_get_bdev_and_sb(const char *device_path, fmode_t flags, void *holder,
error:
*bdev = NULL;
- *bh = NULL;
return ret;
}
@@ -614,7 +613,6 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
{
struct request_queue *q;
struct block_device *bdev;
- struct buffer_head *bh;
struct btrfs_super_block *disk_super;
u64 devid;
int ret;
@@ -625,17 +623,16 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
return -EINVAL;
ret = btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
- &bdev, &bh);
+ &bdev, &disk_super);
if (ret)
return ret;
- disk_super = (struct btrfs_super_block *)bh->b_data;
devid = btrfs_stack_device_id(&disk_super->dev_item);
if (devid != device->devid)
- goto error_brelse;
+ goto error_free_page;
if (memcmp(device->uuid, disk_super->dev_item.uuid, BTRFS_UUID_SIZE))
- goto error_brelse;
+ goto error_free_page;
device->generation = btrfs_super_generation(disk_super);
@@ -644,7 +641,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
BTRFS_FEATURE_INCOMPAT_METADATA_UUID) {
pr_err(
"BTRFS: Invalid seeding and uuid-changed device detected\n");
- goto error_brelse;
+ goto error_free_page;
}
clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
@@ -670,12 +667,12 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
fs_devices->rw_devices++;
list_add_tail(&device->dev_alloc_list, &fs_devices->alloc_list);
}
- brelse(bh);
+ btrfs_release_disk_super(disk_super);
return 0;
-error_brelse:
- brelse(bh);
+error_free_page:
+ btrfs_release_disk_super(disk_super);
blkdev_put(bdev, flags);
return -EINVAL;
@@ -1250,8 +1247,10 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
return ret;
}
-void btrfs_release_disk_super(struct page *page)
+void btrfs_release_disk_super(struct btrfs_super_block *super)
{
+ struct page *page = virt_to_page(super);
+
put_page(page);
}
@@ -1289,7 +1288,7 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
if (btrfs_super_bytenr(*disk_super) != bytenr ||
btrfs_super_magic(*disk_super) != BTRFS_MAGIC) {
- btrfs_release_disk_super(*page);
+ btrfs_release_disk_super(p);
return 1;
}
@@ -1352,7 +1351,7 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags,
btrfs_free_stale_devices(path, device);
}
- btrfs_release_disk_super(page);
+ btrfs_release_disk_super(disk_super);
error_bdev_put:
blkdev_put(bdev, flags);
@@ -2069,7 +2068,8 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
* supers and free the device.
*/
if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state))
- btrfs_scratch_superblocks(device->bdev, device->name->str);
+ btrfs_scratch_superblocks(fs_info, device->bdev,
+ device->name->str);
btrfs_close_bdev(device);
synchronize_rcu();
@@ -2137,7 +2137,8 @@ void btrfs_rm_dev_replace_free_srcdev(struct btrfs_device *srcdev)
if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &srcdev->dev_state)) {
/* zero out the old super if it is writable */
- btrfs_scratch_superblocks(srcdev->bdev, srcdev->name->str);
+ btrfs_scratch_superblocks(fs_info, srcdev->bdev,
+ srcdev->name->str);
}
btrfs_close_bdev(srcdev);
@@ -2196,7 +2197,8 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_device *tgtdev)
* is already out of device list, so we don't have to hold
* the device_list_mutex lock.
*/
- btrfs_scratch_superblocks(tgtdev->bdev, tgtdev->name->str);
+ btrfs_scratch_superblocks(tgtdev->fs_info, tgtdev->bdev,
+ tgtdev->name->str);
btrfs_close_bdev(tgtdev);
synchronize_rcu();
@@ -2211,14 +2213,14 @@ static struct btrfs_device *btrfs_find_device_by_path(
u64 devid;
u8 *dev_uuid;
struct block_device *bdev;
- struct buffer_head *bh;
struct btrfs_device *device;
ret = btrfs_get_bdev_and_sb(device_path, FMODE_READ,
- fs_info->bdev_holder, 0, &bdev, &bh);
+ fs_info->bdev_holder, 0, &bdev,
+ &disk_super);
if (ret)
return ERR_PTR(ret);
- disk_super = (struct btrfs_super_block *)bh->b_data;
+
devid = btrfs_stack_device_id(&disk_super->dev_item);
dev_uuid = disk_super->dev_item.uuid;
if (btrfs_fs_incompat(fs_info, METADATA_UUID))
@@ -2228,7 +2230,7 @@ static struct btrfs_device *btrfs_find_device_by_path(
device = btrfs_find_device(fs_info->fs_devices, devid, dev_uuid,
disk_super->fsid, true);
- brelse(bh);
+ btrfs_release_disk_super(disk_super);
if (!device)
device = ERR_PTR(-ENOENT);
blkdev_put(bdev, FMODE_READ);
@@ -7319,10 +7321,10 @@ int btrfs_get_dev_stats(struct btrfs_fs_info *fs_info,
return 0;
}
-static void btrfs_scratch_superblocks(struct block_device *bdev,
- const char *device_path)
+static void btrfs_scratch_superblocks(struct btrfs_fs_info *fs_info,
+ struct block_device *bdev,
+ const char *device_path)
{
- struct buffer_head *bh;
struct btrfs_super_block *disk_super;
int copy_num;
@@ -7331,16 +7333,24 @@ static void btrfs_scratch_superblocks(struct block_device *bdev,
for (copy_num = 0; copy_num < BTRFS_SUPER_MIRROR_MAX;
copy_num++) {
+ struct page *page;
+ int err;
- if (btrfs_read_dev_one_super(bdev, copy_num, &bh))
+ disk_super = btrfs_read_dev_one_super(bdev, copy_num);
+ if (IS_ERR(disk_super))
continue;
- disk_super = (struct btrfs_super_block *)bh->b_data;
-
memset(&disk_super->magic, 0, sizeof(disk_super->magic));
- set_buffer_dirty(bh);
- sync_dirty_buffer(bh);
- brelse(bh);
+
+ page = virt_to_page(disk_super);
+ set_page_dirty(page);
+ lock_page(page); /* write_on_page() unlocks the page */
+ err = write_one_page(page);
+ if (err)
+ btrfs_warn(fs_info, "error clearing superblock number %d (%d)",
+ copy_num, err);
+ btrfs_release_disk_super(disk_super);
+
}
/* Notify udev that device has changed */
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index a3d86ee6a883..3f52c3a05af8 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -17,8 +17,6 @@ extern struct mutex uuid_mutex;
#define BTRFS_STRIPE_LEN SZ_64K
-struct buffer_head;
-
struct btrfs_io_geometry {
/* remaining bytes before crossing a stripe */
u64 len;
@@ -482,7 +480,7 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle *trans,
int btrfs_remove_chunk(struct btrfs_trans_handle *trans, u64 chunk_offset);
struct extent_map *btrfs_get_chunk_map(struct btrfs_fs_info *fs_info,
u64 logical, u64 length);
-void btrfs_release_disk_super(struct page *page);
+void btrfs_release_disk_super(struct btrfs_super_block *super);
static inline void btrfs_dev_stat_inc(struct btrfs_device *dev,
int index)
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 4/8] btrfs: use the page-cache for super block reading
2020-02-12 7:17 ` [PATCH v7 4/8] btrfs: use the page-cache for super block reading Johannes Thumshirn
@ 2020-02-12 7:28 ` Christoph Hellwig
2020-02-12 7:43 ` Johannes Thumshirn
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2020-02-12 7:28 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: David Sterba, linux-btrfs @ vger . kernel . org
On Wed, Feb 12, 2020 at 04:17:00PM +0900, Johannes Thumshirn wrote:
> Super-block reading in BTRFS is done using buffer_heads. Buffer_heads have
> some drawbacks, like not being able to propagate errors from the lower
> layers.
>
> Directly use the page cache for reading the super-blocks from disk or
> invalidating an on-disk super-block. We have to use the page-cache so to
> avoid races between mkfs and udev. See also 6f60cbd3ae44 ("btrfs: access
> superblock via pagecache in scan_one_device").
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>
> ---
> Changes to v6:
> - Warn if we can't write out a page for a superblock (David)
Shouldn't there be some real error handling instead of just a warning?
At least shut down the file system?
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v7 4/8] btrfs: use the page-cache for super block reading
2020-02-12 7:28 ` Christoph Hellwig
@ 2020-02-12 7:43 ` Johannes Thumshirn
0 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:43 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: David Sterba, linux-btrfs @ vger . kernel . org
On 12/02/2020 08:28, Christoph Hellwig wrote:
>> - Warn if we can't write out a page for a superblock (David)
>
> Shouldn't there be some real error handling instead of just a warning?
> At least shut down the file system?
Looking at the callers of btrfs_scratch_superblock() I don't think so.
btrfs_scratch_superblock() is called by btrfs_rm_device(),
btrfs_rm_dev_replace_free_srcdev() and
btrfs_destroy_dev_replace_tgtdev(). So it's all functions related to
removing a device from a multi device file-system. Do you really want to
shut down the file-system if clearing the magic in one of the
super-blocks of a disk that is to be removed from the file-system
doesn't work?
I think it's better to warn the administrator that something didn't work
all to well and to be cautious with that disk if he/she ever want's to
put it back than shut down the whole FS.
Thanks,
Johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 5/8] btrfs: use BIOs instead of buffer_heads from superblock writeout
2020-02-12 7:16 [PATCH v7 0/8] btrfs: remove buffer heads form superblock handling Johannes Thumshirn
` (3 preceding siblings ...)
2020-02-12 7:17 ` [PATCH v7 4/8] btrfs: use the page-cache for super block reading Johannes Thumshirn
@ 2020-02-12 7:17 ` Johannes Thumshirn
2020-02-12 7:30 ` Christoph Hellwig
2020-02-12 7:17 ` [PATCH v7 6/8] btrfs: remove btrfsic_submit_bh() Johannes Thumshirn
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:17 UTC (permalink / raw)
To: David Sterba
Cc: linux-btrfs @ vger . kernel . org, Johannes Thumshirn,
Nikolay Borisov, Josef Bacik
Similar to the superblock read path, change the write path to using BIOs
and pages instead of buffer_heads. This allows us to skip over the
buffer_head code, for writing the superblock to disk.
This is based on a patch originally authored by Nikolay Borisov.
Co-developed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
---
Changes to v4:
- get rid of op_flags (hch)
- don't use mapping_gfp_constraint() (hch)
- print errno on error (hch)
- use typed pointer (hch)
Changes to v2:
- Don't use bi_set_op_attrs() (David)
Changes to v1:
- Remove left-over buffer_head.h include (David)
---
fs/btrfs/disk-io.c | 117 ++++++++++++++++++++++++++-------------------
1 file changed, 67 insertions(+), 50 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 9f422edd6cce..906a48f6c996 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -7,7 +7,6 @@
#include <linux/blkdev.h>
#include <linux/radix-tree.h>
#include <linux/writeback.h>
-#include <linux/buffer_head.h>
#include <linux/workqueue.h>
#include <linux/kthread.h>
#include <linux/slab.h>
@@ -3398,25 +3397,34 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
}
ALLOW_ERROR_INJECTION(open_ctree, ERRNO);
-static void btrfs_end_buffer_write_sync(struct buffer_head *bh, int uptodate)
+static void btrfs_end_super_write(struct bio *bio)
{
- if (uptodate) {
- set_buffer_uptodate(bh);
- } else {
- struct btrfs_device *device = (struct btrfs_device *)
- bh->b_private;
-
- btrfs_warn_rl_in_rcu(device->fs_info,
- "lost page write due to IO error on %s",
- rcu_str_deref(device->name));
- /* note, we don't set_buffer_write_io_error because we have
- * our own ways of dealing with the IO errors
- */
- clear_buffer_uptodate(bh);
- btrfs_dev_stat_inc_and_print(device, BTRFS_DEV_STAT_WRITE_ERRS);
+ struct btrfs_device *device = bio->bi_private;
+ struct bio_vec *bvec;
+ struct bvec_iter_all iter_all;
+ struct page *page;
+
+ bio_for_each_segment_all(bvec, bio, iter_all) {
+ page = bvec->bv_page;
+
+ if (bio->bi_status) {
+ btrfs_warn_rl_in_rcu(device->fs_info,
+ "lost page write due to IO error on %s (%d)",
+ rcu_str_deref(device->name),
+ blk_status_to_errno(bio->bi_status));
+ ClearPageUptodate(page);
+ SetPageError(page);
+ btrfs_dev_stat_inc_and_print(device,
+ BTRFS_DEV_STAT_WRITE_ERRS);
+ } else {
+ SetPageUptodate(page);
+ }
+
+ put_page(page);
+ unlock_page(page);
}
- unlock_buffer(bh);
- put_bh(bh);
+
+ bio_put(bio);
}
struct btrfs_super_block *btrfs_read_dev_one_super(struct block_device *bdev,
@@ -3482,19 +3490,17 @@ struct btrfs_super_block *btrfs_read_dev_super(struct block_device *bdev)
* the expected device size at commit time. Note that max_mirrors must be
* same for write and wait phases.
*
- * Return number of errors when buffer head is not found or submission fails.
+ * Return number of errors when page is not found or submission fails.
*/
static int write_dev_supers(struct btrfs_device *device,
struct btrfs_super_block *sb, int max_mirrors)
{
struct btrfs_fs_info *fs_info = device->fs_info;
+ struct address_space *mapping = device->bdev->bd_inode->i_mapping;
SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
- struct buffer_head *bh;
int i;
- int ret;
int errors = 0;
u64 bytenr;
- int op_flags;
if (max_mirrors == 0)
max_mirrors = BTRFS_SUPER_MIRROR_MAX;
@@ -3502,6 +3508,10 @@ static int write_dev_supers(struct btrfs_device *device,
shash->tfm = fs_info->csum_shash;
for (i = 0; i < max_mirrors; i++) {
+ struct page *page;
+ struct bio *bio;
+ struct btrfs_super_block *disk_super;
+
bytenr = btrfs_sb_offset(i);
if (bytenr + BTRFS_SUPER_INFO_SIZE >=
device->commit_total_bytes)
@@ -3514,37 +3524,44 @@ static int write_dev_supers(struct btrfs_device *device,
BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
crypto_shash_final(shash, sb->csum);
- /* One reference for us, and we leave it for the caller */
- bh = __getblk(device->bdev, bytenr / BTRFS_BDEV_BLOCKSIZE,
- BTRFS_SUPER_INFO_SIZE);
- if (!bh) {
+ page = find_or_create_page(mapping, bytenr >> PAGE_SHIFT,
+ GFP_NOFS);
+ if (!page) {
btrfs_err(device->fs_info,
- "couldn't get super buffer head for bytenr %llu",
+ "couldn't get superblock page for bytenr %llu",
bytenr);
errors++;
continue;
}
- memcpy(bh->b_data, sb, BTRFS_SUPER_INFO_SIZE);
+ /* Bump the refcount for wait_dev_supers() */
+ get_page(page);
- /* one reference for submit_bh */
- get_bh(bh);
+ disk_super = page_address(page);
+ memcpy(disk_super, sb, BTRFS_SUPER_INFO_SIZE);
- set_buffer_uptodate(bh);
- lock_buffer(bh);
- bh->b_end_io = btrfs_end_buffer_write_sync;
- bh->b_private = device;
+ /*
+ * Directly use BIOs here instead of relying on the page-cache
+ * to do I/O, so we don't loose the ability to do integrity
+ * checking.
+ */
+ bio = bio_alloc(GFP_NOFS, 1);
+ bio_set_dev(bio, device->bdev);
+ bio->bi_iter.bi_sector = bytenr >> SECTOR_SHIFT;
+ bio->bi_private = device;
+ bio->bi_end_io = btrfs_end_super_write;
+ __bio_add_page(bio, page, BTRFS_SUPER_INFO_SIZE,
+ offset_in_page(bytenr));
/*
- * we fua the first super. The others we allow
+ * We fua the first super. The others we allow
* to go down lazy.
*/
- op_flags = REQ_SYNC | REQ_META | REQ_PRIO;
+ bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_META | REQ_PRIO;
if (i == 0 && !btrfs_test_opt(device->fs_info, NOBARRIER))
- op_flags |= REQ_FUA;
- ret = btrfsic_submit_bh(REQ_OP_WRITE, op_flags, bh);
- if (ret)
- errors++;
+ bio->bi_opf |= REQ_FUA;
+
+ btrfsic_submit_bio(bio);
}
return errors < i ? 0 : -1;
}
@@ -3553,12 +3570,11 @@ static int write_dev_supers(struct btrfs_device *device,
* Wait for write completion of superblocks done by write_dev_supers,
* @max_mirrors same for write and wait phases.
*
- * Return number of errors when buffer head is not found or not marked up to
+ * Return number of errors when page is not found or not marked up to
* date.
*/
static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
{
- struct buffer_head *bh;
int i;
int errors = 0;
bool primary_failed = false;
@@ -3568,32 +3584,33 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
max_mirrors = BTRFS_SUPER_MIRROR_MAX;
for (i = 0; i < max_mirrors; i++) {
+ struct page *page;
+
bytenr = btrfs_sb_offset(i);
if (bytenr + BTRFS_SUPER_INFO_SIZE >=
device->commit_total_bytes)
break;
- bh = __find_get_block(device->bdev,
- bytenr / BTRFS_BDEV_BLOCKSIZE,
- BTRFS_SUPER_INFO_SIZE);
- if (!bh) {
+ page = find_get_page(device->bdev->bd_inode->i_mapping,
+ bytenr >> PAGE_SHIFT);
+ if (!page) {
errors++;
if (i == 0)
primary_failed = true;
continue;
}
- wait_on_buffer(bh);
- if (!buffer_uptodate(bh)) {
+ wait_on_page_locked(page);
+ if (PageError(page)) {
errors++;
if (i == 0)
primary_failed = true;
}
/* drop our reference */
- brelse(bh);
+ put_page(page);
/* drop the reference from the writing run */
- brelse(bh);
+ put_page(page);
}
/* log error, force error return */
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 6/8] btrfs: remove btrfsic_submit_bh()
2020-02-12 7:16 [PATCH v7 0/8] btrfs: remove buffer heads form superblock handling Johannes Thumshirn
` (4 preceding siblings ...)
2020-02-12 7:17 ` [PATCH v7 5/8] btrfs: use BIOs instead of buffer_heads from superblock writeout Johannes Thumshirn
@ 2020-02-12 7:17 ` Johannes Thumshirn
2020-02-12 7:17 ` [PATCH v7 7/8] btrfs: remove buffer_heads from btrfsic_process_written_block() Johannes Thumshirn
2020-02-12 7:17 ` [PATCH v7 8/8] btrfs: remove buffer_heads form superblock mirror integrity checking Johannes Thumshirn
7 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:17 UTC (permalink / raw)
To: David Sterba
Cc: linux-btrfs @ vger . kernel . org, Johannes Thumshirn,
Nikolay Borisov, Josef Bacik
Now that the last use of btrfsic_submit_bh() is gone, remove the function
as well.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/check-integrity.c | 57 --------------------------------------
fs/btrfs/check-integrity.h | 2 --
2 files changed, 59 deletions(-)
diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index a0ce69f2d27c..e7507985435e 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -2730,63 +2730,6 @@ static struct btrfsic_dev_state *btrfsic_dev_state_lookup(dev_t dev)
&btrfsic_dev_state_hashtable);
}
-int btrfsic_submit_bh(int op, int op_flags, struct buffer_head *bh)
-{
- struct btrfsic_dev_state *dev_state;
-
- if (!btrfsic_is_initialized)
- return submit_bh(op, op_flags, bh);
-
- mutex_lock(&btrfsic_mutex);
- /* since btrfsic_submit_bh() might also be called before
- * btrfsic_mount(), this might return NULL */
- dev_state = btrfsic_dev_state_lookup(bh->b_bdev->bd_dev);
-
- /* Only called to write the superblock (incl. FLUSH/FUA) */
- if (NULL != dev_state &&
- (op == REQ_OP_WRITE) && bh->b_size > 0) {
- u64 dev_bytenr;
-
- dev_bytenr = BTRFS_BDEV_BLOCKSIZE * bh->b_blocknr;
- if (dev_state->state->print_mask &
- BTRFSIC_PRINT_MASK_SUBMIT_BIO_BH)
- pr_info("submit_bh(op=0x%x,0x%x, blocknr=%llu (bytenr %llu), size=%zu, data=%p, bdev=%p)\n",
- op, op_flags, (unsigned long long)bh->b_blocknr,
- dev_bytenr, bh->b_size, bh->b_data, bh->b_bdev);
- btrfsic_process_written_block(dev_state, dev_bytenr,
- &bh->b_data, 1, NULL,
- NULL, bh, op_flags);
- } else if (NULL != dev_state && (op_flags & REQ_PREFLUSH)) {
- if (dev_state->state->print_mask &
- BTRFSIC_PRINT_MASK_SUBMIT_BIO_BH)
- pr_info("submit_bh(op=0x%x,0x%x FLUSH, bdev=%p)\n",
- op, op_flags, bh->b_bdev);
- if (!dev_state->dummy_block_for_bio_bh_flush.is_iodone) {
- if ((dev_state->state->print_mask &
- (BTRFSIC_PRINT_MASK_SUBMIT_BIO_BH |
- BTRFSIC_PRINT_MASK_VERBOSE)))
- pr_info("btrfsic_submit_bh(%s) with FLUSH but dummy block already in use (ignored)!\n",
- dev_state->name);
- } else {
- struct btrfsic_block *const block =
- &dev_state->dummy_block_for_bio_bh_flush;
-
- block->is_iodone = 0;
- block->never_written = 0;
- block->iodone_w_error = 0;
- block->flush_gen = dev_state->last_flush_gen + 1;
- block->submit_bio_bh_rw = op_flags;
- block->orig_bio_bh_private = bh->b_private;
- block->orig_bio_bh_end_io.bh = bh->b_end_io;
- block->next_in_same_bio = NULL;
- bh->b_private = block;
- bh->b_end_io = btrfsic_bh_end_io;
- }
- }
- mutex_unlock(&btrfsic_mutex);
- return submit_bh(op, op_flags, bh);
-}
-
static void __btrfsic_submit_bio(struct bio *bio)
{
struct btrfsic_dev_state *dev_state;
diff --git a/fs/btrfs/check-integrity.h b/fs/btrfs/check-integrity.h
index 9bf4359cc44c..bcc730a06cb5 100644
--- a/fs/btrfs/check-integrity.h
+++ b/fs/btrfs/check-integrity.h
@@ -7,11 +7,9 @@
#define BTRFS_CHECK_INTEGRITY_H
#ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
-int btrfsic_submit_bh(int op, int op_flags, struct buffer_head *bh);
void btrfsic_submit_bio(struct bio *bio);
int btrfsic_submit_bio_wait(struct bio *bio);
#else
-#define btrfsic_submit_bh submit_bh
#define btrfsic_submit_bio submit_bio
#define btrfsic_submit_bio_wait submit_bio_wait
#endif
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 7/8] btrfs: remove buffer_heads from btrfsic_process_written_block()
2020-02-12 7:16 [PATCH v7 0/8] btrfs: remove buffer heads form superblock handling Johannes Thumshirn
` (5 preceding siblings ...)
2020-02-12 7:17 ` [PATCH v7 6/8] btrfs: remove btrfsic_submit_bh() Johannes Thumshirn
@ 2020-02-12 7:17 ` Johannes Thumshirn
2020-02-12 7:17 ` [PATCH v7 8/8] btrfs: remove buffer_heads form superblock mirror integrity checking Johannes Thumshirn
7 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:17 UTC (permalink / raw)
To: David Sterba
Cc: linux-btrfs @ vger . kernel . org, Johannes Thumshirn,
Josef Bacik, Nikolay Borisov
Now that the last caller of btrfsic_process_written_block() with
buffer_heads is gone, remove the buffer_head processing path from it as
well.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/check-integrity.c | 103 +++++++++----------------------------
1 file changed, 25 insertions(+), 78 deletions(-)
diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index e7507985435e..4f6db2fe482a 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -152,11 +152,8 @@ struct btrfsic_block {
struct list_head ref_to_list; /* list */
struct list_head ref_from_list; /* list */
struct btrfsic_block *next_in_same_bio;
- void *orig_bio_bh_private;
- union {
- bio_end_io_t *bio;
- bh_end_io_t *bh;
- } orig_bio_bh_end_io;
+ void *orig_bio_private;
+ bio_end_io_t *orig_bio_end_io;
int submit_bio_bh_rw;
u64 flush_gen; /* only valid if !never_written */
};
@@ -325,14 +322,12 @@ static void btrfsic_process_written_block(struct btrfsic_dev_state *dev_state,
u64 dev_bytenr, char **mapped_datav,
unsigned int num_pages,
struct bio *bio, int *bio_is_patched,
- struct buffer_head *bh,
int submit_bio_bh_rw);
static int btrfsic_process_written_superblock(
struct btrfsic_state *state,
struct btrfsic_block *const block,
struct btrfs_super_block *const super_hdr);
static void btrfsic_bio_end_io(struct bio *bp);
-static void btrfsic_bh_end_io(struct buffer_head *bh, int uptodate);
static int btrfsic_is_block_ref_by_superblock(const struct btrfsic_state *state,
const struct btrfsic_block *block,
int recursion_level);
@@ -399,8 +394,8 @@ static void btrfsic_block_init(struct btrfsic_block *b)
b->never_written = 0;
b->mirror_num = 0;
b->next_in_same_bio = NULL;
- b->orig_bio_bh_private = NULL;
- b->orig_bio_bh_end_io.bio = NULL;
+ b->orig_bio_private = NULL;
+ b->orig_bio_end_io = NULL;
INIT_LIST_HEAD(&b->collision_resolving_node);
INIT_LIST_HEAD(&b->all_blocks_node);
INIT_LIST_HEAD(&b->ref_to_list);
@@ -1743,7 +1738,6 @@ static void btrfsic_process_written_block(struct btrfsic_dev_state *dev_state,
u64 dev_bytenr, char **mapped_datav,
unsigned int num_pages,
struct bio *bio, int *bio_is_patched,
- struct buffer_head *bh,
int submit_bio_bh_rw)
{
int is_metadata;
@@ -1902,9 +1896,9 @@ static void btrfsic_process_written_block(struct btrfsic_dev_state *dev_state,
block->is_iodone = 0;
BUG_ON(NULL == bio_is_patched);
if (!*bio_is_patched) {
- block->orig_bio_bh_private =
+ block->orig_bio_private =
bio->bi_private;
- block->orig_bio_bh_end_io.bio =
+ block->orig_bio_end_io =
bio->bi_end_io;
block->next_in_same_bio = NULL;
bio->bi_private = block;
@@ -1916,25 +1910,17 @@ static void btrfsic_process_written_block(struct btrfsic_dev_state *dev_state,
bio->bi_private;
BUG_ON(NULL == chained_block);
- block->orig_bio_bh_private =
- chained_block->orig_bio_bh_private;
- block->orig_bio_bh_end_io.bio =
- chained_block->orig_bio_bh_end_io.
- bio;
+ block->orig_bio_private =
+ chained_block->orig_bio_private;
+ block->orig_bio_end_io =
+ chained_block->orig_bio_end_io;
block->next_in_same_bio = chained_block;
bio->bi_private = block;
}
- } else if (NULL != bh) {
- block->is_iodone = 0;
- block->orig_bio_bh_private = bh->b_private;
- block->orig_bio_bh_end_io.bh = bh->b_end_io;
- block->next_in_same_bio = NULL;
- bh->b_private = block;
- bh->b_end_io = btrfsic_bh_end_io;
} else {
block->is_iodone = 1;
- block->orig_bio_bh_private = NULL;
- block->orig_bio_bh_end_io.bio = NULL;
+ block->orig_bio_private = NULL;
+ block->orig_bio_end_io = NULL;
block->next_in_same_bio = NULL;
}
}
@@ -2042,8 +2028,8 @@ static void btrfsic_process_written_block(struct btrfsic_dev_state *dev_state,
block->is_iodone = 0;
BUG_ON(NULL == bio_is_patched);
if (!*bio_is_patched) {
- block->orig_bio_bh_private = bio->bi_private;
- block->orig_bio_bh_end_io.bio = bio->bi_end_io;
+ block->orig_bio_private = bio->bi_private;
+ block->orig_bio_end_io = bio->bi_end_io;
block->next_in_same_bio = NULL;
bio->bi_private = block;
bio->bi_end_io = btrfsic_bio_end_io;
@@ -2054,24 +2040,17 @@ static void btrfsic_process_written_block(struct btrfsic_dev_state *dev_state,
bio->bi_private;
BUG_ON(NULL == chained_block);
- block->orig_bio_bh_private =
- chained_block->orig_bio_bh_private;
- block->orig_bio_bh_end_io.bio =
- chained_block->orig_bio_bh_end_io.bio;
+ block->orig_bio_private =
+ chained_block->orig_bio_private;
+ block->orig_bio_end_io =
+ chained_block->orig_bio_end_io;
block->next_in_same_bio = chained_block;
bio->bi_private = block;
}
- } else if (NULL != bh) {
- block->is_iodone = 0;
- block->orig_bio_bh_private = bh->b_private;
- block->orig_bio_bh_end_io.bh = bh->b_end_io;
- block->next_in_same_bio = NULL;
- bh->b_private = block;
- bh->b_end_io = btrfsic_bh_end_io;
} else {
block->is_iodone = 1;
- block->orig_bio_bh_private = NULL;
- block->orig_bio_bh_end_io.bio = NULL;
+ block->orig_bio_private = NULL;
+ block->orig_bio_end_io = NULL;
block->next_in_same_bio = NULL;
}
if (state->print_mask & BTRFSIC_PRINT_MASK_VERBOSE)
@@ -2112,8 +2091,8 @@ static void btrfsic_bio_end_io(struct bio *bp)
iodone_w_error = 1;
BUG_ON(NULL == block);
- bp->bi_private = block->orig_bio_bh_private;
- bp->bi_end_io = block->orig_bio_bh_end_io.bio;
+ bp->bi_private = block->orig_bio_private;
+ bp->bi_end_io = block->orig_bio_end_io;
do {
struct btrfsic_block *next_block;
@@ -2146,38 +2125,6 @@ static void btrfsic_bio_end_io(struct bio *bp)
bp->bi_end_io(bp);
}
-static void btrfsic_bh_end_io(struct buffer_head *bh, int uptodate)
-{
- struct btrfsic_block *block = (struct btrfsic_block *)bh->b_private;
- int iodone_w_error = !uptodate;
- struct btrfsic_dev_state *dev_state;
-
- BUG_ON(NULL == block);
- dev_state = block->dev_state;
- if ((dev_state->state->print_mask & BTRFSIC_PRINT_MASK_END_IO_BIO_BH))
- pr_info("bh_end_io(error=%d) for %c @%llu (%s/%llu/%d)\n",
- iodone_w_error,
- btrfsic_get_block_type(dev_state->state, block),
- block->logical_bytenr, block->dev_state->name,
- block->dev_bytenr, block->mirror_num);
-
- block->iodone_w_error = iodone_w_error;
- if (block->submit_bio_bh_rw & REQ_PREFLUSH) {
- dev_state->last_flush_gen++;
- if ((dev_state->state->print_mask &
- BTRFSIC_PRINT_MASK_END_IO_BIO_BH))
- pr_info("bh_end_io() new %s flush_gen=%llu\n",
- dev_state->name, dev_state->last_flush_gen);
- }
- if (block->submit_bio_bh_rw & REQ_FUA)
- block->flush_gen = 0; /* FUA completed means block is on disk */
-
- bh->b_private = block->orig_bio_bh_private;
- bh->b_end_io = block->orig_bio_bh_end_io.bh;
- block->is_iodone = 1; /* for FLUSH, this releases the block */
- bh->b_end_io(bh, uptodate);
-}
-
static int btrfsic_process_written_superblock(
struct btrfsic_state *state,
struct btrfsic_block *const superblock,
@@ -2781,7 +2728,7 @@ static void __btrfsic_submit_bio(struct bio *bio)
btrfsic_process_written_block(dev_state, dev_bytenr,
mapped_datav, segs,
bio, &bio_is_patched,
- NULL, bio->bi_opf);
+ bio->bi_opf);
bio_for_each_segment(bvec, bio, iter)
kunmap(bvec.bv_page);
kfree(mapped_datav);
@@ -2805,8 +2752,8 @@ static void __btrfsic_submit_bio(struct bio *bio)
block->iodone_w_error = 0;
block->flush_gen = dev_state->last_flush_gen + 1;
block->submit_bio_bh_rw = bio->bi_opf;
- block->orig_bio_bh_private = bio->bi_private;
- block->orig_bio_bh_end_io.bio = bio->bi_end_io;
+ block->orig_bio_private = bio->bi_private;
+ block->orig_bio_end_io = bio->bi_end_io;
block->next_in_same_bio = NULL;
bio->bi_private = block;
bio->bi_end_io = btrfsic_bio_end_io;
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 8/8] btrfs: remove buffer_heads form superblock mirror integrity checking
2020-02-12 7:16 [PATCH v7 0/8] btrfs: remove buffer heads form superblock handling Johannes Thumshirn
` (6 preceding siblings ...)
2020-02-12 7:17 ` [PATCH v7 7/8] btrfs: remove buffer_heads from btrfsic_process_written_block() Johannes Thumshirn
@ 2020-02-12 7:17 ` Johannes Thumshirn
2020-02-12 17:06 ` David Sterba
7 siblings, 1 reply; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-12 7:17 UTC (permalink / raw)
To: David Sterba
Cc: linux-btrfs @ vger . kernel . org, Johannes Thumshirn,
Josef Bacik, Nikolay Borisov
The integrity checking code for the superblock mirrors is the last remaining
user of buffer_heads in BTRFS, change it to using plain BIOs as well.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
Changes to v4:
- Remove mapping_gfp_constraint()
Changes to v2:
- Open-code kunmap() + put_page() (David)
- Remove __GFP_NOFAIL from allocation (Josef)
- Merge error paths (David)
Changes to v1:
- Convert from alloc_page() to find_or_create_page()
---
fs/btrfs/check-integrity.c | 55 +++++++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 19 deletions(-)
diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 4f6db2fe482a..e10a27d6db08 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -77,7 +77,6 @@
#include <linux/sched.h>
#include <linux/slab.h>
-#include <linux/buffer_head.h>
#include <linux/mutex.h>
#include <linux/genhd.h>
#include <linux/blkdev.h>
@@ -762,29 +761,45 @@ static int btrfsic_process_superblock_dev_mirror(
struct btrfs_fs_info *fs_info = state->fs_info;
struct btrfs_super_block *super_tmp;
u64 dev_bytenr;
- struct buffer_head *bh;
struct btrfsic_block *superblock_tmp;
int pass;
struct block_device *const superblock_bdev = device->bdev;
+ struct page *page;
+ struct bio bio;
+ struct bio_vec bio_vec;
+ struct address_space *mapping = superblock_bdev->bd_inode->i_mapping;
+ int ret;
/* super block bytenr is always the unmapped device bytenr */
dev_bytenr = btrfs_sb_offset(superblock_mirror_num);
if (dev_bytenr + BTRFS_SUPER_INFO_SIZE > device->commit_total_bytes)
return -1;
- bh = __bread(superblock_bdev, dev_bytenr / BTRFS_BDEV_BLOCKSIZE,
- BTRFS_SUPER_INFO_SIZE);
- if (NULL == bh)
+
+ page = find_or_create_page(mapping, dev_bytenr >> PAGE_SHIFT, GFP_NOFS);
+ if (!page)
+ return -1;
+
+ bio_init(&bio, &bio_vec, 1);
+ bio.bi_iter.bi_sector = dev_bytenr >> SECTOR_SHIFT;
+ bio_set_dev(&bio, superblock_bdev);
+ bio_set_op_attrs(&bio, REQ_OP_READ, 0);
+ bio_add_page(&bio, page, BTRFS_SUPER_INFO_SIZE, 0);
+
+ ret = submit_bio_wait(&bio);
+ if (ret)
return -1;
- super_tmp = (struct btrfs_super_block *)
- (bh->b_data + (dev_bytenr & (BTRFS_BDEV_BLOCKSIZE - 1)));
+
+ unlock_page(page);
+
+ super_tmp = kmap(page);
if (btrfs_super_bytenr(super_tmp) != dev_bytenr ||
btrfs_super_magic(super_tmp) != BTRFS_MAGIC ||
memcmp(device->uuid, super_tmp->dev_item.uuid, BTRFS_UUID_SIZE) ||
btrfs_super_nodesize(super_tmp) != state->metablock_size ||
btrfs_super_sectorsize(super_tmp) != state->datablock_size) {
- brelse(bh);
- return 0;
+ ret = 0;
+ goto out_unmap;
}
superblock_tmp =
@@ -795,8 +810,8 @@ static int btrfsic_process_superblock_dev_mirror(
superblock_tmp = btrfsic_block_alloc();
if (NULL == superblock_tmp) {
pr_info("btrfsic: error, kmalloc failed!\n");
- brelse(bh);
- return -1;
+ ret = -1;
+ goto out_unmap;
}
/* for superblock, only the dev_bytenr makes sense */
superblock_tmp->dev_bytenr = dev_bytenr;
@@ -880,8 +895,8 @@ static int btrfsic_process_superblock_dev_mirror(
mirror_num)) {
pr_info("btrfsic: btrfsic_map_block(bytenr @%llu, mirror %d) failed!\n",
next_bytenr, mirror_num);
- brelse(bh);
- return -1;
+ ret = -1;
+ goto out_unmap;
}
next_block = btrfsic_block_lookup_or_add(
@@ -890,8 +905,8 @@ static int btrfsic_process_superblock_dev_mirror(
mirror_num, NULL);
if (NULL == next_block) {
btrfsic_release_block_ctx(&tmp_next_block_ctx);
- brelse(bh);
- return -1;
+ ret = -1;
+ goto out_unmap;
}
next_block->disk_key = tmp_disk_key;
@@ -902,16 +917,18 @@ static int btrfsic_process_superblock_dev_mirror(
BTRFSIC_GENERATION_UNKNOWN);
btrfsic_release_block_ctx(&tmp_next_block_ctx);
if (NULL == l) {
- brelse(bh);
- return -1;
+ ret = -1;
+ goto out_unmap;
}
}
}
if (state->print_mask & BTRFSIC_PRINT_MASK_INITIAL_ALL_TREES)
btrfsic_dump_tree_sub(state, superblock_tmp, 0);
- brelse(bh);
- return 0;
+out_unmap:
+ kunmap(page);
+ put_page(page);
+ return ret;
}
static struct btrfsic_stack_frame *btrfsic_stack_frame_alloc(void)
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 8/8] btrfs: remove buffer_heads form superblock mirror integrity checking
2020-02-12 7:17 ` [PATCH v7 8/8] btrfs: remove buffer_heads form superblock mirror integrity checking Johannes Thumshirn
@ 2020-02-12 17:06 ` David Sterba
2020-02-13 9:12 ` Johannes Thumshirn
0 siblings, 1 reply; 17+ messages in thread
From: David Sterba @ 2020-02-12 17:06 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: David Sterba, linux-btrfs @ vger . kernel . org, Josef Bacik,
Nikolay Borisov
On Wed, Feb 12, 2020 at 04:17:04PM +0900, Johannes Thumshirn wrote:
> @@ -762,29 +761,45 @@ static int btrfsic_process_superblock_dev_mirror(
> struct btrfs_fs_info *fs_info = state->fs_info;
> struct btrfs_super_block *super_tmp;
> u64 dev_bytenr;
> - struct buffer_head *bh;
> struct btrfsic_block *superblock_tmp;
> int pass;
> struct block_device *const superblock_bdev = device->bdev;
> + struct page *page;
> + struct bio bio;
> + struct bio_vec bio_vec;
> + struct address_space *mapping = superblock_bdev->bd_inode->i_mapping;
> + int ret;
>
> /* super block bytenr is always the unmapped device bytenr */
> dev_bytenr = btrfs_sb_offset(superblock_mirror_num);
> if (dev_bytenr + BTRFS_SUPER_INFO_SIZE > device->commit_total_bytes)
> return -1;
> - bh = __bread(superblock_bdev, dev_bytenr / BTRFS_BDEV_BLOCKSIZE,
> - BTRFS_SUPER_INFO_SIZE);
> - if (NULL == bh)
> +
> + page = find_or_create_page(mapping, dev_bytenr >> PAGE_SHIFT, GFP_NOFS);
> + if (!page)
> + return -1;
> +
> + bio_init(&bio, &bio_vec, 1);
> + bio.bi_iter.bi_sector = dev_bytenr >> SECTOR_SHIFT;
> + bio_set_dev(&bio, superblock_bdev);
> + bio_set_op_attrs(&bio, REQ_OP_READ, 0);
> + bio_add_page(&bio, page, BTRFS_SUPER_INFO_SIZE, 0);
> +
> + ret = submit_bio_wait(&bio);
> + if (ret)
> return -1;
> - super_tmp = (struct btrfs_super_block *)
> - (bh->b_data + (dev_bytenr & (BTRFS_BDEV_BLOCKSIZE - 1)));
> +
> + unlock_page(page);
> +
> + super_tmp = kmap(page);
I beleve the same reasoning applies here regarding kmap, it's the bdev
mapping and we won't get a highmem page.
>
> if (btrfs_super_bytenr(super_tmp) != dev_bytenr ||
> btrfs_super_magic(super_tmp) != BTRFS_MAGIC ||
> memcmp(device->uuid, super_tmp->dev_item.uuid, BTRFS_UUID_SIZE) ||
> btrfs_super_nodesize(super_tmp) != state->metablock_size ||
> btrfs_super_sectorsize(super_tmp) != state->datablock_size) {
> - brelse(bh);
> - return 0;
> + ret = 0;
> + goto out_unmap;
> }
>
> superblock_tmp =
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v7 8/8] btrfs: remove buffer_heads form superblock mirror integrity checking
2020-02-12 17:06 ` David Sterba
@ 2020-02-13 9:12 ` Johannes Thumshirn
0 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2020-02-13 9:12 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs @ vger . kernel . org, Josef Bacik, Nikolay Borisov
On 12/02/2020 18:06, David Sterba wrote:
> On Wed, Feb 12, 2020 at 04:17:04PM +0900, Johannes Thumshirn wrote:
>> @@ -762,29 +761,45 @@ static int btrfsic_process_superblock_dev_mirror(
>> struct btrfs_fs_info *fs_info = state->fs_info;
>> struct btrfs_super_block *super_tmp;
>> u64 dev_bytenr;
>> - struct buffer_head *bh;
>> struct btrfsic_block *superblock_tmp;
>> int pass;
>> struct block_device *const superblock_bdev = device->bdev;
>> + struct page *page;
>> + struct bio bio;
>> + struct bio_vec bio_vec;
>> + struct address_space *mapping = superblock_bdev->bd_inode->i_mapping;
>> + int ret;
>>
>> /* super block bytenr is always the unmapped device bytenr */
>> dev_bytenr = btrfs_sb_offset(superblock_mirror_num);
>> if (dev_bytenr + BTRFS_SUPER_INFO_SIZE > device->commit_total_bytes)
>> return -1;
>> - bh = __bread(superblock_bdev, dev_bytenr / BTRFS_BDEV_BLOCKSIZE,
>> - BTRFS_SUPER_INFO_SIZE);
>> - if (NULL == bh)
>> +
>> + page = find_or_create_page(mapping, dev_bytenr >> PAGE_SHIFT, GFP_NOFS);
>> + if (!page)
>> + return -1;
>> +
>> + bio_init(&bio, &bio_vec, 1);
>> + bio.bi_iter.bi_sector = dev_bytenr >> SECTOR_SHIFT;
>> + bio_set_dev(&bio, superblock_bdev);
>> + bio_set_op_attrs(&bio, REQ_OP_READ, 0);
>> + bio_add_page(&bio, page, BTRFS_SUPER_INFO_SIZE, 0);
>> +
>> + ret = submit_bio_wait(&bio);
>> + if (ret)
>> return -1;
>> - super_tmp = (struct btrfs_super_block *)
>> - (bh->b_data + (dev_bytenr & (BTRFS_BDEV_BLOCKSIZE - 1)));
>> +
>> + unlock_page(page);
>> +
>> + super_tmp = kmap(page);
>
> I beleve the same reasoning applies here regarding kmap, it's the bdev
> mapping and we won't get a highmem page.
>
Ah damn it my bad and __bio_add_page() as well. Can I send you a
follow-on patch to fold in?
^ permalink raw reply [flat|nested] 17+ messages in thread