| Message ID | 20190816141952.19369-2-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rework reserve ticket handling | expand |
On 16.08.19 г. 17:19 ч., Josef Bacik wrote: > If we already have tickets on the list we don't want to steal their > reservations. This is a preparation patch for upcoming changes, > technically this shouldn't happen today because of the way we add bytes > to tickets before adding them to the space_info in most cases. nit: IMO this changelog should be a bit more explicit since this commit really makes reservations in FIFO order. Have you also quantified what's the latency impact as I suspect this will introduce such latencies? > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/space-info.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c > index e9406b2133d1..d671d6476eed 100644 > --- a/fs/btrfs/space-info.c > +++ b/fs/btrfs/space-info.c > @@ -938,6 +938,7 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info, > u64 used; > u64 reclaim_bytes = 0; > int ret = 0; > + bool pending_tickets; > > ASSERT(orig_bytes); > ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_ALL); > @@ -945,14 +946,17 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info, > spin_lock(&space_info->lock); > ret = -ENOSPC; > used = btrfs_space_info_used(space_info, true); > + pending_tickets = !list_empty(&space_info->tickets) || > + !list_empty(&space_info->priority_tickets); > > /* > * Carry on if we have enough space (short-circuit) OR call > * can_overcommit() to ensure we can overcommit to continue. > */ > - if ((used + orig_bytes <= space_info->total_bytes) || > - can_overcommit(fs_info, space_info, orig_bytes, flush, > - system_chunk)) { > + if (!pending_tickets && > + ((used + orig_bytes <= space_info->total_bytes) || > + can_overcommit(fs_info, space_info, orig_bytes, flush, > + system_chunk))) { > btrfs_space_info_update_bytes_may_use(fs_info, space_info, > orig_bytes); > trace_btrfs_space_reservation(fs_info, "space_info", >
On Mon, Aug 19, 2019 at 03:54:29PM +0300, Nikolay Borisov wrote: > > > On 16.08.19 г. 17:19 ч., Josef Bacik wrote: > > If we already have tickets on the list we don't want to steal their > > reservations. This is a preparation patch for upcoming changes, > > technically this shouldn't happen today because of the way we add bytes > > to tickets before adding them to the space_info in most cases. > > nit: IMO this changelog should be a bit more explicit since this commit > really makes reservations in FIFO order. Have you also quantified what's > the latency impact as I suspect this will introduce such latencies? Reservations were already in FIFO order because we would add new space to existing tickets. This doesn't change the behavior, just makes it so we get the same behavior without refilling. Thanks, Josef
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c index e9406b2133d1..d671d6476eed 100644 --- a/fs/btrfs/space-info.c +++ b/fs/btrfs/space-info.c @@ -938,6 +938,7 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info, u64 used; u64 reclaim_bytes = 0; int ret = 0; + bool pending_tickets; ASSERT(orig_bytes); ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_ALL); @@ -945,14 +946,17 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info, spin_lock(&space_info->lock); ret = -ENOSPC; used = btrfs_space_info_used(space_info, true); + pending_tickets = !list_empty(&space_info->tickets) || + !list_empty(&space_info->priority_tickets); /* * Carry on if we have enough space (short-circuit) OR call * can_overcommit() to ensure we can overcommit to continue. */ - if ((used + orig_bytes <= space_info->total_bytes) || - can_overcommit(fs_info, space_info, orig_bytes, flush, - system_chunk)) { + if (!pending_tickets && + ((used + orig_bytes <= space_info->total_bytes) || + can_overcommit(fs_info, space_info, orig_bytes, flush, + system_chunk))) { btrfs_space_info_update_bytes_may_use(fs_info, space_info, orig_bytes); trace_btrfs_space_reservation(fs_info, "space_info",
If we already have tickets on the list we don't want to steal their reservations. This is a preparation patch for upcoming changes, technically this shouldn't happen today because of the way we add bytes to tickets before adding them to the space_info in most cases. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/space-info.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)