[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5] 9pfs: use GHashTable for fid table
From: |
Linus Heckemann |
Subject: |
Re: [PATCH v5] 9pfs: use GHashTable for fid table |
Date: |
Tue, 27 Sep 2022 17:14:57 +0200 |
Linus Heckemann <git@sphalerite.org> writes:
> 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);
> - fidp->ref++;
> + /* ... remove from the table, taking over ownership. */
> + g_hash_table_steal_all(s->fids);
>
> - /* Clunk fid */
> - QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
> + /*
> + * 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++;
> fidp->clunked = true;
> -
> put_fid(pdu, fidp);
> }
> }
I'm not sure if this implementation is correct. I'm concerned that it
may result in dangling references, but haven't been able to find a
client that will send the TVERSION request on a connection that's
already been used in other ways, as opposed to when the connection is
first established. I suspect this will be very rare in general, so it
might be good to have a test case somewhere.