All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org>
To: qemu-devel@nongnu.org
Cc: "Stefan Hajnoczi" <stefanha@gmail.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Greg Kurz" <groug@kaod.org>,
	"Antonios Motakis" <antonios.motakis@huawei.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: [Qemu-devel] [PATCH v7 2/3] 9p: stat_to_qid: implement slow path
Date: Thu, 5 Sep 2019 00:05:03 +0200	[thread overview]
Message-ID: <fdbcf8b7735f93f04215b244b36385ac5b6020b7.1567680121.git.qemu_oss@crudebyte.com> (raw)
In-Reply-To: <cover.1567680121.git.qemu_oss@crudebyte.com>

From: Christian Schoenebeck <qemu_oss@crudebyte.com>

stat_to_qid attempts via qid_path_prefixmap to map unique files (which are
identified by 64 bit inode nr and 32 bit device id) to a 64 QID path value.
However this implementation makes some assumptions about inode number
generation on the host.

If qid_path_prefixmap fails, we still have 48 bits available in the QID
path to fall back to a less memory efficient full mapping.

Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com>
[CS: - Rebased to https://github.com/gkurz/qemu/commits/9p-next
       (SHA1 7fc4c49e91).
     - Updated hash calls to new xxhash API.
     - Removed unnecessary parantheses in qpf_lookup_func().
     - Removed unnecessary g_malloc0() result checks.
     - Log error message when running out of prefixes in
       qid_path_fullmap().
     - Log warning message about potential degraded performance in
       qid_path_prefixmap().
     - Wrapped qpf_table initialization to dedicated qpf_table_init()
       function.
     - Fixed typo in comment. ]
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/9pfs/9p.h |  9 +++++++
 2 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 8eb89c5c7d..d9be2d45d3 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -579,23 +579,34 @@ static uint32_t qpp_hash(QppEntry e)
     return qemu_xxhash7(e.ino_prefix, e.dev, 0, 0, 0);
 }
 
+static uint32_t qpf_hash(QpfEntry e)
+{
+    return qemu_xxhash7(e.ino, e.dev, 0, 0, 0);
+}
+
 static bool qpp_lookup_func(const void *obj, const void *userp)
 {
     const QppEntry *e1 = obj, *e2 = userp;
     return e1->dev == e2->dev && e1->ino_prefix == e2->ino_prefix;
 }
 
-static void qpp_table_remove(void *p, uint32_t h, void *up)
+static bool qpf_lookup_func(const void *obj, const void *userp)
+{
+    const QpfEntry *e1 = obj, *e2 = userp;
+    return e1->dev == e2->dev && e1->ino == e2->ino;
+}
+
+static void qp_table_remove(void *p, uint32_t h, void *up)
 {
     g_free(p);
 }
 
-static void qpp_table_destroy(struct qht *ht)
+static void qp_table_destroy(struct qht *ht)
 {
     if (!ht || !ht->map) {
         return;
     }
-    qht_iter(ht, qpp_table_remove, NULL);
+    qht_iter(ht, qp_table_remove, NULL);
     qht_destroy(ht);
 }
 
@@ -604,6 +615,50 @@ static void qpp_table_init(struct qht *ht)
     qht_init(ht, qpp_lookup_func, 1, QHT_MODE_AUTO_RESIZE);
 }
 
+static void qpf_table_init(struct qht *ht)
+{
+    qht_init(ht, qpf_lookup_func, 1 << 16, QHT_MODE_AUTO_RESIZE);
+}
+
+static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
+                            uint64_t *path)
+{
+    QpfEntry lookup = {
+        .dev = stbuf->st_dev,
+        .ino = stbuf->st_ino
+    }, *val;
+    uint32_t hash = qpf_hash(lookup);
+
+    /* most users won't need the fullmap, so init the table lazily */
+    if (!pdu->s->qpf_table.map) {
+        qpf_table_init(&pdu->s->qpf_table);
+    }
+
+    val = qht_lookup(&pdu->s->qpf_table, &lookup, hash);
+
+    if (!val) {
+        if (pdu->s->qp_fullpath_next == 0) {
+            /* no more files can be mapped :'( */
+            error_report_once(
+                "9p: No more prefixes available for remapping inodes from "
+                "host to guest."
+            );
+            return -ENFILE;
+        }
+
+        val = g_malloc0(sizeof(QppEntry));
+        *val = lookup;
+
+        /* new unique inode and device combo */
+        val->path = pdu->s->qp_fullpath_next++;
+        pdu->s->qp_fullpath_next &= QPATH_INO_MASK;
+        qht_insert(&pdu->s->qpf_table, val, hash, NULL);
+    }
+
+    *path = val->path;
+    return 0;
+}
+
 /*
  * stat_to_qid needs to map inode number (64 bits) and device id (32 bits)
  * to a unique QID path (64 bits). To avoid having to map and keep track
@@ -629,9 +684,8 @@ static int qid_path_prefixmap(V9fsPDU *pdu, const struct stat *stbuf,
     if (!val) {
         if (pdu->s->qp_prefix_next == 0) {
             /* we ran out of prefixes */
-            error_report_once(
-                "9p: No more prefixes available for remapping inodes from "
-                "host to guest."
+            warn_report_once(
+                "9p: Potential degraded performance of inode remapping"
             );
             return -ENFILE;
         }
@@ -656,6 +710,10 @@ static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID *qidp)
     if (pdu->s->ctx.export_flags & V9FS_REMAP_INODES) {
         /* map inode+device to qid path (fast path) */
         err = qid_path_prefixmap(pdu, stbuf, &qidp->path);
+        if (err == -ENFILE) {
+            /* fast path didn't work, fall back to full map */
+            err = qid_path_fullmap(pdu, stbuf, &qidp->path);
+        }
         if (err) {
             return err;
         }
@@ -3820,6 +3878,7 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
 
     qpp_table_init(&s->qpp_table);
     s->qp_prefix_next = 1; /* reserve 0 to detect overflow */
+    s->qp_fullpath_next = 1;
 
     s->ctx.fst = &fse->fst;
     fsdev_throttle_init(s->ctx.fst);
@@ -3842,7 +3901,8 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
         fsdev_throttle_cleanup(s->ctx.fst);
     }
     g_free(s->tag);
-    qpp_table_destroy(&s->qpp_table);
+    qp_table_destroy(&s->qpp_table);
+    qp_table_destroy(&s->qpf_table);
     g_free(s->ctx.fs_root);
 }
 
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 7262fe80aa..35a362c0d7 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -245,6 +245,13 @@ typedef struct {
     uint16_t qp_prefix;
 } QppEntry;
 
+/* QID path full entry, as above */
+typedef struct {
+    dev_t dev;
+    ino_t ino;
+    uint64_t path;
+} QpfEntry;
+
 struct V9fsState
 {
     QLIST_HEAD(, V9fsPDU) free_list;
@@ -268,7 +275,9 @@ struct V9fsState
     V9fsQID root_qid;
     dev_t dev_id;
     struct qht qpp_table;
+    struct qht qpf_table;
     uint16_t qp_prefix_next;
+    uint64_t qp_fullpath_next;
 };
 
 /* 9p2000.L open flags */
-- 
2.20.1



  parent reply	other threads:[~2019-09-05 12:12 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-05 10:42 [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
2019-09-04 21:34 ` [Qemu-devel] [PATCH v7 1/3] 9p: Added virtfs option 'multidevs=remap|forbid|warn' Christian Schoenebeck via Qemu-devel
2019-09-04 22:05 ` Christian Schoenebeck via Qemu-devel [this message]
2019-09-04 22:53 ` [Qemu-devel] [PATCH v7 3/3] 9p: Use variable length suffixes for inode remapping Christian Schoenebeck via Qemu-devel
2019-09-13 17:01 ` [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions Greg Kurz
2019-09-23  9:50   ` Christian Schoenebeck via
2019-09-23 12:56     ` Greg Kurz
2019-09-23 14:06       ` Christian Schoenebeck via
2019-09-23 14:46         ` Greg Kurz
2019-09-23 15:03           ` Christian Schoenebeck via
2019-09-23 16:50             ` Greg Kurz
2019-09-24  9:31               ` Christian Schoenebeck via
2019-10-08  9:14                 ` Greg Kurz
2019-10-08 12:05                   ` Christian Schoenebeck
2019-10-08 13:47                     ` Greg Kurz
2019-10-08 14:25                       ` Christian Schoenebeck
2019-10-08 14:45                         ` Greg Kurz
2019-10-15  9:20                     ` Greg Kurz
2019-10-16  9:42                       ` virtio-fs: Fix file ID collisions (was: 9p: Fix file ID collisions) Christian Schoenebeck
2019-10-16 13:44                         ` Dr. David Alan Gilbert
2019-10-18 13:15                           ` Christian Schoenebeck
2019-10-16 14:00                         ` Greg Kurz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fdbcf8b7735f93f04215b244b36385ac5b6020b7.1567680121.git.qemu_oss@crudebyte.com \
    --to=qemu-devel@nongnu.org \
    --cc=antonios.motakis@huawei.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=groug@kaod.org \
    --cc=qemu_oss@crudebyte.com \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.