Message ID | 20200630135921.745612-16-josef@toxicpanda.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Change data reservations to use the ticketing infra | expand |
On 30.06.20 г. 16:59 ч., Josef Bacik wrote: > Now that we have all the infrastructure in place, use the ticketing > infrastructure to make data allocations. This still maintains the exact > same flushing behavior, but now we're using tickets to get our > reservations satisfied. > > Reviewed-by: Nikolay Borisov <nborisov@suse.com> > Tested-by: Nikolay Borisov <nborisov@suse.com> > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/space-info.c | 125 ++++++++++++++++++++++-------------------- > 1 file changed, 67 insertions(+), 58 deletions(-) > > diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c > index 799ee6090693..ee4747917b81 100644 > --- a/fs/btrfs/space-info.c > +++ b/fs/btrfs/space-info.c > @@ -1068,6 +1068,54 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, > } while (flush_state < states_nr); > } > > +static void priority_reclaim_data_space(struct btrfs_fs_info *fs_info, > + struct btrfs_space_info *space_info, > + struct reserve_ticket *ticket, > + const enum btrfs_flush_state *states, > + int states_nr) > +{ > + int flush_state = 0; > + int commit_cycles = 2; > + > + while (!space_info->full) { > + flush_space(fs_info, space_info, U64_MAX, ALLOC_CHUNK_FORCE); > + spin_lock(&space_info->lock); > + if (ticket->bytes == 0) { > + spin_unlock(&space_info->lock); > + return; > + } > + spin_unlock(&space_info->lock); > + } > +again: > + while (flush_state < states_nr) { > + u64 flush_bytes = U64_MAX; > + > + if (!commit_cycles) { > + if (states[flush_state] == FLUSH_DELALLOC_WAIT) { > + flush_state++; > + continue; > + } > + if (states[flush_state] == COMMIT_TRANS) > + flush_bytes = ticket->bytes; > + } > + > + flush_space(fs_info, space_info, flush_bytes, > + states[flush_state]); > + spin_lock(&space_info->lock); > + if (ticket->bytes == 0) { > + spin_unlock(&space_info->lock); > + return; > + } > + spin_unlock(&space_info->lock); > + flush_state++; > + } > + if (commit_cycles) { > + commit_cycles--; > + flush_state = 0; > + goto again; > + } > +} > + > static void wait_reserve_ticket(struct btrfs_fs_info *fs_info, > struct btrfs_space_info *space_info, > struct reserve_ticket *ticket) > @@ -1134,6 +1182,15 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info, > evict_flush_states, > ARRAY_SIZE(evict_flush_states)); > break; > + case BTRFS_RESERVE_FLUSH_DATA: > + priority_reclaim_data_space(fs_info, space_info, ticket, > + data_flush_states, > + ARRAY_SIZE(data_flush_states)); > + break; > + case BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE: > + priority_reclaim_data_space(fs_info, space_info, ticket, > + NULL, 0); > + break; > default: > ASSERT(0); > break; > @@ -1341,78 +1398,30 @@ int btrfs_reserve_data_bytes(struct btrfs_fs_info *fs_info, u64 bytes, > enum btrfs_reserve_flush_enum flush) > { > struct btrfs_space_info *data_sinfo = fs_info->data_sinfo; > - const enum btrfs_flush_state *states = NULL; > u64 used; > - int states_nr = 0; > - int commit_cycles = 2; > int ret = -ENOSPC; > > ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_DATA); > > - if (flush == BTRFS_RESERVE_FLUSH_DATA) { > - states = data_flush_states; > - states_nr = ARRAY_SIZE(data_flush_states); > - } > - > spin_lock(&data_sinfo->lock); > -again: > used = btrfs_space_info_used(data_sinfo, true); > > if (used + bytes > data_sinfo->total_bytes) { > - u64 prev_total_bytes = data_sinfo->total_bytes; > - int flush_state = 0; > + struct reserve_ticket ticket; > > + init_waitqueue_head(&ticket.wait); > + ticket.bytes = bytes; > + ticket.error = 0; > + list_add_tail(&ticket.list, &data_sinfo->priority_tickets); nit: Shouldn't adding the ticket also be recorded in spac_info->reclaim_size? I see later that you are removing this code and relying on the existing logic in __reserve_metadata_bytes( renamed to reserve_bytes) which correctly modifies reclaim_size, but this just means this particular patch is slightly broken. > spin_unlock(&data_sinfo->lock); > > - /* > - * Everybody can force chunk allocation, so try this first to > - * see if we can just bail here and make our reservation. > - */ > - flush_space(fs_info, data_sinfo, bytes, ALLOC_CHUNK_FORCE); > - spin_lock(&data_sinfo->lock); > - if (prev_total_bytes < data_sinfo->total_bytes) > - goto again; > + ret = handle_reserve_ticket(fs_info, data_sinfo, &ticket, > + flush); > + } else { > + btrfs_space_info_update_bytes_may_use(fs_info, data_sinfo, bytes); > + ret = 0; <snip>
On 7/7/20 10:46 AM, Nikolay Borisov wrote: > > > On 30.06.20 г. 16:59 ч., Josef Bacik wrote: >> Now that we have all the infrastructure in place, use the ticketing >> infrastructure to make data allocations. This still maintains the exact >> same flushing behavior, but now we're using tickets to get our >> reservations satisfied. >> >> Reviewed-by: Nikolay Borisov <nborisov@suse.com> >> Tested-by: Nikolay Borisov <nborisov@suse.com> >> Signed-off-by: Josef Bacik <josef@toxicpanda.com> >> --- >> fs/btrfs/space-info.c | 125 ++++++++++++++++++++++-------------------- >> 1 file changed, 67 insertions(+), 58 deletions(-) >> >> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c >> index 799ee6090693..ee4747917b81 100644 >> --- a/fs/btrfs/space-info.c >> +++ b/fs/btrfs/space-info.c >> @@ -1068,6 +1068,54 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, >> } while (flush_state < states_nr); >> } >> >> +static void priority_reclaim_data_space(struct btrfs_fs_info *fs_info, >> + struct btrfs_space_info *space_info, >> + struct reserve_ticket *ticket, >> + const enum btrfs_flush_state *states, >> + int states_nr) >> +{ >> + int flush_state = 0; >> + int commit_cycles = 2; >> + >> + while (!space_info->full) { >> + flush_space(fs_info, space_info, U64_MAX, ALLOC_CHUNK_FORCE); >> + spin_lock(&space_info->lock); >> + if (ticket->bytes == 0) { >> + spin_unlock(&space_info->lock); >> + return; >> + } >> + spin_unlock(&space_info->lock); >> + } >> +again: >> + while (flush_state < states_nr) { >> + u64 flush_bytes = U64_MAX; >> + >> + if (!commit_cycles) { >> + if (states[flush_state] == FLUSH_DELALLOC_WAIT) { >> + flush_state++; >> + continue; >> + } >> + if (states[flush_state] == COMMIT_TRANS) >> + flush_bytes = ticket->bytes; >> + } >> + >> + flush_space(fs_info, space_info, flush_bytes, >> + states[flush_state]); >> + spin_lock(&space_info->lock); >> + if (ticket->bytes == 0) { >> + spin_unlock(&space_info->lock); >> + return; >> + } >> + spin_unlock(&space_info->lock); >> + flush_state++; >> + } >> + if (commit_cycles) { >> + commit_cycles--; >> + flush_state = 0; >> + goto again; >> + } >> +} >> + >> static void wait_reserve_ticket(struct btrfs_fs_info *fs_info, >> struct btrfs_space_info *space_info, >> struct reserve_ticket *ticket) >> @@ -1134,6 +1182,15 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info, >> evict_flush_states, >> ARRAY_SIZE(evict_flush_states)); >> break; >> + case BTRFS_RESERVE_FLUSH_DATA: >> + priority_reclaim_data_space(fs_info, space_info, ticket, >> + data_flush_states, >> + ARRAY_SIZE(data_flush_states)); >> + break; >> + case BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE: >> + priority_reclaim_data_space(fs_info, space_info, ticket, >> + NULL, 0); >> + break; >> default: >> ASSERT(0); >> break; >> @@ -1341,78 +1398,30 @@ int btrfs_reserve_data_bytes(struct btrfs_fs_info *fs_info, u64 bytes, >> enum btrfs_reserve_flush_enum flush) >> { >> struct btrfs_space_info *data_sinfo = fs_info->data_sinfo; >> - const enum btrfs_flush_state *states = NULL; >> u64 used; >> - int states_nr = 0; >> - int commit_cycles = 2; >> int ret = -ENOSPC; >> >> ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_DATA); >> >> - if (flush == BTRFS_RESERVE_FLUSH_DATA) { >> - states = data_flush_states; >> - states_nr = ARRAY_SIZE(data_flush_states); >> - } >> - >> spin_lock(&data_sinfo->lock); >> -again: >> used = btrfs_space_info_used(data_sinfo, true); >> >> if (used + bytes > data_sinfo->total_bytes) { >> - u64 prev_total_bytes = data_sinfo->total_bytes; >> - int flush_state = 0; >> + struct reserve_ticket ticket; >> >> + init_waitqueue_head(&ticket.wait); >> + ticket.bytes = bytes; >> + ticket.error = 0; >> + list_add_tail(&ticket.list, &data_sinfo->priority_tickets); > > nit: Shouldn't adding the ticket also be recorded in > spac_info->reclaim_size? > I see later that you are removing this code and relying on the existing > logic in __reserve_metadata_bytes( renamed to reserve_bytes) which > correctly modifies reclaim_size, but this just means this particular > patch is slightly broken. Yup I'll fix this up, thanks. Thanks, Josef
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c index 799ee6090693..ee4747917b81 100644 --- a/fs/btrfs/space-info.c +++ b/fs/btrfs/space-info.c @@ -1068,6 +1068,54 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, } while (flush_state < states_nr); } +static void priority_reclaim_data_space(struct btrfs_fs_info *fs_info, + struct btrfs_space_info *space_info, + struct reserve_ticket *ticket, + const enum btrfs_flush_state *states, + int states_nr) +{ + int flush_state = 0; + int commit_cycles = 2; + + while (!space_info->full) { + flush_space(fs_info, space_info, U64_MAX, ALLOC_CHUNK_FORCE); + spin_lock(&space_info->lock); + if (ticket->bytes == 0) { + spin_unlock(&space_info->lock); + return; + } + spin_unlock(&space_info->lock); + } +again: + while (flush_state < states_nr) { + u64 flush_bytes = U64_MAX; + + if (!commit_cycles) { + if (states[flush_state] == FLUSH_DELALLOC_WAIT) { + flush_state++; + continue; + } + if (states[flush_state] == COMMIT_TRANS) + flush_bytes = ticket->bytes; + } + + flush_space(fs_info, space_info, flush_bytes, + states[flush_state]); + spin_lock(&space_info->lock); + if (ticket->bytes == 0) { + spin_unlock(&space_info->lock); + return; + } + spin_unlock(&space_info->lock); + flush_state++; + } + if (commit_cycles) { + commit_cycles--; + flush_state = 0; + goto again; + } +} + static void wait_reserve_ticket(struct btrfs_fs_info *fs_info, struct btrfs_space_info *space_info, struct reserve_ticket *ticket) @@ -1134,6 +1182,15 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info, evict_flush_states, ARRAY_SIZE(evict_flush_states)); break; + case BTRFS_RESERVE_FLUSH_DATA: + priority_reclaim_data_space(fs_info, space_info, ticket, + data_flush_states, + ARRAY_SIZE(data_flush_states)); + break; + case BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE: + priority_reclaim_data_space(fs_info, space_info, ticket, + NULL, 0); + break; default: ASSERT(0); break; @@ -1341,78 +1398,30 @@ int btrfs_reserve_data_bytes(struct btrfs_fs_info *fs_info, u64 bytes, enum btrfs_reserve_flush_enum flush) { struct btrfs_space_info *data_sinfo = fs_info->data_sinfo; - const enum btrfs_flush_state *states = NULL; u64 used; - int states_nr = 0; - int commit_cycles = 2; int ret = -ENOSPC; ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_DATA); - if (flush == BTRFS_RESERVE_FLUSH_DATA) { - states = data_flush_states; - states_nr = ARRAY_SIZE(data_flush_states); - } - spin_lock(&data_sinfo->lock); -again: used = btrfs_space_info_used(data_sinfo, true); if (used + bytes > data_sinfo->total_bytes) { - u64 prev_total_bytes = data_sinfo->total_bytes; - int flush_state = 0; + struct reserve_ticket ticket; + init_waitqueue_head(&ticket.wait); + ticket.bytes = bytes; + ticket.error = 0; + list_add_tail(&ticket.list, &data_sinfo->priority_tickets); spin_unlock(&data_sinfo->lock); - /* - * Everybody can force chunk allocation, so try this first to - * see if we can just bail here and make our reservation. - */ - flush_space(fs_info, data_sinfo, bytes, ALLOC_CHUNK_FORCE); - spin_lock(&data_sinfo->lock); - if (prev_total_bytes < data_sinfo->total_bytes) - goto again; + ret = handle_reserve_ticket(fs_info, data_sinfo, &ticket, + flush); + } else { + btrfs_space_info_update_bytes_may_use(fs_info, data_sinfo, bytes); + ret = 0; spin_unlock(&data_sinfo->lock); - - /* - * Cycle through the rest of the flushing options for our flush - * type, then try again. - */ - while (flush_state < states_nr) { - u64 flush_bytes = U64_MAX; - - /* - * Previously we unconditionally committed the - * transaction twice before finally checking against - * pinned space before committing the final time. We - * also skipped flushing delalloc the final pass - * through. - */ - if (!commit_cycles) { - if (states[flush_state] == FLUSH_DELALLOC_WAIT) { - flush_state++; - continue; - } - if (states[flush_state] == COMMIT_TRANS) - flush_bytes = bytes; - } - - flush_space(fs_info, data_sinfo, flush_bytes, - states[flush_state]); - flush_state++; - } - - if (!commit_cycles) - goto out; - - commit_cycles--; - spin_lock(&data_sinfo->lock); - goto again; } - btrfs_space_info_update_bytes_may_use(fs_info, data_sinfo, bytes); - ret = 0; - spin_unlock(&data_sinfo->lock); -out: if (ret) trace_btrfs_space_reservation(fs_info, "space_info:enospc",