Hi On Tue, Jul 21, 2026 at 11:58 AM Marc-André Lureau <[email protected]> wrote: > > Hi > > On Tue, Jul 21, 2026 at 12:54 AM Philippe Mathieu-Daudé > <[email protected]> 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 <[email protected]> > > >>> --- > > >>> 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 > > >> 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 > > >> > > >> > > >> > > >> > > > > > > > >
