Skip to content

Commit aa695ae

Browse files
fdmananakdave
authored andcommitted
btrfs: avoid space_info locking when checking if tickets are served
When checking if a ticket was served, we take the space_info's spinlock. If the ticket was served (its ->bytes is 0) or had an error (its ->error it not 0) then we just unlock the space_info and return. This however causes contention on the space_info's spinlock, which is heavily used (space reservation, space flushing, allocating and deallocating an extent from a block group (btrfs_update_block_group()), etc). Instead of using the space_info's spinlock to check if a ticket was served, use a per ticket spinlock which isn't used by anyone other than the task that created the ticket (stack allocated) and the task that serves the ticket (a reclaim task or any task deallocating space that ends up at btrfs_try_granting_tickets()). After applying this patch and all previous patches from the same patchset (many attempt to reduce space_info critical sections), lockstat showed some improvements for a fs_mark test regarding the space_info's spinlock 'lock'. The lockstat results: Before patchset: con-bounces: 13733858 contentions: 15902322 waittime-total: 264902529.72 acq-bounces: 28161791 acquisitions: 38679282 After patchset: con-bounces: 12032220 contentions: 1359803 waittime-total: 221806127.28 acq-bounces: 24717947 acquisitions: 34103281 Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> Signed-off-by: Filipe Manana <fdmanana@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent 902d696 commit aa695ae

File tree

2 files changed

+34
-23
lines changed

2 files changed

+34
-23
lines changed

fs/btrfs/space-info.c

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -517,18 +517,22 @@ bool btrfs_can_overcommit(const struct btrfs_space_info *space_info, u64 bytes,
517517
static void remove_ticket(struct btrfs_space_info *space_info,
518518
struct reserve_ticket *ticket, int error)
519519
{
520+
lockdep_assert_held(&space_info->lock);
521+
520522
if (!list_empty(&ticket->list)) {
521523
list_del_init(&ticket->list);
522524
ASSERT(space_info->reclaim_size >= ticket->bytes);
523525
space_info->reclaim_size -= ticket->bytes;
524526
}
525527

528+
spin_lock(&ticket->lock);
526529
if (error)
527530
ticket->error = error;
528531
else
529532
ticket->bytes = 0;
530533

531534
wake_up(&ticket->wait);
535+
spin_unlock(&ticket->lock);
532536
}
533537

534538
/*
@@ -1495,6 +1499,17 @@ static const enum btrfs_flush_state evict_flush_states[] = {
14951499
RESET_ZONES,
14961500
};
14971501

1502+
static bool is_ticket_served(struct reserve_ticket *ticket)
1503+
{
1504+
bool ret;
1505+
1506+
spin_lock(&ticket->lock);
1507+
ret = (ticket->bytes == 0);
1508+
spin_unlock(&ticket->lock);
1509+
1510+
return ret;
1511+
}
1512+
14981513
static void priority_reclaim_metadata_space(struct btrfs_space_info *space_info,
14991514
struct reserve_ticket *ticket,
15001515
const enum btrfs_flush_state *states,
@@ -1504,29 +1519,25 @@ static void priority_reclaim_metadata_space(struct btrfs_space_info *space_info,
15041519
u64 to_reclaim;
15051520
int flush_state = 0;
15061521

1507-
spin_lock(&space_info->lock);
15081522
/*
15091523
* This is the priority reclaim path, so to_reclaim could be >0 still
15101524
* because we may have only satisfied the priority tickets and still
15111525
* left non priority tickets on the list. We would then have
15121526
* to_reclaim but ->bytes == 0.
15131527
*/
1514-
if (ticket->bytes == 0) {
1515-
spin_unlock(&space_info->lock);
1528+
if (is_ticket_served(ticket))
15161529
return;
1517-
}
15181530

1531+
spin_lock(&space_info->lock);
15191532
to_reclaim = btrfs_calc_reclaim_metadata_size(space_info);
15201533

15211534
while (flush_state < states_nr) {
15221535
spin_unlock(&space_info->lock);
15231536
flush_space(space_info, to_reclaim, states[flush_state], false);
1537+
if (is_ticket_served(ticket))
1538+
return;
15241539
flush_state++;
15251540
spin_lock(&space_info->lock);
1526-
if (ticket->bytes == 0) {
1527-
spin_unlock(&space_info->lock);
1528-
return;
1529-
}
15301541
}
15311542

15321543
/*
@@ -1554,22 +1565,17 @@ static void priority_reclaim_metadata_space(struct btrfs_space_info *space_info,
15541565
static void priority_reclaim_data_space(struct btrfs_space_info *space_info,
15551566
struct reserve_ticket *ticket)
15561567
{
1557-
spin_lock(&space_info->lock);
1558-
15591568
/* We could have been granted before we got here. */
1560-
if (ticket->bytes == 0) {
1561-
spin_unlock(&space_info->lock);
1569+
if (is_ticket_served(ticket))
15621570
return;
1563-
}
15641571

1572+
spin_lock(&space_info->lock);
15651573
while (!space_info->full) {
15661574
spin_unlock(&space_info->lock);
15671575
flush_space(space_info, U64_MAX, ALLOC_CHUNK_FORCE, false);
1568-
spin_lock(&space_info->lock);
1569-
if (ticket->bytes == 0) {
1570-
spin_unlock(&space_info->lock);
1576+
if (is_ticket_served(ticket))
15711577
return;
1572-
}
1578+
spin_lock(&space_info->lock);
15731579
}
15741580

15751581
remove_ticket(space_info, ticket, -ENOSPC);
@@ -1582,11 +1588,13 @@ static void wait_reserve_ticket(struct btrfs_space_info *space_info,
15821588

15831589
{
15841590
DEFINE_WAIT(wait);
1585-
int ret = 0;
15861591

1587-
spin_lock(&space_info->lock);
1592+
spin_lock(&ticket->lock);
15881593
while (ticket->bytes > 0 && ticket->error == 0) {
1594+
int ret;
1595+
15891596
ret = prepare_to_wait_event(&ticket->wait, &wait, TASK_KILLABLE);
1597+
spin_unlock(&ticket->lock);
15901598
if (ret) {
15911599
/*
15921600
* Delete us from the list. After we unlock the space
@@ -1596,17 +1604,18 @@ static void wait_reserve_ticket(struct btrfs_space_info *space_info,
15961604
* despite getting an error, resulting in a space leak
15971605
* (bytes_may_use counter of our space_info).
15981606
*/
1607+
spin_lock(&space_info->lock);
15991608
remove_ticket(space_info, ticket, -EINTR);
1600-
break;
1609+
spin_unlock(&space_info->lock);
1610+
return;
16011611
}
1602-
spin_unlock(&space_info->lock);
16031612

16041613
schedule();
16051614

16061615
finish_wait(&ticket->wait, &wait);
1607-
spin_lock(&space_info->lock);
1616+
spin_lock(&ticket->lock);
16081617
}
1609-
spin_unlock(&space_info->lock);
1618+
spin_unlock(&ticket->lock);
16101619
}
16111620

16121621
/*
@@ -1804,6 +1813,7 @@ static int reserve_bytes(struct btrfs_space_info *space_info, u64 orig_bytes,
18041813
ticket.error = 0;
18051814
space_info->reclaim_size += ticket.bytes;
18061815
init_waitqueue_head(&ticket.wait);
1816+
spin_lock_init(&ticket.lock);
18071817
ticket.steal = can_steal(flush);
18081818
if (trace_btrfs_reserve_ticket_enabled())
18091819
start_ns = ktime_get_ns();

fs/btrfs/space-info.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ struct reserve_ticket {
230230
bool steal;
231231
struct list_head list;
232232
wait_queue_head_t wait;
233+
spinlock_t lock;
233234
};
234235

235236
static inline bool btrfs_mixed_space_info(const struct btrfs_space_info *space_info)

0 commit comments

Comments
 (0)