On Thu, Apr 29, 2021 at 11:32 AM Pearson, Robert B <rpearsonhpe@xxxxxxxxx> wrote: > > > On 4/28/2021 10:20 PM, Zhu Yanjun wrote: > > On Thu, Apr 29, 2021 at 11:07 AM Pearson, Robert B > > <rpearsonhpe@xxxxxxxxx> wrote: > >> > >> On 4/28/2021 7:54 PM, Zhu Yanjun wrote: > >>> Adding a prefix makes debug easy. This can help filter the functions. > >>> And a prefix can help us to verify the location of the function. > >>> > >>> Zhu Yanjun > >> For comparison here are all the subroutine names from a well known > >> driver file. As you can see, 100% of the non static subroutines *do* > >> have mlx5 in their name but the majority of the static ones *do not*, > >> and these are by definition not shared anywhere else but this file. This > >> is representative of the typical style in linux-rdma. > > With perf or ftrace debug tools, a lot of functions will pop out. > > A prefix can help the developer to locate the functions source code easily. > > And a prefix can help the developer filter the functions conveniently. > > > > This is why I recommend a prefix. > > > > Zhu Yanjun > > Yes, I can tell. We're still debating this. Obviously the developers of > mr.c don't entirely agree and I don't either. There are places where it > makes sense but the code is ugly IMHO. I think you should let developers A prefix can make code easy to debug, then easy to maintain. This is not ugly code.^o^ On the contrary, it is beautiful code. ^o^ > write in the style they are most effective with, especially in the > context of a local static subroutine which have a very narrow scope. Even though the local static functions still possibly appear in kernel debug tools, it is necessary to add a prefix to make it easy to locate and filter. This can make maintenance easy. Zhu Yanjun > > Bob > > > > >> Bob > >> > >> static TYPE add_keys() > >> static TYPE alloc_cacheable_mr() > >> static TYPE alloc_cache_mr() > >> static TYPE assign_mkey_variant() > >> static TYPE __cache_work_func() > >> static TYPE cache_work_func() > >> static TYPE can_use_umr_rereg_access() > >> static TYPE can_use_umr_rereg_pas() > >> static TYPE clean_keys() > >> static TYPE create_cache_mr() > >> static TYPE create_mkey_callback() > >> static TYPE create_real_mr() > >> static TYPE create_user_odp_mr() > >> static TYPE delayed_cache_work_func() > >> static TYPE delay_time_func() > >> static TYPE destroy_mkey() > >> static TYPE get_cache_mr() > >> static TYPE get_octo_len() > >> static TYPE limit_read() > >> static TYPE limit_write() > >> static TYPE mlx5_alloc_integrity_descs() > >> static TYPE mlx5_alloc_mem_reg_descs() > >> static TYPE _mlx5_alloc_mkey_descs() > >> static TYPE mlx5_alloc_priv_descs() > >> static TYPE mlx5_alloc_sg_gaps_descs() > >> static TYPE mlx5_free_priv_descs() > >> static TYPE __mlx5_ib_alloc_mr() > >> static TYPE mlx5_ib_alloc_pi_mr() > >> static TYPE *mlx5_ib_alloc_xlt() > >> static TYPE mlx5_ib_create_mkey() > >> static TYPE mlx5_ib_create_mkey_cb() > >> static TYPE *mlx5_ib_create_xlt_wr() > >> static TYPE mlx5_ib_dmabuf_invalidate_cb() > >> static TYPE mlx5_ib_free_xlt() > >> static TYPE mlx5_ib_get_dm_mr() > >> static TYPE mlx5_ib_init_umr_context() > >> static TYPE mlx5_ib_map_klm_mr_sg_pi() > >> static TYPE mlx5_ib_map_mtt_mr_sg_pi() > >> static TYPE mlx5_ib_map_pa_mr_sg_pi() > >> static TYPE mlx5_ib_post_send_wait() > >> static TYPE mlx5_ib_sg_to_klms() > >> static TYPE mlx5_ib_umr_done() > >> static TYPE mlx5_ib_unmap_free_xlt() > >> static TYPE mlx5_mr_cache_debugfs_cleanup() > >> static TYPE mlx5_mr_cache_debugfs_init() > >> static TYPE mlx5_mr_cache_free() > >> static TYPE mlx5_set_page() > >> static TYPE mlx5_set_page_pi() > >> static TYPE mlx5_set_umr_free_mkey() > >> static TYPE mlx5_umem_dmabuf_default_pgsz() > >> static TYPE mr_cache_ent_from_order() > >> static TYPE mr_cache_max_order() > >> static TYPE queue_adjust_cache_locked() > >> static TYPE reg_create() > >> static TYPE remove_cache_mr_locked() > >> static TYPE resize_available_mrs() > >> static TYPE revoke_mr() > >> static TYPE set_mkc_access_pd_addr_fields() > >> static TYPE set_mr_fields() > >> static TYPE size_read() > >> static TYPE size_write() > >> static TYPE someone_adding() > >> static TYPE umr_can_use_indirect_mkey() > >> static TYPE umr_rereg_pas() > >> static TYPE umr_rereg_pd_access() > >> static TYPE xlt_wr_final_send_flags() > >> TYPE mlx5_ib_advise_mr() > >> TYPE mlx5_ib_alloc_mr() > >> TYPE mlx5_ib_alloc_mr_integrity() > >> TYPE mlx5_ib_alloc_mw() > >> TYPE mlx5_ib_check_mr_status() > >> TYPE mlx5_ib_dealloc_mw() > >> TYPE mlx5_ib_dereg_mr() > >> TYPE mlx5_ib_get_dma_mr() > >> TYPE mlx5_ib_map_mr_sg() > >> TYPE mlx5_ib_map_mr_sg_pi() > >> TYPE mlx5_ib_reg_dm_mr() > >> TYPE mlx5_ib_reg_user_mr() > >> TYPE mlx5_ib_reg_user_mr_dmabuf() > >> TYPE mlx5_ib_rereg_user_mr() > >> TYPE mlx5_ib_update_mr_pas() > >> TYPE mlx5_ib_update_xlt() > >> TYPE mlx5_mr_cache_cleanup() > >> TYPE mlx5_mr_cache_init() > >>
