Minor cleanups in GFP_NOFS flags use
David Sterba
maintainercover Minor cleanups in GFP_NOFS flags use
Hi,
the first part of patchset mostly moves the GFP_NOFS flags from several callsites into the callers. The point is to move all GFP_NOFS to the actual point of use. If all the callers pass the same flag, there's no point in doing that, which as a small benefit saves a few bytes on the stack.
The second part tries to improve extent_state preallocation logic in the set/clear/convert functions. Depending on the internal state, we might not need the allocation at all, the actual failure is left to the place where we really need the memory, that's done in a locked section with GFP_ATOMIC. If we leave this section, we speculatively try to allocate new extent state unless we already have one spare.
I'd like to get this to 4.7, the patchset has low functionality impact but helps me to continue in the NOFS crusade.
David Sterba (16):
btrfs: sink gfp parameter to set_extent_bits
btrfs: sink gfp parameter to clear_extent_bits
btrfs: sink gfp parameter to clear_record_extent_bits
btrfs: sink gfp parameter to clear_extent_dirty
btrfs: sink gfp parameter to set_extent_delalloc
btrfs: sink gfp parameter to set_extent_defrag
btrfs: sink gfp parameter to set_extent_new
btrfs: sink gfp parameter to set_record_extent_bits
btrfs: untangle gotos a bit in __set_extent_bit
btrfs: untangle gotos a bit in __clear_extent_bit
btrfs: untangle gotos a bit in convert_extent_bit
btrfs: make state preallocation more speculative in __set_extent_bit
btrfs: __set_extent_bit, try preallocation out of locked section with
lighter gfp flags
btrfs: __clear_extent_bit, try preallocation out of locked section
with lighter gfp flags
btrfs: convert_extent_bit, try preallocation out of locked section
with lighter gfp flags
btrfs: sink gfp parameter to convert_extent_bit
fs/btrfs/disk-io.c | 4 +- fs/btrfs/extent-tree.c | 16 +++--- fs/btrfs/extent_io.c | 105 +++++++++++++++++++++++---------------- fs/btrfs/extent_io.h | 34 ++++++------- fs/btrfs/file-item.c | 2 +- fs/btrfs/inode.c | 5 +- fs/btrfs/ioctl.c | 2 +- fs/btrfs/qgroup.c | 8 ++- fs/btrfs/relocation.c | 7 ++- fs/btrfs/scrub.c | 4 +- fs/btrfs/tests/extent-io-tests.c | 8 +-- fs/btrfs/transaction.c | 2 +- fs/btrfs/tree-log.c | 2 +- 13 files changed, 108 insertions(+), 91 deletions(-)
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 1/16 btrfs: sink gfp parameter to set_extent_bits
All callers pass GFP_NOFS.
--- fs/btrfs/extent-tree.c | 4 ++-- fs/btrfs/extent_io.c | 5 ++--- fs/btrfs/extent_io.h | 4 ++-- fs/btrfs/file-item.c | 2 +- fs/btrfs/relocation.c | 4 ++-- fs/btrfs/scrub.c | 2 +- 6 files changed, 10 insertions(+), 11 deletions(-)
fs/btrfs/extent-tree.c +2 -2
@@ -231,9 +231,9 @@ static int add_excluded_extent(struct btrfs_root *root, {u64 end = start + num_bytes -1 ;set_extent_bits (&root->fs_info->freed_extents[0 ], - start, end,EXTENT_UPTODATE ,GFP_NOFS ); + start, end,EXTENT_UPTODATE );set_extent_bits (&root->fs_info->freed_extents[1 ], - start, end,EXTENT_UPTODATE ,GFP_NOFS ); + start, end,EXTENT_UPTODATE );return 0 ; }
fs/btrfs/extent_io.c +2 -3
@@ -2232,13 +2232,12 @@ int btrfs_get_io_failure_record(struct inode *inode, u64 start, u64 end,/* set the bits in the private failure tree */ ret =set_extent_bits (failure_tree, start, end, -EXTENT_LOCKED |EXTENT_DIRTY ,GFP_NOFS ); +EXTENT_LOCKED |EXTENT_DIRTY );if (ret >=0 ) ret =set_state_failrec (failure_tree, start, failrec);/* set the bits in the inode's tree */ if (ret >=0 ) - ret =set_extent_bits (tree, start, end,EXTENT_DAMAGED , -GFP_NOFS ); + ret =set_extent_bits (tree, start, end,EXTENT_DAMAGED );if (ret <0 ) {kfree (failrec);return ret;
fs/btrfs/extent_io.h +2 -2
@@ -259,9 +259,9 @@ int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, struct extent_state **cached_state, gfp_t mask);static inline int set_extent_bits (struct extent_io_tree *tree,u64 start, -u64 end,unsigned bits,gfp_t mask) +u64 end,unsigned bits) { -return set_extent_bit (tree, start, end, bits,NULL ,NULL , mask); +return set_extent_bit (tree, start, end, bits,NULL ,NULL ,GFP_NOFS ); } static inline int clear_extent_uptodate(struct extent_io_tree *tree, u64 start,
fs/btrfs/file-item.c +1 -1
@@ -248,7 +248,7 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root *root, BTRFS_DATA_RELOC_TREE_OBJECTID) { set_extent_bits(io_tree, offset, offset + root->sectorsize - 1, - EXTENT_NODATASUM, GFP_NOFS); + EXTENT_NODATASUM); } else { btrfs_info(BTRFS_I(inode)->root->fs_info, "no csum found for inode %llu start %llu",
fs/btrfs/relocation.c +2 -2
@@ -2814,7 +2814,7 @@ static void mark_block_processed(struct reloc_control *rc, u64 bytenr, u32 blocksize) {set_extent_bits (&rc->processed_blocks, bytenr, bytenr + blocksize -1 , -EXTENT_DIRTY ,GFP_NOFS ); +EXTENT_DIRTY ); } static void __mark_block_processed(struct reloc_control *rc,
@@ -3182,7 +3182,7 @@ static int relocate_file_extent_cluster(struct inode *inode, page_start + offset == cluster->boundary[nr]) {set_extent_bits (&BTRFS_I (inode)->io_tree, page_start, page_end, -EXTENT_BOUNDARY ,GFP_NOFS ); +EXTENT_BOUNDARY ); nr++; }
fs/btrfs/scrub.c +1 -1
@@ -745,7 +745,7 @@ static int scrub_fixup_readpage(u64 inum, u64 offset, u64 root, void *fixup_ctx)* sure we read the bad mirror. */ ret =set_extent_bits (&BTRFS_I (inode)->io_tree, offset, end, -EXTENT_DAMAGED ,GFP_NOFS ); +EXTENT_DAMAGED );if (ret) {/* set_extent_bits should give proper error */ WARN_ON (ret >0 );
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 2/16 btrfs: sink gfp parameter to clear_extent_bits
In reply to David SterbaCallers pass GFP_NOFS and GFP_KERNEL. No need to pass the flags around.
--- fs/btrfs/disk-io.c | 2 +- fs/btrfs/extent-tree.c | 8 ++++---- fs/btrfs/extent_io.c | 4 ++-- fs/btrfs/extent_io.h | 5 +++-- fs/btrfs/inode.c | 3 +-- fs/btrfs/relocation.c | 3 +-- fs/btrfs/scrub.c | 2 +- fs/btrfs/tests/extent-io-tests.c | 2 +- fs/btrfs/tree-log.c | 2 +- 9 files changed, 15 insertions(+), 16 deletions(-)
fs/btrfs/disk-io.c +1 -1
@@ -4357,7 +4357,7 @@ static int btrfs_destroy_marked_extents(struct btrfs_root *root,if (ret)break ; -clear_extent_bits (dirty_pages, start, end, mark,GFP_NOFS ); +clear_extent_bits (dirty_pages, start, end, mark);while (start <= end) { eb =btrfs_find_tree_block (root->fs_info, start); start += root->nodesize;
fs/btrfs/extent-tree.c +4 -4
@@ -246,9 +246,9 @@ static void free_excluded_extents(struct btrfs_root *root, end = start + cache->key.offset -1 ;clear_extent_bits (&root->fs_info->freed_extents[0 ], - start, end,EXTENT_UPTODATE ,GFP_NOFS ); + start, end,EXTENT_UPTODATE );clear_extent_bits (&root->fs_info->freed_extents[1 ], - start, end,EXTENT_UPTODATE ,GFP_NOFS ); + start, end,EXTENT_UPTODATE ); } static int exclude_super_stripes(struct btrfs_root *root,
@@ -10526,14 +10526,14 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info)*/ mutex_lock (&fs_info->unused_bg_unpin_mutex); ret =clear_extent_bits (&fs_info->freed_extents[0 ], start, end, -EXTENT_DIRTY ,GFP_NOFS ); +EXTENT_DIRTY );if (ret) {mutex_unlock (&fs_info->unused_bg_unpin_mutex);btrfs_dec_block_group_ro (root, block_group); goto end_trans; } ret =clear_extent_bits (&fs_info->freed_extents[1 ], start, end, -EXTENT_DIRTY ,GFP_NOFS ); +EXTENT_DIRTY );if (ret) {mutex_unlock (&fs_info->unused_bg_unpin_mutex);btrfs_dec_block_group_ro (root, block_group);
fs/btrfs/extent_io.c +2 -2
@@ -1975,13 +1975,13 @@ int free_io_failure(struct inode *inode, struct io_failure_record *rec)set_state_failrec (failure_tree, rec->start,NULL ); ret =clear_extent_bits (failure_tree, rec->start, rec->start + rec->len -1 , -EXTENT_LOCKED |EXTENT_DIRTY ,GFP_NOFS ); +EXTENT_LOCKED |EXTENT_DIRTY );if (ret) err = ret; ret =clear_extent_bits (&BTRFS_I (inode)->io_tree, rec->start, rec->start + rec->len -1 , -EXTENT_DAMAGED ,GFP_NOFS ); +EXTENT_DAMAGED );if (ret && !err) err = ret;
fs/btrfs/extent_io.h +3 -2
@@ -241,14 +241,15 @@ static inline int unlock_extent_cached(struct extent_io_tree *tree, u64 start, }static inline int clear_extent_bits (struct extent_io_tree *tree,u64 start, -u64 end,unsigned bits,gfp_t mask) +u64 end,unsigned bits) {int wake =0 ;if (bits &EXTENT_LOCKED ) wake =1 ; -return clear_extent_bit (tree, start, end, bits, wake,0 ,NULL , mask); +return clear_extent_bit (tree, start, end, bits, wake,0 ,NULL , +GFP_NOFS ); } int set_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
fs/btrfs/inode.c +1 -2
@@ -3103,8 +3103,7 @@ static int btrfs_readpage_end_io_hook(struct btrfs_io_bio *io_bio,if (root->root_key.objectid ==BTRFS_DATA_RELOC_TREE_OBJECTID &&test_range_bit (io_tree, start, end,EXTENT_NODATASUM ,1 ,NULL )) { -clear_extent_bits (io_tree, start, end,EXTENT_NODATASUM , -GFP_NOFS ); +clear_extent_bits (io_tree, start, end,EXTENT_NODATASUM );return 0 ; }
fs/btrfs/relocation.c +1 -2
@@ -4059,8 +4059,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc) }btrfs_release_path (path); -clear_extent_bits (&rc->processed_blocks,0 , (u64)-1 ,EXTENT_DIRTY , -GFP_NOFS ); +clear_extent_bits (&rc->processed_blocks,0 , (u64)-1 ,EXTENT_DIRTY );if (trans) {btrfs_end_transaction_throttle (trans, rc->extent_root);
fs/btrfs/scrub.c +1 -1
@@ -763,7 +763,7 @@ static int scrub_fixup_readpage(u64 inum, u64 offset, u64 root, void *fixup_ctx) end, EXTENT_DAMAGED, 0, NULL);if (!corrected)clear_extent_bits (&BTRFS_I (inode)->io_tree, offset, end, -EXTENT_DAMAGED ,GFP_NOFS ); +EXTENT_DAMAGED ); } out:
fs/btrfs/tests/extent-io-tests.c +1 -1
@@ -262,7 +262,7 @@ static int test_find_delalloc(void) } ret =0 ; out_bits: -clear_extent_bits (&tmp,0 , total_dirty -1 , (unsigned )-1 ,GFP_KERNEL ); +clear_extent_bits (&tmp,0 , total_dirty -1 , (unsigned )-1 ); out:if (locked_page)put_page (locked_page);
fs/btrfs/tree-log.c +1 -1
@@ -3001,7 +3001,7 @@ static void free_log_tree(struct btrfs_trans_handle *trans,break ;clear_extent_bits (&log->dirty_log_pages, start, end, -EXTENT_DIRTY |EXTENT_NEW ,GFP_NOFS ); +EXTENT_DIRTY |EXTENT_NEW ); }/*
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 3/16 btrfs: sink gfp parameter to clear_record_extent_bits
In reply to David SterbaCallers pass GFP_NOFS. No need to pass the flags around.
--- fs/btrfs/extent_io.c | 5 ++--- fs/btrfs/extent_io.h | 3 +-- fs/btrfs/qgroup.c | 5 ++--- 3 files changed, 5 insertions(+), 8 deletions(-)
fs/btrfs/extent_io.c +2 -3
@@ -1308,8 +1308,7 @@ int clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, }int clear_record_extent_bits (struct extent_io_tree *tree,u64 start,u64 end, -unsigned bits,gfp_t mask, -struct extent_changeset *changeset) +unsigned bits,struct extent_changeset *changeset) {/* * Don't support EXTENT_LOCKED case, same reason as
@@ -1317,7 +1316,7 @@ int clear_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,*/ BUG_ON (bits &EXTENT_LOCKED ); -return __clear_extent_bit (tree, start, end, bits,0 ,0 ,NULL , mask, +return __clear_extent_bit (tree, start, end, bits,0 ,0 ,NULL ,GFP_NOFS , changeset); }
fs/btrfs/extent_io.h +1 -2
@@ -221,8 +221,7 @@ int test_range_bit(struct extent_io_tree *tree, u64 start, u64 end, unsigned bits, int filled, struct extent_state *cached_state);int clear_record_extent_bits (struct extent_io_tree *tree,u64 start,u64 end, -unsigned bits,gfp_t mask, -struct extent_changeset *changeset); +unsigned bits,struct extent_changeset *changeset);int clear_extent_bit (struct extent_io_tree *tree,u64 start,u64 end,unsigned bits,int wake,int delete,struct extent_state **cached,gfp_t mask);
fs/btrfs/qgroup.c +2 -3
@@ -2580,8 +2580,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, u64 start, u64 len,return -ENOMEM ; ret =clear_record_extent_bits (&BTRFS_I (inode)->io_tree, start, - start + len -1 ,EXTENT_QGROUP_RESERVED ,GFP_NOFS , - &changeset); + start + len -1 ,EXTENT_QGROUP_RESERVED , &changeset);if (ret <0 ) goto out;
@@ -2688,7 +2687,7 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode)return ; ret =clear_record_extent_bits (&BTRFS_I (inode)->io_tree,0 , (u64)-1 , -EXTENT_QGROUP_RESERVED ,GFP_NOFS , &changeset); +EXTENT_QGROUP_RESERVED , &changeset);WARN_ON (ret <0 );if (WARN_ON (changeset.bytes_changed)) {
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 4/16 btrfs: sink gfp parameter to clear_extent_dirty
In reply to David SterbaCallers pass GFP_NOFS. No need to pass the flags around.
--- fs/btrfs/disk-io.c | 2 +- fs/btrfs/extent-tree.c | 2 +- fs/btrfs/extent_io.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)
fs/btrfs/disk-io.c +1 -1
@@ -4392,7 +4392,7 @@ static int btrfs_destroy_pinned_extent(struct btrfs_root *root,if (ret)break ; -clear_extent_dirty (unpin, start, end,GFP_NOFS ); +clear_extent_dirty (unpin, start, end);btrfs_error_unpin_extent_range (root, start, end);cond_resched (); }
fs/btrfs/extent-tree.c +1 -1
@@ -6408,7 +6408,7 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans, ret =btrfs_discard_extent (root, start, end +1 - start,NULL ); -clear_extent_dirty (unpin, start, end,GFP_NOFS ); +clear_extent_dirty (unpin, start, end);unpin_extent_range (root, start, end, true);mutex_unlock (&fs_info->unused_bg_unpin_mutex);cond_resched ();
fs/btrfs/extent_io.h +2 -2
@@ -279,11 +279,11 @@ static inline int set_extent_dirty(struct extent_io_tree *tree, u64 start, }static inline int clear_extent_dirty (struct extent_io_tree *tree,u64 start, -u64 end,gfp_t mask) +u64 end) {return clear_extent_bit (tree, start, end,EXTENT_DIRTY |EXTENT_DELALLOC | -EXTENT_DO_ACCOUNTING ,0 ,0 ,NULL , mask); +EXTENT_DO_ACCOUNTING ,0 ,0 ,NULL ,GFP_NOFS ); } int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 5/16 btrfs: sink gfp parameter to set_extent_delalloc
In reply to David SterbaCallers pass GFP_NOFS and tests pass GFP_KERNEL, but using NOFS there does not hurt. No need to pass the flags around.
--- fs/btrfs/extent_io.h | 4 ++-- fs/btrfs/inode.c | 2 +- fs/btrfs/tests/extent-io-tests.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-)
fs/btrfs/extent_io.h +2 -2
@@ -291,11 +291,11 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, struct extent_state **cached_state, gfp_t mask);static inline int set_extent_delalloc (struct extent_io_tree *tree,u64 start, -u64 end,struct extent_state **cached_state,gfp_t mask) +u64 end,struct extent_state **cached_state) {return set_extent_bit (tree, start, end,EXTENT_DELALLOC |EXTENT_UPTODATE , -NULL , cached_state, mask); +NULL , cached_state,GFP_NOFS ); } static inline int set_extent_defrag(struct extent_io_tree *tree, u64 start,
fs/btrfs/inode.c +1 -1
@@ -1962,7 +1962,7 @@ int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end, {WARN_ON ((end & (PAGE_SIZE -1 )) ==0 );return set_extent_delalloc (&BTRFS_I (inode)->io_tree, start, end, - cached_state,GFP_NOFS ); + cached_state); }/* see btrfs_writepage_start_hook for details on why this is required */
fs/btrfs/tests/extent-io-tests.c +3 -3
@@ -113,7 +113,7 @@ static int test_find_delalloc(void)* |--- delalloc ---| * |--- search ---| */ -set_extent_delalloc (&tmp,0 ,4095 ,NULL ,GFP_KERNEL ); +set_extent_delalloc (&tmp,0 ,4095 ,NULL ); start =0 ; end =0 ; found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
@@ -144,7 +144,7 @@ static int test_find_delalloc(void)test_msg ("Couldn't find the locked page\n" ); goto out_bits; } -set_extent_delalloc (&tmp,4096 , max_bytes -1 ,NULL ,GFP_KERNEL ); +set_extent_delalloc (&tmp,4096 , max_bytes -1 ,NULL ); start = test_start; end =0 ; found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
@@ -199,7 +199,7 @@ static int test_find_delalloc(void)* * We are re-using our test_start from above since it works out well. */ -set_extent_delalloc (&tmp, max_bytes, total_dirty -1 ,NULL ,GFP_KERNEL ); +set_extent_delalloc (&tmp, max_bytes, total_dirty -1 ,NULL ); start = test_start; end =0 ; found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 6/16 btrfs: sink gfp parameter to set_extent_defrag
In reply to David SterbaSingle caller passes GFP_NOFS.
--- fs/btrfs/extent_io.h | 4 ++-- fs/btrfs/ioctl.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
fs/btrfs/extent_io.h +2 -2
@@ -299,11 +299,11 @@ static inline int set_extent_delalloc(struct extent_io_tree *tree, u64 start, }static inline int set_extent_defrag (struct extent_io_tree *tree,u64 start, -u64 end,struct extent_state **cached_state,gfp_t mask) +u64 end,struct extent_state **cached_state) {return set_extent_bit (tree, start, end,EXTENT_DELALLOC |EXTENT_UPTODATE |EXTENT_DEFRAG , -NULL , cached_state, mask); +NULL , cached_state,GFP_NOFS ); } static inline int set_extent_new(struct extent_io_tree *tree, u64 start,
fs/btrfs/ioctl.c +1 -1
@@ -1228,7 +1228,7 @@ static int cluster_pages_for_defrag(struct inode *inode,set_extent_defrag (&BTRFS_I (inode)->io_tree, page_start, page_end -1 , - &cached_state,GFP_NOFS ); + &cached_state); unlock_extent_cached(&BTRFS_I(inode)->io_tree, page_start, page_end - 1, &cached_state,
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 7/16 btrfs: sink gfp parameter to set_extent_new
In reply to David SterbaSingle caller passes GFP_NOFS.
--- fs/btrfs/extent-tree.c | 2 +- fs/btrfs/extent_io.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-)
fs/btrfs/extent-tree.c +1 -1
@@ -7923,7 +7923,7 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root, buf->start + buf->len - 1, GFP_NOFS); else set_extent_new(&root->dirty_log_pages, buf->start, - buf->start + buf->len - 1, GFP_NOFS); + buf->start + buf->len - 1); }else { buf->log_index =-1 ; set_extent_dirty(&trans->transaction->dirty_pages, buf->start,
fs/btrfs/extent_io.h +3 -2
@@ -307,9 +307,10 @@ static inline int set_extent_defrag(struct extent_io_tree *tree, u64 start, }static inline int set_extent_new (struct extent_io_tree *tree,u64 start, -u64 end,gfp_t mask) +u64 end) { -return set_extent_bit (tree, start, end,EXTENT_NEW ,NULL ,NULL , mask); +return set_extent_bit (tree, start, end,EXTENT_NEW ,NULL ,NULL , +GFP_NOFS ); } static inline int set_extent_uptodate(struct extent_io_tree *tree, u64 start,
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 8/16 btrfs: sink gfp parameter to set_record_extent_bits
In reply to David SterbaSingle caller passes GFP_NOFS.
--- fs/btrfs/extent_io.c | 5 ++--- fs/btrfs/extent_io.h | 3 +-- fs/btrfs/qgroup.c | 3 +-- 3 files changed, 4 insertions(+), 7 deletions(-)
fs/btrfs/extent_io.c +2 -3
@@ -1284,8 +1284,7 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,/* wrappers around set/clear extent bit */ int set_record_extent_bits (struct extent_io_tree *tree,u64 start,u64 end, -unsigned bits,gfp_t mask, -struct extent_changeset *changeset) +unsigned bits,struct extent_changeset *changeset) {/* * We don't support EXTENT_LOCKED yet, as current changeset will
@@ -1295,7 +1294,7 @@ int set_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,*/ BUG_ON (bits &EXTENT_LOCKED ); -return __set_extent_bit (tree, start, end, bits,0 ,NULL ,NULL , mask, +return __set_extent_bit (tree, start, end, bits,0 ,NULL ,NULL ,GFP_NOFS , changeset); }
fs/btrfs/extent_io.h +1 -2
@@ -252,8 +252,7 @@ static inline int clear_extent_bits(struct extent_io_tree *tree, u64 start, }int set_record_extent_bits (struct extent_io_tree *tree,u64 start,u64 end, -unsigned bits,gfp_t mask, -struct extent_changeset *changeset); +unsigned bits,struct extent_changeset *changeset);int set_extent_bit (struct extent_io_tree *tree,u64 start,u64 end,unsigned bits,u64 *failed_start,struct extent_state **cached_state,gfp_t mask);
fs/btrfs/qgroup.c +1 -2
@@ -2542,8 +2542,7 @@ int btrfs_qgroup_reserve_data(struct inode *inode, u64 start, u64 len) changeset.bytes_changed =0 ; changeset.range_changed =ulist_alloc (GFP_NOFS ); ret =set_record_extent_bits (&BTRFS_I (inode)->io_tree, start, - start + len -1 ,EXTENT_QGROUP_RESERVED ,GFP_NOFS , - &changeset); + start + len -1 ,EXTENT_QGROUP_RESERVED , &changeset);trace_btrfs_qgroup_reserve_data (inode, start, len, changeset.bytes_changed,QGROUP_RESERVE );
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 9/16 btrfs: untangle gotos a bit in __set_extent_bit
In reply to David Sterba--- fs/btrfs/extent_io.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
fs/btrfs/extent_io.c +7 -8
@@ -1037,7 +1037,13 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, goto out; } - goto search_again; +search_again: +if (start > end) + goto out; +spin_unlock (&tree->lock); +if (gfpflags_allow_blocking (mask)) +cond_resched (); + goto again; out:spin_unlock (&tree->lock);
@@ -1046,13 +1052,6 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,return err; -search_again: -if (start > end) - goto out; -spin_unlock (&tree->lock); -if (gfpflags_allow_blocking (mask)) -cond_resched (); - goto again; } int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 10/16 btrfs: untangle gotos a bit in __clear_extent_bit
In reply to David Sterba--- fs/btrfs/extent_io.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
fs/btrfs/extent_io.c +8 -8
@@ -726,14 +726,6 @@ static int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, start = last_end +1 ;if (start <= end && state && !need_resched ()) goto hit_next; - goto search_again; - -out: -spin_unlock (&tree->lock); -if (prealloc) -free_extent_state (prealloc); - -return 0 ; search_again:if (start > end)
@@ -742,6 +734,14 @@ static int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,if (gfpflags_allow_blocking (mask))cond_resched (); goto again; + +out: +spin_unlock (&tree->lock); +if (prealloc) +free_extent_state (prealloc); + +return 0 ; + } static void wait_on_state(struct extent_io_tree *tree,
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 11/16 btrfs: untangle gotos a bit in convert_extent_bit
In reply to David Sterba--- fs/btrfs/extent_io.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
fs/btrfs/extent_io.c +7 -9
@@ -1262,15 +1262,6 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, goto out; } - goto search_again; - -out: -spin_unlock (&tree->lock); -if (prealloc) -free_extent_state (prealloc); - -return err; - search_again:if (start > end) goto out;
@@ -1279,6 +1270,13 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,cond_resched (); first_iteration = false; goto again; + +out: +spin_unlock (&tree->lock); +if (prealloc) +free_extent_state (prealloc); + +return err; }/* wrappers around set/clear extent bit */
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 12/16 btrfs: make state preallocation more speculative in __set_extent_bit
In reply to David SterbaSimilar to __clear_extent_bit, do not fail if the state preallocation fails as we might not need it. One less BUG_ON.
--- fs/btrfs/extent_io.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
fs/btrfs/extent_io.c +7 -1
@@ -873,8 +873,14 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, bits |=EXTENT_FIRST_DELALLOC ; again:if (!prealloc &&gfpflags_allow_blocking (mask)) { +/* +* Don't care for allocation failure here because we might end +* up not needing the pre-allocated extent state at all, which +* is the case if we only have in the tree extent states that +* cover our input range and don't cover too any other range. +* If we end up needing a new extent state we allocate it later. +*/ prealloc =alloc_extent_state (mask); -BUG_ON (!prealloc); }spin_lock (&tree->lock);
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 13/16 btrfs: __set_extent_bit, try preallocation out of locked section with lighter gfp flags
In reply to David SterbaIn __set_extent_bit we allocate with GFP_ATOMIC with the tree lock held, this takes away allocator opportunities to satisfy the allocation. In some cases we leave the locked section and we could repeat the preallocation with less strict flags. It could lead to unnecessary allocation, but we won't fail until we really need it.
--- fs/btrfs/extent_io.c | 7 +++++++ 1 file changed, 7 insertions(+)
fs/btrfs/extent_io.c +7 -0
@@ -1049,6 +1049,13 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,spin_unlock (&tree->lock);if (gfpflags_allow_blocking (mask))cond_resched (); +/* +* If we used the preallocated state, try again here out of the +* locked section so we can avoid GFP_ATOMIC. No error checking +* as we might not need it in the end. +*/ +if (!prealloc) + prealloc =alloc_extent_state (mask); goto again; out:
Signature · 1 line
2.7.1
Re: [PATCH 13/16] btrfs: __set_extent_bit, try preallocation out of locked section with lighter gfp flags
Quoted 16 lines · David Sterba
In
__set_extent_bitwe allocate withGFP_ATOMICwith the tree lock held, this takes away allocator opportunities to satisfy the allocation. In some cases we leave the locked section and we could repeat the preallocation with less strict flags. It could lead to unnecessary allocation, but we won't fail until we really need it.Signed-off-by David Sterba <[email protected]>--- fs/btrfs/extent_io.c | 7 +++++++ 1 file changed, 7 insertions(+)fs/btrfs/extent_io.c +0
-0
fs/btrfs/extent_io.c __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, · line 1058
Quoted 10 lines
fs/btrfs/extent_io.c +7
-0@@ -1049,6 +1049,13 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,spin_unlock (&tree->lock);if (gfpflags_allow_blocking (mask))cond_resched (); +/* +* If we used the preallocated state, try again here out of the +* locked section so we can avoid GFP_ATOMIC. No error checking +* as we might not need it in the end. +*/ +if (!prealloc) + prealloc =alloc_extent_state (mask);
Under the again label we already try to allocate in non atomic mode (gfpflags_allow_blocking() returns us true for GFP_NOFS), and some other patch in the series removes the BUG_ON() there.
So this extra allocation attempt seems unnecessary to me. Or did I miss something?
At least I'm seeing the following in 4.5's gfp.h:
#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */
#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM)) #define GFP_NOFS (__GFP_RECLAIM | __GFP_IO)
static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
{
return !!(gfp_flags & __GFP_DIRECT_RECLAIM);
}So:
GFP_NOFS == ___GFP_DIRECT_RECLAIM | ___GFP_KSWAPD_RECLAIM | __GFP_IO __GFP_DIRECT_RECLAIM == ___GFP_DIRECT_RECLAIM
So, as we always use GFP_NOFS for the gfp flags value...
Some comment goes for the other similar patches.
Quoted 10 lines
fs/btrfs/extent_io.c +0
-1goto again; out: --2.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 5 lines
Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men."
Re: [PATCH 13/16] btrfs: __set_extent_bit, try preallocation out of locked section with lighter gfp flags
Quoted 25 lines · Filipe Manana
Quoted 24 lines · David Sterba
In
__set_extent_bitwe allocate withGFP_ATOMICwith the tree lock held, this takes away allocator opportunities to satisfy the allocation. In some cases we leave the locked section and we could repeat the preallocation with less strict flags. It could lead to unnecessary allocation, but we won't fail until we really need it.Signed-off-by David Sterba <[email protected]>--- fs/btrfs/extent_io.c | 7 +++++++ 1 file changed, 7 insertions(+)fs/btrfs/extent_io.c +5
-0@@ -1049,6 +1049,13 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,spin_unlock (&tree->lock);if (gfpflags_allow_blocking (mask))cond_resched (); +/* +* If we used the preallocated state, try again here out of the +* locked section so we can avoid GFP_ATOMIC. No error checking +* as we might not need it in the end. +*/
Quoted 8 lines
fs/btrfs/extent_io.c __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end, · line 1058
Quoted 2 lines
fs/btrfs/extent_io.c +2
-0+if (!prealloc) + prealloc =alloc_extent_state (mask);Under the again label we already try to allocate in non atomic mode (
gfpflags_allow_blocking()returns us true forGFP_NOFS), and some other patch in the series removes theBUG_ON()there.
So this extra allocation attempt seems unnecessary to me. Or did I miss something?
No, you're right, I've duplicated the preallocation, the gfpflags_allow_blocking check gets dropped in the last patch.
Quoted 13 lines
At least I'm seeing the following in 4.5's
gfp.h:#define
__GFP_DIRECT_RECLAIM((__forcegfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */
#define__GFP_RECLAIM((__forcegfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM)) #defineGFP_NOFS(__GFP_RECLAIM|__GFP_IO)static inline bool
gfpflags_allow_blocking(const gfp_t gfp_flags){ return !!(gfp_flags & __GFP_DIRECT_RECLAIM); }
Quoted 8 lines
So:
GFP_NOFS==___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM|__GFP_IO__GFP_DIRECT_RECLAIM==___GFP_DIRECT_RECLAIMSo, as we always use
GFP_NOFSfor the gfp flags value...Some comment goes for the other similar patches.
I'll drop the patches, thanks for spotting it.
David Sterba
maintainerPATCH 14/16 btrfs: __clear_extent_bit, try preallocation out of locked section with lighter gfp flags
In reply to David SterbaIn __clear_extent_bit we allocate with GFP_ATOMIC with the tree lock held, this takes away allocator opportunities to satisfy the allocation. In some cases we leave the locked section and we could repeat the preallocation with less strict flags. It could lead to unnecessary allocation, but we won't fail until we really need it.
--- fs/btrfs/extent_io.c | 7 +++++++ 1 file changed, 7 insertions(+)
fs/btrfs/extent_io.c +7 -0
@@ -733,6 +733,13 @@ static int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,spin_unlock (&tree->lock);if (gfpflags_allow_blocking (mask))cond_resched (); +/* +* If we used the preallocated state, try again here out of the +* locked section so we can avoid GFP_ATOMIC. No error checking +* as we might not need it in the end. +*/ +if (!prealloc) + prealloc =alloc_extent_state (mask); goto again; out:
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 15/16 btrfs: convert_extent_bit, try preallocation out of locked section with lighter gfp flags
In reply to David SterbaIn convert_extent_bit we allocate with GFP_ATOMIC with the tree lock held, this takes away allocator opportunities to satisfy the allocation. In some cases we leave the locked section and we could repeat the preallocation with less strict flags. It could lead to unnecessary allocation, but we won't fail until we really need it.
--- fs/btrfs/extent_io.c | 7 +++++++ 1 file changed, 7 insertions(+)
fs/btrfs/extent_io.c +7 -0
@@ -1288,6 +1288,13 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,spin_unlock (&tree->lock);if (gfpflags_allow_blocking (mask))cond_resched (); +/* +* If we used the preallocated state, try again here out of the +* locked section so we can avoid GFP_ATOMIC. No error checking +* as we might not need it in the end. +*/ +if (!prealloc) + prealloc =alloc_extent_state (mask); first_iteration = false; goto again;
Signature · 1 line
2.7.1
David Sterba
maintainerPATCH 16/16 btrfs: sink gfp parameter to convert_extent_bit
In reply to David SterbaSingle caller passes GFP_NOFS. We can get rid of the gfpflags_allow_blocking checks as NOFS can block but does not recurse to filesystem through reclaim.
--- fs/btrfs/extent_io.c | 14 +++++++------- fs/btrfs/extent_io.h | 2 +- fs/btrfs/transaction.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-)
fs/btrfs/extent_io.c +7 -7
@@ -1092,17 +1092,18 @@ int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,* @bits: the bits to set in this range * @clear_bits: the bits to clear in this range * @cached_state: state that we're going to cache -* @mask: the allocation mask * * This will go through and set bits for the given range. If any states exist * already in this range they are set with the given bit and cleared of the * clear_bits. This is only meant to be used by things that are mergeable, ie * converting from say DELALLOC to DIRTY. This is not meant to be used with * boundary bits like LOCK. +* +* All allocations are done with GFP_NOFS. */ int convert_extent_bit (struct extent_io_tree *tree,u64 start,u64 end,unsigned bits,unsigned clear_bits, -struct extent_state **cached_state,gfp_t mask) +struct extent_state **cached_state) {struct extent_state *state;struct extent_state *prealloc =NULL ;
@@ -1117,7 +1118,7 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,btrfs_debug_check_extent_io_range (tree, start, end); again: -if (!prealloc &&gfpflags_allow_blocking (mask)) { +if (!prealloc) {/* * Best effort, don't worry if extent state allocation fails * here for the first iteration. We might have a cached state
@@ -1125,7 +1126,7 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,* extent state allocations are needed. We'll only know this * after locking the tree. */ - prealloc =alloc_extent_state (mask); + prealloc =alloc_extent_state (GFP_NOFS );if (!prealloc && !first_iteration)return -ENOMEM ; }
@@ -1286,15 +1287,14 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,if (start > end) goto out;spin_unlock (&tree->lock); -if (gfpflags_allow_blocking (mask)) -cond_resched (); +cond_resched ();/* * If we used the preallocated state, try again here out of the * locked section so we can avoid GFP_ATOMIC. No error checking * as we might not need it in the end. */ if (!prealloc) - prealloc =alloc_extent_state (mask); + prealloc =alloc_extent_state (GFP_NOFS ); first_iteration = false; goto again;
fs/btrfs/extent_io.h +1 -1
@@ -287,7 +287,7 @@ static inline int clear_extent_dirty(struct extent_io_tree *tree, u64 start,int convert_extent_bit (struct extent_io_tree *tree,u64 start,u64 end,unsigned bits,unsigned clear_bits, -struct extent_state **cached_state,gfp_t mask); +struct extent_state **cached_state);static inline int set_extent_delalloc (struct extent_io_tree *tree,u64 start,u64 end,struct extent_state **cached_state)
fs/btrfs/transaction.c +1 -1
@@ -943,7 +943,7 @@ int btrfs_write_marked_extents(struct btrfs_root *root, err =convert_extent_bit (dirty_pages, start, end,EXTENT_NEED_WAIT , - mark, &cached_state,GFP_NOFS ); + mark, &cached_state);/* * convert_extent_bit can return -ENOMEM, which is most of the * time a temporary error. So when it happens, ignore the error
Signature · 1 line
2.7.1