Re: [PATCH v2 2/9] block-copy: add missing coroutine_fn annotations

[Paolo Bonzini]

On 11/15/22 16:41, Emanuele Giuseppe Esposito wrote:
> To sum up on what was discussed in this serie, I don't really see any
> strong objection against these patches, so I will soon send v3 which is
> pretty much the same except for patch 1, which will be removed.
>
> I think these patches are useful and will be even more meaningful to the
> reviewer when in the next few days I send all the rwlock patches.

Yes, I agree.

FWIW I implemented path search in vrc and it found 133 candidates 
(functions that are only called by coroutine_fn are not coroutine_fns 
themselves).  I only list them after the signature because as expected, 
most of them are pointless; however there are some are obviously correct:

1) some have _co_ in their name :)

2) these five directly call a generated_co_wrapper so they're an easy catch:

vhdx_log_write_and_flush    -> bdrv_flush
vhdx_log_write_and_flush    -> bdrv_pread
vhdx_log_write_and_flush    -> bdrv_pwrite
mirror_flush                -> blk_flush
qcow2_check_refcounts       -> bdrv_pwrite
qcow2_check_refcounts       -> bdrv_pwrite_sync
qcow2_check_refcounts       -> bdrv_pread
qcow2_read_extensions       -> bdrv_pread
check_directory_consistency -> bdrv_pwrite

(vrc lets me query this with "paths [coroutine_fn_candidate] 
[no_coroutine_fn]")

3) I can also query (with "paths [coroutine_fn_candidate] ... 
[no_coroutine_fn]") those that end up calling a generated_co_wrapper. 
Among these, vrc catches block_copy_reset_unallocated from this patch:

block_copy_reset_unallocated
block_crypto_co_create_generic
calculate_l2_meta
check_directory_consistency
commit_direntries
commit_one_file
is_zero
mirror_flush
qcow2_alloc_bytes
qcow2_alloc_cluster_abort
qcow2_alloc_clusters_at
qcow2_check_refcounts
qcow2_get_last_cluster
qcow2_read_extensions
qcow2_read_snapshots
qcow2_truncate_bitmaps_check
qcow2_update_options
vhdx_log_write_and_flush
vmdk_is_cid_valid
zero_l2_subclusters

Another possibility is to identify common "entry points" in the paths to 
the no_coroutine_fn and make them generated_co_wrappers.  For example in 
qcow2 these include bitmap_list_load, update_refcount and 
get_cluster_table and the qcow2_snapshot_* functions.

Of course the analysis would have to be rerun after doing every change.

The most time consuming part is labeling coroutine_fn/no_coroutine_fn, 
which would be useful to do with clang (and at this point you might as 
well extract the CFG with it).  Doing the queries totally by hand 
doesn't quite scale (for example vrc's blind spot is inlining and I 
forgot to disable it, but I only noticed too late...), but it should be 
scriptable since after all VRC is just a Python package + a nice CLI.

Thanks,

Paolo



label coroutine_fn_candidate aio_get_thread_pool
label coroutine_fn_candidate aio_task_pool_free
label coroutine_fn_candidate aio_task_pool_status
label coroutine_fn_candidate bdrv_bsc_fill
label coroutine_fn_candidate bdrv_bsc_invalidate_range
label coroutine_fn_candidate bdrv_bsc_is_data
label coroutine_fn_candidate bdrv_can_write_zeroes_with_unmap
label coroutine_fn_candidate bdrv_check_request
label coroutine_fn_candidate bdrv_dirty_bitmap_get
label coroutine_fn_candidate bdrv_dirty_bitmap_get_locked
label coroutine_fn_candidate bdrv_dirty_bitmap_lock
label coroutine_fn_candidate bdrv_dirty_bitmap_next_dirty_area
label coroutine_fn_candidate bdrv_dirty_bitmap_next_zero
label coroutine_fn_candidate bdrv_dirty_bitmap_set_inconsistent
label coroutine_fn_candidate bdrv_dirty_bitmap_status
label coroutine_fn_candidate bdrv_dirty_bitmap_truncate
label coroutine_fn_candidate bdrv_dirty_bitmap_unlock
label coroutine_fn_candidate bdrv_dirty_iter_free
label coroutine_fn_candidate bdrv_dirty_iter_new
label coroutine_fn_candidate bdrv_dirty_iter_next
label coroutine_fn_candidate bdrv_has_readonly_bitmaps
label coroutine_fn_candidate bdrv_inc_in_flight
label coroutine_fn_candidate bdrv_min_mem_align
label coroutine_fn_candidate bdrv_pad_request
label coroutine_fn_candidate bdrv_probe_all
label coroutine_fn_candidate bdrv_reset_dirty_bitmap_locked
label coroutine_fn_candidate bdrv_round_to_clusters
label coroutine_fn_candidate bdrv_set_dirty
label coroutine_fn_candidate bdrv_set_dirty_iter
label coroutine_fn_candidate bdrv_write_threshold_check_write
label coroutine_fn_candidate blk_check_byte_request
label coroutine_fn_candidate blkverify_err
label coroutine_fn_candidate block_copy_async
label coroutine_fn_candidate block_copy_call_cancel
label coroutine_fn_candidate block_copy_call_cancelled
label coroutine_fn_candidate block_copy_call_failed
label coroutine_fn_candidate block_copy_call_finished
label coroutine_fn_candidate block_copy_call_free
label coroutine_fn_candidate block_copy_call_status
label coroutine_fn_candidate block_copy_call_succeeded
label coroutine_fn_candidate block_copy_reset_unallocated
label coroutine_fn_candidate block_copy_set_skip_unallocated
label coroutine_fn_candidate block_crypto_co_create_generic
label coroutine_fn_candidate BlockDriver.bdrv_aio_flush
label coroutine_fn_candidate BlockDriver.bdrv_aio_ioctl
label coroutine_fn_candidate BlockDriver.bdrv_aio_pdiscard
label coroutine_fn_candidate BlockDriver.bdrv_aio_preadv
label coroutine_fn_candidate BlockDriver.bdrv_aio_pwritev
label coroutine_fn_candidate BlockDriver.bdrv_co_drain_end
label coroutine_fn_candidate block_job_ratelimit_get_delay
label coroutine_fn_candidate block_job_ratelimit_get_delay
label coroutine_fn_candidate block_status
label coroutine_fn_candidate calculate_l2_meta
label coroutine_fn_candidate check_directory_consistency
label coroutine_fn_candidate commit_direntries
label coroutine_fn_candidate commit_one_file
label coroutine_fn_candidate count_single_write_clusters
label coroutine_fn_candidate iov_send_recv
label coroutine_fn_candidate is_allocated_sectors
label coroutine_fn_candidate iscsi_allocmap_is_allocated
label coroutine_fn_candidate iscsi_allocmap_is_valid
label coroutine_fn_candidate iscsi_allocmap_update
label coroutine_fn_candidate iscsi_populate_target_desc
label coroutine_fn_candidate is_sector_request_lun_aligned
label coroutine_fn_candidate is_zero
label coroutine_fn_candidate job_cancel_requested
label coroutine_fn_candidate job_enter
label coroutine_fn_candidate job_is_cancelled
label coroutine_fn_candidate job_progress_increase_remaining
label coroutine_fn_candidate job_progress_set_remaining
label coroutine_fn_candidate job_progress_update
label coroutine_fn_candidate job_transition_to_ready
label coroutine_fn_candidate mirror_flush
label coroutine_fn_candidate mirror_perform
label coroutine_fn_candidate nbd_client_will_reconnect
label coroutine_fn_candidate nbd_client_will_reconnect
label coroutine_fn_candidate nbd_err_lookup
label coroutine_fn_candidate nbd_errno_to_system_errno
label coroutine_fn_candidate nbd_iter_channel_error
label coroutine_fn_candidate nfs_file_co_create
label coroutine_fn_candidate nvme_get_free_req
label coroutine_fn_candidate nvme_put_free_req_and_wake
label coroutine_fn_candidate pstrcat
label coroutine_fn_candidate qapi_event_send_quorum_failure
label coroutine_fn_candidate qcow2_alloc_bytes
label coroutine_fn_candidate qcow2_alloc_cluster_abort
label coroutine_fn_candidate qcow2_alloc_clusters_at
label coroutine_fn_candidate qcow2_check_refcounts
label coroutine_fn_candidate qcow2_check_vmstate_request
label coroutine_fn_candidate qcow2_get_last_cluster
label coroutine_fn_candidate qcow2_read_extensions
label coroutine_fn_candidate qcow2_read_snapshots
label coroutine_fn_candidate qcow2_truncate_bitmaps_check
label coroutine_fn_candidate qcow2_update_options
label coroutine_fn_candidate qcrypto_block_decrypt
label coroutine_fn_candidate qcrypto_block_encrypt
label coroutine_fn_candidate qdict_rename_keys
label coroutine_fn_candidate qed_alloc_l2_cache_entry
label coroutine_fn_candidate qed_alloc_table
label coroutine_fn_candidate qed_commit_l2_cache_entry
label coroutine_fn_candidate qed_set_used_clusters
label coroutine_fn_candidate qemu_blockalign0
label coroutine_fn_candidate qemu_coroutine_enter_if_inactive
label coroutine_fn_candidate qemu_iovec_clone
label coroutine_fn_candidate qemu_iovec_compare
label coroutine_fn_candidate qemu_iovec_init_slice
label coroutine_fn_candidate qemu_iovec_is_zero
label coroutine_fn_candidate qemu_iovec_subvec_niov
label coroutine_fn_candidate qemu_iovec_to_buf
label coroutine_fn_candidate qemu_rbd_co_create
label coroutine_fn_candidate qio_channel_readv
label coroutine_fn_candidate qio_channel_readv_all
label coroutine_fn_candidate qio_channel_readv_all_eof
label coroutine_fn_candidate quorum_copy_qiov
label coroutine_fn_candidate quorum_report_bad_acb
label coroutine_fn_candidate quorum_vote_error
label coroutine_fn_candidate reqlist_find_conflict
label coroutine_fn_candidate reqlist_init_req
label coroutine_fn_candidate rule_check
label coroutine_fn_candidate throttle_account
label coroutine_fn_candidate tracked_request_set_serialising
label coroutine_fn_candidate vhdx_block_translate
label coroutine_fn_candidate vhdx_log_write_and_flush
label coroutine_fn_candidate vhdx_metadata_entry_le_export
label coroutine_fn_candidate vhdx_metadata_header_le_export
label coroutine_fn_candidate vhdx_region_entry_le_export
label coroutine_fn_candidate vhdx_region_header_le_export
label coroutine_fn_candidate vhdx_update_bat_table_entry
label coroutine_fn_candidate vmdk_is_cid_valid
label coroutine_fn_candidate warn_reportf_err
label coroutine_fn_candidate yank_register_function
label coroutine_fn_candidate zero_l2_subclusters