On Wed, 3 Nov 2021 22:40:51 +0200 Tudor Cornea <tudor.cor...@gmail.com> wrote:
> On Tue, 2 Nov 2021 at 17:53, Stephen Hemminger <step...@networkplumber.org> > wrote: > > > On Tue, 2 Nov 2021 17:51:13 +0200 > > Tudor Cornea <tudor.cor...@gmail.com> wrote: > > > > > +#ifdef RTE_KNI_PREEMPT_DEFAULT > > > +module_param(min_scheduling_interval, long, 0644); > > > +MODULE_PARM_DESC(min_scheduling_interval, > > > +"Kni thread min scheduling interval (default=100 microseconds):\n" > > > +"\t\t" > > > +); > > > > Why the non-standard newline's and tab's? > > Please try to make KNI look like other kernel code. > > > > Hi Stephen, > > I tried to base the description of the new parameters on an existing > parameter implemented for the rte_kni module - carrier. > > module_param(carrier, charp, 0644); > MODULE_PARM_DESC(carrier, > "Default carrier state for KNI interface (default=off):\n" > "\t\toff Interfaces will be created with carrier state set to off.\n" > "\t\ton Interfaces will be created with carrier state set to on.\n" > "\t\t" > ); > > I thought about keeping the compatibility in terms of coding style with the > existing Kni module parameters. > Upon browsing the Linux tree, I realise it might not be standard ( > checkpatch.pl , interestingly didn't seem to complain about the patch) > > I also realise now, that I missed two tabs at the beginning of the params > description. > Should I add the missing tabs, so that the new parameters that I intend to > add through this patch are similar in style to the existing ones, or should > I remove the newlines and tabs altogether, when specifying the description > for min_scheduling_interval and max_scheduling_interval ? > > Thanks, > Tudor Although KNI is unlikely to ever get upstream code review, there is no reason to deviate from common practice in kernel drivers. The original module parameter was doing something unconventional. Not wrong, just different.
