[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5] 9pfs: use GHashTable for fid table
From: |
Christian Schoenebeck |
Subject: |
Re: [PATCH v5] 9pfs: use GHashTable for fid table |
Date: |
Tue, 27 Sep 2022 21:10:52 +0200 |
On Dienstag, 27. September 2022 17:14:57 CEST Linus Heckemann wrote:
> 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.
Always welcome! :)
https://wiki.qemu.org/Documentation/9p#Test_Cases
If you do, then please add the test as a separate patch.
Best regards,
Christian Schoenebeck