Skip to content

Commit a0f3262

Browse files
cschoenebeckMichael Tokarev
authored andcommitted
9pfs: fix FD leak and reduce latency of v9fs_reclaim_fd()
This patch fixes two different bugs in v9fs_reclaim_fd(): 1. Reduce latency: This function calls v9fs_co_close() and v9fs_co_closedir() in a loop. Each one of the calls adds two thread hops (between main thread and a fs driver background thread). Each thread hop adds latency, which sums up in function's loop to a significant duration. Reduce overall latency by open coding what v9fs_co_close() and v9fs_co_closedir() do, executing those and the loop itself altogether in only one background thread block, hence reducing the total amount of thread hops to only two. 2. Fix file descriptor leak: The existing code called v9fs_co_close() and v9fs_co_closedir() to close file descriptors. Both functions check right at the beginning if the 9p request was cancelled: if (v9fs_request_cancelled(pdu)) { return -EINTR; } So if client sent a 'Tflush' message, v9fs_co_close() / v9fs_co_closedir() returned without having closed the file descriptor and v9fs_reclaim_fd() subsequently freed the FID without its file descriptor being closed, hence leaking those file descriptors. This 2nd bug is fixed by this patch as well by open coding v9fs_co_close() and v9fs_co_closedir() inside of v9fs_reclaim_fd() and not performing the v9fs_request_cancelled(pdu) check there. Fixes: 7a46274 ('hw/9pfs: Add file descriptor reclaim support') Fixes: bccacf6 ('hw/9pfs: Implement TFLUSH operation') Signed-off-by: Christian Schoenebeck <[email protected]> Reviewed-by: Greg Kurz <[email protected]> Message-Id: <5747469d3f039c53147e850b456943a1d4b5485c.1741339452.git.qemu_oss@crudebyte.com> (cherry picked from commit 89f7b4d) Signed-off-by: Michael Tokarev <[email protected]>
1 parent 5081dc5 commit a0f3262

File tree

1 file changed

+20
-9
lines changed

1 file changed

+20
-9
lines changed

hw/9pfs/9p.c

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,8 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
434434
V9fsFidState *f;
435435
GHashTableIter iter;
436436
gpointer fid;
437+
int err;
438+
int nclosed = 0;
437439

438440
/* prevent multiple coroutines running this function simultaniously */
439441
if (s->reclaiming) {
@@ -446,10 +448,10 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
446448
QSLIST_HEAD(, V9fsFidState) reclaim_list =
447449
QSLIST_HEAD_INITIALIZER(reclaim_list);
448450

451+
/* Pick FIDs to be closed, collect them on reclaim_list. */
449452
while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &f)) {
450453
/*
451-
* Unlink fids cannot be reclaimed. Check
452-
* for them and skip them. Also skip fids
454+
* Unlinked fids cannot be reclaimed, skip those, and also skip fids
453455
* currently being operated on.
454456
*/
455457
if (f->ref || f->flags & FID_NON_RECLAIMABLE) {
@@ -499,17 +501,26 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
499501
}
500502
}
501503
/*
502-
* Now close the fid in reclaim list. Free them if they
503-
* are already clunked.
504+
* Close the picked FIDs altogether on a background I/O driver thread. Do
505+
* this all at once to keep latency (i.e. amount of thread hops between main
506+
* thread <-> fs driver background thread) as low as possible.
504507
*/
508+
v9fs_co_run_in_worker({
509+
QSLIST_FOREACH(f, &reclaim_list, reclaim_next) {
510+
err = (f->fid_type == P9_FID_DIR) ?
511+
s->ops->closedir(&s->ctx, &f->fs_reclaim) :
512+
s->ops->close(&s->ctx, &f->fs_reclaim);
513+
if (!err) {
514+
/* total_open_fd must only be mutated on main thread */
515+
nclosed++;
516+
}
517+
}
518+
});
519+
total_open_fd -= nclosed;
520+
/* Free the closed FIDs. */
505521
while (!QSLIST_EMPTY(&reclaim_list)) {
506522
f = QSLIST_FIRST(&reclaim_list);
507523
QSLIST_REMOVE(&reclaim_list, f, V9fsFidState, reclaim_next);
508-
if (f->fid_type == P9_FID_FILE) {
509-
v9fs_co_close(pdu, &f->fs_reclaim);
510-
} else if (f->fid_type == P9_FID_DIR) {
511-
v9fs_co_closedir(pdu, &f->fs_reclaim);
512-
}
513524
/*
514525
* Now drop the fid reference, free it
515526
* if clunked.

0 commit comments

Comments
 (0)