9pfs: performance, Windows host prep, tests restructure

* Highlight of this PR is Linus Heckemann's GHashTable patch which
   brings massive general performance improvements of 9p server
   somewhere between factor 6 .. 12.
 
 * Bin Meng's g_mkdir patch is a preparatory patch for upcoming
   Windows host support of 9p server.
 
 * The rest of the patches in this PR are 9p test code restructuring
   and refactoring changes to improve readability and to ease
   maintenance of 9p test code on the long-term.
 -----BEGIN PGP SIGNATURE-----
 
 iQJLBAABCgA1FiEEltjREM96+AhPiFkBNMK1h2Wkc5UFAmNWbs8XHHFlbXVfb3Nz
 QGNydWRlYnl0ZS5jb20ACgkQNMK1h2Wkc5V4cw/8CqoSJqoJixlP8kAGDYWq3CgF
 SKd09rIzLSWyyufAoZr1TqLwRrvEQRlZJSpL4fGvRpQLv0IQCu4x59ohHRob25Tm
 Fe7IxYBNuBwLW4yu+Y7FaujeGoYAi9Qw5q4ijq3/aSSiIeuXySKB2JmW71CQ+Tbe
 uwivsnMtWzQ7qsNwrtXYbxDs7UGkdsiW2sEQUS26GMApAXZoB+38hwtTW2Y9MOrC
 58JuZza/fUVPzo0V1D0ggRawb5O2VTF5fz8aGFG4FvoyIW6DDZFSfnyre9QxivOl
 5McWwSQ/D04vdEK9ornGPYr9YRGuP8g07p1EW9OfKeie4I41e9pS3UminK5lVCgo
 SfBHzz96efM5XR+Wnl4yVKowivmTqjwUU8lDqW2eB/7YBRuYUzrpxYe//UPv4q1J
 zaQV3pgwFAVkVJCnkcLCa1JQbH581bXSsuRlDdYqoRYfyzXoxbywNjvn9BXE0PrG
 WRecS//GyN3GVZYxMwb3H052110pYsYIg2YZ2H4QiqCwpEHHvy+L/ZXm19vbDm7B
 GYJQPUK8/y0NGwZsUYcUSx1TWlU9ZPwrbqZfv7e7+B6FL4VNjdaqb8PvS9admWSq
 LOSzrVVIus+nb7tP99d1Fb6oRyCy3x8E48gTr5UtTJHC4SAw/OBJmem6GOc/D490
 H7Dq8Y27qsQ6fT7iPm8=
 =MxSG
 -----END PGP SIGNATURE-----

Merge tag 'pull-9p-20221024' of https://github.com/cschoenebeck/qemu into staging

9pfs: performance, Windows host prep, tests restructure

* Highlight of this PR is Linus Heckemann's GHashTable patch which
  brings massive general performance improvements of 9p server
  somewhere between factor 6 .. 12.

* Bin Meng's g_mkdir patch is a preparatory patch for upcoming
  Windows host support of 9p server.

* The rest of the patches in this PR are 9p test code restructuring
  and refactoring changes to improve readability and to ease
  maintenance of 9p test code on the long-term.

# -----BEGIN PGP SIGNATURE-----
#
# iQJLBAABCgA1FiEEltjREM96+AhPiFkBNMK1h2Wkc5UFAmNWbs8XHHFlbXVfb3Nz
# QGNydWRlYnl0ZS5jb20ACgkQNMK1h2Wkc5V4cw/8CqoSJqoJixlP8kAGDYWq3CgF
# SKd09rIzLSWyyufAoZr1TqLwRrvEQRlZJSpL4fGvRpQLv0IQCu4x59ohHRob25Tm
# Fe7IxYBNuBwLW4yu+Y7FaujeGoYAi9Qw5q4ijq3/aSSiIeuXySKB2JmW71CQ+Tbe
# uwivsnMtWzQ7qsNwrtXYbxDs7UGkdsiW2sEQUS26GMApAXZoB+38hwtTW2Y9MOrC
# 58JuZza/fUVPzo0V1D0ggRawb5O2VTF5fz8aGFG4FvoyIW6DDZFSfnyre9QxivOl
# 5McWwSQ/D04vdEK9ornGPYr9YRGuP8g07p1EW9OfKeie4I41e9pS3UminK5lVCgo
# SfBHzz96efM5XR+Wnl4yVKowivmTqjwUU8lDqW2eB/7YBRuYUzrpxYe//UPv4q1J
# zaQV3pgwFAVkVJCnkcLCa1JQbH581bXSsuRlDdYqoRYfyzXoxbywNjvn9BXE0PrG
# WRecS//GyN3GVZYxMwb3H052110pYsYIg2YZ2H4QiqCwpEHHvy+L/ZXm19vbDm7B
# GYJQPUK8/y0NGwZsUYcUSx1TWlU9ZPwrbqZfv7e7+B6FL4VNjdaqb8PvS9admWSq
# LOSzrVVIus+nb7tP99d1Fb6oRyCy3x8E48gTr5UtTJHC4SAw/OBJmem6GOc/D490
# H7Dq8Y27qsQ6fT7iPm8=
# =MxSG
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 24 Oct 2022 06:54:07 EDT
# gpg:                using RSA key 96D8D110CF7AF8084F88590134C2B58765A47395
# gpg:                issuer "qemu_oss@crudebyte.com"
# gpg: Good signature from "Christian Schoenebeck <qemu_oss@crudebyte.com>" [unknown]
# gpg: WARNING: The key's User ID is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: ECAB 1A45 4014 1413 BA38  4926 30DB 47C3 A012 D5F4
#      Subkey fingerprint: 96D8 D110 CF7A F808 4F88  5901 34C2 B587 65A4 7395

* tag 'pull-9p-20221024' of https://github.com/cschoenebeck/qemu: (23 commits)
  tests/9p: remove unnecessary g_strdup() calls
  tests/9p: merge v9fs_tunlinkat() and do_unlinkat()
  tests/9p: merge v9fs_tlink() and do_hardlink()
  tests/9p: merge v9fs_tsymlink() and do_symlink()
  tests/9p: merge v9fs_tlcreate() and do_lcreate()
  tests/9p: merge v9fs_tmkdir() and do_mkdir()
  tests/9p: convert v9fs_tflush() to declarative arguments
  tests/9p: simplify callers of twrite()
  tests/9p: convert v9fs_twrite() to declarative arguments
  tests/9p: simplify callers of tlopen()
  tests/9p: convert v9fs_tlopen() to declarative arguments
  tests/9p: simplify callers of treaddir()
  tests/9p: convert v9fs_treaddir() to declarative arguments
  tests/9p: simplify callers of tgetattr()
  tests/9p: convert v9fs_tgetattr() to declarative arguments
  tests/9p: simplify callers of tattach()
  tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid()
  tests/9p: merge v9fs_tversion() and do_version()
  tests/9p: simplify callers of twalk()
  tests/9p: merge *walk*() functions
  ...

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This commit is contained in:
Stefan Hajnoczi 2022-10-24 14:27:12 -04:00
commit e750a7ace4
7 changed files with 1867 additions and 1177 deletions

View file

@ -256,7 +256,8 @@ static size_t v9fs_string_size(V9fsString *str)
}
/*
* returns 0 if fid got re-opened, 1 if not, < 0 on error */
* returns 0 if fid got re-opened, 1 if not, < 0 on error
*/
static int coroutine_fn v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f)
{
int err = 1;
@ -282,33 +283,32 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, int32_t fid)
V9fsFidState *f;
V9fsState *s = pdu->s;
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
if (f) {
BUG_ON(f->clunked);
if (f->fid == fid) {
/*
* Update the fid ref upfront so that
* we don't get reclaimed when we yield
* in open later.
*/
f->ref++;
/*
* check whether we need to reopen the
* file. We might have closed the fd
* while trying to free up some file
* descriptors.
*/
err = v9fs_reopen_fid(pdu, f);
if (err < 0) {
f->ref--;
return NULL;
}
/*
* Mark the fid as referenced so that the LRU
* reclaim won't close the file descriptor
*/
f->flags |= FID_REFERENCED;
return f;
/*
* Update the fid ref upfront so that
* we don't get reclaimed when we yield
* in open later.
*/
f->ref++;
/*
* check whether we need to reopen the
* file. We might have closed the fd
* while trying to free up some file
* descriptors.
*/
err = v9fs_reopen_fid(pdu, f);
if (err < 0) {
f->ref--;
return NULL;
}
/*
* Mark the fid as referenced so that the LRU
* reclaim won't close the file descriptor
*/
f->flags |= FID_REFERENCED;
return f;
}
return NULL;
}
@ -317,12 +317,11 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *f;
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
if (f) {
/* If fid is already there return NULL */
BUG_ON(f->clunked);
if (f->fid == fid) {
return NULL;
}
return NULL;
}
f = g_new0(V9fsFidState, 1);
f->fid = fid;
@ -333,7 +332,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
* reclaim won't close the file descriptor
*/
f->flags |= FID_REFERENCED;
QSIMPLEQ_INSERT_TAIL(&s->fid_list, f, next);
g_hash_table_insert(s->fids, GINT_TO_POINTER(fid), f);
v9fs_readdir_init(s->proto_version, &f->fs.dir);
v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir);
@ -424,12 +423,12 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *fidp;
QSIMPLEQ_FOREACH(fidp, &s->fid_list, next) {
if (fidp->fid == fid) {
QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
fidp->clunked = true;
return fidp;
}
/* TODO: Use g_hash_table_steal_extended() instead? */
fidp = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
if (fidp) {
g_hash_table_remove(s->fids, GINT_TO_POINTER(fid));
fidp->clunked = true;
return fidp;
}
return NULL;
}
@ -439,10 +438,15 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
int reclaim_count = 0;
V9fsState *s = pdu->s;
V9fsFidState *f;
GHashTableIter iter;
gpointer fid;
g_hash_table_iter_init(&iter, s->fids);
QSLIST_HEAD(, V9fsFidState) reclaim_list =
QSLIST_HEAD_INITIALIZER(reclaim_list);
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &f)) {
/*
* Unlink fids cannot be reclaimed. Check
* for them and skip them. Also skip fids
@ -514,72 +518,85 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
}
}
/*
* This is used when a path is removed from the directory tree. Any
* fids that still reference it must not be closed from then on, since
* they cannot be reopened.
*/
static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path)
{
int err;
int err = 0;
V9fsState *s = pdu->s;
V9fsFidState *fidp, *fidp_next;
V9fsFidState *fidp;
gpointer fid;
GHashTableIter iter;
/*
* The most common case is probably that we have exactly one
* fid for the given path, so preallocate exactly one.
*/
g_autoptr(GArray) to_reopen = g_array_sized_new(FALSE, FALSE,
sizeof(V9fsFidState *), 1);
gint i;
fidp = QSIMPLEQ_FIRST(&s->fid_list);
if (!fidp) {
return 0;
}
g_hash_table_iter_init(&iter, s->fids);
/*
* v9fs_reopen_fid() can yield : a reference on the fid must be held
* to ensure its pointer remains valid and we can safely pass it to
* QSIMPLEQ_NEXT(). The corresponding put_fid() can also yield so
* we must keep a reference on the next fid as well. So the logic here
* is to get a reference on a fid and only put it back during the next
* iteration after we could get a reference on the next fid. Start with
* the first one.
* We iterate over the fid table looking for the entries we need
* to reopen, and store them in to_reopen. This is because
* v9fs_reopen_fid() and put_fid() yield. This allows the fid table
* to be modified in the meantime, invalidating our iterator.
*/
for (fidp->ref++; fidp; fidp = fidp_next) {
while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &fidp)) {
if (fidp->path.size == path->size &&
!memcmp(fidp->path.data, path->data, path->size)) {
/* Mark the fid non reclaimable. */
fidp->flags |= FID_NON_RECLAIMABLE;
/* reopen the file/dir if already closed */
err = v9fs_reopen_fid(pdu, fidp);
if (err < 0) {
put_fid(pdu, fidp);
return err;
}
}
fidp_next = QSIMPLEQ_NEXT(fidp, next);
if (fidp_next) {
/*
* Ensure the next fid survives a potential clunk request during
* put_fid() below and v9fs_reopen_fid() in the next iteration.
* Ensure the fid survives a potential clunk request during
* v9fs_reopen_fid or put_fid.
*/
fidp_next->ref++;
fidp->ref++;
fidp->flags |= FID_NON_RECLAIMABLE;
g_array_append_val(to_reopen, fidp);
}
/* We're done with this fid */
put_fid(pdu, fidp);
}
return 0;
for (i = 0; i < to_reopen->len; i++) {
fidp = g_array_index(to_reopen, V9fsFidState*, i);
/* reopen the file/dir if already closed */
err = v9fs_reopen_fid(pdu, fidp);
if (err < 0) {
break;
}
}
for (i = 0; i < to_reopen->len; i++) {
put_fid(pdu, g_array_index(to_reopen, V9fsFidState*, i));
}
return err;
}
static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
{
V9fsState *s = pdu->s;
V9fsFidState *fidp;
GList *freeing;
/*
* Get a list of all the values (fid states) in the table, which
* we then...
*/
g_autoptr(GList) fids = g_hash_table_get_values(s->fids);
/* Free all fids */
while (!QSIMPLEQ_EMPTY(&s->fid_list)) {
/* Get fid */
fidp = QSIMPLEQ_FIRST(&s->fid_list);
/* ... remove from the table, taking over ownership. */
g_hash_table_steal_all(s->fids);
/*
* This allows us to release our references to them asynchronously without
* iterating over the hash table and risking iterator invalidation
* through concurrent modifications.
*/
for (freeing = fids; freeing; freeing = freeing->next) {
fidp = freeing->data;
fidp->ref++;
/* Clunk fid */
QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
fidp->clunked = true;
put_fid(pdu, fidp);
}
}
@ -3205,6 +3222,8 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
V9fsFidState *tfidp;
V9fsState *s = pdu->s;
V9fsFidState *dirfidp = NULL;
GHashTableIter iter;
gpointer fid;
v9fs_path_init(&new_path);
if (newdirfid != -1) {
@ -3238,11 +3257,13 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
if (err < 0) {
goto out;
}
/*
* Fixup fid's pointing to the old name to
* start pointing to the new name
*/
QSIMPLEQ_FOREACH(tfidp, &s->fid_list, next) {
g_hash_table_iter_init(&iter, s->fids);
while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &tfidp)) {
if (v9fs_path_is_ancestor(&fidp->path, &tfidp->path)) {
/* replace the name */
v9fs_fix_path(&tfidp->path, &new_path, strlen(fidp->path.data));
@ -3320,6 +3341,8 @@ static int coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir,
V9fsPath oldpath, newpath;
V9fsState *s = pdu->s;
int err;
GHashTableIter iter;
gpointer fid;
v9fs_path_init(&oldpath);
v9fs_path_init(&newpath);
@ -3336,7 +3359,8 @@ static int coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir,
* Fixup fid's pointing to the old name to
* start pointing to the new name
*/
QSIMPLEQ_FOREACH(tfidp, &s->fid_list, next) {
g_hash_table_iter_init(&iter, s->fids);
while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &tfidp)) {
if (v9fs_path_is_ancestor(&oldpath, &tfidp->path)) {
/* replace the name */
v9fs_fix_path(&tfidp->path, &newpath, strlen(oldpath.data));
@ -4226,7 +4250,7 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
s->ctx.fmode = fse->fmode;
s->ctx.dmode = fse->dmode;
QSIMPLEQ_INIT(&s->fid_list);
s->fids = g_hash_table_new(NULL, NULL);
qemu_co_rwlock_init(&s->rename_lock);
if (s->ops->init(&s->ctx, errp) < 0) {
@ -4286,6 +4310,10 @@ void v9fs_device_unrealize_common(V9fsState *s)
if (s->ctx.fst) {
fsdev_throttle_cleanup(s->ctx.fst);
}
if (s->fids) {
g_hash_table_destroy(s->fids);
s->fids = NULL;
}
g_free(s->tag);
qp_table_destroy(&s->qpd_table);
qp_table_destroy(&s->qpp_table);

View file

@ -339,7 +339,7 @@ typedef struct {
struct V9fsState {
QLIST_HEAD(, V9fsPDU) free_list;
QLIST_HEAD(, V9fsPDU) active_list;
QSIMPLEQ_HEAD(, V9fsFidState) fid_list;
GHashTable *fids;
FileOperations *ops;
FsContext ctx;
char *tag;