* [RFC PATCH v3 04/12] btrfs: get rid of trivial __btrfs_lookup_bio_sums() wrappers
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Currently, we have two wrappers for __btrfs_lookup_bio_sums():
btrfs_lookup_bio_sums_dio(), which is used for direct I/O, and
btrfs_lookup_bio_sums(), which is used everywhere else. The only
difference is that the _dio variant looks up csums starting at the given
offset instead of using the page index, which isn't actually direct
I/O-specific. Let's clean up the signature and return value of
__btrfs_lookup_bio_sums(), rename it to btrfs_lookup_bio_sums(), and get
rid of the trivial helpers.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/compression.c | 4 ++--
fs/btrfs/ctree.h | 4 +---
fs/btrfs/file-item.c | 35 +++++++++++++++++------------------
fs/btrfs/inode.c | 6 +++---
4 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index b05b361e2062..4df6f0c58dc9 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -660,7 +660,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
ret = btrfs_lookup_bio_sums(inode, comp_bio,
- sums);
+ false, 0, sums);
BUG_ON(ret); /* -ENOMEM */
}
@@ -689,7 +689,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
BUG_ON(ret); /* -ENOMEM */
if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
- ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
+ ret = btrfs_lookup_bio_sums(inode, comp_bio, false, 0, sums);
BUG_ON(ret); /* -ENOMEM */
}
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index fe2b8765d9e6..4bc40bf49b0e 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2787,9 +2787,7 @@ struct btrfs_dio_private;
int btrfs_del_csums(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info, u64 bytenr, u64 len);
blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
- u8 *dst);
-blk_status_t btrfs_lookup_bio_sums_dio(struct inode *inode, struct bio *bio,
- u64 logical_offset);
+ bool at_offset, u64 offset, u8 *dst);
int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
u64 objectid, u64 pos,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 1a599f50837b..a87c40502267 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -148,8 +148,21 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
return ret;
}
-static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
- u64 logical_offset, u8 *dst, int dio)
+/**
+ * btrfs_lookup_bio_sums - Look up checksums for a bio.
+ * @inode: inode that the bio is for.
+ * @bio: bio embedded in btrfs_io_bio.
+ * @at_offset: If true, look up checksums for the extent at @c offset.
+ * If false, use the page offsets from the bio.
+ * @offset: If @at_offset is true, offset in file to look up checksums for.
+ * Ignored otherwise.
+ * @dst: Buffer of size btrfs_super_csum_size() used to return checksum. If
+ * NULL, the checksum is returned in btrfs_io_bio(bio)->csum instead.
+ *
+ * Return: BLK_STS_RESOURCE if allocating memory fails, BLK_STS_OK otherwise.
+ */
+blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
+ bool at_offset, u64 offset, u8 *dst)
{
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct bio_vec bvec;
@@ -159,7 +172,6 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
struct btrfs_path *path;
u8 *csum;
- u64 offset = 0;
u64 item_start_offset = 0;
u64 item_last_offset = 0;
u64 disk_bytenr;
@@ -205,15 +217,13 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
}
disk_bytenr = (u64)bio->bi_iter.bi_sector << 9;
- if (dio)
- offset = logical_offset;
bio_for_each_segment(bvec, bio, iter) {
page_bytes_left = bvec.bv_len;
if (count)
goto next;
- if (!dio)
+ if (!at_offset)
offset = page_offset(bvec.bv_page) + bvec.bv_offset;
count = btrfs_find_ordered_sum(inode, offset, disk_bytenr,
csum, nblocks);
@@ -285,18 +295,7 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
WARN_ON_ONCE(count);
btrfs_free_path(path);
- return 0;
-}
-
-blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
- u8 *dst)
-{
- return __btrfs_lookup_bio_sums(inode, bio, 0, dst, 0);
-}
-
-blk_status_t btrfs_lookup_bio_sums_dio(struct inode *inode, struct bio *bio, u64 offset)
-{
- return __btrfs_lookup_bio_sums(inode, bio, offset, NULL, 1);
+ return BLK_STS_OK;
}
int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 015910079e73..ad5bffb24199 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2090,7 +2090,7 @@ static blk_status_t btrfs_submit_bio_hook(struct inode *inode, struct bio *bio,
bio_flags);
goto out;
} else if (!skip_sum) {
- ret = btrfs_lookup_bio_sums(inode, bio, NULL);
+ ret = btrfs_lookup_bio_sums(inode, bio, false, 0, NULL);
if (ret)
goto out;
}
@@ -8332,8 +8332,8 @@ static inline blk_status_t btrfs_lookup_and_bind_dio_csum(struct inode *inode,
* contention.
*/
if (dip->logical_offset == file_offset) {
- ret = btrfs_lookup_bio_sums_dio(inode, dip->orig_bio,
- file_offset);
+ ret = btrfs_lookup_bio_sums(inode, dip->orig_bio, true,
+ file_offset, NULL);
if (ret)
return ret;
}
--
2.24.0
^ permalink raw reply related
* [RFC PATCH v3 05/12] btrfs: don't advance offset for compressed bios in btrfs_csum_one_bio()
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
btrfs_csum_one_bio() loops over each sector in the bio while keeping a
cursor of its current logical position in the file in order to look up
the ordered extent to add the checksums to. However, this doesn't make
much sense for compressed extents, as a sector on disk does not
correspond to a sector of decompressed file data. It happens to work
because 1) the compressed bio always covers one ordered extent and 2)
the size of the bio is always less than the size of the ordered extent.
However, the second point will not always be true for encoded writes.
Let's add a boolean parameter to btrfs_csum_one_bio() to indicate that
it can assume that the bio only covers one ordered extent. Since we're
already changing the signature, let's make contig bool instead of int,
too.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/compression.c | 5 +++--
fs/btrfs/ctree.h | 2 +-
fs/btrfs/file-item.c | 19 +++++++++++--------
fs/btrfs/inode.c | 8 ++++----
4 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 4df6f0c58dc9..05b6e404a291 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -374,7 +374,8 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
BUG_ON(ret); /* -ENOMEM */
if (!skip_sum) {
- ret = btrfs_csum_one_bio(inode, bio, start, 1);
+ ret = btrfs_csum_one_bio(inode, bio, start,
+ true, true);
BUG_ON(ret); /* -ENOMEM */
}
@@ -405,7 +406,7 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
BUG_ON(ret); /* -ENOMEM */
if (!skip_sum) {
- ret = btrfs_csum_one_bio(inode, bio, start, 1);
+ ret = btrfs_csum_one_bio(inode, bio, start, true, true);
BUG_ON(ret); /* -ENOMEM */
}
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 4bc40bf49b0e..c32741879088 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2802,7 +2802,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
struct btrfs_ordered_sum *sums);
blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
- u64 file_start, int contig);
+ u64 file_start, bool contig, bool one_ordered);
int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
struct list_head *list, int search_commit);
void btrfs_extent_item_to_extent_map(struct btrfs_inode *inode,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index a87c40502267..c95772949b00 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -423,13 +423,14 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
* @inode: Owner of the data inside the bio
* @bio: Contains the data to be checksummed
* @file_start: offset in file this bio begins to describe
- * @contig: Boolean. If true/1 means all bio vecs in this bio are
- * contiguous and they begin at @file_start in the file. False/0
- * means this bio can contains potentially discontigous bio vecs
- * so the logical offset of each should be calculated separately.
+ * @contig: If true, all bio vecs in @bio are contiguous and they begin at
+ * @file_start in the file. If false, @bio may contain
+ * discontigous bio vecs, so the logical offset of each should be
+ * calculated separately (@file_start is ignored).
+ * @one_ordered: If true, @bio only refers to one ordered extent.
*/
blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
- u64 file_start, int contig)
+ u64 file_start, bool contig, bool one_ordered)
{
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
@@ -482,8 +483,9 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
- 1);
for (i = 0; i < nr_sectors; i++) {
- if (offset >= ordered->file_offset + ordered->len ||
- offset < ordered->file_offset) {
+ if (!one_ordered &&
+ (offset >= ordered->file_offset + ordered->len ||
+ offset < ordered->file_offset)) {
unsigned long bytes_left;
sums->len = this_sum_bytes;
@@ -515,7 +517,8 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
kunmap_atomic(data);
crypto_shash_final(shash, (char *)(sums->sums + index));
index += csum_size;
- offset += fs_info->sectorsize;
+ if (!one_ordered)
+ offset += fs_info->sectorsize;
this_sum_bytes += fs_info->sectorsize;
total_bytes += fs_info->sectorsize;
}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ad5bffb24199..4c1ed6dddfd8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2039,7 +2039,7 @@ static blk_status_t btrfs_submit_bio_start(void *private_data, struct bio *bio,
struct inode *inode = private_data;
blk_status_t ret = 0;
- ret = btrfs_csum_one_bio(inode, bio, 0, 0);
+ ret = btrfs_csum_one_bio(inode, bio, 0, false, false);
BUG_ON(ret); /* -ENOMEM */
return 0;
}
@@ -2104,7 +2104,7 @@ static blk_status_t btrfs_submit_bio_hook(struct inode *inode, struct bio *bio,
0, inode, btrfs_submit_bio_start);
goto out;
} else if (!skip_sum) {
- ret = btrfs_csum_one_bio(inode, bio, 0, 0);
+ ret = btrfs_csum_one_bio(inode, bio, 0, false, false);
if (ret)
goto out;
}
@@ -8272,7 +8272,7 @@ static blk_status_t btrfs_submit_bio_start_direct_io(void *private_data,
{
struct inode *inode = private_data;
blk_status_t ret;
- ret = btrfs_csum_one_bio(inode, bio, offset, 1);
+ ret = btrfs_csum_one_bio(inode, bio, offset, true, false);
BUG_ON(ret); /* -ENOMEM */
return 0;
}
@@ -8379,7 +8379,7 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio,
* If we aren't doing async submit, calculate the csum of the
* bio now.
*/
- ret = btrfs_csum_one_bio(inode, bio, file_offset, 1);
+ ret = btrfs_csum_one_bio(inode, bio, file_offset, true, false);
if (ret)
goto err;
} else {
--
2.24.0
^ permalink raw reply related
* [RFC PATCH v3 06/12] btrfs: remove dead snapshot-aware defrag code
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Snapshot-aware defrag has been disabled since commit 8101c8dbf624
("Btrfs: disable snapshot aware defrag for now") almost 6 years ago.
Let's remove the dead code. If someone is up to the task of bringing it
back, they can dig it up from git.
This is logically a revert of commit 38c227d87c49 ("Btrfs:
snapshot-aware defrag") except that now we have to clear the
EXTENT_DEFRAG bit to avoid need_force_cow() returning true forever.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/inode.c | 695 +----------------------------------------------
1 file changed, 11 insertions(+), 684 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 4c1ed6dddfd8..707b4d86409f 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -44,7 +44,6 @@
#include "locking.h"
#include "free-space-cache.h"
#include "inode-map.h"
-#include "backref.h"
#include "props.h"
#include "qgroup.h"
#include "delalloc-space.h"
@@ -2353,649 +2352,6 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
return ret;
}
-/* snapshot-aware defrag */
-struct sa_defrag_extent_backref {
- struct rb_node node;
- struct old_sa_defrag_extent *old;
- u64 root_id;
- u64 inum;
- u64 file_pos;
- u64 extent_offset;
- u64 num_bytes;
- u64 generation;
-};
-
-struct old_sa_defrag_extent {
- struct list_head list;
- struct new_sa_defrag_extent *new;
-
- u64 extent_offset;
- u64 bytenr;
- u64 offset;
- u64 len;
- int count;
-};
-
-struct new_sa_defrag_extent {
- struct rb_root root;
- struct list_head head;
- struct btrfs_path *path;
- struct inode *inode;
- u64 file_pos;
- u64 len;
- u64 bytenr;
- u64 disk_len;
- u8 compress_type;
-};
-
-static int backref_comp(struct sa_defrag_extent_backref *b1,
- struct sa_defrag_extent_backref *b2)
-{
- if (b1->root_id < b2->root_id)
- return -1;
- else if (b1->root_id > b2->root_id)
- return 1;
-
- if (b1->inum < b2->inum)
- return -1;
- else if (b1->inum > b2->inum)
- return 1;
-
- if (b1->file_pos < b2->file_pos)
- return -1;
- else if (b1->file_pos > b2->file_pos)
- return 1;
-
- /*
- * [------------------------------] ===> (a range of space)
- * |<--->| |<---->| =============> (fs/file tree A)
- * |<---------------------------->| ===> (fs/file tree B)
- *
- * A range of space can refer to two file extents in one tree while
- * refer to only one file extent in another tree.
- *
- * So we may process a disk offset more than one time(two extents in A)
- * and locate at the same extent(one extent in B), then insert two same
- * backrefs(both refer to the extent in B).
- */
- return 0;
-}
-
-static void backref_insert(struct rb_root *root,
- struct sa_defrag_extent_backref *backref)
-{
- struct rb_node **p = &root->rb_node;
- struct rb_node *parent = NULL;
- struct sa_defrag_extent_backref *entry;
- int ret;
-
- while (*p) {
- parent = *p;
- entry = rb_entry(parent, struct sa_defrag_extent_backref, node);
-
- ret = backref_comp(backref, entry);
- if (ret < 0)
- p = &(*p)->rb_left;
- else
- p = &(*p)->rb_right;
- }
-
- rb_link_node(&backref->node, parent, p);
- rb_insert_color(&backref->node, root);
-}
-
-/*
- * Note the backref might has changed, and in this case we just return 0.
- */
-static noinline int record_one_backref(u64 inum, u64 offset, u64 root_id,
- void *ctx)
-{
- struct btrfs_file_extent_item *extent;
- struct old_sa_defrag_extent *old = ctx;
- struct new_sa_defrag_extent *new = old->new;
- struct btrfs_path *path = new->path;
- struct btrfs_key key;
- struct btrfs_root *root;
- struct sa_defrag_extent_backref *backref;
- struct extent_buffer *leaf;
- struct inode *inode = new->inode;
- struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
- int slot;
- int ret;
- u64 extent_offset;
- u64 num_bytes;
-
- if (BTRFS_I(inode)->root->root_key.objectid == root_id &&
- inum == btrfs_ino(BTRFS_I(inode)))
- return 0;
-
- key.objectid = root_id;
- key.type = BTRFS_ROOT_ITEM_KEY;
- key.offset = (u64)-1;
-
- root = btrfs_read_fs_root_no_name(fs_info, &key);
- if (IS_ERR(root)) {
- if (PTR_ERR(root) == -ENOENT)
- return 0;
- WARN_ON(1);
- btrfs_debug(fs_info, "inum=%llu, offset=%llu, root_id=%llu",
- inum, offset, root_id);
- return PTR_ERR(root);
- }
-
- key.objectid = inum;
- key.type = BTRFS_EXTENT_DATA_KEY;
- if (offset > (u64)-1 << 32)
- key.offset = 0;
- else
- key.offset = offset;
-
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (WARN_ON(ret < 0))
- return ret;
- ret = 0;
-
- while (1) {
- cond_resched();
-
- leaf = path->nodes[0];
- slot = path->slots[0];
-
- if (slot >= btrfs_header_nritems(leaf)) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0) {
- goto out;
- } else if (ret > 0) {
- ret = 0;
- goto out;
- }
- continue;
- }
-
- path->slots[0]++;
-
- btrfs_item_key_to_cpu(leaf, &key, slot);
-
- if (key.objectid > inum)
- goto out;
-
- if (key.objectid < inum || key.type != BTRFS_EXTENT_DATA_KEY)
- continue;
-
- extent = btrfs_item_ptr(leaf, slot,
- struct btrfs_file_extent_item);
-
- if (btrfs_file_extent_disk_bytenr(leaf, extent) != old->bytenr)
- continue;
-
- /*
- * 'offset' refers to the exact key.offset,
- * NOT the 'offset' field in btrfs_extent_data_ref, ie.
- * (key.offset - extent_offset).
- */
- if (key.offset != offset)
- continue;
-
- extent_offset = btrfs_file_extent_offset(leaf, extent);
- num_bytes = btrfs_file_extent_num_bytes(leaf, extent);
-
- if (extent_offset >= old->extent_offset + old->offset +
- old->len || extent_offset + num_bytes <=
- old->extent_offset + old->offset)
- continue;
- break;
- }
-
- backref = kmalloc(sizeof(*backref), GFP_NOFS);
- if (!backref) {
- ret = -ENOENT;
- goto out;
- }
-
- backref->root_id = root_id;
- backref->inum = inum;
- backref->file_pos = offset;
- backref->num_bytes = num_bytes;
- backref->extent_offset = extent_offset;
- backref->generation = btrfs_file_extent_generation(leaf, extent);
- backref->old = old;
- backref_insert(&new->root, backref);
- old->count++;
-out:
- btrfs_release_path(path);
- WARN_ON(ret);
- return ret;
-}
-
-static noinline bool record_extent_backrefs(struct btrfs_path *path,
- struct new_sa_defrag_extent *new)
-{
- struct btrfs_fs_info *fs_info = btrfs_sb(new->inode->i_sb);
- struct old_sa_defrag_extent *old, *tmp;
- int ret;
-
- new->path = path;
-
- list_for_each_entry_safe(old, tmp, &new->head, list) {
- ret = iterate_inodes_from_logical(old->bytenr +
- old->extent_offset, fs_info,
- path, record_one_backref,
- old, false);
- if (ret < 0 && ret != -ENOENT)
- return false;
-
- /* no backref to be processed for this extent */
- if (!old->count) {
- list_del(&old->list);
- kfree(old);
- }
- }
-
- if (list_empty(&new->head))
- return false;
-
- return true;
-}
-
-static int relink_is_mergable(struct extent_buffer *leaf,
- struct btrfs_file_extent_item *fi,
- struct new_sa_defrag_extent *new)
-{
- if (btrfs_file_extent_disk_bytenr(leaf, fi) != new->bytenr)
- return 0;
-
- if (btrfs_file_extent_type(leaf, fi) != BTRFS_FILE_EXTENT_REG)
- return 0;
-
- if (btrfs_file_extent_compression(leaf, fi) != new->compress_type)
- return 0;
-
- if (btrfs_file_extent_encryption(leaf, fi) ||
- btrfs_file_extent_other_encoding(leaf, fi))
- return 0;
-
- return 1;
-}
-
-/*
- * Note the backref might has changed, and in this case we just return 0.
- */
-static noinline int relink_extent_backref(struct btrfs_path *path,
- struct sa_defrag_extent_backref *prev,
- struct sa_defrag_extent_backref *backref)
-{
- struct btrfs_file_extent_item *extent;
- struct btrfs_file_extent_item *item;
- struct btrfs_ordered_extent *ordered;
- struct btrfs_trans_handle *trans;
- struct btrfs_ref ref = { 0 };
- struct btrfs_root *root;
- struct btrfs_key key;
- struct extent_buffer *leaf;
- struct old_sa_defrag_extent *old = backref->old;
- struct new_sa_defrag_extent *new = old->new;
- struct btrfs_fs_info *fs_info = btrfs_sb(new->inode->i_sb);
- struct inode *inode;
- struct extent_state *cached = NULL;
- int ret = 0;
- u64 start;
- u64 len;
- u64 lock_start;
- u64 lock_end;
- bool merge = false;
- int index;
-
- if (prev && prev->root_id == backref->root_id &&
- prev->inum == backref->inum &&
- prev->file_pos + prev->num_bytes == backref->file_pos)
- merge = true;
-
- /* step 1: get root */
- key.objectid = backref->root_id;
- key.type = BTRFS_ROOT_ITEM_KEY;
- key.offset = (u64)-1;
-
- index = srcu_read_lock(&fs_info->subvol_srcu);
-
- root = btrfs_read_fs_root_no_name(fs_info, &key);
- if (IS_ERR(root)) {
- srcu_read_unlock(&fs_info->subvol_srcu, index);
- if (PTR_ERR(root) == -ENOENT)
- return 0;
- return PTR_ERR(root);
- }
-
- if (btrfs_root_readonly(root)) {
- srcu_read_unlock(&fs_info->subvol_srcu, index);
- return 0;
- }
-
- /* step 2: get inode */
- key.objectid = backref->inum;
- key.type = BTRFS_INODE_ITEM_KEY;
- key.offset = 0;
-
- inode = btrfs_iget(fs_info->sb, &key, root, NULL);
- if (IS_ERR(inode)) {
- srcu_read_unlock(&fs_info->subvol_srcu, index);
- return 0;
- }
-
- srcu_read_unlock(&fs_info->subvol_srcu, index);
-
- /* step 3: relink backref */
- lock_start = backref->file_pos;
- lock_end = backref->file_pos + backref->num_bytes - 1;
- lock_extent_bits(&BTRFS_I(inode)->io_tree, lock_start, lock_end,
- &cached);
-
- ordered = btrfs_lookup_first_ordered_extent(inode, lock_end);
- if (ordered) {
- btrfs_put_ordered_extent(ordered);
- goto out_unlock;
- }
-
- trans = btrfs_join_transaction(root);
- if (IS_ERR(trans)) {
- ret = PTR_ERR(trans);
- goto out_unlock;
- }
-
- key.objectid = backref->inum;
- key.type = BTRFS_EXTENT_DATA_KEY;
- key.offset = backref->file_pos;
-
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0) {
- goto out_free_path;
- } else if (ret > 0) {
- ret = 0;
- goto out_free_path;
- }
-
- extent = btrfs_item_ptr(path->nodes[0], path->slots[0],
- struct btrfs_file_extent_item);
-
- if (btrfs_file_extent_generation(path->nodes[0], extent) !=
- backref->generation)
- goto out_free_path;
-
- btrfs_release_path(path);
-
- start = backref->file_pos;
- if (backref->extent_offset < old->extent_offset + old->offset)
- start += old->extent_offset + old->offset -
- backref->extent_offset;
-
- len = min(backref->extent_offset + backref->num_bytes,
- old->extent_offset + old->offset + old->len);
- len -= max(backref->extent_offset, old->extent_offset + old->offset);
-
- ret = btrfs_drop_extents(trans, root, inode, start,
- start + len, 1);
- if (ret)
- goto out_free_path;
-again:
- key.objectid = btrfs_ino(BTRFS_I(inode));
- key.type = BTRFS_EXTENT_DATA_KEY;
- key.offset = start;
-
- path->leave_spinning = 1;
- if (merge) {
- struct btrfs_file_extent_item *fi;
- u64 extent_len;
- struct btrfs_key found_key;
-
- ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
- if (ret < 0)
- goto out_free_path;
-
- path->slots[0]--;
- leaf = path->nodes[0];
- btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
-
- fi = btrfs_item_ptr(leaf, path->slots[0],
- struct btrfs_file_extent_item);
- extent_len = btrfs_file_extent_num_bytes(leaf, fi);
-
- if (extent_len + found_key.offset == start &&
- relink_is_mergable(leaf, fi, new)) {
- btrfs_set_file_extent_num_bytes(leaf, fi,
- extent_len + len);
- btrfs_mark_buffer_dirty(leaf);
- inode_add_bytes(inode, len);
-
- ret = 1;
- goto out_free_path;
- } else {
- merge = false;
- btrfs_release_path(path);
- goto again;
- }
- }
-
- ret = btrfs_insert_empty_item(trans, root, path, &key,
- sizeof(*extent));
- if (ret) {
- btrfs_abort_transaction(trans, ret);
- goto out_free_path;
- }
-
- leaf = path->nodes[0];
- item = btrfs_item_ptr(leaf, path->slots[0],
- struct btrfs_file_extent_item);
- btrfs_set_file_extent_disk_bytenr(leaf, item, new->bytenr);
- btrfs_set_file_extent_disk_num_bytes(leaf, item, new->disk_len);
- btrfs_set_file_extent_offset(leaf, item, start - new->file_pos);
- btrfs_set_file_extent_num_bytes(leaf, item, len);
- btrfs_set_file_extent_ram_bytes(leaf, item, new->len);
- btrfs_set_file_extent_generation(leaf, item, trans->transid);
- btrfs_set_file_extent_type(leaf, item, BTRFS_FILE_EXTENT_REG);
- btrfs_set_file_extent_compression(leaf, item, new->compress_type);
- btrfs_set_file_extent_encryption(leaf, item, 0);
- btrfs_set_file_extent_other_encoding(leaf, item, 0);
-
- btrfs_mark_buffer_dirty(leaf);
- inode_add_bytes(inode, len);
- btrfs_release_path(path);
-
- btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, new->bytenr,
- new->disk_len, 0);
- btrfs_init_data_ref(&ref, backref->root_id, backref->inum,
- new->file_pos); /* start - extent_offset */
- ret = btrfs_inc_extent_ref(trans, &ref);
- if (ret) {
- btrfs_abort_transaction(trans, ret);
- goto out_free_path;
- }
-
- ret = 1;
-out_free_path:
- btrfs_release_path(path);
- path->leave_spinning = 0;
- btrfs_end_transaction(trans);
-out_unlock:
- unlock_extent_cached(&BTRFS_I(inode)->io_tree, lock_start, lock_end,
- &cached);
- iput(inode);
- return ret;
-}
-
-static void free_sa_defrag_extent(struct new_sa_defrag_extent *new)
-{
- struct old_sa_defrag_extent *old, *tmp;
-
- if (!new)
- return;
-
- list_for_each_entry_safe(old, tmp, &new->head, list) {
- kfree(old);
- }
- kfree(new);
-}
-
-static void relink_file_extents(struct new_sa_defrag_extent *new)
-{
- struct btrfs_fs_info *fs_info = btrfs_sb(new->inode->i_sb);
- struct btrfs_path *path;
- struct sa_defrag_extent_backref *backref;
- struct sa_defrag_extent_backref *prev = NULL;
- struct rb_node *node;
- int ret;
-
- path = btrfs_alloc_path();
- if (!path)
- return;
-
- if (!record_extent_backrefs(path, new)) {
- btrfs_free_path(path);
- goto out;
- }
- btrfs_release_path(path);
-
- while (1) {
- node = rb_first(&new->root);
- if (!node)
- break;
- rb_erase(node, &new->root);
-
- backref = rb_entry(node, struct sa_defrag_extent_backref, node);
-
- ret = relink_extent_backref(path, prev, backref);
- WARN_ON(ret < 0);
-
- kfree(prev);
-
- if (ret == 1)
- prev = backref;
- else
- prev = NULL;
- cond_resched();
- }
- kfree(prev);
-
- btrfs_free_path(path);
-out:
- free_sa_defrag_extent(new);
-
- atomic_dec(&fs_info->defrag_running);
- wake_up(&fs_info->transaction_wait);
-}
-
-static struct new_sa_defrag_extent *
-record_old_file_extents(struct inode *inode,
- struct btrfs_ordered_extent *ordered)
-{
- struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
- struct btrfs_root *root = BTRFS_I(inode)->root;
- struct btrfs_path *path;
- struct btrfs_key key;
- struct old_sa_defrag_extent *old;
- struct new_sa_defrag_extent *new;
- int ret;
-
- new = kmalloc(sizeof(*new), GFP_NOFS);
- if (!new)
- return NULL;
-
- new->inode = inode;
- new->file_pos = ordered->file_offset;
- new->len = ordered->len;
- new->bytenr = ordered->start;
- new->disk_len = ordered->disk_len;
- new->compress_type = ordered->compress_type;
- new->root = RB_ROOT;
- INIT_LIST_HEAD(&new->head);
-
- path = btrfs_alloc_path();
- if (!path)
- goto out_kfree;
-
- key.objectid = btrfs_ino(BTRFS_I(inode));
- key.type = BTRFS_EXTENT_DATA_KEY;
- key.offset = new->file_pos;
-
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto out_free_path;
- if (ret > 0 && path->slots[0] > 0)
- path->slots[0]--;
-
- /* find out all the old extents for the file range */
- while (1) {
- struct btrfs_file_extent_item *extent;
- struct extent_buffer *l;
- int slot;
- u64 num_bytes;
- u64 offset;
- u64 end;
- u64 disk_bytenr;
- u64 extent_offset;
-
- l = path->nodes[0];
- slot = path->slots[0];
-
- if (slot >= btrfs_header_nritems(l)) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- goto out_free_path;
- else if (ret > 0)
- break;
- continue;
- }
-
- btrfs_item_key_to_cpu(l, &key, slot);
-
- if (key.objectid != btrfs_ino(BTRFS_I(inode)))
- break;
- if (key.type != BTRFS_EXTENT_DATA_KEY)
- break;
- if (key.offset >= new->file_pos + new->len)
- break;
-
- extent = btrfs_item_ptr(l, slot, struct btrfs_file_extent_item);
-
- num_bytes = btrfs_file_extent_num_bytes(l, extent);
- if (key.offset + num_bytes < new->file_pos)
- goto next;
-
- disk_bytenr = btrfs_file_extent_disk_bytenr(l, extent);
- if (!disk_bytenr)
- goto next;
-
- extent_offset = btrfs_file_extent_offset(l, extent);
-
- old = kmalloc(sizeof(*old), GFP_NOFS);
- if (!old)
- goto out_free_path;
-
- offset = max(new->file_pos, key.offset);
- end = min(new->file_pos + new->len, key.offset + num_bytes);
-
- old->bytenr = disk_bytenr;
- old->extent_offset = extent_offset;
- old->offset = offset - key.offset;
- old->len = end - offset;
- old->new = new;
- old->count = 0;
- list_add_tail(&old->list, &new->head);
-next:
- path->slots[0]++;
- cond_resched();
- }
-
- btrfs_free_path(path);
- atomic_inc(&fs_info->defrag_running);
-
- return new;
-
-out_free_path:
- btrfs_free_path(path);
-out_kfree:
- free_sa_defrag_extent(new);
- return NULL;
-}
-
static void btrfs_release_delalloc_bytes(struct btrfs_fs_info *fs_info,
u64 start, u64 len)
{
@@ -3023,7 +2379,6 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
struct btrfs_trans_handle *trans = NULL;
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
struct extent_state *cached_state = NULL;
- struct new_sa_defrag_extent *new = NULL;
int compress_type = 0;
int ret = 0;
u64 logical_len = ordered_extent->len;
@@ -3032,6 +2387,7 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
bool range_locked = false;
bool clear_new_delalloc_bytes = false;
bool clear_reserved_extent = true;
+ unsigned int clear_bits;
if (!test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags) &&
!test_bit(BTRFS_ORDERED_PREALLOC, &ordered_extent->flags) &&
@@ -3090,20 +2446,6 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
ordered_extent->file_offset + ordered_extent->len - 1,
&cached_state);
- ret = test_range_bit(io_tree, ordered_extent->file_offset,
- ordered_extent->file_offset + ordered_extent->len - 1,
- EXTENT_DEFRAG, 0, cached_state);
- if (ret) {
- u64 last_snapshot = btrfs_root_last_snapshot(&root->root_item);
- if (0 && last_snapshot >= BTRFS_I(inode)->generation)
- /* the inode is shared */
- new = record_old_file_extents(inode, ordered_extent);
-
- clear_extent_bit(io_tree, ordered_extent->file_offset,
- ordered_extent->file_offset + ordered_extent->len - 1,
- EXTENT_DEFRAG, 0, 0, &cached_state);
- }
-
if (nolock)
trans = btrfs_join_transaction_nolock(root);
else
@@ -3164,21 +2506,16 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
}
ret = 0;
out:
- if (range_locked || clear_new_delalloc_bytes) {
- unsigned int clear_bits = 0;
-
- if (range_locked)
- clear_bits |= EXTENT_LOCKED;
- if (clear_new_delalloc_bytes)
- clear_bits |= EXTENT_DELALLOC_NEW;
- clear_extent_bit(&BTRFS_I(inode)->io_tree,
- ordered_extent->file_offset,
- ordered_extent->file_offset +
- ordered_extent->len - 1,
- clear_bits,
- (clear_bits & EXTENT_LOCKED) ? 1 : 0,
- 0, &cached_state);
- }
+ clear_bits = EXTENT_DEFRAG;
+ if (range_locked)
+ clear_bits |= EXTENT_LOCKED;
+ if (clear_new_delalloc_bytes)
+ clear_bits |= EXTENT_DELALLOC_NEW;
+ clear_extent_bit(&BTRFS_I(inode)->io_tree,
+ ordered_extent->file_offset,
+ ordered_extent->file_offset + ordered_extent->len - 1,
+ clear_bits, (clear_bits & EXTENT_LOCKED) ? 1 : 0, 0,
+ &cached_state);
if (trans)
btrfs_end_transaction(trans);
@@ -3222,16 +2559,6 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
*/
btrfs_remove_ordered_extent(inode, ordered_extent);
- /* for snapshot-aware defrag */
- if (new) {
- if (ret) {
- free_sa_defrag_extent(new);
- atomic_dec(&fs_info->defrag_running);
- } else {
- relink_file_extents(new);
- }
- }
-
/* once for us */
btrfs_put_ordered_extent(ordered_extent);
/* once for the tree */
--
2.24.0
^ permalink raw reply related
* [RFC PATCH v3 07/12] btrfs: make btrfs_ordered_extent naming consistent with btrfs_file_extent_item
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
ordered->start, ordered->len, and ordered->disk_len correspond to
fi->disk_bytenr, fi->num_bytes, and fi->disk_num_bytes, respectively.
It's confusing to translate between the two naming schemes. Since a
btrfs_ordered_extent is basically a pending btrfs_file_extent_item,
let's make the former use the naming from the latter.
Note that I didn't touch the names in tracepoints just in case there are
scripts depending on the current naming.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/file-item.c | 2 +-
fs/btrfs/file.c | 6 ++--
fs/btrfs/inode.c | 67 ++++++++++++++++------------------
fs/btrfs/ordered-data.c | 69 ++++++++++++++++++------------------
fs/btrfs/ordered-data.h | 26 +++++++-------
fs/btrfs/relocation.c | 5 +--
include/trace/events/btrfs.h | 6 ++--
7 files changed, 89 insertions(+), 92 deletions(-)
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index c95772949b00..aae674b37bed 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -484,7 +484,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
for (i = 0; i < nr_sectors; i++) {
if (!one_ordered &&
- (offset >= ordered->file_offset + ordered->len ||
+ (offset >= ordered->file_offset + ordered->num_bytes ||
offset < ordered->file_offset)) {
unsigned long bytes_left;
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 435a502a3226..34c1a2284e03 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1503,7 +1503,7 @@ lock_and_cleanup_extent_if_need(struct btrfs_inode *inode, struct page **pages,
ordered = btrfs_lookup_ordered_range(inode, start_pos,
last_pos - start_pos + 1);
if (ordered &&
- ordered->file_offset + ordered->len > start_pos &&
+ ordered->file_offset + ordered->num_bytes > start_pos &&
ordered->file_offset <= last_pos) {
unlock_extent_cached(&inode->io_tree, start_pos,
last_pos, cached_state);
@@ -2428,7 +2428,7 @@ static int btrfs_punch_hole_lock_range(struct inode *inode,
* we need to try again.
*/
if ((!ordered ||
- (ordered->file_offset + ordered->len <= lockstart ||
+ (ordered->file_offset + ordered->num_bytes <= lockstart ||
ordered->file_offset > lockend)) &&
!filemap_range_has_page(inode->i_mapping,
lockstart, lockend)) {
@@ -3250,7 +3250,7 @@ static long btrfs_fallocate(struct file *file, int mode,
ordered = btrfs_lookup_first_ordered_extent(inode, locked_end);
if (ordered &&
- ordered->file_offset + ordered->len > alloc_start &&
+ ordered->file_offset + ordered->num_bytes > alloc_start &&
ordered->file_offset < alloc_end) {
btrfs_put_ordered_extent(ordered);
unlock_extent_cached(&BTRFS_I(inode)->io_tree,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 707b4d86409f..62d6aaccc202 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2379,9 +2379,10 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
struct btrfs_trans_handle *trans = NULL;
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
struct extent_state *cached_state = NULL;
+ u64 start, end;
int compress_type = 0;
int ret = 0;
- u64 logical_len = ordered_extent->len;
+ u64 logical_len = ordered_extent->num_bytes;
bool nolock;
bool truncated = false;
bool range_locked = false;
@@ -2389,6 +2390,9 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
bool clear_reserved_extent = true;
unsigned int clear_bits;
+ start = ordered_extent->file_offset;
+ end = start + ordered_extent->num_bytes - 1;
+
if (!test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags) &&
!test_bit(BTRFS_ORDERED_PREALLOC, &ordered_extent->flags) &&
!test_bit(BTRFS_ORDERED_DIRECT, &ordered_extent->flags))
@@ -2401,10 +2405,7 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
goto out;
}
- btrfs_free_io_failure_record(BTRFS_I(inode),
- ordered_extent->file_offset,
- ordered_extent->file_offset +
- ordered_extent->len - 1);
+ btrfs_free_io_failure_record(BTRFS_I(inode), start, end);
if (test_bit(BTRFS_ORDERED_TRUNCATED, &ordered_extent->flags)) {
truncated = true;
@@ -2422,8 +2423,8 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
* space for NOCOW range.
* As NOCOW won't cause a new delayed ref, just free the space
*/
- btrfs_qgroup_free_data(inode, NULL, ordered_extent->file_offset,
- ordered_extent->len);
+ btrfs_qgroup_free_data(inode, NULL, start,
+ ordered_extent->num_bytes);
btrfs_ordered_update_i_size(inode, 0, ordered_extent);
if (nolock)
trans = btrfs_join_transaction_nolock(root);
@@ -2442,9 +2443,7 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
}
range_locked = true;
- lock_extent_bits(io_tree, ordered_extent->file_offset,
- ordered_extent->file_offset + ordered_extent->len - 1,
- &cached_state);
+ lock_extent_bits(io_tree, start, end, &cached_state);
if (nolock)
trans = btrfs_join_transaction_nolock(root);
@@ -2462,31 +2461,30 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
compress_type = ordered_extent->compress_type;
if (test_bit(BTRFS_ORDERED_PREALLOC, &ordered_extent->flags)) {
BUG_ON(compress_type);
- btrfs_qgroup_free_data(inode, NULL, ordered_extent->file_offset,
- ordered_extent->len);
+ btrfs_qgroup_free_data(inode, NULL, start,
+ ordered_extent->num_bytes);
ret = btrfs_mark_extent_written(trans, BTRFS_I(inode),
ordered_extent->file_offset,
ordered_extent->file_offset +
logical_len);
} else {
BUG_ON(root == fs_info->tree_root);
- ret = insert_reserved_file_extent(trans, inode,
- ordered_extent->file_offset,
- ordered_extent->start,
- ordered_extent->disk_len,
+ ret = insert_reserved_file_extent(trans, inode, start,
+ ordered_extent->disk_bytenr,
+ ordered_extent->disk_num_bytes,
logical_len, logical_len,
compress_type, 0, 0,
BTRFS_FILE_EXTENT_REG);
if (!ret) {
clear_reserved_extent = false;
btrfs_release_delalloc_bytes(fs_info,
- ordered_extent->start,
- ordered_extent->disk_len);
+ ordered_extent->disk_bytenr,
+ ordered_extent->disk_num_bytes);
}
}
unpin_extent_cache(&BTRFS_I(inode)->extent_tree,
- ordered_extent->file_offset, ordered_extent->len,
- trans->transid);
+ ordered_extent->file_offset,
+ ordered_extent->num_bytes, trans->transid);
if (ret < 0) {
btrfs_abort_transaction(trans, ret);
goto out;
@@ -2511,27 +2509,23 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
clear_bits |= EXTENT_LOCKED;
if (clear_new_delalloc_bytes)
clear_bits |= EXTENT_DELALLOC_NEW;
- clear_extent_bit(&BTRFS_I(inode)->io_tree,
- ordered_extent->file_offset,
- ordered_extent->file_offset + ordered_extent->len - 1,
- clear_bits, (clear_bits & EXTENT_LOCKED) ? 1 : 0, 0,
+ clear_extent_bit(&BTRFS_I(inode)->io_tree, start, end, clear_bits,
+ (clear_bits & EXTENT_LOCKED) ? 1 : 0, 0,
&cached_state);
if (trans)
btrfs_end_transaction(trans);
if (ret || truncated) {
- u64 start, end;
+ u64 unwritten_start = start;
if (truncated)
- start = ordered_extent->file_offset + logical_len;
- else
- start = ordered_extent->file_offset;
- end = ordered_extent->file_offset + ordered_extent->len - 1;
- clear_extent_uptodate(io_tree, start, end, NULL);
+ unwritten_start += logical_len;
+ clear_extent_uptodate(io_tree, unwritten_start, end, NULL);
/* Drop the cache for the part of the extent we didn't write. */
- btrfs_drop_extent_cache(BTRFS_I(inode), start, end, 0);
+ btrfs_drop_extent_cache(BTRFS_I(inode), unwritten_start, end,
+ 0);
/*
* If the ordered extent had an IOERR or something else went
@@ -2548,11 +2542,11 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
!test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags) &&
!test_bit(BTRFS_ORDERED_PREALLOC, &ordered_extent->flags))
btrfs_free_reserved_extent(fs_info,
- ordered_extent->start,
- ordered_extent->disk_len, 1);
+ ordered_extent->disk_bytenr,
+ ordered_extent->disk_num_bytes,
+ 1);
}
-
/*
* This needs to be done to make sure anybody waiting knows we are done
* updating everything for this ordered extent.
@@ -8173,7 +8167,8 @@ static void btrfs_invalidatepage(struct page *page, unsigned int offset,
ordered = btrfs_lookup_ordered_range(BTRFS_I(inode), start,
page_end - start + 1);
if (ordered) {
- end = min(page_end, ordered->file_offset + ordered->len - 1);
+ end = min(page_end,
+ ordered->file_offset + ordered->num_bytes - 1);
/*
* IO on this page will never be started, so we need
* to account for any ordered extents now
@@ -8702,7 +8697,7 @@ void btrfs_destroy_inode(struct inode *inode)
else {
btrfs_err(fs_info,
"found ordered extent %llu %llu on inode cleanup",
- ordered->file_offset, ordered->len);
+ ordered->file_offset, ordered->num_bytes);
btrfs_remove_ordered_extent(inode, ordered);
btrfs_put_ordered_extent(ordered);
btrfs_put_ordered_extent(ordered);
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 24b6c72b9a59..94e2485006ab 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -20,9 +20,9 @@ static struct kmem_cache *btrfs_ordered_extent_cache;
static u64 entry_end(struct btrfs_ordered_extent *entry)
{
- if (entry->file_offset + entry->len < entry->file_offset)
+ if (entry->file_offset + entry->num_bytes < entry->file_offset)
return (u64)-1;
- return entry->file_offset + entry->len;
+ return entry->file_offset + entry->num_bytes;
}
/* returns NULL if the insertion worked, or it returns the node it did find
@@ -120,7 +120,7 @@ static struct rb_node *__tree_search(struct rb_root *root, u64 file_offset,
static int offset_in_entry(struct btrfs_ordered_extent *entry, u64 file_offset)
{
if (file_offset < entry->file_offset ||
- entry->file_offset + entry->len <= file_offset)
+ entry->file_offset + entry->num_bytes <= file_offset)
return 0;
return 1;
}
@@ -129,7 +129,7 @@ static int range_overlaps(struct btrfs_ordered_extent *entry, u64 file_offset,
u64 len)
{
if (file_offset + len <= entry->file_offset ||
- entry->file_offset + entry->len <= file_offset)
+ entry->file_offset + entry->num_bytes <= file_offset)
return 0;
return 1;
}
@@ -161,19 +161,14 @@ static inline struct rb_node *tree_search(struct btrfs_ordered_inode_tree *tree,
}
/* allocate and add a new ordered_extent into the per-inode tree.
- * file_offset is the logical offset in the file
- *
- * start is the disk block number of an extent already reserved in the
- * extent allocation tree
- *
- * len is the length of the extent
*
* The tree is given a single reference on the ordered extent that was
* inserted.
*/
static int __btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
- u64 start, u64 len, u64 disk_len,
- int type, int dio, int compress_type)
+ u64 disk_bytenr, u64 num_bytes,
+ u64 disk_num_bytes, int type, int dio,
+ int compress_type)
{
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct btrfs_root *root = BTRFS_I(inode)->root;
@@ -187,10 +182,10 @@ static int __btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
return -ENOMEM;
entry->file_offset = file_offset;
- entry->start = start;
- entry->len = len;
- entry->disk_len = disk_len;
- entry->bytes_left = len;
+ entry->disk_bytenr = disk_bytenr;
+ entry->num_bytes = num_bytes;
+ entry->disk_num_bytes = disk_num_bytes;
+ entry->bytes_left = num_bytes;
entry->inode = igrab(inode);
entry->compress_type = compress_type;
entry->truncated_len = (u64)-1;
@@ -198,7 +193,7 @@ static int __btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
set_bit(type, &entry->flags);
if (dio) {
- percpu_counter_add_batch(&fs_info->dio_bytes, len,
+ percpu_counter_add_batch(&fs_info->dio_bytes, num_bytes,
fs_info->delalloc_batch);
set_bit(BTRFS_ORDERED_DIRECT, &entry->flags);
}
@@ -247,27 +242,30 @@ static int __btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
}
int btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
- u64 start, u64 len, u64 disk_len, int type)
+ u64 disk_bytenr, u64 num_bytes, u64 disk_num_bytes,
+ int type)
{
- return __btrfs_add_ordered_extent(inode, file_offset, start, len,
- disk_len, type, 0,
+ return __btrfs_add_ordered_extent(inode, file_offset, disk_bytenr,
+ num_bytes, disk_num_bytes, type, 0,
BTRFS_COMPRESS_NONE);
}
int btrfs_add_ordered_extent_dio(struct inode *inode, u64 file_offset,
- u64 start, u64 len, u64 disk_len, int type)
+ u64 disk_bytenr, u64 num_bytes,
+ u64 disk_num_bytes, int type)
{
- return __btrfs_add_ordered_extent(inode, file_offset, start, len,
- disk_len, type, 1,
+ return __btrfs_add_ordered_extent(inode, file_offset, disk_bytenr,
+ num_bytes, disk_num_bytes, type, 1,
BTRFS_COMPRESS_NONE);
}
int btrfs_add_ordered_extent_compress(struct inode *inode, u64 file_offset,
- u64 start, u64 len, u64 disk_len,
- int type, int compress_type)
+ u64 disk_bytenr, u64 num_bytes,
+ u64 disk_num_bytes, int type,
+ int compress_type)
{
- return __btrfs_add_ordered_extent(inode, file_offset, start, len,
- disk_len, type, 0,
+ return __btrfs_add_ordered_extent(inode, file_offset, disk_bytenr,
+ num_bytes, disk_num_bytes, type, 0,
compress_type);
}
@@ -328,8 +326,8 @@ int btrfs_dec_test_first_ordered_pending(struct inode *inode,
}
dec_start = max(*file_offset, entry->file_offset);
- dec_end = min(*file_offset + io_size, entry->file_offset +
- entry->len);
+ dec_end = min(*file_offset + io_size,
+ entry->file_offset + entry->num_bytes);
*file_offset = dec_end;
if (dec_start > dec_end) {
btrfs_crit(fs_info, "bad ordering dec_start %llu end %llu",
@@ -471,10 +469,11 @@ void btrfs_remove_ordered_extent(struct inode *inode,
btrfs_mod_outstanding_extents(btrfs_inode, -1);
spin_unlock(&btrfs_inode->lock);
if (root != fs_info->tree_root)
- btrfs_delalloc_release_metadata(btrfs_inode, entry->len, false);
+ btrfs_delalloc_release_metadata(btrfs_inode, entry->num_bytes,
+ false);
if (test_bit(BTRFS_ORDERED_DIRECT, &entry->flags))
- percpu_counter_add_batch(&fs_info->dio_bytes, -entry->len,
+ percpu_counter_add_batch(&fs_info->dio_bytes, -entry->num_bytes,
fs_info->delalloc_batch);
tree = &btrfs_inode->ordered_tree;
@@ -534,8 +533,8 @@ u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
ordered = list_first_entry(&splice, struct btrfs_ordered_extent,
root_extent_list);
- if (range_end <= ordered->start ||
- ordered->start + ordered->disk_len <= range_start) {
+ if (range_end <= ordered->disk_bytenr ||
+ ordered->disk_bytenr + ordered->disk_num_bytes <= range_start) {
list_move_tail(&ordered->root_extent_list, &skipped);
cond_resched_lock(&root->ordered_extent_lock);
continue;
@@ -624,7 +623,7 @@ void btrfs_start_ordered_extent(struct inode *inode,
int wait)
{
u64 start = entry->file_offset;
- u64 end = start + entry->len - 1;
+ u64 end = start + entry->num_bytes - 1;
trace_btrfs_ordered_extent_start(inode, entry);
@@ -685,7 +684,7 @@ int btrfs_wait_ordered_range(struct inode *inode, u64 start, u64 len)
btrfs_put_ordered_extent(ordered);
break;
}
- if (ordered->file_offset + ordered->len <= start) {
+ if (ordered->file_offset + ordered->num_bytes <= start) {
btrfs_put_ordered_extent(ordered);
break;
}
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index 5204171ea962..4a3dd80e776c 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -67,14 +67,13 @@ struct btrfs_ordered_extent {
/* logical offset in the file */
u64 file_offset;
- /* disk byte number */
- u64 start;
-
- /* ram length of the extent in bytes */
- u64 len;
-
- /* extent length on disk */
- u64 disk_len;
+ /*
+ * These fields directly correspond to the same fields in
+ * btrfs_file_extent_item.
+ */
+ u64 disk_bytenr;
+ u64 num_bytes;
+ u64 disk_num_bytes;
/* number of bytes that still need writing */
u64 bytes_left;
@@ -161,12 +160,15 @@ int btrfs_dec_test_first_ordered_pending(struct inode *inode,
u64 *file_offset, u64 io_size,
int uptodate);
int btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
- u64 start, u64 len, u64 disk_len, int type);
+ u64 disk_bytenr, u64 num_bytes, u64 disk_num_bytes,
+ int type);
int btrfs_add_ordered_extent_dio(struct inode *inode, u64 file_offset,
- u64 start, u64 len, u64 disk_len, int type);
+ u64 disk_bytenr, u64 num_bytes,
+ u64 disk_num_bytes, int type);
int btrfs_add_ordered_extent_compress(struct inode *inode, u64 file_offset,
- u64 start, u64 len, u64 disk_len,
- int type, int compress_type);
+ u64 disk_bytenr, u64 num_bytes,
+ u64 disk_num_bytes, int type,
+ int compress_type);
void btrfs_add_ordered_sum(struct btrfs_ordered_extent *entry,
struct btrfs_ordered_sum *sum);
struct btrfs_ordered_extent *btrfs_lookup_ordered_extent(struct inode *inode,
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 5cd42b66818c..e3cec29813ee 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4617,7 +4617,7 @@ int btrfs_reloc_clone_csums(struct inode *inode, u64 file_pos, u64 len)
LIST_HEAD(list);
ordered = btrfs_lookup_ordered_extent(inode, file_pos);
- BUG_ON(ordered->file_offset != file_pos || ordered->len != len);
+ BUG_ON(ordered->file_offset != file_pos || ordered->num_bytes != len);
disk_bytenr = file_pos + BTRFS_I(inode)->index_cnt;
ret = btrfs_lookup_csums_range(fs_info->csum_root, disk_bytenr,
@@ -4641,7 +4641,8 @@ int btrfs_reloc_clone_csums(struct inode *inode, u64 file_pos, u64 len)
* disk_len vs real len like with real inodes since it's all
* disk length.
*/
- new_bytenr = ordered->start + (sums->bytenr - disk_bytenr);
+ new_bytenr = (ordered->disk_bytenr +
+ (sums->bytenr - disk_bytenr));
sums->bytenr = new_bytenr;
btrfs_add_ordered_sum(ordered, sums);
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 75ae1899452b..3a0f172cfc8f 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -496,9 +496,9 @@ DECLARE_EVENT_CLASS(btrfs__ordered_extent,
TP_fast_assign_btrfs(btrfs_sb(inode->i_sb),
__entry->ino = btrfs_ino(BTRFS_I(inode));
__entry->file_offset = ordered->file_offset;
- __entry->start = ordered->start;
- __entry->len = ordered->len;
- __entry->disk_len = ordered->disk_len;
+ __entry->start = ordered->disk_bytenr;
+ __entry->len = ordered->num_bytes;
+ __entry->disk_len = ordered->disk_num_bytes;
__entry->bytes_left = ordered->bytes_left;
__entry->flags = ordered->flags;
__entry->compress_type = ordered->compress_type;
--
2.24.0
^ permalink raw reply related
* [RFC PATCH v3 08/12] btrfs: add ram_bytes and offset to btrfs_ordered_extent
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Currently, we only create ordered extents when ram_bytes == num_bytes
and offset == 0. However, RWF_ENCODED writes may create extents which
only refer to a subset of the full unencoded extent, so we need to plumb
these fields through the ordered extent infrastructure and pass them
down to insert_reserved_file_extent().
Since we're changing the btrfs_add_ordered_extent* signature, let's get
rid of the trivial wrappers and add a kernel-doc.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/inode.c | 65 +++++++++++++++++++++++------------------
fs/btrfs/ordered-data.c | 65 +++++++++++++++--------------------------
fs/btrfs/ordered-data.h | 16 ++++------
3 files changed, 67 insertions(+), 79 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 62d6aaccc202..d53580ad2c46 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -846,13 +846,12 @@ static noinline void submit_compressed_extents(struct async_chunk *async_chunk)
goto out_free_reserve;
free_extent_map(em);
- ret = btrfs_add_ordered_extent_compress(inode,
- async_extent->start,
- ins.objectid,
- async_extent->ram_size,
- ins.offset,
- BTRFS_ORDERED_COMPRESSED,
- async_extent->compress_type);
+ ret = btrfs_add_ordered_extent(inode, async_extent->start,
+ async_extent->ram_size,
+ async_extent->ram_size,
+ ins.objectid, ins.offset, 0,
+ 1 << BTRFS_ORDERED_COMPRESSED,
+ async_extent->compress_type);
if (ret) {
btrfs_drop_extent_cache(BTRFS_I(inode),
async_extent->start,
@@ -1046,8 +1045,9 @@ static noinline int cow_file_range(struct inode *inode,
}
free_extent_map(em);
- ret = btrfs_add_ordered_extent(inode, start, ins.objectid,
- ram_size, cur_alloc_size, 0);
+ ret = btrfs_add_ordered_extent(inode, start, ram_size, ram_size,
+ ins.objectid, cur_alloc_size, 0,
+ 0, BTRFS_COMPRESS_NONE);
if (ret)
goto out_drop_extent_cache;
@@ -1584,10 +1584,11 @@ static noinline int run_delalloc_nocow(struct inode *inode,
goto error;
}
free_extent_map(em);
- ret = btrfs_add_ordered_extent(inode, cur_offset,
- disk_bytenr, num_bytes,
- num_bytes,
- BTRFS_ORDERED_PREALLOC);
+ ret = btrfs_add_ordered_extent(inode,
+ cur_offset, num_bytes, num_bytes,
+ disk_bytenr, num_bytes, 0,
+ 1 << BTRFS_ORDERED_PREALLOC,
+ BTRFS_COMPRESS_NONE);
if (ret) {
btrfs_drop_extent_cache(BTRFS_I(inode),
cur_offset,
@@ -1597,9 +1598,11 @@ static noinline int run_delalloc_nocow(struct inode *inode,
}
} else {
ret = btrfs_add_ordered_extent(inode, cur_offset,
+ num_bytes, num_bytes,
disk_bytenr, num_bytes,
- num_bytes,
- BTRFS_ORDERED_NOCOW);
+ 0,
+ 1 << BTRFS_ORDERED_NOCOW,
+ BTRFS_COMPRESS_NONE);
if (ret)
goto error;
}
@@ -2269,7 +2272,7 @@ int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end)
static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
struct inode *inode, u64 file_pos,
u64 disk_bytenr, u64 disk_num_bytes,
- u64 num_bytes, u64 ram_bytes,
+ u64 offset, u64 num_bytes, u64 ram_bytes,
u8 compression, u8 encryption,
u16 other_encoding, int extent_type)
{
@@ -2319,7 +2322,7 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
btrfs_set_file_extent_type(leaf, fi, extent_type);
btrfs_set_file_extent_disk_bytenr(leaf, fi, disk_bytenr);
btrfs_set_file_extent_disk_num_bytes(leaf, fi, disk_num_bytes);
- btrfs_set_file_extent_offset(leaf, fi, 0);
+ btrfs_set_file_extent_offset(leaf, fi, offset);
btrfs_set_file_extent_num_bytes(leaf, fi, num_bytes);
btrfs_set_file_extent_ram_bytes(leaf, fi, ram_bytes);
btrfs_set_file_extent_compression(leaf, fi, compression);
@@ -2345,7 +2348,8 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
qg_released = ret;
ret = btrfs_alloc_reserved_file_extent(trans, root,
btrfs_ino(BTRFS_I(inode)),
- file_pos, qg_released, &ins);
+ file_pos - offset, qg_released,
+ &ins);
out:
btrfs_free_path(path);
@@ -2382,7 +2386,8 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
u64 start, end;
int compress_type = 0;
int ret = 0;
- u64 logical_len = ordered_extent->num_bytes;
+ u64 num_bytes = ordered_extent->num_bytes;
+ u64 ram_bytes = ordered_extent->ram_bytes;
bool nolock;
bool truncated = false;
bool range_locked = false;
@@ -2409,9 +2414,9 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
if (test_bit(BTRFS_ORDERED_TRUNCATED, &ordered_extent->flags)) {
truncated = true;
- logical_len = ordered_extent->truncated_len;
+ num_bytes = ram_bytes = ordered_extent->truncated_len;
/* Truncated the entire extent, don't bother adding */
- if (!logical_len)
+ if (!num_bytes)
goto out;
}
@@ -2466,13 +2471,14 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
ret = btrfs_mark_extent_written(trans, BTRFS_I(inode),
ordered_extent->file_offset,
ordered_extent->file_offset +
- logical_len);
+ num_bytes);
} else {
BUG_ON(root == fs_info->tree_root);
ret = insert_reserved_file_extent(trans, inode, start,
ordered_extent->disk_bytenr,
ordered_extent->disk_num_bytes,
- logical_len, logical_len,
+ ordered_extent->offset,
+ num_bytes, ram_bytes,
compress_type, 0, 0,
BTRFS_FILE_EXTENT_REG);
if (!ret) {
@@ -2520,7 +2526,7 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
u64 unwritten_start = start;
if (truncated)
- unwritten_start += logical_len;
+ unwritten_start += num_bytes;
clear_extent_uptodate(io_tree, unwritten_start, end, NULL);
/* Drop the cache for the part of the extent we didn't write. */
@@ -2537,7 +2543,7 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
* errored out then we don't need to do this as the accounting
* has already been done.
*/
- if ((ret || !logical_len) &&
+ if ((ret || !num_bytes) &&
clear_reserved_extent &&
!test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags) &&
!test_bit(BTRFS_ORDERED_PREALLOC, &ordered_extent->flags))
@@ -6609,8 +6615,11 @@ static struct extent_map *btrfs_create_dio_extent(struct inode *inode,
if (IS_ERR(em))
goto out;
}
- ret = btrfs_add_ordered_extent_dio(inode, start, block_start,
- len, block_len, type);
+ ret = btrfs_add_ordered_extent(inode, start, len, len, block_start,
+ block_len, 0,
+ (1 << type) |
+ (1 << BTRFS_ORDERED_DIRECT),
+ BTRFS_COMPRESS_NONE);
if (ret) {
if (em) {
free_extent_map(em);
@@ -9743,7 +9752,7 @@ static int __btrfs_prealloc_file_range(struct inode *inode, int mode,
last_alloc = ins.offset;
ret = insert_reserved_file_extent(trans, inode,
cur_offset, ins.objectid,
- ins.offset, ins.offset,
+ ins.offset, 0, ins.offset,
ins.offset, 0, 0, 0,
BTRFS_FILE_EXTENT_PREALLOC);
if (ret) {
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 94e2485006ab..3c6edc307657 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -160,15 +160,27 @@ static inline struct rb_node *tree_search(struct btrfs_ordered_inode_tree *tree,
return ret;
}
-/* allocate and add a new ordered_extent into the per-inode tree.
+/**
+ * btrfs_add_ordered_extent - Add an ordered extent to the per-inode tree.
+ * @inode: inode that this extent is for.
+ * @file_offset: Logical offset in file where the extent starts.
+ * @num_bytes: Logical length of extent in file.
+ * @ram_bytes: Full length of unencoded data.
+ * @disk_bytenr: Offset of extent on disk.
+ * @disk_num_bytes: Size of extent on disk.
+ * @offset: Offset into unencoded data where file data starts.
+ * @flags: Flags specifying type of extent (1 << BTRFS_ORDERED_*).
+ * @compress_type: Compression algorithm used for data.
*
- * The tree is given a single reference on the ordered extent that was
- * inserted.
+ * Most of these parameters correspond to &struct btrfs_file_extent_item. The
+ * tree is given a single reference on the ordered extent that was inserted.
+ *
+ * Return: 0 or -ENOMEM.
*/
-static int __btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
- u64 disk_bytenr, u64 num_bytes,
- u64 disk_num_bytes, int type, int dio,
- int compress_type)
+int btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
+ u64 num_bytes, u64 ram_bytes, u64 disk_bytenr,
+ u64 disk_num_bytes, u64 offset, int flags,
+ int compress_type)
{
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct btrfs_root *root = BTRFS_I(inode)->root;
@@ -182,20 +194,19 @@ static int __btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
return -ENOMEM;
entry->file_offset = file_offset;
- entry->disk_bytenr = disk_bytenr;
entry->num_bytes = num_bytes;
+ entry->ram_bytes = ram_bytes;
+ entry->disk_bytenr = disk_bytenr;
entry->disk_num_bytes = disk_num_bytes;
+ entry->offset = offset;
entry->bytes_left = num_bytes;
entry->inode = igrab(inode);
entry->compress_type = compress_type;
entry->truncated_len = (u64)-1;
- if (type != BTRFS_ORDERED_IO_DONE && type != BTRFS_ORDERED_COMPLETE)
- set_bit(type, &entry->flags);
-
- if (dio) {
+ entry->flags = flags;
+ if (flags & (1 << BTRFS_ORDERED_DIRECT)) {
percpu_counter_add_batch(&fs_info->dio_bytes, num_bytes,
fs_info->delalloc_batch);
- set_bit(BTRFS_ORDERED_DIRECT, &entry->flags);
}
/* one ref for the tree */
@@ -241,34 +252,6 @@ static int __btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
return 0;
}
-int btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
- u64 disk_bytenr, u64 num_bytes, u64 disk_num_bytes,
- int type)
-{
- return __btrfs_add_ordered_extent(inode, file_offset, disk_bytenr,
- num_bytes, disk_num_bytes, type, 0,
- BTRFS_COMPRESS_NONE);
-}
-
-int btrfs_add_ordered_extent_dio(struct inode *inode, u64 file_offset,
- u64 disk_bytenr, u64 num_bytes,
- u64 disk_num_bytes, int type)
-{
- return __btrfs_add_ordered_extent(inode, file_offset, disk_bytenr,
- num_bytes, disk_num_bytes, type, 1,
- BTRFS_COMPRESS_NONE);
-}
-
-int btrfs_add_ordered_extent_compress(struct inode *inode, u64 file_offset,
- u64 disk_bytenr, u64 num_bytes,
- u64 disk_num_bytes, int type,
- int compress_type)
-{
- return __btrfs_add_ordered_extent(inode, file_offset, disk_bytenr,
- num_bytes, disk_num_bytes, type, 0,
- compress_type);
-}
-
/*
* Add a struct btrfs_ordered_sum into the list of checksums to be inserted
* when an ordered extent is finished. If the list covers more than one
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index 4a3dd80e776c..a038bda16fdf 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -71,9 +71,11 @@ struct btrfs_ordered_extent {
* These fields directly correspond to the same fields in
* btrfs_file_extent_item.
*/
- u64 disk_bytenr;
u64 num_bytes;
+ u64 ram_bytes;
+ u64 disk_bytenr;
u64 disk_num_bytes;
+ u64 offset;
/* number of bytes that still need writing */
u64 bytes_left;
@@ -160,15 +162,9 @@ int btrfs_dec_test_first_ordered_pending(struct inode *inode,
u64 *file_offset, u64 io_size,
int uptodate);
int btrfs_add_ordered_extent(struct inode *inode, u64 file_offset,
- u64 disk_bytenr, u64 num_bytes, u64 disk_num_bytes,
- int type);
-int btrfs_add_ordered_extent_dio(struct inode *inode, u64 file_offset,
- u64 disk_bytenr, u64 num_bytes,
- u64 disk_num_bytes, int type);
-int btrfs_add_ordered_extent_compress(struct inode *inode, u64 file_offset,
- u64 disk_bytenr, u64 num_bytes,
- u64 disk_num_bytes, int type,
- int compress_type);
+ u64 num_bytes, u64 ram_bytes, u64 disk_bytenr,
+ u64 disk_num_bytes, u64 offset, int flags,
+ int compress_type);
void btrfs_add_ordered_sum(struct btrfs_ordered_extent *entry,
struct btrfs_ordered_sum *sum);
struct btrfs_ordered_extent *btrfs_lookup_ordered_extent(struct inode *inode,
--
2.24.0
^ permalink raw reply related
* [RFC PATCH v3 09/12] btrfs: support different disk extent size for delalloc
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Currently, we always reserve the same extent size in the file and extent
size on disk for delalloc because the former is the worst case for the
latter. For RWF_ENCODED writes, we know the exact size of the extent on
disk, which may be less than or greater than (for bookends) the size in
the file. Add a disk_num_bytes parameter to
btrfs_delalloc_reserve_metadata() so that we can reserve the correct
amount of csum bytes. Additionally, make
btrfs_free_reserve_data_space_noquota() take a number of bytes instead
of a range, as it refers to the extent size on disk, not in the file. No
functional change.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/ctree.h | 3 ++-
fs/btrfs/delalloc-space.c | 38 +++++++++++++++++---------------------
fs/btrfs/delalloc-space.h | 4 ++--
fs/btrfs/file.c | 3 ++-
fs/btrfs/inode.c | 7 ++-----
fs/btrfs/relocation.c | 4 ++--
6 files changed, 27 insertions(+), 32 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index c32741879088..f9ac05d1ca60 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2489,7 +2489,8 @@ void btrfs_subvolume_release_metadata(struct btrfs_fs_info *fs_info,
struct btrfs_block_rsv *rsv);
void btrfs_delalloc_release_extents(struct btrfs_inode *inode, u64 num_bytes);
-int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes);
+int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes,
+ u64 disk_num_bytes);
u64 btrfs_account_ro_block_groups_free_space(struct btrfs_space_info *sinfo);
int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
u64 start, u64 end);
diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c
index db9f2c58eb4a..720b246772fb 100644
--- a/fs/btrfs/delalloc-space.c
+++ b/fs/btrfs/delalloc-space.c
@@ -153,34 +153,28 @@ int btrfs_check_data_free_space(struct inode *inode,
/* Use new btrfs_qgroup_reserve_data to reserve precious data space. */
ret = btrfs_qgroup_reserve_data(inode, reserved, start, len);
if (ret < 0)
- btrfs_free_reserved_data_space_noquota(inode, start, len);
+ btrfs_free_reserved_data_space_noquota(fs_info, len);
else
ret = 0;
return ret;
}
/*
- * Called if we need to clear a data reservation for this inode
- * Normally in a error case.
+ * Called if we need to clear a data reservation, normally in an error case.
*
* This one will *NOT* use accurate qgroup reserved space API, just for case
* which we can't sleep and is sure it won't affect qgroup reserved space.
* Like clear_bit_hook().
*/
-void btrfs_free_reserved_data_space_noquota(struct inode *inode, u64 start,
- u64 len)
+void btrfs_free_reserved_data_space_noquota(struct btrfs_fs_info *fs_info,
+ u64 num_bytes)
{
- struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct btrfs_space_info *data_sinfo;
- /* Make sure the range is aligned to sectorsize */
- len = round_up(start + len, fs_info->sectorsize) -
- round_down(start, fs_info->sectorsize);
- start = round_down(start, fs_info->sectorsize);
-
+ num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
data_sinfo = fs_info->data_sinfo;
spin_lock(&data_sinfo->lock);
- btrfs_space_info_update_bytes_may_use(fs_info, data_sinfo, -len);
+ btrfs_space_info_update_bytes_may_use(fs_info, data_sinfo, -num_bytes);
spin_unlock(&data_sinfo->lock);
}
@@ -201,7 +195,7 @@ void btrfs_free_reserved_data_space(struct inode *inode,
round_down(start, root->fs_info->sectorsize);
start = round_down(start, root->fs_info->sectorsize);
- btrfs_free_reserved_data_space_noquota(inode, start, len);
+ btrfs_free_reserved_data_space_noquota(root->fs_info, len);
btrfs_qgroup_free_data(inode, reserved, start, len);
}
@@ -280,11 +274,11 @@ static void btrfs_calculate_inode_block_rsv_size(struct btrfs_fs_info *fs_info,
}
static void calc_inode_reservations(struct btrfs_fs_info *fs_info,
- u64 num_bytes, u64 *meta_reserve,
- u64 *qgroup_reserve)
+ u64 num_bytes, u64 disk_num_bytes,
+ u64 *meta_reserve, u64 *qgroup_reserve)
{
u64 nr_extents = count_max_extents(num_bytes);
- u64 csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, num_bytes);
+ u64 csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, disk_num_bytes);
u64 inode_update = btrfs_calc_metadata_size(fs_info, 1);
*meta_reserve = btrfs_calc_insert_metadata_size(fs_info,
@@ -298,7 +292,8 @@ static void calc_inode_reservations(struct btrfs_fs_info *fs_info,
*qgroup_reserve = nr_extents * fs_info->nodesize;
}
-int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes)
+int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes,
+ u64 disk_num_bytes)
{
struct btrfs_root *root = inode->root;
struct btrfs_fs_info *fs_info = root->fs_info;
@@ -333,6 +328,7 @@ int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes)
mutex_lock(&inode->delalloc_mutex);
num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
+ disk_num_bytes = ALIGN(disk_num_bytes, fs_info->sectorsize);
/*
* We always want to do it this way, every other way is wrong and ends
@@ -344,8 +340,8 @@ int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes)
* everything out and try again, which is bad. This way we just
* over-reserve slightly, and clean up the mess when we are done.
*/
- calc_inode_reservations(fs_info, num_bytes, &meta_reserve,
- &qgroup_reserve);
+ calc_inode_reservations(fs_info, num_bytes, disk_num_bytes,
+ &meta_reserve, &qgroup_reserve);
ret = btrfs_qgroup_reserve_meta_prealloc(root, qgroup_reserve, true);
if (ret)
goto out_fail;
@@ -362,7 +358,7 @@ int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes)
spin_lock(&inode->lock);
nr_extents = count_max_extents(num_bytes);
btrfs_mod_outstanding_extents(inode, nr_extents);
- inode->csum_bytes += num_bytes;
+ inode->csum_bytes += disk_num_bytes;
btrfs_calculate_inode_block_rsv_size(fs_info, inode);
spin_unlock(&inode->lock);
@@ -474,7 +470,7 @@ int btrfs_delalloc_reserve_space(struct inode *inode,
ret = btrfs_check_data_free_space(inode, reserved, start, len);
if (ret < 0)
return ret;
- ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), len);
+ ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), len, len);
if (ret < 0)
btrfs_free_reserved_data_space(inode, *reserved, start, len);
return ret;
diff --git a/fs/btrfs/delalloc-space.h b/fs/btrfs/delalloc-space.h
index 54466fbd7075..f847f0a80409 100644
--- a/fs/btrfs/delalloc-space.h
+++ b/fs/btrfs/delalloc-space.h
@@ -13,8 +13,8 @@ void btrfs_free_reserved_data_space(struct inode *inode,
void btrfs_delalloc_release_space(struct inode *inode,
struct extent_changeset *reserved,
u64 start, u64 len, bool qgroup_free);
-void btrfs_free_reserved_data_space_noquota(struct inode *inode, u64 start,
- u64 len);
+void btrfs_free_reserved_data_space_noquota(struct btrfs_fs_info *fs_info,
+ u64 num_bytes);
void btrfs_delalloc_release_metadata(struct btrfs_inode *inode, u64 num_bytes,
bool qgroup_free);
int btrfs_delalloc_reserve_space(struct inode *inode,
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 34c1a2284e03..bc7ee7c4180e 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1669,7 +1669,8 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
WARN_ON(reserve_bytes == 0);
ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode),
- reserve_bytes);
+ reserve_bytes,
+ reserve_bytes);
if (ret) {
if (!only_release_metadata)
btrfs_free_reserved_data_space(inode,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index d53580ad2c46..a8bc193c99ca 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1963,9 +1963,7 @@ void btrfs_clear_delalloc_extent(struct inode *vfs_inode,
if (root->root_key.objectid != BTRFS_DATA_RELOC_TREE_OBJECTID &&
do_list && !(state->state & EXTENT_NORESERVE) &&
(*bits & EXTENT_CLEAR_DATA_RESV))
- btrfs_free_reserved_data_space_noquota(
- &inode->vfs_inode,
- state->start, len);
+ btrfs_free_reserved_data_space_noquota(fs_info, len);
percpu_counter_add_batch(&fs_info->delalloc_bytes, -len,
fs_info->delalloc_batch);
@@ -7025,8 +7023,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
* use the existing or preallocated extent, so does not
* need to adjust btrfs_space_info's bytes_may_use.
*/
- btrfs_free_reserved_data_space_noquota(inode, start,
- len);
+ btrfs_free_reserved_data_space_noquota(fs_info, len);
goto skip_cow;
}
}
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index e3cec29813ee..af61e07b5094 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3262,8 +3262,8 @@ static int relocate_file_extent_cluster(struct inode *inode,
index = (cluster->start - offset) >> PAGE_SHIFT;
last_index = (cluster->end - offset) >> PAGE_SHIFT;
while (index <= last_index) {
- ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode),
- PAGE_SIZE);
+ ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), PAGE_SIZE,
+ PAGE_SIZE);
if (ret)
goto out;
--
2.24.0
^ permalink raw reply related
* [RFC PATCH v3 10/12] btrfs: optionally extend i_size in cow_file_range_inline()
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent. Add an
update_i_size parameter to cow_file_range_inline() and
insert_inline_extent() and pass in the size of the extent rather than
determining it from i_size. Since the start parameter is always passed
as 0, get rid of it and simplify the logic in these two functions. While
we're here, let's document the requirements for creating an inline
extent.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/inode.c | 94 +++++++++++++++++++++++-------------------------
1 file changed, 44 insertions(+), 50 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a8bc193c99ca..c1c37549155e 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -165,9 +165,10 @@ static int btrfs_init_inode_security(struct btrfs_trans_handle *trans,
static int insert_inline_extent(struct btrfs_trans_handle *trans,
struct btrfs_path *path, int extent_inserted,
struct btrfs_root *root, struct inode *inode,
- u64 start, size_t size, size_t compressed_size,
+ size_t size, size_t compressed_size,
int compress_type,
- struct page **compressed_pages)
+ struct page **compressed_pages,
+ bool update_i_size)
{
struct extent_buffer *leaf;
struct page *page = NULL;
@@ -176,7 +177,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,
struct btrfs_file_extent_item *ei;
int ret;
size_t cur_size = size;
- unsigned long offset;
+ u64 i_size;
ASSERT((compressed_size > 0 && compressed_pages) ||
(compressed_size == 0 && !compressed_pages));
@@ -191,7 +192,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,
size_t datasize;
key.objectid = btrfs_ino(BTRFS_I(inode));
- key.offset = start;
+ key.offset = 0;
key.type = BTRFS_EXTENT_DATA_KEY;
datasize = btrfs_file_extent_calc_inline_size(cur_size);
@@ -230,12 +231,10 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,
btrfs_set_file_extent_compression(leaf, ei,
compress_type);
} else {
- page = find_get_page(inode->i_mapping,
- start >> PAGE_SHIFT);
+ page = find_get_page(inode->i_mapping, 0);
btrfs_set_file_extent_compression(leaf, ei, 0);
kaddr = kmap_atomic(page);
- offset = offset_in_page(start);
- write_extent_buffer(leaf, kaddr + offset, ptr, size);
+ write_extent_buffer(leaf, kaddr, ptr, size);
kunmap_atomic(kaddr);
put_page(page);
}
@@ -251,7 +250,12 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,
* before we unlock the pages. Otherwise we
* could end up racing with unlink.
*/
- BTRFS_I(inode)->disk_i_size = inode->i_size;
+ i_size = i_size_read(inode);
+ if (update_i_size && size > i_size) {
+ i_size_write(inode, size);
+ i_size = size;
+ }
+ BTRFS_I(inode)->disk_i_size = i_size;
ret = btrfs_update_inode(trans, root, inode);
fail:
@@ -264,36 +268,31 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,
* does the checks required to make sure the data is small enough
* to fit as an inline extent.
*/
-static noinline int cow_file_range_inline(struct inode *inode, u64 start,
- u64 end, size_t compressed_size,
+static noinline int cow_file_range_inline(struct inode *inode, u64 size,
+ size_t compressed_size,
int compress_type,
- struct page **compressed_pages)
+ struct page **compressed_pages,
+ bool update_i_size)
{
struct btrfs_root *root = BTRFS_I(inode)->root;
struct btrfs_fs_info *fs_info = root->fs_info;
struct btrfs_trans_handle *trans;
- u64 isize = i_size_read(inode);
- u64 actual_end = min(end + 1, isize);
- u64 inline_len = actual_end - start;
- u64 aligned_end = ALIGN(end, fs_info->sectorsize);
- u64 data_len = inline_len;
+ u64 data_len = compressed_size ? compressed_size : size;
int ret;
struct btrfs_path *path;
int extent_inserted = 0;
u32 extent_item_size;
- if (compressed_size)
- data_len = compressed_size;
-
- if (start > 0 ||
- actual_end > fs_info->sectorsize ||
+ /*
+ * We can create an inline extent if it ends at or beyond the current
+ * i_size, is no larger than a sector (decompressed), and the (possibly
+ * compressed) data fits in a leaf and the configured maximum inline
+ * size.
+ */
+ if (size < i_size_read(inode) || size > fs_info->sectorsize ||
data_len > BTRFS_MAX_INLINE_DATA_SIZE(fs_info) ||
- (!compressed_size &&
- (actual_end & (fs_info->sectorsize - 1)) == 0) ||
- end + 1 < isize ||
- data_len > fs_info->max_inline) {
+ data_len > fs_info->max_inline)
return 1;
- }
path = btrfs_alloc_path();
if (!path)
@@ -306,27 +305,18 @@ static noinline int cow_file_range_inline(struct inode *inode, u64 start,
}
trans->block_rsv = &BTRFS_I(inode)->block_rsv;
- if (compressed_size && compressed_pages)
- extent_item_size = btrfs_file_extent_calc_inline_size(
- compressed_size);
- else
- extent_item_size = btrfs_file_extent_calc_inline_size(
- inline_len);
-
- ret = __btrfs_drop_extents(trans, root, inode, path,
- start, aligned_end, NULL,
- 1, 1, extent_item_size, &extent_inserted);
+ extent_item_size = btrfs_file_extent_calc_inline_size(data_len);
+ ret = __btrfs_drop_extents(trans, root, inode, path, 0,
+ fs_info->sectorsize, NULL, 1, 1,
+ extent_item_size, &extent_inserted);
if (ret) {
btrfs_abort_transaction(trans, ret);
goto out;
}
- if (isize > actual_end)
- inline_len = min_t(u64, isize, actual_end);
- ret = insert_inline_extent(trans, path, extent_inserted,
- root, inode, start,
- inline_len, compressed_size,
- compress_type, compressed_pages);
+ ret = insert_inline_extent(trans, path, extent_inserted, root, inode,
+ size, compressed_size, compress_type,
+ compressed_pages, update_i_size);
if (ret && ret != -ENOSPC) {
btrfs_abort_transaction(trans, ret);
goto out;
@@ -336,7 +326,7 @@ static noinline int cow_file_range_inline(struct inode *inode, u64 start,
}
set_bit(BTRFS_INODE_NEEDS_FULL_SYNC, &BTRFS_I(inode)->runtime_flags);
- btrfs_drop_extent_cache(BTRFS_I(inode), start, aligned_end - 1, 0);
+ btrfs_drop_extent_cache(BTRFS_I(inode), 0, fs_info->sectorsize - 1, 0);
out:
/*
* Don't forget to free the reserved space, as for inlined extent
@@ -605,13 +595,15 @@ static noinline int compress_file_range(struct async_chunk *async_chunk)
/* we didn't compress the entire range, try
* to make an uncompressed inline extent.
*/
- ret = cow_file_range_inline(inode, start, end, 0,
- BTRFS_COMPRESS_NONE, NULL);
+ ret = cow_file_range_inline(inode, actual_end, 0,
+ BTRFS_COMPRESS_NONE, NULL,
+ false);
} else {
/* try making a compressed inline extent */
- ret = cow_file_range_inline(inode, start, end,
+ ret = cow_file_range_inline(inode, actual_end,
total_compressed,
- compress_type, pages);
+ compress_type, pages,
+ false);
}
if (ret <= 0) {
unsigned long clear_flags = EXTENT_DELALLOC |
@@ -991,9 +983,11 @@ static noinline int cow_file_range(struct inode *inode,
inode_should_defrag(BTRFS_I(inode), start, end, num_bytes, SZ_64K);
if (start == 0) {
+ u64 actual_end = min_t(u64, i_size_read(inode), end + 1);
+
/* lets try to make an inline extent */
- ret = cow_file_range_inline(inode, start, end, 0,
- BTRFS_COMPRESS_NONE, NULL);
+ ret = cow_file_range_inline(inode, actual_end, 0,
+ BTRFS_COMPRESS_NONE, NULL, false);
if (ret == 0) {
/*
* We use DO_ACCOUNTING here because we need the
--
2.24.0
^ permalink raw reply related
* [RFC PATCH v3 11/12] btrfs: implement RWF_ENCODED reads
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
There are 4 main cases:
1. Inline extents: we copy the data straight out of the extent buffer.
2. Hole/preallocated extents: we indicate the size of the extent
starting from the read position; we don't need to copy zeroes.
3. Regular, uncompressed extents: we read the sectors we need directly
from disk.
4. Regular, compressed extents: we read the entire compressed extent
from disk and indicate what subset of the decompressed extent is in
the file.
This initial implementation simplifies a few things that can be improved
in the future:
- We hold the inode lock during the operation.
- Cases 1, 3, and 4 allocate temporary memory to read into before
copying out to userspace.
- Cases 3 and 4 do not implement repair yet.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/ctree.h | 2 +
fs/btrfs/file.c | 12 +-
fs/btrfs/inode.c | 454 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 467 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index f9ac05d1ca60..3be72a6e022e 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2901,6 +2901,8 @@ int btrfs_run_delalloc_range(struct inode *inode, struct page *locked_page,
int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end);
void btrfs_writepage_endio_finish_ordered(struct page *page, u64 start,
u64 end, int uptodate);
+ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter);
+
extern const struct dentry_operations btrfs_dentry_operations;
/* ioctl.c */
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index bc7ee7c4180e..5425200092c2 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -390,6 +390,16 @@ int btrfs_run_defrag_inodes(struct btrfs_fs_info *fs_info)
return 0;
}
+static ssize_t btrfs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+ if (iocb->ki_flags & IOCB_ENCODED) {
+ if (iocb->ki_flags & IOCB_NOWAIT)
+ return -EOPNOTSUPP;
+ return btrfs_encoded_read(iocb, iter);
+ }
+ return generic_file_read_iter(iocb, iter);
+}
+
/* simple helper to fault in pages and copy. This should go away
* and be replaced with calls into generic code.
*/
@@ -3455,7 +3465,7 @@ static int btrfs_file_open(struct inode *inode, struct file *filp)
const struct file_operations btrfs_file_operations = {
.llseek = btrfs_file_llseek,
- .read_iter = generic_file_read_iter,
+ .read_iter = btrfs_file_read_iter,
.splice_read = generic_file_splice_read,
.write_iter = btrfs_file_write_iter,
.mmap = btrfs_file_mmap,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c1c37549155e..698e24aa8b21 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9943,6 +9943,460 @@ void btrfs_set_range_writeback(struct extent_io_tree *tree, u64 start, u64 end)
}
}
+static int encoded_iov_compression_from_btrfs(struct encoded_iov *encoded,
+ unsigned int compress_type)
+{
+ switch (compress_type) {
+ case BTRFS_COMPRESS_NONE:
+ encoded->compression = ENCODED_IOV_COMPRESSION_NONE;
+ break;
+ case BTRFS_COMPRESS_ZLIB:
+ encoded->compression = ENCODED_IOV_COMPRESSION_ZLIB;
+ break;
+ case BTRFS_COMPRESS_LZO:
+ encoded->compression = ENCODED_IOV_COMPRESSION_LZO;
+ break;
+ case BTRFS_COMPRESS_ZSTD:
+ encoded->compression = ENCODED_IOV_COMPRESSION_ZSTD;
+ break;
+ default:
+ return -EIO;
+ }
+ return 0;
+}
+
+static ssize_t btrfs_encoded_read_inline(struct kiocb *iocb,
+ struct iov_iter *iter, u64 start,
+ u64 lockend,
+ struct extent_state **cached_state,
+ u64 extent_start, size_t count,
+ struct encoded_iov *encoded,
+ bool *unlocked)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+ struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
+ struct btrfs_path *path;
+ struct extent_buffer *leaf;
+ struct btrfs_file_extent_item *item;
+ u64 ram_bytes;
+ unsigned long ptr;
+ void *tmp;
+ ssize_t ret;
+
+ path = btrfs_alloc_path();
+ if (!path) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ ret = btrfs_lookup_file_extent(NULL, BTRFS_I(inode)->root, path,
+ btrfs_ino(BTRFS_I(inode)), extent_start,
+ 0);
+ if (ret) {
+ if (ret > 0) {
+ /* The extent item disappeared? */
+ ret = -EIO;
+ }
+ goto out;
+ }
+ leaf = path->nodes[0];
+ item = btrfs_item_ptr(leaf, path->slots[0],
+ struct btrfs_file_extent_item);
+
+ ram_bytes = btrfs_file_extent_ram_bytes(leaf, item);
+ ptr = btrfs_file_extent_inline_start(item);
+
+ encoded->len = (min_t(u64, extent_start + ram_bytes, inode->i_size) -
+ iocb->ki_pos);
+ ret = encoded_iov_compression_from_btrfs(encoded,
+ btrfs_file_extent_compression(leaf, item));
+ if (ret)
+ goto out;
+ if (encoded->compression) {
+ size_t inline_size;
+
+ inline_size = btrfs_file_extent_inline_item_len(leaf,
+ btrfs_item_nr(path->slots[0]));
+ if (inline_size > count) {
+ ret = -ENOBUFS;
+ goto out;
+ }
+ count = inline_size;
+ encoded->unencoded_len = ram_bytes;
+ encoded->unencoded_offset = iocb->ki_pos - extent_start;
+ } else {
+ encoded->len = encoded->unencoded_len = count =
+ min_t(u64, count, encoded->len);
+ ptr += iocb->ki_pos - extent_start;
+ }
+
+ tmp = kmalloc(count, GFP_NOFS);
+ if (!tmp) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ read_extent_buffer(leaf, tmp, ptr, count);
+ btrfs_free_path(path);
+ path = NULL;
+ unlock_extent_cached(io_tree, start, lockend, cached_state);
+ inode_unlock(inode);
+ *unlocked = true;
+
+ ret = copy_encoded_iov_to_iter(encoded, iter);
+ if (ret)
+ goto out_free;
+ if (copy_to_iter(tmp, count, iter) == count)
+ ret = count;
+ else
+ ret = -EFAULT;
+out_free:
+ kfree(tmp);
+out:
+ btrfs_free_path(path);
+ return ret;
+}
+
+struct btrfs_encoded_read_private {
+ struct inode *inode;
+ wait_queue_head_t wait;
+ atomic_t pending;
+ bool uptodate;
+ bool skip_csum;
+};
+
+static bool btrfs_encoded_read_check_csums(struct btrfs_io_bio *io_bio)
+{
+ struct btrfs_encoded_read_private *priv = io_bio->bio.bi_private;
+ struct inode *inode = priv->inode;
+ struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+ u32 sectorsize = fs_info->sectorsize;
+ struct bio_vec *bvec;
+ struct bvec_iter_all iter_all;
+ u64 offset = 0;
+
+ if (priv->skip_csum)
+ return true;
+ bio_for_each_segment_all(bvec, &io_bio->bio, iter_all) {
+ unsigned int i, nr_sectors, pgoff;
+
+ nr_sectors = BTRFS_BYTES_TO_BLKS(fs_info, bvec->bv_len);
+ pgoff = bvec->bv_offset;
+ for (i = 0; i < nr_sectors; i++) {
+ int csum_pos;
+
+ csum_pos = BTRFS_BYTES_TO_BLKS(fs_info, offset);
+ if (__readpage_endio_check(inode, io_bio, csum_pos,
+ bvec->bv_page, pgoff,
+ io_bio->logical + offset,
+ sectorsize))
+ return false;
+ offset += sectorsize;
+ pgoff += sectorsize;
+ }
+ }
+ return true;
+}
+
+static void btrfs_encoded_read_endio(struct bio *bio)
+{
+ struct btrfs_encoded_read_private *priv = bio->bi_private;
+ struct btrfs_io_bio *io_bio = btrfs_io_bio(bio);
+
+ if (bio->bi_status || !btrfs_encoded_read_check_csums(io_bio))
+ priv->uptodate = false;
+ if (!atomic_dec_return(&priv->pending))
+ wake_up(&priv->wait);
+ btrfs_io_bio_free_csum(io_bio);
+ bio_put(bio);
+}
+
+static bool btrfs_submit_encoded_read(struct bio *bio)
+{
+ struct btrfs_encoded_read_private *priv = bio->bi_private;
+ struct inode *inode = priv->inode;
+ struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+ blk_status_t status;
+
+ atomic_inc(&priv->pending);
+
+ if (!priv->skip_csum) {
+ status = btrfs_lookup_bio_sums(inode, bio, true,
+ btrfs_io_bio(bio)->logical,
+ NULL);
+ if (status)
+ goto out;
+ }
+
+ status = btrfs_bio_wq_end_io(fs_info, bio, BTRFS_WQ_ENDIO_DATA);
+ if (status)
+ goto out;
+
+ status = btrfs_map_bio(fs_info, bio, 0, 0);
+out:
+ if (status) {
+ bio->bi_status = status;
+ bio_endio(bio);
+ return false;
+ }
+ return true;
+}
+
+static ssize_t btrfs_encoded_read_regular(struct kiocb *iocb,
+ struct iov_iter *iter,
+ u64 start, u64 lockend,
+ struct extent_state **cached_state,
+ struct block_device *bdev,
+ u64 offset, u64 disk_io_size,
+ size_t count,
+ const struct encoded_iov *encoded,
+ bool *unlocked)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+ struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
+ struct btrfs_encoded_read_private priv = {
+ .inode = inode,
+ .wait = __WAIT_QUEUE_HEAD_INITIALIZER(priv.wait),
+ .pending = ATOMIC_INIT(1),
+ .uptodate = true,
+ .skip_csum = BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM,
+ };
+ struct page **pages;
+ unsigned long nr_pages, i;
+ struct bio *bio = NULL;
+ u64 cur;
+ size_t page_offset;
+ ssize_t ret;
+
+ nr_pages = DIV_ROUND_UP(disk_io_size, PAGE_SIZE);
+ pages = kcalloc(nr_pages, sizeof(struct page *), GFP_NOFS);
+ if (!pages)
+ return -ENOMEM;
+ for (i = 0; i < nr_pages; i++) {
+ pages[i] = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+ if (!pages[i]) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+
+ i = 0;
+ cur = 0;
+ while (cur < disk_io_size) {
+ size_t bytes = min_t(u64, disk_io_size - cur,
+ PAGE_SIZE);
+
+ if (!bio) {
+ bio = btrfs_bio_alloc(offset + cur);
+ bio_set_dev(bio, bdev);
+ bio->bi_end_io = btrfs_encoded_read_endio;
+ bio->bi_private = &priv;
+ bio->bi_opf = REQ_OP_READ;
+ btrfs_io_bio(bio)->logical = start + cur;
+ }
+
+ if (bio_add_page(bio, pages[i], bytes, 0) < bytes) {
+ bool success;
+
+ success = btrfs_submit_encoded_read(bio);
+ bio = NULL;
+ if (!success)
+ break;
+ continue;
+ }
+ i++;
+ cur += bytes;
+ }
+
+ if (bio)
+ btrfs_submit_encoded_read(bio);
+ if (atomic_dec_return(&priv.pending))
+ wait_event(priv.wait, !atomic_read(&priv.pending));
+ if (!priv.uptodate) {
+ ret = -EIO;
+ goto out;
+ }
+
+ unlock_extent_cached(io_tree, start, lockend, cached_state);
+ inode_unlock(inode);
+ *unlocked = true;
+
+ ret = copy_encoded_iov_to_iter(encoded, iter);
+ if (ret)
+ goto out;
+ if (encoded->compression) {
+ i = 0;
+ page_offset = 0;
+ } else {
+ i = (iocb->ki_pos - start) >> PAGE_SHIFT;
+ page_offset = (iocb->ki_pos - start) & (PAGE_SIZE - 1);
+ }
+ cur = 0;
+ while (cur < count) {
+ size_t bytes = min_t(size_t, count - cur,
+ PAGE_SIZE - page_offset);
+
+ if (copy_page_to_iter(pages[i], page_offset, bytes,
+ iter) != bytes) {
+ ret = -EFAULT;
+ goto out;
+ }
+ i++;
+ cur += bytes;
+ page_offset = 0;
+ }
+ ret = count;
+out:
+ for (i = 0; i < nr_pages; i++) {
+ if (pages[i])
+ put_page(pages[i]);
+ }
+ kfree(pages);
+ return ret;
+}
+
+ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+ struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+ struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
+ ssize_t ret;
+ size_t count;
+ struct block_device *em_bdev;
+ u64 start, lockend, offset, disk_io_size;
+ struct extent_state *cached_state = NULL;
+ struct extent_map *em;
+ struct encoded_iov encoded = {};
+ bool unlocked = false;
+
+ ret = generic_encoded_read_checks(iocb, iter);
+ if (ret < 0)
+ return ret;
+ if (ret == 0)
+ return copy_encoded_iov_to_iter(&encoded, iter);
+ count = ret;
+
+ file_accessed(iocb->ki_filp);
+
+ inode_lock_shared(inode);
+
+ if (iocb->ki_pos >= inode->i_size) {
+ inode_unlock_shared(inode);
+ return copy_encoded_iov_to_iter(&encoded, iter);
+ }
+ start = ALIGN_DOWN(iocb->ki_pos, fs_info->sectorsize);
+ /*
+ * We don't know how long the extent containing iocb->ki_pos is, but if
+ * it's compressed we know that it won't be longer than this.
+ */
+ lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;
+
+ for (;;) {
+ struct btrfs_ordered_extent *ordered;
+
+ ret = btrfs_wait_ordered_range(inode, start,
+ lockend - start + 1);
+ if (ret)
+ goto out_unlock_inode;
+ lock_extent_bits(io_tree, start, lockend, &cached_state);
+ ordered = btrfs_lookup_ordered_range(BTRFS_I(inode), start,
+ lockend - start + 1);
+ if (!ordered)
+ break;
+ btrfs_put_ordered_extent(ordered);
+ unlock_extent_cached(io_tree, start, lockend, &cached_state);
+ cond_resched();
+ }
+
+ em = btrfs_get_extent(BTRFS_I(inode), NULL, 0, start,
+ lockend - start + 1, 0);
+ if (IS_ERR(em)) {
+ ret = PTR_ERR(em);
+ goto out_unlock_extent;
+ }
+ em_bdev = em->bdev;
+
+ if (em->block_start == EXTENT_MAP_INLINE) {
+ u64 extent_start = em->start;
+
+ /*
+ * For inline extents we get everything we need out of the
+ * extent item.
+ */
+ free_extent_map(em);
+ em = NULL;
+ ret = btrfs_encoded_read_inline(iocb, iter, start, lockend,
+ &cached_state, extent_start,
+ count, &encoded, &unlocked);
+ goto out;
+ }
+
+ /*
+ * We only want to return up to EOF even if the extent extends beyond
+ * that.
+ */
+ encoded.len = (min_t(u64, extent_map_end(em), inode->i_size) -
+ iocb->ki_pos);
+ if (em->block_start == EXTENT_MAP_HOLE ||
+ test_bit(EXTENT_FLAG_PREALLOC, &em->flags)) {
+ offset = EXTENT_MAP_HOLE;
+ } else if (test_bit(EXTENT_FLAG_COMPRESSED, &em->flags)) {
+ offset = em->block_start;
+ /*
+ * Bail if the buffer isn't large enough to return the whole
+ * compressed extent.
+ */
+ if (em->block_len > count) {
+ ret = -ENOBUFS;
+ goto out_em;
+ }
+ disk_io_size = count = em->block_len;
+ encoded.unencoded_len = em->ram_bytes;
+ encoded.unencoded_offset = iocb->ki_pos - em->orig_start;
+ ret = encoded_iov_compression_from_btrfs(&encoded,
+ em->compress_type);
+ if (ret)
+ goto out_em;
+ } else {
+ offset = em->block_start + (start - em->start);
+ if (encoded.len > count)
+ encoded.len = count;
+ /*
+ * Don't read beyond what we locked. This also limits the page
+ * allocations that we'll do.
+ */
+ disk_io_size = min(lockend + 1, iocb->ki_pos + encoded.len) - start;
+ encoded.len = encoded.unencoded_len = count =
+ start + disk_io_size - iocb->ki_pos;
+ disk_io_size = ALIGN(disk_io_size, fs_info->sectorsize);
+ }
+ free_extent_map(em);
+ em = NULL;
+
+ if (offset == EXTENT_MAP_HOLE) {
+ unlock_extent_cached(io_tree, start, lockend, &cached_state);
+ inode_unlock_shared(inode);
+ unlocked = true;
+ ret = copy_encoded_iov_to_iter(&encoded, iter);
+ } else {
+ ret = btrfs_encoded_read_regular(iocb, iter, start, lockend,
+ &cached_state, em_bdev, offset,
+ disk_io_size, count, &encoded,
+ &unlocked);
+ }
+
+out:
+ if (ret >= 0)
+ iocb->ki_pos += encoded.len;
+out_em:
+ free_extent_map(em);
+out_unlock_extent:
+ if (!unlocked)
+ unlock_extent_cached(io_tree, start, lockend, &cached_state);
+out_unlock_inode:
+ if (!unlocked)
+ inode_unlock_shared(inode);
+ return ret;
+}
+
#ifdef CONFIG_SWAP
/*
* Add an entry indicating a block group or device which is pinned by a
--
2.24.0
^ permalink raw reply related
* [RFC PATCH v3 12/12] btrfs: implement RWF_ENCODED writes
From: Omar Sandoval @ 2019-11-20 18:24 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <cover.1574273658.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
The implementation resembles direct I/O: we have to flush any ordered
extents, invalidate the page cache, and do the io tree/delalloc/extent
map/ordered extent dance. From there, we can reuse the compression code
with a minor modification to distinguish the write from writeback. This
also creates inline extents when possible.
Now that read and write are implemented, this also sets the
FMODE_ENCODED_IO flag in btrfs_file_open().
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/compression.c | 6 +-
fs/btrfs/compression.h | 5 +-
fs/btrfs/ctree.h | 2 +
fs/btrfs/file.c | 40 +++++--
fs/btrfs/inode.c | 243 +++++++++++++++++++++++++++++++++++++++-
fs/btrfs/ordered-data.c | 12 +-
fs/btrfs/ordered-data.h | 2 +
7 files changed, 293 insertions(+), 17 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 05b6e404a291..ae24e8c5ea34 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -276,7 +276,8 @@ static void end_compressed_bio_write(struct bio *bio)
bio->bi_status == BLK_STS_OK);
cb->compressed_pages[0]->mapping = NULL;
- end_compressed_writeback(inode, cb);
+ if (cb->writeback)
+ end_compressed_writeback(inode, cb);
/* note, our inode could be gone now */
/*
@@ -311,7 +312,7 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
unsigned long compressed_len,
struct page **compressed_pages,
unsigned long nr_pages,
- unsigned int write_flags)
+ unsigned int write_flags, bool writeback)
{
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct bio *bio = NULL;
@@ -336,6 +337,7 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
cb->mirror_num = 0;
cb->compressed_pages = compressed_pages;
cb->compressed_len = compressed_len;
+ cb->writeback = writeback;
cb->orig_bio = NULL;
cb->nr_pages = nr_pages;
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 4cb8be9ff88b..d4176384ec15 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -47,6 +47,9 @@ struct compressed_bio {
/* the compression algorithm for this bio */
int compress_type;
+ /* Whether this is a write for writeback. */
+ bool writeback;
+
/* number of compressed pages in the array */
unsigned long nr_pages;
@@ -93,7 +96,7 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
unsigned long compressed_len,
struct page **compressed_pages,
unsigned long nr_pages,
- unsigned int write_flags);
+ unsigned int write_flags, bool writeback);
blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
int mirror_num, unsigned long bio_flags);
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3be72a6e022e..3c020dbe894a 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2902,6 +2902,8 @@ int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end);
void btrfs_writepage_endio_finish_ordered(struct page *page, u64 start,
u64 end, int uptodate);
ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter);
+ssize_t btrfs_encoded_write(struct kiocb *iocb, struct iov_iter *from,
+ struct encoded_iov *encoded);
extern const struct dentry_operations btrfs_dentry_operations;
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 5425200092c2..16d8df66378f 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1893,8 +1893,7 @@ static void update_time_for_write(struct inode *inode)
inode_inc_iversion(inode);
}
-static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
- struct iov_iter *from)
+static ssize_t btrfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct file *file = iocb->ki_filp;
struct inode *inode = file_inode(file);
@@ -1904,30 +1903,51 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
u64 end_pos;
ssize_t num_written = 0;
const bool sync = iocb->ki_flags & IOCB_DSYNC;
+ struct encoded_iov encoded;
ssize_t err;
loff_t pos;
size_t count;
loff_t oldsize;
int clean_page = 0;
- if (!(iocb->ki_flags & IOCB_DIRECT) &&
- (iocb->ki_flags & IOCB_NOWAIT))
+ if ((iocb->ki_flags & IOCB_NOWAIT) &&
+ (!(iocb->ki_flags & IOCB_DIRECT) ||
+ (iocb->ki_flags & IOCB_ENCODED)))
return -EOPNOTSUPP;
+ if (iocb->ki_flags & IOCB_ENCODED) {
+ err = copy_encoded_iov_from_iter(&encoded, from);
+ if (err)
+ return err;
+ }
+
if (!inode_trylock(inode)) {
if (iocb->ki_flags & IOCB_NOWAIT)
return -EAGAIN;
inode_lock(inode);
}
- err = generic_write_checks(iocb, from);
- if (err <= 0) {
+ if (iocb->ki_flags & IOCB_ENCODED) {
+ err = generic_encoded_write_checks(iocb, &encoded);
+ if (err) {
+ inode_unlock(inode);
+ return err;
+ }
+ count = encoded.len;
+ } else {
+ err = generic_write_checks(iocb, from);
+ if (err < 0) {
+ inode_unlock(inode);
+ return err;
+ }
+ count = iov_iter_count(from);
+ }
+ if (count == 0) {
inode_unlock(inode);
return err;
}
pos = iocb->ki_pos;
- count = iov_iter_count(from);
if (iocb->ki_flags & IOCB_NOWAIT) {
/*
* We will allocate space in case nodatacow is not set,
@@ -1986,7 +2006,9 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
if (sync)
atomic_inc(&BTRFS_I(inode)->sync_writers);
- if (iocb->ki_flags & IOCB_DIRECT) {
+ if (iocb->ki_flags & IOCB_ENCODED) {
+ num_written = btrfs_encoded_write(iocb, from, &encoded);
+ } else if (iocb->ki_flags & IOCB_DIRECT) {
num_written = __btrfs_direct_write(iocb, from);
} else {
num_written = btrfs_buffered_write(iocb, from);
@@ -3459,7 +3481,7 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int whence)
static int btrfs_file_open(struct inode *inode, struct file *filp)
{
- filp->f_mode |= FMODE_NOWAIT;
+ filp->f_mode |= FMODE_NOWAIT | FMODE_ENCODED_IO;
return generic_file_open(inode, filp);
}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 698e24aa8b21..b9b410af6d0d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -868,7 +868,7 @@ static noinline void submit_compressed_extents(struct async_chunk *async_chunk)
ins.objectid,
ins.offset, async_extent->pages,
async_extent->nr_pages,
- async_chunk->write_flags)) {
+ async_chunk->write_flags, true)) {
struct page *p = async_extent->pages[0];
const u64 start = async_extent->start;
const u64 end = start + async_extent->ram_size - 1;
@@ -2392,7 +2392,8 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
if (!test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags) &&
!test_bit(BTRFS_ORDERED_PREALLOC, &ordered_extent->flags) &&
- !test_bit(BTRFS_ORDERED_DIRECT, &ordered_extent->flags))
+ !test_bit(BTRFS_ORDERED_DIRECT, &ordered_extent->flags) &&
+ !test_bit(BTRFS_ORDERED_ENCODED, &ordered_extent->flags))
clear_new_delalloc_bytes = true;
nolock = btrfs_is_free_space_inode(BTRFS_I(inode));
@@ -10397,6 +10398,244 @@ ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter)
return ret;
}
+ssize_t btrfs_encoded_write(struct kiocb *iocb, struct iov_iter *from,
+ struct encoded_iov *encoded)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+ struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+ struct btrfs_root *root = BTRFS_I(inode)->root;
+ struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
+ struct extent_changeset *data_reserved = NULL;
+ struct extent_state *cached_state = NULL;
+ int compression;
+ size_t orig_count;
+ u64 start, end;
+ u64 num_bytes, ram_bytes, disk_num_bytes;
+ unsigned long nr_pages, i;
+ struct page **pages;
+ struct btrfs_key ins;
+ bool extent_reserved = false;
+ struct extent_map *em;
+ ssize_t ret;
+
+ switch (encoded->compression) {
+ case ENCODED_IOV_COMPRESSION_ZLIB:
+ compression = BTRFS_COMPRESS_ZLIB;
+ break;
+ case ENCODED_IOV_COMPRESSION_LZO:
+ compression = BTRFS_COMPRESS_LZO;
+ break;
+ case ENCODED_IOV_COMPRESSION_ZSTD:
+ compression = BTRFS_COMPRESS_ZSTD;
+ break;
+ default:
+ return -EINVAL;
+ }
+ if (encoded->encryption != ENCODED_IOV_ENCRYPTION_NONE)
+ return -EINVAL;
+
+ orig_count = iov_iter_count(from);
+
+ /* The extent size must be sane. */
+ if (encoded->unencoded_len > BTRFS_MAX_UNCOMPRESSED ||
+ orig_count > BTRFS_MAX_COMPRESSED || orig_count == 0)
+ return -EINVAL;
+
+ /*
+ * The compressed data must be smaller than the decompressed data.
+ *
+ * It's of course possible for data to compress to larger or the same
+ * size, but the buffered I/O path falls back to no compression for such
+ * data, and we don't want to break any assumptions by creating these
+ * extents.
+ *
+ * Note that this is less strict than the current check we have that the
+ * compressed data must be at least one sector smaller than the
+ * decompressed data. We only want to enforce the weaker requirement
+ * from old kernels that it is at least one byte smaller.
+ */
+ if (orig_count >= encoded->unencoded_len)
+ return -EINVAL;
+
+ /* The extent must start on a sector boundary. */
+ start = iocb->ki_pos;
+ if (!IS_ALIGNED(start, fs_info->sectorsize))
+ return -EINVAL;
+
+ /*
+ * The extent must end on a sector boundary. However, we allow a write
+ * which ends at or extends i_size to have an unaligned length; we round
+ * up the extent size and set i_size to the unaligned end.
+ */
+ if (start + encoded->len < inode->i_size &&
+ !IS_ALIGNED(start + encoded->len, fs_info->sectorsize))
+ return -EINVAL;
+
+ /* Finally, the offset in the unencoded data must be sector-aligned. */
+ if (!IS_ALIGNED(encoded->unencoded_offset, fs_info->sectorsize))
+ return -EINVAL;
+
+ num_bytes = ALIGN(encoded->len, fs_info->sectorsize);
+ ram_bytes = ALIGN(encoded->unencoded_len, fs_info->sectorsize);
+ end = start + num_bytes - 1;
+
+ /*
+ * If the extent cannot be inline, the compressed data on disk must be
+ * sector-aligned. For convenience, we extend it with zeroes if it
+ * isn't.
+ */
+ disk_num_bytes = ALIGN(orig_count, fs_info->sectorsize);
+ nr_pages = DIV_ROUND_UP(disk_num_bytes, PAGE_SIZE);
+ pages = kvcalloc(nr_pages, sizeof(struct page *), GFP_KERNEL_ACCOUNT);
+ if (!pages)
+ return -ENOMEM;
+ for (i = 0; i < nr_pages; i++) {
+ size_t bytes = min_t(size_t, PAGE_SIZE, iov_iter_count(from));
+ char *kaddr;
+
+ pages[i] = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_HIGHMEM);
+ if (!pages[i]) {
+ ret = -ENOMEM;
+ goto out_pages;
+ }
+ kaddr = kmap(pages[i]);
+ if (copy_from_iter(kaddr, bytes, from) != bytes) {
+ kunmap(pages[i]);
+ ret = -EFAULT;
+ goto out_pages;
+ }
+ if (bytes < PAGE_SIZE)
+ memset(kaddr + bytes, 0, PAGE_SIZE - bytes);
+ kunmap(pages[i]);
+ }
+
+ for (;;) {
+ struct btrfs_ordered_extent *ordered;
+
+ ret = btrfs_wait_ordered_range(inode, start, num_bytes);
+ if (ret)
+ goto out_pages;
+ ret = invalidate_inode_pages2_range(inode->i_mapping,
+ start >> PAGE_SHIFT,
+ end >> PAGE_SHIFT);
+ if (ret)
+ goto out_pages;
+ lock_extent_bits(io_tree, start, end, &cached_state);
+ ordered = btrfs_lookup_ordered_range(BTRFS_I(inode), start,
+ num_bytes);
+ if (!ordered &&
+ !filemap_range_has_page(inode->i_mapping, start, end))
+ break;
+ if (ordered)
+ btrfs_put_ordered_extent(ordered);
+ unlock_extent_cached(io_tree, start, end, &cached_state);
+ cond_resched();
+ }
+
+ ret = btrfs_alloc_data_chunk_ondemand(BTRFS_I(inode), disk_num_bytes);
+ if (ret)
+ goto out_unlock;
+ ret = btrfs_qgroup_reserve_data(inode, &data_reserved, start,
+ num_bytes);
+ if (ret)
+ goto out_free_data_space;
+ ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), num_bytes,
+ disk_num_bytes);
+ if (ret)
+ goto out_qgroup_free_data;
+
+ /* Try an inline extent first. */
+ if (start == 0 && encoded->unencoded_len == encoded->len &&
+ encoded->unencoded_offset == 0) {
+ ret = cow_file_range_inline(inode, encoded->len, orig_count,
+ compression, pages, true);
+ if (ret <= 0) {
+ if (ret == 0)
+ ret = orig_count;
+ goto out_delalloc_release;
+ }
+ }
+
+ ret = btrfs_reserve_extent(root, disk_num_bytes, disk_num_bytes,
+ disk_num_bytes, 0, 0, &ins, 1, 1);
+ if (ret)
+ goto out_delalloc_release;
+ extent_reserved = true;
+
+ em = create_io_em(inode, start, num_bytes,
+ start - encoded->unencoded_offset, ins.objectid,
+ ins.offset, ins.offset, ram_bytes, compression,
+ BTRFS_ORDERED_COMPRESSED);
+ if (IS_ERR(em)) {
+ ret = PTR_ERR(em);
+ goto out_free_reserved;
+ }
+ free_extent_map(em);
+
+ ret = btrfs_add_ordered_extent(inode, start, num_bytes, ram_bytes,
+ ins.objectid, ins.offset,
+ encoded->unencoded_offset,
+ (1 << BTRFS_ORDERED_ENCODED) |
+ (1 << BTRFS_ORDERED_COMPRESSED),
+ compression);
+ if (ret) {
+ btrfs_drop_extent_cache(BTRFS_I(inode), start, end, 0);
+ goto out_free_reserved;
+ }
+ btrfs_dec_block_group_reservations(fs_info, ins.objectid);
+
+ if (start + encoded->len > inode->i_size)
+ i_size_write(inode, start + encoded->len);
+
+ unlock_extent_cached(io_tree, start, end, &cached_state);
+
+ btrfs_delalloc_release_extents(BTRFS_I(inode), num_bytes);
+
+ if (btrfs_submit_compressed_write(inode, start, num_bytes, ins.objectid,
+ ins.offset, pages, nr_pages, 0,
+ false)) {
+ struct page *page = pages[0];
+
+ page->mapping = inode->i_mapping;
+ btrfs_writepage_endio_finish_ordered(page, start, end, 0);
+ page->mapping = NULL;
+ ret = -EIO;
+ goto out_pages;
+ }
+ ret = orig_count;
+ goto out;
+
+out_free_reserved:
+ btrfs_dec_block_group_reservations(fs_info, ins.objectid);
+ btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset, 1);
+out_delalloc_release:
+ btrfs_delalloc_release_extents(BTRFS_I(inode), num_bytes);
+ btrfs_delalloc_release_metadata(BTRFS_I(inode), disk_num_bytes,
+ ret < 0);
+out_qgroup_free_data:
+ if (ret < 0)
+ btrfs_qgroup_free_data(inode, data_reserved, start, num_bytes);
+out_free_data_space:
+ /*
+ * If btrfs_reserve_extent() succeeded, then we already decremented
+ * bytes_may_use.
+ */
+ if (!extent_reserved)
+ btrfs_free_reserved_data_space_noquota(fs_info, disk_num_bytes);
+out_unlock:
+ unlock_extent_cached(io_tree, start, end, &cached_state);
+out_pages:
+ for (i = 0; i < nr_pages; i++) {
+ if (pages[i])
+ put_page(pages[i]);
+ }
+ kvfree(pages);
+out:
+ if (ret >= 0)
+ iocb->ki_pos += encoded->len;
+ return ret;
+}
+
#ifdef CONFIG_SWAP
/*
* Add an entry indicating a block group or device which is pinned by a
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 3c6edc307657..1e52105c4c1a 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -451,9 +451,15 @@ void btrfs_remove_ordered_extent(struct inode *inode,
spin_lock(&btrfs_inode->lock);
btrfs_mod_outstanding_extents(btrfs_inode, -1);
spin_unlock(&btrfs_inode->lock);
- if (root != fs_info->tree_root)
- btrfs_delalloc_release_metadata(btrfs_inode, entry->num_bytes,
- false);
+ if (root != fs_info->tree_root) {
+ u64 release;
+
+ if (test_bit(BTRFS_ORDERED_ENCODED, &entry->flags))
+ release = entry->disk_num_bytes;
+ else
+ release = entry->num_bytes;
+ btrfs_delalloc_release_metadata(btrfs_inode, release, false);
+ }
if (test_bit(BTRFS_ORDERED_DIRECT, &entry->flags))
percpu_counter_add_batch(&fs_info->dio_bytes, -entry->num_bytes,
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index a038bda16fdf..0079ce49bc5e 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -61,6 +61,8 @@ enum {
BTRFS_ORDERED_TRUNCATED,
/* Regular IO for COW */
BTRFS_ORDERED_REGULAR,
+ /* RWF_ENCODED I/O */
+ BTRFS_ORDERED_ENCODED,
};
struct btrfs_ordered_extent {
--
2.24.0
^ permalink raw reply related
* Re: Suggested Patch is not working for 22851 Bugzilla issue
From: Kees Cook @ 2019-11-20 18:39 UTC (permalink / raw)
To: Michal Hocko; +Cc: Bala S, linux-kernel, linux-api, Linus Torvalds
In-Reply-To: <20191119095708.GB21113@dhcp22.suse.cz>
On Tue, Nov 19, 2019 at 10:57:08AM +0100, Michal Hocko wrote:
> let me add Kees Cook and Linus to the cc list. I didn't have much
> time to study the bug report and cannot really comment on the security
> aspect of it. But let me point out that a big part of
> MAP_FIXED_NOREPLACE usage has been removed from the loader code just
> recently because it has caused some regressions
> http://lkml.kernel.org/r/20191005233227.GB25745@shell.armlinux.org.uk
> b212921b13bd ("elf: don't use MAP_FIXED_NOREPLACE for elf executable mappings").
> So you definitely want to look at the current Linus tree for your future
> experiments.
Hi!
Yes, as Michal mentions, there were legitimate binaries that expected to
overlap mappings, so we had to revert the MAP_FIXED_NOREPLACE logic for
now. At the time I added a TODO item for fixing this up correctly here:
https://github.com/KSPP/linux/issues/17
Speaking to the ldd issue (not the kernel binfmt_elf.c loader, which
is very separate), there isn't a security issue here: ldd can in many
cases _execute_ the binaries it is examining. This is a well known flaw
(as Florian points out in the bug report).
Is there some other piece of this puzzle you're trying to solve? I'm
always open to hearing new ideas in this space.
Thanks!
-Kees
>
> On Tue 19-11-19 10:37:44, Bala S wrote:
> > Hi Mhocko,
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=22851
> > For the above issue, I have found the patch.
> >
> > Patch link:
> > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1561935.html
> >
> > Only change i noticed is 'MAP_FIXED_NOREPLACE' is used instead of
> > 'MAP_FIXED_SAFE'.
> >
> > I ran test case on the following targets with this patch:
> >
> > 1. For X86-64, Still i could see the reported issue( 'libevil.so' just
> > runs ‘cat /etc/passwd')
> >
> > 2. For MIPS-64, i am not seeing the malicious file content as
> > reported. But ‘ldd’ could not found ‘libevil.so’.
> >
> > root@qemumips64:~/LIN1019-1806# ldd ./main
> > linux-vdso.so.1 (0x000000fff1f20000)
> > libevil.so => not found
> > libc.so.6 => /lib/libc.so.6 (0x0000005e46f70000)
> > /lib/ld.so.1 (0x000000fff7888000)
> >
> > I am not clear why this patch is not working for X86-64? But it is
> > working for MIPS-64 with some issue.
> > Please let me know, if anything is pending on this patch for the reported issue.
> >
> > Thanks,
> > Bala
>
> --
> Michal Hocko
> SUSE Labs
--
Kees Cook
^ permalink raw reply
* Re: Suggested Patch is not working for 22851 Bugzilla issue
From: Linus Torvalds @ 2019-11-20 19:45 UTC (permalink / raw)
To: Kees Cook; +Cc: Michal Hocko, Bala S, Linux Kernel Mailing List, Linux API
In-Reply-To: <201911201032.67566C6BF@keescook>
On Wed, Nov 20, 2019 at 10:39 AM Kees Cook <keescook@chromium.org> wrote:
> Yes, as Michal mentions, there were legitimate binaries that expected to
> overlap mappings
I'm not sure they were really overlapping as much as "the ELF sections
were in a bad order and we don't do the whole "optimize loading"
thing.
It's one of those things that *might* be fixed by first creating a
"simplified/combined map of the ELF sections", and them using mmap()
on that simplified one. But that code is nasty and hairy.
Linus
^ permalink raw reply
* Re: [PATCH v25 10/12] LRNG - add TRNG support
From: Stephan Müller @ 2019-11-20 19:51 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andy Lutomirski, Arnd Bergmann, Linux Crypto Mailing List, LKML,
Linux API, Eric W. Biederman, Alexander E. Patrakov,
Ahmed S. Darwish, Theodore Y. Ts'o, Willy Tarreau,
Matthew Garrett, Vito Caputo, Andreas Dilger, Jan Kara,
Ray Strode, William Jon McCann, zhangjs, Florian Weimer,
Lennart Poettering, Nicolai Stange
In-Reply-To: <20191120132918.GA2892197@kroah.com>
Am Mittwoch, 20. November 2019, 14:29:18 CET schrieb Greg Kroah-Hartman:
Hi Greg,
> On Wed, Nov 20, 2019 at 09:58:35AM +0100, Stephan Müller wrote:
> > Am Dienstag, 19. November 2019, 13:41:50 CET schrieb Greg Kroah-Hartman:
> >
> > Hi Greg,
> >
> > > On Tue, Nov 19, 2019 at 02:07:40AM -0800, Andy Lutomirski wrote:
> > > > > As this would introduce a new device file now, is there a special
> > > > > process that I need to follow or do I need to copy? Which
> > > > > major/minor
> > > > > number should I use?
> > > > >
> > > > > Looking into static const struct memdev devlist[] I see
> > > > >
> > > > > [8] = { "random", 0666, &random_fops, 0 },
> > > > > [9] = { "urandom", 0666, &urandom_fops, 0 },
> > > > >
> > > > > Shall a true_random be added here with [10]?
> > > >
> > > > I am not at all an expert on chardevs, but this sounds generally
> > > > reasonable. gregkh is probably the real authority here.
> > >
> > > [10] is the aio char device node, so you better not try to overlap it or
> > > bad things will happen :(
> >
> > Thanks for your insights.
> >
> > Which device minor number could we use?
>
> Get your own dynamic one by using a misc device if you _REALLY_ want to
> add yet-another-char-node-for-random-data.
>
> But I would have thought that we all realize that this is not the way to
> do things. Let's not have "random", "urandom", and "true_random" be
> something we want to totally confuse userspace with, that way is insane.
>
> Please just make the existing userspace api "just work", don't add to
> the mess.
Thank you, I think we should follow that advise.
With that and considering Alexander's rightful remark we have a challenge. So,
changing the syscall may not be the right way unless we find a way to restrict
the permissions somehow (capability? LSM? None of that seems to be a good
fit).
What about providing a /sys file? I.e. adding a file that:
a) has permissions 440 per default and maybe the ownership of root:root
b) allow user space to perform a chown/chgrp
c) only supports reading of data from user space
But then, how could we provide a common interface for the existing random.c
and the LRNG?
Or should we use a proc file for that? If yes, I guess it should not be a
sysctl, but a "regular" proc file that should allow a chown(2) operation. On
the other hand, is proc the right place to provide a user space interface for
exporting data to user?
Thanks a lot.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v25 10/12] LRNG - add TRNG support
From: Alexander E. Patrakov @ 2019-11-20 19:57 UTC (permalink / raw)
To: Stephan Müller, Greg Kroah-Hartman
Cc: Andy Lutomirski, Arnd Bergmann, Linux Crypto Mailing List, LKML,
Linux API, Eric W. Biederman, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Florian Weimer, Lennart Poettering, Nicolai Stange,
Peter, Matthias
In-Reply-To: <1695782.oZ5Vf4nH9s@positron.chronox.de>
[-- Attachment #1: Type: text/plain, Size: 2952 bytes --]
21.11.2019 00:51, Stephan Müller пишет:
> Am Mittwoch, 20. November 2019, 14:29:18 CET schrieb Greg Kroah-Hartman:
>
> Hi Greg,
>
>> On Wed, Nov 20, 2019 at 09:58:35AM +0100, Stephan Müller wrote:
>>> Am Dienstag, 19. November 2019, 13:41:50 CET schrieb Greg Kroah-Hartman:
>>>
>>> Hi Greg,
>>>
>>>> On Tue, Nov 19, 2019 at 02:07:40AM -0800, Andy Lutomirski wrote:
>>>>>> As this would introduce a new device file now, is there a special
>>>>>> process that I need to follow or do I need to copy? Which
>>>>>> major/minor
>>>>>> number should I use?
>>>>>>
>>>>>> Looking into static const struct memdev devlist[] I see
>>>>>>
>>>>>> [8] = { "random", 0666, &random_fops, 0 },
>>>>>> [9] = { "urandom", 0666, &urandom_fops, 0 },
>>>>>>
>>>>>> Shall a true_random be added here with [10]?
>>>>>
>>>>> I am not at all an expert on chardevs, but this sounds generally
>>>>> reasonable. gregkh is probably the real authority here.
>>>>
>>>> [10] is the aio char device node, so you better not try to overlap it or
>>>> bad things will happen :(
>>>
>>> Thanks for your insights.
>>>
>>> Which device minor number could we use?
>>
>> Get your own dynamic one by using a misc device if you _REALLY_ want to
>> add yet-another-char-node-for-random-data.
>>
>> But I would have thought that we all realize that this is not the way to
>> do things. Let's not have "random", "urandom", and "true_random" be
>> something we want to totally confuse userspace with, that way is insane.
>>
>> Please just make the existing userspace api "just work", don't add to
>> the mess.
>
> Thank you, I think we should follow that advise.
>
> With that and considering Alexander's rightful remark we have a challenge. So,
> changing the syscall may not be the right way unless we find a way to restrict
> the permissions somehow (capability? LSM? None of that seems to be a good
> fit).
>
> What about providing a /sys file? I.e. adding a file that:
>
> a) has permissions 440 per default and maybe the ownership of root:root
>
> b) allow user space to perform a chown/chgrp
>
> c) only supports reading of data from user space
>
> But then, how could we provide a common interface for the existing random.c
> and the LRNG?
>
> Or should we use a proc file for that? If yes, I guess it should not be a
> sysctl, but a "regular" proc file that should allow a chown(2) operation. On
> the other hand, is proc the right place to provide a user space interface for
> exporting data to user?
>
> Thanks a lot.
>
> Ciao
> Stephan
>
>
I'd say that a sys or proc file is worse than a device node, because the
wanted semantics are exactly those of a device node. Besides, a chown of
a sysfs file is something not friendly to containers. We may need
different uids in different containers to be able to access true random
data.
--
Alexander E. Patrakov
[-- Attachment #2: Криптографическая подпись S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4052 bytes --]
^ permalink raw reply
* Re: [PATCH v25 09/12] LRNG - add Jitter RNG fast noise source
From: Stephan Müller @ 2019-11-20 20:07 UTC (permalink / raw)
To: Neil Horman
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange
In-Reply-To: <20191120133303.GA28341@hmswarspite.think-freely.org>
Am Mittwoch, 20. November 2019, 14:33:03 CET schrieb Neil Horman:
Hi Neil,
> On Sat, Nov 16, 2019 at 10:36:52AM +0100, Stephan Müller wrote:
> > The Jitter RNG fast noise source implemented as part of the kernel
> > crypto API is queried for 256 bits of entropy at the time the seed
> > buffer managed by the LRNG is about to be filled.
> >
> > CC: "Eric W. Biederman" <ebiederm@xmission.com>
> > CC: "Alexander E. Patrakov" <patrakov@gmail.com>
> > CC: "Ahmed S. Darwish" <darwish.07@gmail.com>
> > CC: "Theodore Y. Ts'o" <tytso@mit.edu>
> > CC: Willy Tarreau <w@1wt.eu>
> > CC: Matthew Garrett <mjg59@srcf.ucam.org>
> > CC: Vito Caputo <vcaputo@pengaru.com>
> > CC: Andreas Dilger <adilger.kernel@dilger.ca>
> > CC: Jan Kara <jack@suse.cz>
> > CC: Ray Strode <rstrode@redhat.com>
> > CC: William Jon McCann <mccann@jhu.edu>
> > CC: zhangjs <zachary@baishancloud.com>
> > CC: Andy Lutomirski <luto@kernel.org>
> > CC: Florian Weimer <fweimer@redhat.com>
> > CC: Lennart Poettering <mzxreary@0pointer.de>
> > CC: Nicolai Stange <nstange@suse.de>
> > Reviewed-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > Reviewed-by: Roman Drahtmueller <draht@schaltsekun.de>
> > Tested-by: Roman Drahtmüller <draht@schaltsekun.de>
> > Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > Tested-by: Neil Horman <nhorman@redhat.com>
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> >
> > drivers/char/lrng/Kconfig | 11 +++++
> > drivers/char/lrng/Makefile | 1 +
> > drivers/char/lrng/lrng_jent.c | 88 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 100 insertions(+)
> > create mode 100644 drivers/char/lrng/lrng_jent.c
> >
> > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > index 03e6e2ec356b..80fc723c67d2 100644
> > --- a/drivers/char/lrng/Kconfig
> > +++ b/drivers/char/lrng/Kconfig
> > @@ -80,4 +80,15 @@ config LRNG_KCAPI
> >
> > provided by the selected kernel crypto API RNG.
> >
> > endif # LRNG_DRNG_SWITCH
> >
> > +config LRNG_JENT
> > + bool "Enable Jitter RNG as LRNG Seed Source"
> > + select CRYPTO_JITTERENTROPY
> > + help
> > + The Linux RNG may use the Jitter RNG as noise source. Enabling
> > + this option enables the use of the Jitter RNG. Its default
> > + entropy level is 16 bits of entropy per 256 data bits delivered
> > + by the Jitter RNG. This entropy level can be changed at boot
> > + time or at runtime with the lrng_base.jitterrng configuration
> > + variable.
> > +
> >
> > endif # LRNG
> >
> > diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
> > index 027b6ea51c20..a87d800c9aae 100644
> > --- a/drivers/char/lrng/Makefile
> > +++ b/drivers/char/lrng/Makefile
> > @@ -13,3 +13,4 @@ obj-$(CONFIG_SYSCTL) += lrng_proc.o
> >
> > obj-$(CONFIG_LRNG_DRNG_SWITCH) += lrng_switch.o
> > obj-$(CONFIG_LRNG_DRBG) += lrng_drbg.o
> > obj-$(CONFIG_LRNG_KCAPI) += lrng_kcapi.o
> >
> > +obj-$(CONFIG_LRNG_JENT) += lrng_jent.o
> > diff --git a/drivers/char/lrng/lrng_jent.c b/drivers/char/lrng/lrng_jent.c
> > new file mode 100644
> > index 000000000000..43114a44b8f5
> > --- /dev/null
> > +++ b/drivers/char/lrng/lrng_jent.c
> > @@ -0,0 +1,88 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > +/*
> > + * LRNG Fast Noise Source: Jitter RNG
> > + *
> > + * Copyright (C) 2016 - 2019, Stephan Mueller <smueller@chronox.de>
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include "lrng_internal.h"
> > +
> > +/*
> > + * Estimated entropy of data is a 16th of
> > LRNG_DRNG_SECURITY_STRENGTH_BITS. + * Albeit a full entropy assessment is
> > provided for the noise source indicating + * that it provides high
> > entropy rates and considering that it deactivates + * when it detects
> > insufficient hardware, the chosen under estimation of + * entropy is
> > considered to be acceptable to all reviewers.
> > + */
> > +static u32 jitterrng = LRNG_DRNG_SECURITY_STRENGTH_BITS>>4;
> > +module_param(jitterrng, uint, 0644);
> > +MODULE_PARM_DESC(jitterrng, "Entropy in bits of 256 data bits from Jitter
> > " + "RNG noise source");
> > +
> > +/**
> > + * Get Jitter RNG entropy
> > + *
> > + * @outbuf buffer to store entropy
> > + * @outbuflen length of buffer
> > + * @return > 0 on success where value provides the added entropy in bits
> > + * 0 if no fast source was available
> > + */
> > +struct rand_data;
> > +struct rand_data *jent_lrng_entropy_collector(void);
> > +int jent_read_entropy(struct rand_data *ec, unsigned char *data,
> > + unsigned int len);
> > +static struct rand_data *lrng_jent_state;
> > +
> > +u32 lrng_get_jent(u8 *outbuf, unsigned int outbuflen)
> > +{
> > + int ret;
> > + u32 ent_bits = jitterrng;
> > + unsigned long flags;
> > + static DEFINE_SPINLOCK(lrng_jent_lock);
> > + static int lrng_jent_initialized = 0;
> > +
> > + spin_lock_irqsave(&lrng_jent_lock, flags);
> > +
> > + if (!ent_bits || (lrng_jent_initialized == -1)) {
> > + spin_unlock_irqrestore(&lrng_jent_lock, flags);
> > + return 0;
> > + }
> > +
>
> this works, but I think you can avoid the use of the spin lock on the read
> calls here. If you assign a global pointer to the value of
> &lrng_jent_state on init, you can just take the spinlock on assignment, and
> assume its stable after that (which it should be given that its only ever
> going to point to a static data structure).
It is correct that the lock protects the assignment of the data structure.
But the Jitter RNG itself is not multi-threaded. So, a form of serialization
is needed to also "read" data from the Jitter RNG using one and the same
state.
Granted, there is a serialization in the current code as the
lrng_pool_trylock() is taken before the Jitter RNG is called by
lrng_fill_seed_buffer which effectively serializes all requests to also the
Jitter RNG. But this is coincidence in this case. I would think, however, that
this coincidence could easily lead to programming errors further down the road
when the spinlock is not present and that trylock() is moved to some place
else considering that this trylock() is meant to protect reading the entropy
pool and not the Jitter RNG.
As the reading of the Jitter RNG is always performed in process context, I
think having this additional spin lock against possible programming errors
should not lead to performance regressions.
What do you think?
Thank you for your review!
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v25 10/12] LRNG - add TRNG support
From: Greg Kroah-Hartman @ 2019-11-20 20:32 UTC (permalink / raw)
To: Stephan Müller
Cc: Andy Lutomirski, Arnd Bergmann, Linux Crypto Mailing List, LKML,
Linux API, Eric W. Biederman, Alexander E. Patrakov,
Ahmed S. Darwish, Theodore Y. Ts'o, Willy Tarreau,
Matthew Garrett, Vito Caputo, Andreas Dilger, Jan Kara,
Ray Strode, William Jon McCann, zhangjs, Florian Weimer,
Lennart Poettering, Nicolai Stange
In-Reply-To: <1695782.oZ5Vf4nH9s@positron.chronox.de>
On Wed, Nov 20, 2019 at 08:51:11PM +0100, Stephan Müller wrote:
> Am Mittwoch, 20. November 2019, 14:29:18 CET schrieb Greg Kroah-Hartman:
>
> Hi Greg,
>
> > On Wed, Nov 20, 2019 at 09:58:35AM +0100, Stephan Müller wrote:
> > > Am Dienstag, 19. November 2019, 13:41:50 CET schrieb Greg Kroah-Hartman:
> > >
> > > Hi Greg,
> > >
> > > > On Tue, Nov 19, 2019 at 02:07:40AM -0800, Andy Lutomirski wrote:
> > > > > > As this would introduce a new device file now, is there a special
> > > > > > process that I need to follow or do I need to copy? Which
> > > > > > major/minor
> > > > > > number should I use?
> > > > > >
> > > > > > Looking into static const struct memdev devlist[] I see
> > > > > >
> > > > > > [8] = { "random", 0666, &random_fops, 0 },
> > > > > > [9] = { "urandom", 0666, &urandom_fops, 0 },
> > > > > >
> > > > > > Shall a true_random be added here with [10]?
> > > > >
> > > > > I am not at all an expert on chardevs, but this sounds generally
> > > > > reasonable. gregkh is probably the real authority here.
> > > >
> > > > [10] is the aio char device node, so you better not try to overlap it or
> > > > bad things will happen :(
> > >
> > > Thanks for your insights.
> > >
> > > Which device minor number could we use?
> >
> > Get your own dynamic one by using a misc device if you _REALLY_ want to
> > add yet-another-char-node-for-random-data.
> >
> > But I would have thought that we all realize that this is not the way to
> > do things. Let's not have "random", "urandom", and "true_random" be
> > something we want to totally confuse userspace with, that way is insane.
> >
> > Please just make the existing userspace api "just work", don't add to
> > the mess.
>
> Thank you, I think we should follow that advise.
>
> With that and considering Alexander's rightful remark we have a challenge. So,
> changing the syscall may not be the right way unless we find a way to restrict
> the permissions somehow (capability? LSM? None of that seems to be a good
> fit).
>
> What about providing a /sys file? I.e. adding a file that:
>
> a) has permissions 440 per default and maybe the ownership of root:root
>
> b) allow user space to perform a chown/chgrp
>
> c) only supports reading of data from user space
>
> But then, how could we provide a common interface for the existing random.c
> and the LRNG?
>
> Or should we use a proc file for that? If yes, I guess it should not be a
> sysctl, but a "regular" proc file that should allow a chown(2) operation. On
> the other hand, is proc the right place to provide a user space interface for
> exporting data to user?
No, do not abuse sysfs or procfs for something like this. Use a real
syscall please if you really need it.
greg k-h
^ permalink raw reply
* Re: [PATCH] clone.2: Mention that CLONE_PARENT is off-limits for inits
From: Michael Kerrisk (man-pages) @ 2019-11-21 9:53 UTC (permalink / raw)
To: Christian Brauner
Cc: mtk.manpages, adrian, akpm, arnd, avagin, christian.brauner,
dhowells, fweimer, jannh, keescook, linux-api, linux-kernel,
linux-man, mingo, oleg, xemul
In-Reply-To: <20191120104504.22411-1-christian@brauner.io>
Hello Christian,
On 11/20/19 11:45 AM, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> The CLONE_PARENT flag cannot but used by init processes. Let's mention
> this in the manpages to prevent suprises.
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> man2/clone.2 | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/man2/clone.2 b/man2/clone.2
> index f0f29d6f1..aa98ab79b 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -646,6 +646,13 @@ if
> .B CLONE_PARENT
> is set, then the parent of the calling process, rather than the
> calling process itself, will be signaled.
> +.IP
> +The kernel will not allow global init and init processes in pid
> +namespaces to use the
> +.B CLONE_PARENT
> +flag. This is done to prevent the creation of multi-rooted process
> +trees. It also avoids unreapable zombies in the initial pid
> +namespace.
> .TP
> .BR CLONE_PARENT_SETTID " (since Linux 2.5.49)"
> Store the child thread ID at the location pointed to by
Thank. I applied, and then tweaked the text a little,
and noted the associated EINVAL error. In the end, the
change is as below.
Cheers,
Michael
index 60e746151..382f6b791 100644
--- a/man2/clone.2
+++ b/man2/clone.2
@@ -648,6 +648,14 @@ if
.B CLONE_PARENT
is set, then the parent of the calling process, rather than the
calling process itself, will be signaled.
+.IP
+The
+.B CLONE_PARENT
+flag can't be used in clone calls by the
+global init process (PID 1 in the initial PID namespace)
+and init processes in other PID namespaces.
+This restriction prevents the creation of multi-rooted process trees
+as well as the creation of unreapable zombies in the initial PID namespace.
.TP
.BR CLONE_PARENT_SETTID " (since Linux 2.5.49)"
Store the child thread ID at the location pointed to by
@@ -1273,6 +1281,11 @@ were specified in the
.IR flags
mask.
.TP
+.BR EINVAL " (since Linux 2.6.32)"
+.\" commit 123be07b0b399670a7cc3d82fef0cb4f93ef885c
+.BR CLONE_PARENT
+was specified, and the caller is an init process.
+.TP
.B EINVAL
Returned by the glibc
.BR clone ()
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply related
* [PATCH] block: add iostat counters for flush requests
From: Konstantin Khlebnikov @ 2019-11-21 10:40 UTC (permalink / raw)
To: linux-block, Jens Axboe, linux-kernel, linux-doc
Cc: linux-fsdevel, linux-api, Dmitry Monakhov
Requests that triggers flushing volatile writeback cache to disk (barriers)
have significant effect to overall performance.
Block layer has sophisticated engine for combining several flush requests
into one. But there is no statistics for actual flushes executed by disk.
Requests which trigger flushes usually are barriers - zero-size writes.
This patch adds two iostat counters into /sys/class/block/$dev/stat and
/proc/diskstats - count of completed flush requests and their total time.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
Documentation/ABI/testing/procfs-diskstats | 5 +++++
Documentation/ABI/testing/sysfs-block | 6 ++++++
Documentation/admin-guide/iostats.rst | 9 +++++++++
Documentation/block/stat.rst | 14 ++++++++++++--
block/blk-flush.c | 15 ++++++++++++++-
block/genhd.c | 8 ++++++--
block/partition-generic.c | 7 +++++--
include/linux/blk_types.h | 1 +
8 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/Documentation/ABI/testing/procfs-diskstats b/Documentation/ABI/testing/procfs-diskstats
index 2c44b4f1b060..70dcaf2481f4 100644
--- a/Documentation/ABI/testing/procfs-diskstats
+++ b/Documentation/ABI/testing/procfs-diskstats
@@ -29,4 +29,9 @@ Description:
17 - sectors discarded
18 - time spent discarding
+ Kernel 5.5+ appends two more fields for flush requests:
+
+ 19 - flush requests completed successfully
+ 20 - time spent flushing
+
For more details refer to Documentation/admin-guide/iostats.rst
diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index f8c7c7126bb1..ed8c14f161ee 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -15,6 +15,12 @@ Description:
9 - I/Os currently in progress
10 - time spent doing I/Os (ms)
11 - weighted time spent doing I/Os (ms)
+ 12 - discards completed
+ 13 - discards merged
+ 14 - sectors discarded
+ 15 - time spent discarding (ms)
+ 16 - flush requests completed
+ 17 - time spent flushing (ms)
For more details refer Documentation/admin-guide/iostats.rst
diff --git a/Documentation/admin-guide/iostats.rst b/Documentation/admin-guide/iostats.rst
index 5d63b18bd6d1..4f0462af3ca7 100644
--- a/Documentation/admin-guide/iostats.rst
+++ b/Documentation/admin-guide/iostats.rst
@@ -121,6 +121,15 @@ Field 15 -- # of milliseconds spent discarding
This is the total number of milliseconds spent by all discards (as
measured from __make_request() to end_that_request_last()).
+Field 16 -- # of flush requests completed
+ This is the total number of flush requests completed successfully.
+
+ Block layer combines flush requests and executes at most one at a time.
+ This counts flush requests executed by disk. Not tracked for partitions.
+
+Field 17 -- # of milliseconds spent flushing
+ This is the total number of milliseconds spent by all flush requests.
+
To avoid introducing performance bottlenecks, no locks are held while
modifying these counters. This implies that minor inaccuracies may be
introduced when changes collide, so (for instance) adding up all the
diff --git a/Documentation/block/stat.rst b/Documentation/block/stat.rst
index 9c07bc22b0bc..77311335c08b 100644
--- a/Documentation/block/stat.rst
+++ b/Documentation/block/stat.rst
@@ -41,6 +41,8 @@ discard I/Os requests number of discard I/Os processed
discard merges requests number of discard I/Os merged with in-queue I/O
discard sectors sectors number of sectors discarded
discard ticks milliseconds total wait time for discard requests
+flush I/Os requests number of flush I/Os processed
+flush ticks milliseconds total wait time for flush requests
=============== ============= =================================================
read I/Os, write I/Os, discard I/0s
@@ -48,6 +50,14 @@ read I/Os, write I/Os, discard I/0s
These values increment when an I/O request completes.
+flush I/Os
+==========
+
+These values increment when an flush I/O request completes.
+
+Block layer combines flush requests and executes at most one at a time.
+This counts flush requests executed by disk. Not tracked for partitions.
+
read merges, write merges, discard merges
=========================================
@@ -62,8 +72,8 @@ discarded from this block device. The "sectors" in question are the
standard UNIX 512-byte sectors, not any device- or filesystem-specific
block size. The counters are incremented when the I/O completes.
-read ticks, write ticks, discard ticks
-======================================
+read ticks, write ticks, discard ticks, flush ticks
+===================================================
These values count the number of milliseconds that I/O requests have
waited on this block device. If there are multiple I/O requests waiting,
diff --git a/block/blk-flush.c b/block/blk-flush.c
index 1eec9cbe5a0a..1777346baf06 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -136,6 +136,17 @@ static void blk_flush_queue_rq(struct request *rq, bool add_front)
blk_mq_add_to_requeue_list(rq, add_front, true);
}
+static void blk_account_io_flush(struct request *rq)
+{
+ struct hd_struct *part = &rq->rq_disk->part0;
+
+ part_stat_lock();
+ part_stat_inc(part, ios[STAT_FLUSH]);
+ part_stat_add(part, nsecs[STAT_FLUSH],
+ ktime_get_ns() - rq->start_time_ns);
+ part_stat_unlock();
+}
+
/**
* blk_flush_complete_seq - complete flush sequence
* @rq: PREFLUSH/FUA request being sequenced
@@ -185,7 +196,7 @@ static void blk_flush_complete_seq(struct request *rq,
case REQ_FSEQ_DONE:
/*
- * @rq was previously adjusted by blk_flush_issue() for
+ * @rq was previously adjusted by blk_insert_flush() for
* flush sequencing and may already have gone through the
* flush data request completion path. Restore @rq for
* normal completion and end it.
@@ -212,6 +223,8 @@ static void flush_end_io(struct request *flush_rq, blk_status_t error)
struct blk_flush_queue *fq = blk_get_flush_queue(q, flush_rq->mq_ctx);
struct blk_mq_hw_ctx *hctx;
+ blk_account_io_flush(flush_rq);
+
/* release the tag's ownership to the req cloned from */
spin_lock_irqsave(&fq->mq_flush_lock, flags);
diff --git a/block/genhd.c b/block/genhd.c
index 26b31fcae217..ff6268970ddc 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1385,7 +1385,9 @@ static int diskstats_show(struct seq_file *seqf, void *v)
"%lu %lu %lu %u "
"%lu %lu %lu %u "
"%u %u %u "
- "%lu %lu %lu %u\n",
+ "%lu %lu %lu %u "
+ "%lu %u"
+ "\n",
MAJOR(part_devt(hd)), MINOR(part_devt(hd)),
disk_name(gp, hd->partno, buf),
part_stat_read(hd, ios[STAT_READ]),
@@ -1402,7 +1404,9 @@ static int diskstats_show(struct seq_file *seqf, void *v)
part_stat_read(hd, ios[STAT_DISCARD]),
part_stat_read(hd, merges[STAT_DISCARD]),
part_stat_read(hd, sectors[STAT_DISCARD]),
- (unsigned int)part_stat_read_msecs(hd, STAT_DISCARD)
+ (unsigned int)part_stat_read_msecs(hd, STAT_DISCARD),
+ part_stat_read(hd, ios[STAT_FLUSH]),
+ (unsigned int)part_stat_read_msecs(hd, STAT_FLUSH)
);
}
disk_part_iter_exit(&piter);
diff --git a/block/partition-generic.c b/block/partition-generic.c
index aee643ce13d1..3db8b73a96b1 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -127,7 +127,8 @@ ssize_t part_stat_show(struct device *dev,
"%8lu %8lu %8llu %8u "
"%8lu %8lu %8llu %8u "
"%8u %8u %8u "
- "%8lu %8lu %8llu %8u"
+ "%8lu %8lu %8llu %8u "
+ "%8lu %8u"
"\n",
part_stat_read(p, ios[STAT_READ]),
part_stat_read(p, merges[STAT_READ]),
@@ -143,7 +144,9 @@ ssize_t part_stat_show(struct device *dev,
part_stat_read(p, ios[STAT_DISCARD]),
part_stat_read(p, merges[STAT_DISCARD]),
(unsigned long long)part_stat_read(p, sectors[STAT_DISCARD]),
- (unsigned int)part_stat_read_msecs(p, STAT_DISCARD));
+ (unsigned int)part_stat_read_msecs(p, STAT_DISCARD),
+ part_stat_read(p, ios[STAT_FLUSH]),
+ (unsigned int)part_stat_read_msecs(p, STAT_FLUSH));
}
ssize_t part_inflight_show(struct device *dev, struct device_attribute *attr,
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index d688b96d1d63..b811a673a300 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -371,6 +371,7 @@ enum stat_group {
STAT_READ,
STAT_WRITE,
STAT_DISCARD,
+ STAT_FLUSH,
NR_STAT_GROUPS
};
^ permalink raw reply related
* [PATCH] clone.2: Fix typo
From: Christian Brauner @ 2019-11-21 11:06 UTC (permalink / raw)
To: mtk.manpages
Cc: adrian, akpm, arnd, avagin, christian.brauner, dhowells, fweimer,
jannh, keescook, linux-api, linux-kernel, linux-man, mingo, oleg,
xemul
From: Christian Brauner <christian.brauner@ubuntu.com>
This surely meant to say clone3() and not clone(3).
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
man2/clone.2 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man2/clone.2 b/man2/clone.2
index 382f6b791..e5ab2a096 100644
--- a/man2/clone.2
+++ b/man2/clone.2
@@ -252,7 +252,7 @@ argument supplied to
lb lb lb
l l l
li li l.
-clone() clone(3) Notes
+clone() clone3() Notes
\fIcl_args\fP field
flags & ~0xff flags For most flags; details below
parent_tid pidfd See CLONE_PIDFD
base-commit: be479fdf027a3288e88d53dbe05ba76bf202776e
--
2.24.0
^ permalink raw reply related
* Re: [PATCH] clone.2: Fix typo
From: Michael Kerrisk (man-pages) @ 2019-11-21 11:25 UTC (permalink / raw)
To: Christian Brauner
Cc: mtk.manpages, adrian, akpm, arnd, avagin, christian.brauner,
dhowells, fweimer, jannh, keescook, linux-api, linux-kernel,
linux-man, mingo, oleg, xemul
In-Reply-To: <20191121110646.1398-1-christian@brauner.io>
Hi Christian,
On 11/21/19 12:06 PM, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> This surely meant to say clone3() and not clone(3).
Thanks. Patch applied.
Cheers,
Michael
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> man2/clone.2 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man2/clone.2 b/man2/clone.2
> index 382f6b791..e5ab2a096 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -252,7 +252,7 @@ argument supplied to
> lb lb lb
> l l l
> li li l.
> -clone() clone(3) Notes
> +clone() clone3() Notes
> \fIcl_args\fP field
> flags & ~0xff flags For most flags; details below
> parent_tid pidfd See CLONE_PIDFD
>
> base-commit: be479fdf027a3288e88d53dbe05ba76bf202776e
>
Hi Christian,,
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH] clone.2: Mention that CLONE_PARENT is off-limits for inits
From: Christian Brauner @ 2019-11-21 11:27 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: Christian Brauner, adrian, akpm, arnd, avagin, dhowells, fweimer,
jannh, keescook, linux-api, linux-kernel, linux-man, mingo, oleg,
xemul
In-Reply-To: <ac6c1644-c6d3-c7eb-48b1-28eb9342a468@gmail.com>
On Thu, Nov 21, 2019 at 10:53:50AM +0100, Michael Kerrisk (man-pages) wrote:
> Hello Christian,
>
> On 11/20/19 11:45 AM, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> >
> > The CLONE_PARENT flag cannot but used by init processes. Let's mention
> > this in the manpages to prevent suprises.
> >
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > ---
> > man2/clone.2 | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/man2/clone.2 b/man2/clone.2
> > index f0f29d6f1..aa98ab79b 100644
> > --- a/man2/clone.2
> > +++ b/man2/clone.2
> > @@ -646,6 +646,13 @@ if
> > .B CLONE_PARENT
> > is set, then the parent of the calling process, rather than the
> > calling process itself, will be signaled.
> > +.IP
> > +The kernel will not allow global init and init processes in pid
> > +namespaces to use the
> > +.B CLONE_PARENT
> > +flag. This is done to prevent the creation of multi-rooted process
> > +trees. It also avoids unreapable zombies in the initial pid
> > +namespace.
> > .TP
> > .BR CLONE_PARENT_SETTID " (since Linux 2.5.49)"
> > Store the child thread ID at the location pointed to by
>
> Thank. I applied, and then tweaked the text a little,
> and noted the associated EINVAL error. In the end, the
> change is as below.
Thanks!
Christian
^ permalink raw reply
* Re: [PATCH v25 12/12] LRNG - add interface for gathering of raw entropy
From: Nicolai Stange @ 2019-11-21 12:18 UTC (permalink / raw)
To: Stephan Müller
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange
In-Reply-To: <3610406.x8mDjznOIz@positron.chronox.de>
Hi Stephan,
two general remarks on debugfs usage below
Stephan Müller <smueller@chronox.de> writes:
> diff --git a/drivers/char/lrng/lrng_testing.c b/drivers/char/lrng/lrng_testing.c
> new file mode 100644
> index 000000000000..5c33d3bd2172
> --- /dev/null
> +++ b/drivers/char/lrng/lrng_testing.c
<snip>
> +/*
> + * This data structure holds the dentry's of the debugfs files establishing
> + * the interface to user space.
> + */
> +struct lrng_raw_debugfs {
> + struct dentry *lrng_raw_debugfs_root; /* root dentry */
> + struct dentry *lrng_raw_debugfs_lrng_raw; /* .../lrng_raw */
> +};
> +
> +static struct lrng_raw_debugfs lrng_raw_debugfs;
> +
> +/* DebugFS operations and definition of the debugfs files */
> +static ssize_t lrng_raw_read(struct file *file, char __user *to,
> + size_t count, loff_t *ppos)
> +{
> + loff_t pos = *ppos;
> + int ret;
> +
> + if (!count)
> + return 0;
> + lrng_raw_entropy_init();
> + ret = lrng_raw_extract_user(to, count);
> + lrng_raw_entropy_fini();
> + if (ret < 0)
> + return ret;
> + count -= ret;
> + *ppos = pos + count;
> + return ret;
> +}
> +
> +/* Module init: allocate memory, register the debugfs files */
> +static int lrng_raw_debugfs_init(void)
> +{
> + lrng_raw_debugfs.lrng_raw_debugfs_root =
> + debugfs_create_dir(KBUILD_MODNAME, NULL);
> + if (IS_ERR(lrng_raw_debugfs.lrng_raw_debugfs_root)) {
> + lrng_raw_debugfs.lrng_raw_debugfs_root = NULL;
> + return PTR_ERR(lrng_raw_debugfs.lrng_raw_debugfs_root);
> + }
I think pointers returned by the debugfs API are not supposed to get
checked for NULL/IS_ERR(), c.f commit ff9fb72bc077 ("debugfs: return
error values, not NULL") or the the output from
git log --pretty=oneline | grep 'no need to check return value of debugfs_create'
(Also the above code is dubious: you're effectively returning
PTR_ERR(NULL)).
> + return 0;
> +}
> +
> +static struct file_operations lrng_raw_name_fops = {
> + .owner = THIS_MODULE,
> + .read = lrng_raw_read,
> +};
> +
> +static int lrng_raw_debugfs_init_name(void)
> +{
> + lrng_raw_debugfs.lrng_raw_debugfs_lrng_raw =
> + debugfs_create_file("lrng_raw", 0400,
> + lrng_raw_debugfs.lrng_raw_debugfs_root,
> + NULL, &lrng_raw_name_fops);q
CONFIG_LRNG_TESTING is a bool and thus, this debugfs file can't ever get
removed. Even if it could, this inode hasn't got any data associated
with it and so file removal would not be a problem for lrng_raw_read().
Please consider using debugfs_create_file_unsafe() instead to save
debugfs from kmalloc()ing a proxy file_operations protecting your fops
against concurrent file removal.
> + if (IS_ERR(lrng_raw_debugfs.lrng_raw_debugfs_lrng_raw)) {
> + lrng_raw_debugfs.lrng_raw_debugfs_lrng_raw = NULL;
> + return PTR_ERR(lrng_raw_debugfs.lrng_raw_debugfs_lrng_raw);
> + }
Same comment regarding return value checking applies here.
Thanks,
Nicolai
> + return 0;
> +}
> +
> +static int __init lrng_raw_init(void)
> +{
> + int ret = lrng_raw_debugfs_init();
> +
> + if (ret < 0)
> + return ret;
> +
> + ret = lrng_raw_debugfs_init_name();
> + if (ret < 0)
> + debugfs_remove_recursive(
> + lrng_raw_debugfs.lrng_raw_debugfs_root);
> +
> + return ret;
> +}
> +
> +static void __exit lrng_raw_exit(void)
> +{
> + debugfs_remove_recursive(lrng_raw_debugfs.lrng_raw_debugfs_root);
> +}
> +
> +module_init(lrng_raw_init);
> +module_exit(lrng_raw_exit);
> +
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
> +MODULE_DESCRIPTION("Kernel module for gathering raw entropy");
--
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply
* Re: [PATCH v25 10/12] LRNG - add TRNG support
From: Stephan Müller @ 2019-11-21 13:06 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andy Lutomirski, Arnd Bergmann, Linux Crypto Mailing List, LKML,
Linux API, Eric W. Biederman, Alexander E. Patrakov,
Ahmed S. Darwish, Theodore Y. Ts'o, Willy Tarreau,
Matthew Garrett, Vito Caputo, Andreas Dilger, Jan Kara,
Ray Strode, William Jon McCann, zhangjs, Florian Weimer,
Lennart Poettering, Nicolai Stange
In-Reply-To: <20191120203232.GB3109949@kroah.com>
Am Mittwoch, 20. November 2019, 21:32:32 CET schrieb Greg Kroah-Hartman:
Hi Greg,
> No, do not abuse sysfs or procfs for something like this. Use a real
> syscall please if you really need it.
You are right.
Ok, let us get back to the drawing board. What are our requirements? We need
to have an interface for the TRNG that should ensure other users of entropy
are not starved by unprivileged users.
What about the following: we use the getrandom(2) system call and add
GRND_TRUERANDOM as already indicated. However, there is one more caveat we
would add:
- if the caller of GRND_TRUERANDOM is !CAP_SYS_ADMIN the entropy pool can only
be depleted to the point where at least one or two full seeding operations
worth of entropy is left.
- if the caller of GRND_TRUERANDOM is CAP_SYS_ADMIN, the entropy can be
depleted completely
At runtime, the LRNG would then behave like the following:
- calling getrandom(..., 0), /dev/random or /dev/urandom would deplete the
entropy pool during reseeding operations but leaving an emergency level of 512
bits of entropy in the pool. If equal or less are in the pool, reseeding would
be skipped.
- calling getrandom(..., GRND_TRUERANDOM) with CAP_SYS_ADMIN allows the
entropy pool to be fully depleted.
- calling getrandom(..., GRND_TRUERANDOM) without CAP_SYS_ADMIN allows the
entropy pool to be depleted down to 1024 bits of entropy. If the pool has
equal or less, the caller is blocked. This allows the DRNG feeding /dev/
random, /dev/urandom or getrandom(..., 0) with 512 bits of entropy (i.e. two
reseed operations are possible). Only if the entropy pool has more than 1024
bits of entropy, the getrandom call would unblock and provide data.
With that approach, I think we can honor the request from Greg to not add any
new interface and yet honor the note from Alexander to not allow unprivileged
user space to deplete the entropy pool to the extent that other users of
entropy are too much affected.
If GRND_TRUERANDOM is not implemented, EOPNOTSUPP is returned.
Thank you.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v25 09/12] LRNG - add Jitter RNG fast noise source
From: Neil Horman @ 2019-11-21 14:19 UTC (permalink / raw)
To: Stephan Müller
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange
In-Reply-To: <1844272.AK0ElEJLVa@positron.chronox.de>
On Wed, Nov 20, 2019 at 09:07:13PM +0100, Stephan Müller wrote:
> Am Mittwoch, 20. November 2019, 14:33:03 CET schrieb Neil Horman:
>
> Hi Neil,
>
> > On Sat, Nov 16, 2019 at 10:36:52AM +0100, Stephan Müller wrote:
> > > The Jitter RNG fast noise source implemented as part of the kernel
> > > crypto API is queried for 256 bits of entropy at the time the seed
> > > buffer managed by the LRNG is about to be filled.
> > >
> > > CC: "Eric W. Biederman" <ebiederm@xmission.com>
> > > CC: "Alexander E. Patrakov" <patrakov@gmail.com>
> > > CC: "Ahmed S. Darwish" <darwish.07@gmail.com>
> > > CC: "Theodore Y. Ts'o" <tytso@mit.edu>
> > > CC: Willy Tarreau <w@1wt.eu>
> > > CC: Matthew Garrett <mjg59@srcf.ucam.org>
> > > CC: Vito Caputo <vcaputo@pengaru.com>
> > > CC: Andreas Dilger <adilger.kernel@dilger.ca>
> > > CC: Jan Kara <jack@suse.cz>
> > > CC: Ray Strode <rstrode@redhat.com>
> > > CC: William Jon McCann <mccann@jhu.edu>
> > > CC: zhangjs <zachary@baishancloud.com>
> > > CC: Andy Lutomirski <luto@kernel.org>
> > > CC: Florian Weimer <fweimer@redhat.com>
> > > CC: Lennart Poettering <mzxreary@0pointer.de>
> > > CC: Nicolai Stange <nstange@suse.de>
> > > Reviewed-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > > Reviewed-by: Roman Drahtmueller <draht@schaltsekun.de>
> > > Tested-by: Roman Drahtmüller <draht@schaltsekun.de>
> > > Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > > Tested-by: Neil Horman <nhorman@redhat.com>
> > > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > > ---
> > >
> > > drivers/char/lrng/Kconfig | 11 +++++
> > > drivers/char/lrng/Makefile | 1 +
> > > drivers/char/lrng/lrng_jent.c | 88 +++++++++++++++++++++++++++++++++++
> > > 3 files changed, 100 insertions(+)
> > > create mode 100644 drivers/char/lrng/lrng_jent.c
> > >
> > > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > > index 03e6e2ec356b..80fc723c67d2 100644
> > > --- a/drivers/char/lrng/Kconfig
> > > +++ b/drivers/char/lrng/Kconfig
> > > @@ -80,4 +80,15 @@ config LRNG_KCAPI
> > >
> > > provided by the selected kernel crypto API RNG.
> > >
> > > endif # LRNG_DRNG_SWITCH
> > >
> > > +config LRNG_JENT
> > > + bool "Enable Jitter RNG as LRNG Seed Source"
> > > + select CRYPTO_JITTERENTROPY
> > > + help
> > > + The Linux RNG may use the Jitter RNG as noise source. Enabling
> > > + this option enables the use of the Jitter RNG. Its default
> > > + entropy level is 16 bits of entropy per 256 data bits delivered
> > > + by the Jitter RNG. This entropy level can be changed at boot
> > > + time or at runtime with the lrng_base.jitterrng configuration
> > > + variable.
> > > +
> > >
> > > endif # LRNG
> > >
> > > diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
> > > index 027b6ea51c20..a87d800c9aae 100644
> > > --- a/drivers/char/lrng/Makefile
> > > +++ b/drivers/char/lrng/Makefile
> > > @@ -13,3 +13,4 @@ obj-$(CONFIG_SYSCTL) += lrng_proc.o
> > >
> > > obj-$(CONFIG_LRNG_DRNG_SWITCH) += lrng_switch.o
> > > obj-$(CONFIG_LRNG_DRBG) += lrng_drbg.o
> > > obj-$(CONFIG_LRNG_KCAPI) += lrng_kcapi.o
> > >
> > > +obj-$(CONFIG_LRNG_JENT) += lrng_jent.o
> > > diff --git a/drivers/char/lrng/lrng_jent.c b/drivers/char/lrng/lrng_jent.c
> > > new file mode 100644
> > > index 000000000000..43114a44b8f5
> > > --- /dev/null
> > > +++ b/drivers/char/lrng/lrng_jent.c
> > > @@ -0,0 +1,88 @@
> > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > > +/*
> > > + * LRNG Fast Noise Source: Jitter RNG
> > > + *
> > > + * Copyright (C) 2016 - 2019, Stephan Mueller <smueller@chronox.de>
> > > + */
> > > +
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > +#include "lrng_internal.h"
> > > +
> > > +/*
> > > + * Estimated entropy of data is a 16th of
> > > LRNG_DRNG_SECURITY_STRENGTH_BITS. + * Albeit a full entropy assessment is
> > > provided for the noise source indicating + * that it provides high
> > > entropy rates and considering that it deactivates + * when it detects
> > > insufficient hardware, the chosen under estimation of + * entropy is
> > > considered to be acceptable to all reviewers.
> > > + */
> > > +static u32 jitterrng = LRNG_DRNG_SECURITY_STRENGTH_BITS>>4;
> > > +module_param(jitterrng, uint, 0644);
> > > +MODULE_PARM_DESC(jitterrng, "Entropy in bits of 256 data bits from Jitter
> > > " + "RNG noise source");
> > > +
> > > +/**
> > > + * Get Jitter RNG entropy
> > > + *
> > > + * @outbuf buffer to store entropy
> > > + * @outbuflen length of buffer
> > > + * @return > 0 on success where value provides the added entropy in bits
> > > + * 0 if no fast source was available
> > > + */
> > > +struct rand_data;
> > > +struct rand_data *jent_lrng_entropy_collector(void);
> > > +int jent_read_entropy(struct rand_data *ec, unsigned char *data,
> > > + unsigned int len);
> > > +static struct rand_data *lrng_jent_state;
> > > +
> > > +u32 lrng_get_jent(u8 *outbuf, unsigned int outbuflen)
> > > +{
> > > + int ret;
> > > + u32 ent_bits = jitterrng;
> > > + unsigned long flags;
> > > + static DEFINE_SPINLOCK(lrng_jent_lock);
> > > + static int lrng_jent_initialized = 0;
> > > +
> > > + spin_lock_irqsave(&lrng_jent_lock, flags);
> > > +
> > > + if (!ent_bits || (lrng_jent_initialized == -1)) {
> > > + spin_unlock_irqrestore(&lrng_jent_lock, flags);
> > > + return 0;
> > > + }
> > > +
> >
> > this works, but I think you can avoid the use of the spin lock on the read
> > calls here. If you assign a global pointer to the value of
> > &lrng_jent_state on init, you can just take the spinlock on assignment, and
> > assume its stable after that (which it should be given that its only ever
> > going to point to a static data structure).
>
> It is correct that the lock protects the assignment of the data structure.
>
> But the Jitter RNG itself is not multi-threaded. So, a form of serialization
> is needed to also "read" data from the Jitter RNG using one and the same
> state.
>
> Granted, there is a serialization in the current code as the
> lrng_pool_trylock() is taken before the Jitter RNG is called by
> lrng_fill_seed_buffer which effectively serializes all requests to also the
> Jitter RNG. But this is coincidence in this case. I would think, however, that
> this coincidence could easily lead to programming errors further down the road
> when the spinlock is not present and that trylock() is moved to some place
> else considering that this trylock() is meant to protect reading the entropy
> pool and not the Jitter RNG.
>
> As the reading of the Jitter RNG is always performed in process context, I
> think having this additional spin lock against possible programming errors
> should not lead to performance regressions.
>
> What do you think?
>
I take your meaning that each random device needs protection, and yes, each of
the random devices (trng and sdrng) have their own locking. But it also appears
to me that each of those random devices contains its own private copy of the
entropy_buf (they're statically declared on the stack in lrng_trng_seed and
_lrng_sdrng_seed), so while the additional locking doesn't necessecarily hurt,
I'm struggling to see why the additional work is needed. If ever you have a
situation in which multiple rngs want want to share an entropy buffer, yes, you
would need that lock, or some other protection, but I don't see the need
immediately.
Neil
> Thank you for your review!
>
> Ciao
> Stephan
>
>
^ permalink raw reply
* Re: [PATCH v25 09/12] LRNG - add Jitter RNG fast noise source
From: Stephan Mueller @ 2019-11-21 14:33 UTC (permalink / raw)
To: Neil Horman
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange
In-Reply-To: <20191121141930.GB27405@hmswarspite.think-freely.org>
Am Donnerstag, 21. November 2019, 15:19:30 CET schrieb Neil Horman:
Hi Neil,
> On Wed, Nov 20, 2019 at 09:07:13PM +0100, Stephan Müller wrote:
> > Am Mittwoch, 20. November 2019, 14:33:03 CET schrieb Neil Horman:
> >
> > Hi Neil,
> >
> > > On Sat, Nov 16, 2019 at 10:36:52AM +0100, Stephan Müller wrote:
> > > > The Jitter RNG fast noise source implemented as part of the kernel
> > > > crypto API is queried for 256 bits of entropy at the time the seed
> > > > buffer managed by the LRNG is about to be filled.
> > > >
> > > > CC: "Eric W. Biederman" <ebiederm@xmission.com>
> > > > CC: "Alexander E. Patrakov" <patrakov@gmail.com>
> > > > CC: "Ahmed S. Darwish" <darwish.07@gmail.com>
> > > > CC: "Theodore Y. Ts'o" <tytso@mit.edu>
> > > > CC: Willy Tarreau <w@1wt.eu>
> > > > CC: Matthew Garrett <mjg59@srcf.ucam.org>
> > > > CC: Vito Caputo <vcaputo@pengaru.com>
> > > > CC: Andreas Dilger <adilger.kernel@dilger.ca>
> > > > CC: Jan Kara <jack@suse.cz>
> > > > CC: Ray Strode <rstrode@redhat.com>
> > > > CC: William Jon McCann <mccann@jhu.edu>
> > > > CC: zhangjs <zachary@baishancloud.com>
> > > > CC: Andy Lutomirski <luto@kernel.org>
> > > > CC: Florian Weimer <fweimer@redhat.com>
> > > > CC: Lennart Poettering <mzxreary@0pointer.de>
> > > > CC: Nicolai Stange <nstange@suse.de>
> > > > Reviewed-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > > > Reviewed-by: Roman Drahtmueller <draht@schaltsekun.de>
> > > > Tested-by: Roman Drahtmüller <draht@schaltsekun.de>
> > > > Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > > > Tested-by: Neil Horman <nhorman@redhat.com>
> > > > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > > > ---
> > > >
> > > > drivers/char/lrng/Kconfig | 11 +++++
> > > > drivers/char/lrng/Makefile | 1 +
> > > > drivers/char/lrng/lrng_jent.c | 88
> > > > +++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 100 insertions(+)
> > > > create mode 100644 drivers/char/lrng/lrng_jent.c
> > > >
> > > > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > > > index 03e6e2ec356b..80fc723c67d2 100644
> > > > --- a/drivers/char/lrng/Kconfig
> > > > +++ b/drivers/char/lrng/Kconfig
> > > > @@ -80,4 +80,15 @@ config LRNG_KCAPI
> > > >
> > > > provided by the selected kernel crypto API RNG.
> > > >
> > > > endif # LRNG_DRNG_SWITCH
> > > >
> > > > +config LRNG_JENT
> > > > + bool "Enable Jitter RNG as LRNG Seed Source"
> > > > + select CRYPTO_JITTERENTROPY
> > > > + help
> > > > + The Linux RNG may use the Jitter RNG as noise source.
Enabling
> > > > + this option enables the use of the Jitter RNG. Its default
> > > > + entropy level is 16 bits of entropy per 256 data bits
delivered
> > > > + by the Jitter RNG. This entropy level can be changed at
boot
> > > > + time or at runtime with the lrng_base.jitterrng
configuration
> > > > + variable.
> > > > +
> > > >
> > > > endif # LRNG
> > > >
> > > > diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
> > > > index 027b6ea51c20..a87d800c9aae 100644
> > > > --- a/drivers/char/lrng/Makefile
> > > > +++ b/drivers/char/lrng/Makefile
> > > > @@ -13,3 +13,4 @@ obj-$(CONFIG_SYSCTL) += lrng_proc.o
> > > >
> > > > obj-$(CONFIG_LRNG_DRNG_SWITCH) += lrng_switch.o
> > > > obj-$(CONFIG_LRNG_DRBG) += lrng_drbg.o
> > > > obj-$(CONFIG_LRNG_KCAPI) += lrng_kcapi.o
> > > >
> > > > +obj-$(CONFIG_LRNG_JENT) += lrng_jent.o
> > > > diff --git a/drivers/char/lrng/lrng_jent.c
> > > > b/drivers/char/lrng/lrng_jent.c
> > > > new file mode 100644
> > > > index 000000000000..43114a44b8f5
> > > > --- /dev/null
> > > > +++ b/drivers/char/lrng/lrng_jent.c
> > > > @@ -0,0 +1,88 @@
> > > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > > > +/*
> > > > + * LRNG Fast Noise Source: Jitter RNG
> > > > + *
> > > > + * Copyright (C) 2016 - 2019, Stephan Mueller <smueller@chronox.de>
> > > > + */
> > > > +
> > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > +
> > > > +#include "lrng_internal.h"
> > > > +
> > > > +/*
> > > > + * Estimated entropy of data is a 16th of
> > > > LRNG_DRNG_SECURITY_STRENGTH_BITS. + * Albeit a full entropy assessment
> > > > is
> > > > provided for the noise source indicating + * that it provides high
> > > > entropy rates and considering that it deactivates + * when it detects
> > > > insufficient hardware, the chosen under estimation of + * entropy is
> > > > considered to be acceptable to all reviewers.
> > > > + */
> > > > +static u32 jitterrng = LRNG_DRNG_SECURITY_STRENGTH_BITS>>4;
> > > > +module_param(jitterrng, uint, 0644);
> > > > +MODULE_PARM_DESC(jitterrng, "Entropy in bits of 256 data bits from
> > > > Jitter
> > > > " + "RNG noise source");
> > > > +
> > > > +/**
> > > > + * Get Jitter RNG entropy
> > > > + *
> > > > + * @outbuf buffer to store entropy
> > > > + * @outbuflen length of buffer
> > > > + * @return > 0 on success where value provides the added entropy in
> > > > bits
> > > > + * 0 if no fast source was available
> > > > + */
> > > > +struct rand_data;
> > > > +struct rand_data *jent_lrng_entropy_collector(void);
> > > > +int jent_read_entropy(struct rand_data *ec, unsigned char *data,
> > > > + unsigned int len);
> > > > +static struct rand_data *lrng_jent_state;
> > > > +
> > > > +u32 lrng_get_jent(u8 *outbuf, unsigned int outbuflen)
> > > > +{
> > > > + int ret;
> > > > + u32 ent_bits = jitterrng;
> > > > + unsigned long flags;
> > > > + static DEFINE_SPINLOCK(lrng_jent_lock);
> > > > + static int lrng_jent_initialized = 0;
> > > > +
> > > > + spin_lock_irqsave(&lrng_jent_lock, flags);
> > > > +
> > > > + if (!ent_bits || (lrng_jent_initialized == -1)) {
> > > > + spin_unlock_irqrestore(&lrng_jent_lock, flags);
> > > > + return 0;
> > > > + }
> > > > +
> > >
> > > this works, but I think you can avoid the use of the spin lock on the
> > > read
> > > calls here. If you assign a global pointer to the value of
> > > &lrng_jent_state on init, you can just take the spinlock on assignment,
> > > and
> > > assume its stable after that (which it should be given that its only
> > > ever
> > > going to point to a static data structure).
> >
> > It is correct that the lock protects the assignment of the data structure.
> >
> > But the Jitter RNG itself is not multi-threaded. So, a form of
> > serialization is needed to also "read" data from the Jitter RNG using one
> > and the same state.
> >
> > Granted, there is a serialization in the current code as the
> > lrng_pool_trylock() is taken before the Jitter RNG is called by
> > lrng_fill_seed_buffer which effectively serializes all requests to also
> > the
> > Jitter RNG. But this is coincidence in this case. I would think, however,
> > that this coincidence could easily lead to programming errors further
> > down the road when the spinlock is not present and that trylock() is
> > moved to some place else considering that this trylock() is meant to
> > protect reading the entropy pool and not the Jitter RNG.
> >
> > As the reading of the Jitter RNG is always performed in process context, I
> > think having this additional spin lock against possible programming errors
> > should not lead to performance regressions.
> >
> > What do you think?
>
> I take your meaning that each random device needs protection, and yes, each
> of the random devices (trng and sdrng) have their own locking. But it also
> appears to me that each of those random devices contains its own private
> copy of the entropy_buf (they're statically declared on the stack in
> lrng_trng_seed and _lrng_sdrng_seed), so while the additional locking
> doesn't necessecarily hurt, I'm struggling to see why the additional work
> is needed.
This lock around the Jitter RNG is needed if:
1) TRNG is not configured
2) NUMA is configured, and
3) we have more than one NUMA node.
In this case it is possible that the lrng_fill_seed_buf is called in parallel
by parallel executions of the secondary DRNG seeding operation on the
different NUMA nodes.
In this case it is possible that the one global Jitter RNG state is used to
request random numbers in parallel. This scenario requires the lock.
Thank you.
> If ever you have a situation in which multiple rngs want want
> to share an entropy buffer, yes, you would need that lock, or some other
> protection, but I don't see the need immediately.
>
> Neil
>
> > Thank you for your review!
> >
> > Ciao
> > Stephan
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v25 12/12] LRNG - add interface for gathering of raw entropy
From: Stephan Müller @ 2019-11-21 15:18 UTC (permalink / raw)
To: Nicolai Stange
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
"Peter, Matthias" <matthias>
In-Reply-To: <87a78pl8xp.fsf@suse.de>
Am Donnerstag, 21. November 2019, 13:18:10 CET schrieb Nicolai Stange:
Hi Nicolai,
> Hi Stephan,
>
> two general remarks on debugfs usage below
>
> Stephan Müller <smueller@chronox.de> writes:
> > diff --git a/drivers/char/lrng/lrng_testing.c
> > b/drivers/char/lrng/lrng_testing.c new file mode 100644
> > index 000000000000..5c33d3bd2172
> > --- /dev/null
> > +++ b/drivers/char/lrng/lrng_testing.c
>
> <snip>
>
> > +/*
> > + * This data structure holds the dentry's of the debugfs files
> > establishing + * the interface to user space.
> > + */
> > +struct lrng_raw_debugfs {
> > + struct dentry *lrng_raw_debugfs_root; /* root dentry */
> > + struct dentry *lrng_raw_debugfs_lrng_raw; /* .../lrng_raw */
> > +};
> > +
> > +static struct lrng_raw_debugfs lrng_raw_debugfs;
> > +
> > +/* DebugFS operations and definition of the debugfs files */
> > +static ssize_t lrng_raw_read(struct file *file, char __user *to,
> > + size_t count, loff_t *ppos)
> > +{
> > + loff_t pos = *ppos;
> > + int ret;
> > +
> > + if (!count)
> > + return 0;
> > + lrng_raw_entropy_init();
> > + ret = lrng_raw_extract_user(to, count);
> > + lrng_raw_entropy_fini();
> > + if (ret < 0)
> > + return ret;
> > + count -= ret;
> > + *ppos = pos + count;
> > + return ret;
> > +}
> > +
> > +/* Module init: allocate memory, register the debugfs files */
> > +static int lrng_raw_debugfs_init(void)
> > +{
> > + lrng_raw_debugfs.lrng_raw_debugfs_root =
> > + debugfs_create_dir(KBUILD_MODNAME, NULL);
> > + if (IS_ERR(lrng_raw_debugfs.lrng_raw_debugfs_root)) {
> > + lrng_raw_debugfs.lrng_raw_debugfs_root = NULL;
> > + return PTR_ERR(lrng_raw_debugfs.lrng_raw_debugfs_root);
> > + }
>
> I think pointers returned by the debugfs API are not supposed to get
> checked for NULL/IS_ERR(), c.f commit ff9fb72bc077 ("debugfs: return
> error values, not NULL") or the the output from
>
> git log --pretty=oneline | grep 'no need to check return value of
> debugfs_create'
>
> (Also the above code is dubious: you're effectively returning
> PTR_ERR(NULL)).
Removed the check compliant to the mentioned patches.
>
> > + return 0;
> > +}
> > +
> > +static struct file_operations lrng_raw_name_fops = {
> > + .owner = THIS_MODULE,
> > + .read = lrng_raw_read,
> > +};
> > +
> > +static int lrng_raw_debugfs_init_name(void)
> > +{
> > + lrng_raw_debugfs.lrng_raw_debugfs_lrng_raw =
> > + debugfs_create_file("lrng_raw", 0400,
> > + lrng_raw_debugfs.lrng_raw_debugfs_root,
> > + NULL, &lrng_raw_name_fops);q
>
> CONFIG_LRNG_TESTING is a bool and thus, this debugfs file can't ever get
> removed. Even if it could, this inode hasn't got any data associated
> with it and so file removal would not be a problem for lrng_raw_read().
Correct.
>
> Please consider using debugfs_create_file_unsafe() instead to save
> debugfs from kmalloc()ing a proxy file_operations protecting your fops
> against concurrent file removal.
Yes, you are correct. Changed.
>
> > + if (IS_ERR(lrng_raw_debugfs.lrng_raw_debugfs_lrng_raw)) {
> > + lrng_raw_debugfs.lrng_raw_debugfs_lrng_raw = NULL;
> > + return PTR_ERR(lrng_raw_debugfs.lrng_raw_debugfs_lrng_raw);
> > + }
>
> Same comment regarding return value checking applies here.
Same here: I removed the check.
With that, I also removed the static variable to maintain the two dentries
following the examples seen in other kernel code. Also, the __exit function is
removed as we do not need it as you pointed out.
Thanks a lot.
>
> Thanks,
>
> Nicolai
>
> > + return 0;
> > +}
> > +
> > +static int __init lrng_raw_init(void)
> > +{
> > + int ret = lrng_raw_debugfs_init();
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = lrng_raw_debugfs_init_name();
> > + if (ret < 0)
> > + debugfs_remove_recursive(
> > + lrng_raw_debugfs.lrng_raw_debugfs_root);
> > +
> > + return ret;
> > +}
> > +
> > +static void __exit lrng_raw_exit(void)
> > +{
> > + debugfs_remove_recursive(lrng_raw_debugfs.lrng_raw_debugfs_root);
> > +}
> > +
> > +module_init(lrng_raw_init);
> > +module_exit(lrng_raw_exit);
> > +
> > +MODULE_LICENSE("Dual BSD/GPL");
> > +MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
> > +MODULE_DESCRIPTION("Kernel module for gathering raw entropy");
Ciao
Stephan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox