Messages in this thread | | | Date | Wed, 29 Aug 2018 06:43:16 +0200 | From | Dominique Martinet <> | Subject | Re: [V9fs-developer] [PATCH v2 2/2] 9p: Add refcount to p9_req_t |
| |
Dominique Martinet wrote on Tue, Aug 28, 2018: > I think I've found why (see below), so I'll push a fixed version after > some more testing and another thorough read -- at some point today, but > this hasn't been 'approved' explicitely so please review! :)
While the issue I pointed at was real, it wasn't what was causing the refcount leak I was observing -- the problem is that we didn't drop a ref when the request was successfully cancelled (e.g. the reply to the flush came and the original request didn't get replied to)
The reason for this was that there were multiple versions of the patch which alternated between doing the put in client.c after the cancelled callback inconditionally, and doing the put in each transport's cancelled() function, but virtio does not have this callback so that didn't get added in the final version (codeveloping is hard); so I've added an else() close to just issue a put if there is no callback.
(In the end, it felt better to have the req_put in the transport because trans_fd is making refcounting difficult with its list handling, and separating the put from the list removal would be more confusing than is gained by sharing code)
Anyway, that's starting to be quite different from the v2 so I'll send a v3 keeping Tomas as the author -- please check my edits are alright with you, Tomas.
Meanwhile I'll keep running tests, I'm now confident about virtio but want to spend more time on other transports again, so delaying the push to linux-next for a few more days...
-- Dominique
| |