log: add function name, line number prefix

[kernelchuk]

See also #250
Add "function-name:line-number" prefix and a new-line suffix for all messages. Use do-while(0) to define the metal_log macro. Add convenience macros ML_ERR, ML_WRN, ML_INF, ML_DBG to avoid using excessively long and redundant "metal_log(METAL_LOG_*".

I had to change s/irq/IRQ/ to fix the checkpatch warning:
EMBEDDED_FUNCTION_NAME: Prefer using '"%s...", __func__' to using 'irq', this function's name, in a string

[arnopo]

@kernelchuk do you have some measurement of the increase of the footprint adding this?

Any reason to not merge this PR with #250?

[kernelchuk]

Any reason to not merge this PR with #250?

None. I just pushed a new branch as a replacement for #250. Should I go back to #250?

@kernelchuk do you have some measurement of the increase of the footprint adding this?

I estimate the upper bound to be less than 1000 bytes:

total len M     function_name 
===== === = =======================; M = number of log messages
   23  23 1 metal_bus_register
   48  25 1 metal_bus_unregister
   72  24 1 metal_get_timestamp
   98  26 1 metal_init_page_sizes
  125  27 1 metal_linux_irq_notify
  146  21 1 metal_shmem_open
  173  27 1 metal_softirq_allocate
  199  26 1 metal_uio_dev_dma_map
  261  31 2 metal_linux_irq_set_enable
  319  29 2 metal_linux_probe_driver
  371  26 2 metal_uio_dev_irq_ack
  411  20 2 metal_virt2phys
  459  24 2 metal_xlnx_irq_init
  519  30 2 metal_xlnx_irq_set_enable
  591  24 3 metal_add_page_size
  666  25 3 metal_linux_irq_init
  753  29 3 metal_linux_irq_shutdown
  804  17 3 metal_mktemp
  876  24 3 metal_shmem_try_map
  952  19 4 metal_sys_init
 1097  29 5 metal_linux_irq_handling
 1235  23 6 metal_uio_dev_bind
 1419  23 8 metal_uio_dev_open

Where 'total' is a running total of bytes; 'len' is the length of "function-name:line-number"; M is the number of log messages in the function. I'm assuming that all line numbers are 3 digits long, so counting as 5 for ":nnn ". We get under 1000 bytes if we adjust for the exiting __func__ strings in the following functions:

total len N     function_name
===== === = =======================; N = number of __func__ in the replaced messages
   24  24 1 metal_linux_irq_notify
   50  26 1 metal_linux_irq_shutdown
   73  23 1 metal_uio_dev_dma_map
   93  20 1 metal_uio_dev_open
  149  28 2 metal_linux_irq_set_enable
  201  26 2 metal_linux_probe_driver
  247  23 2 metal_uio_dev_irq_ack
  289  21 2 metal_xlnx_irq_init
  343  27 2 metal_xlnx_irq_set_enable
  473  26 5 metal_linux_irq_handling

We can mitigate the increased memory footprint with conditional compilation, borrowing from printk in the kernel.

[edmooring]

This should be conditional. Many deployments of OpenAMP will not have any log output visible to the user, so adding extra size is not useful for them.

[kernelchuk]

This should be conditional. Many deployments of OpenAMP will not have any log output visible to the user, so adding extra size is not useful for them.

Yes, I'm working on a version to make it conditional.

[kernelchuk]

Hi @arnopo,
I got the openamp-docs-libmetal-prs build error:

The configuration file required to build documentation is missing from your project.
Add a configuration file to your project to make it build successfully. Read more at
https://docs.readthedocs.io/en/stable/config-file/v2.html

I have scanned the readthedocs.io link, but still not sure how to avoid it. Any hints?

[arnopo]

Hi @arnopo, I got the openamp-docs-libmetal-prs build error:

The configuration file required to build documentation is missing from your project.
Add a configuration file to your project to make it build successfully. Read more at
https://docs.readthedocs.io/en/stable/config-file/v2.html

I have scanned the readthedocs.io link, but still not sure how to avoid it. Any hints?

@wmamills : I guess we can ignore the doc generation issue for now, right?

[wmamills]

@arnopo, yes ignore it until that enabling PR is accepted. The hook is installed already but the needed file is not yet in main.

[arnopo]

Much better§
Few new remarks

[tnmysh]

@arnopo If all looks good, can we include the change in coming release as well?

[tnmysh]

@kernelchuk Please close #250 as it's duplicate of this.