[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 7/8] hw/9pfs: annotate V9fsTransport callbacks as coroutine_fn
- To: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
- From: Philippe Mathieu-Daudé <philmd@xxxxxxxxxxxxxxxx>
- Date: Tue, 21 Jul 2026 10:25:34 +0200
- Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=qcppdkim1 header.d=qualcomm.com header.i="@qualcomm.com" header.h="Cc:Content-Transfer-Encoding:Content-Type:Date:From:In-Reply-To:Message-ID:MIME-Version:References:Subject:To"; dkim=pass header.s=google header.d=oss.qualcomm.com header.i="@oss.qualcomm.com" header.h="Content-Transfer-Encoding:Content-Type:In-Reply-To:From:References:Cc:To:Content-Language:Subject:User-Agent:MIME-Version:Date:Message-ID"
- Cc: Christian Schoenebeck <qemu_oss@xxxxxxxxxxxxx>, qemu-devel@xxxxxxxxxx, Markus Armbruster <armbru@xxxxxxxxxx>, Peter Xu <peterx@xxxxxxxxxx>, Fabiano Rosas <farosas@xxxxxxx>, Daniel P. Berrangé <berrange@xxxxxxxxxx>, John Snow <jsnow@xxxxxxxxxx>, Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxxx>, Kevin Wolf <kwolf@xxxxxxxxxx>, Hanna Reitz <hreitz@xxxxxxxxxx>, qemu-block@xxxxxxxxxx, Stefan Hajnoczi <stefanha@xxxxxxxxxx>, Fam Zheng <fam@xxxxxxxxxx>, "Denis V. Lunev" <den@xxxxxxxxxx>, Greg Kurz <groug@xxxxxxxx>, "Michael S. Tsirkin" <mst@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Anthony PERARD <anthony@xxxxxxxxxxxxxx>, "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Li Zhijian <lizhijian@xxxxxxxxxxx>
- Delivery-date: Tue, 21 Jul 2026 08:25:43 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 21/7/26 10:06, Marc-André Lureau wrote:
Hi
On Tue, Jul 21, 2026 at 11:58 AM Marc-André Lureau
<marcandre.lureau@xxxxxxxxxx> wrote:
Hi
On Tue, Jul 21, 2026 at 12:54 AM Philippe Mathieu-Daudé
<philmd@xxxxxxxxxxxxxxxx> wrote:
On 20/7/26 22:42, Philippe Mathieu-Daudé wrote:
On 20/7/26 16:02, Christian Schoenebeck wrote:
On Monday, 20 July 2026 09:55:36 CEST Marc-André Lureau wrote:
All V9fsTransport callbacks are invoked exclusively from coroutine
context (the v9fs_* PDU handlers). Annotate the function pointer
types in V9fsTransport and all implementations (virtio and xen
backends), as well as intermediate callers in 9p.c (pdu_marshal,
pdu_unmarshal, v9fs_init_qiov_from_pdu, etc.).
Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
---
hw/9pfs/9p.h | 26 +++++++++++++++-----------
hw/9pfs/9p.c | 31 +++++++++++++++++++------------
hw/9pfs/virtio-9p-device.c | 24 +++++++++++++-----------
hw/9pfs/xen-9p-backend.c | 34 +++++++++++++++++-----------------
4 files changed, 64 insertions(+), 51 deletions(-)
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 1a309664f6e..0e52ffbdf38 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -472,17 +472,21 @@ void pdu_submit(V9fsPDU *pdu, P9MsgHeader *hdr);
void v9fs_reset(V9fsState *s);
struct V9fsTransport {
- ssize_t (*pdu_vmarshal)(V9fsPDU *pdu, size_t offset, const
char *fmt,
- va_list ap);
- ssize_t (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const
char *fmt,
- va_list ap);
- void (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec
**piov,
- unsigned int *pniov, size_t
size);
- void (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec
**piov,
- unsigned int *pniov, size_t
size);
- void (*push_and_notify)(V9fsPDU *pdu);
- size_t (*msize_limit)(V9fsState *s);
- size_t (*response_buffer_size)(V9fsPDU *pdu);
+ ssize_t coroutine_fn (*pdu_vmarshal)(V9fsPDU *pdu, size_t
offset,
+ const char *fmt,
va_list ap);
+ ssize_t coroutine_fn (*pdu_vunmarshal)(V9fsPDU *pdu, size_t
offset,
+ const char *fmt,
va_list ap);
+ void coroutine_fn (*init_in_iov_from_pdu)(V9fsPDU *pdu,
+ struct iovec
**piov,
+ unsigned int
*pniov,
+ size_t size);
+ void coroutine_fn (*init_out_iov_from_pdu)(V9fsPDU *pdu,
+ struct iovec
**piov,
+ unsigned int
*pniov,
+ size_t size);
+ void coroutine_fn (*push_and_notify)(V9fsPDU *pdu);
+ size_t coroutine_fn (*msize_limit)(V9fsState *s);
+ size_t coroutine_fn (*response_buffer_size)(V9fsPDU *pdu);
};
I wonder whether that could be made more diff/blame-friendly.
Especially as
this currently only serves documentation purposes.
They way you are positioning coroutine_fn it usually resolves to an
attribute
attached to the function's return type, not to the function itself, even
though clang/gcc handling it flexible. For attaching an attribute to the
function itself the attribute should either be attached before the
function
declaration or after the function declaration.
I tend to aggree it would be clearer to have these attributes /before/
the function prototype declaration.
$ git grep -E '^(static )?coroutine_(mixed_)?fn' | wc -l
90
More like
$ git grep -rnE '\b(coroutine_fn|co_wrapper[a-z_]*)\b' | wc -l
1343
That matching is not accurate, it's actually hard enough to match all
annotations instances.. but this should be a rough estimation.
most of them are "return-type co_annotation fn(..)"
we even have scripts/block-coroutine-wrapper.py that relies and
produce this order..
a bit late to change the style imho...
At least, let's not make this a requirement for this series, please
Not a requirement on my side, but if we had to pick a style,
I'd vote for "attribute before return type".
block/block-copy.c:47:static coroutine_fn int
block_copy_task_entry(AioTask *task);
block/copy-before-write.c:106:static coroutine_fn int
cbw_do_copy_before_write(BlockDriverState *bs,
block/nvme.c:1351:static coroutine_fn int nvme_co_flush(BlockDriverState
*bs)
block/ssh.c:1223:static coroutine_fn int ssh_flush(BDRVSSHState *s,
BlockDriverState *bs)
nbd/server.c:3077:static coroutine_fn void nbd_trip(void *opaque)
block/block-copy.c:581:static coroutine_fn int
block_copy_task_entry(AioTask *task)
...
In both cases you could then
add it to its own, separate line, avoiding a lot of noise.
/Christian
|
