[net-next,v2,1/2] can-isotp: implement cleanups / improvements from review

As pointed out by Jakub Kicinski here:
http://lore.kernel.org/r/20201009175751.5c54097f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com
this patch addresses the remarked issues:

- remove empty line in comment
- remove default=y for CAN_ISOTP in Kconfig
- make use of pr_notice_once()
- use GFP_KERNEL instead of gfp_any() in soft hrtimer context
- make use of pr_fmt() [suggested my Marc Kleine-Budde]

The version strings in the CAN subsystem are removed by a separate patch.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 include/uapi/linux/can/isotp.h |  1 -
 net/can/Kconfig                |  3 ++-
 net/can/isotp.c                | 19 ++++++++++---------
 3 files changed, 12 insertions(+), 11 deletions(-)

On Sun, 11 Oct 2020 11:24:07 +0200 Oliver Hartkopp wrote:
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index e6ff032b5426..22187669c5c9 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -79,6 +79,8 @@ MODULE_LICENSE("Dual BSD/GPL");
>  MODULE_AUTHOR("Oliver Hartkopp <socketcan@hartkopp.net>");
>  MODULE_ALIAS("can-proto-6");
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

You need to move this before the includes:

net/can/isotp.c:82: warning: "pr_fmt" redefined
   82 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
      | 
In file included from ../include/linux/kernel.h:15,
                 from ../include/linux/list.h:9,
                 from ../include/linux/module.h:12,
                 from ../net/can/isotp.c:56:
include/linux/printk.h:297: note: this is the location of the previous definition
  297 | #define pr_fmt(fmt) fmt
      | 
net/can/isotp.c:82:9: warning: preprocessor token pr_fmt redefined
net/can/isotp.c: note: in included file (through ../include/linux/kernel.h, ../include/linux/list.h, ../include/linux/module.h):
include/linux/printk.h:297:9: this was the original definition

On Sun, 11 Oct 2020 11:24:07 +0200 Oliver Hartkopp wrote:
> @@ -769,7 +771,7 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)

>  

>  isotp_tx_burst:

>  		skb = alloc_skb(so->ll.mtu + sizeof(struct can_skb_priv),

> -				gfp_any());

> +				GFP_KERNEL);


hrtimer will need GFP_ATOMIC

Hi Oliver,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Oliver-Hartkopp/can-isotp-implement-cleanups-improvements-from-review/20201012-000055
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git df41c19abbeaae6e24c8e31cff5fdb48632be380
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/693c9b4e8c5710e7b536ec9d6a969a7d7343a100
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oliver-Hartkopp/can-isotp-implement-cleanups-improvements-from-review/20201012-000055
        git checkout 693c9b4e8c5710e7b536ec9d6a969a7d7343a100
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/can/isotp.c:82: warning: "pr_fmt" redefined

      82 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
         | 
   In file included from include/linux/kernel.h:15,
                    from include/linux/list.h:9,
                    from include/linux/module.h:12,
                    from net/can/isotp.c:56:
   include/linux/printk.h:297: note: this is the location of the previous definition
     297 | #define pr_fmt(fmt) fmt
         | 

vim +/pr_fmt +82 net/can/isotp.c

    81	
  > 82	#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

    83	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

On 11.10.20 17:44, Jakub Kicinski wrote:
> On Sun, 11 Oct 2020 11:24:07 +0200 Oliver Hartkopp wrote:
>> diff --git a/net/can/isotp.c b/net/can/isotp.c
>> index e6ff032b5426..22187669c5c9 100644
>> --- a/net/can/isotp.c
>> +++ b/net/can/isotp.c
>> @@ -79,6 +79,8 @@ MODULE_LICENSE("Dual BSD/GPL");
>>   MODULE_AUTHOR("Oliver Hartkopp <socketcan@hartkopp.net>");
>>   MODULE_ALIAS("can-proto-6");
>>   
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> You need to move this before the includes:
> 
> net/can/isotp.c:82: warning: "pr_fmt" redefined
>     82 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>        |
> In file included from ../include/linux/kernel.h:15,
>                   from ../include/linux/list.h:9,
>                   from ../include/linux/module.h:12,
>                   from ../net/can/isotp.c:56:
> include/linux/printk.h:297: note: this is the location of the previous definition
>    297 | #define pr_fmt(fmt) fmt
>        |
> net/can/isotp.c:82:9: warning: preprocessor token pr_fmt redefined
> net/can/isotp.c: note: in included file (through ../include/linux/kernel.h, ../include/linux/list.h, ../include/linux/module.h):
> include/linux/printk.h:297:9: this was the original definition
> 

Hm - don't know why my build process didn't complain about it. Or why I 
possibly overlooked it.

I'll do the cosmetic pr_fmt() improvements for the entire CAN network 
layer stuff later as it is not relevant for this net-next window.

The v3 patch will fix the GFP_ATOMIC too.

Thanks for your patience,
Oliver