[PATCH] vringh: add MODULE_DESCRIPTION()

[Jeff Johnson]

Fix the allmodconfig 'make w=1' issue:

WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/vhost/vringh.o

Signed-off-by: Jeff Johnson 
---
 drivers/vhost/vringh.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 7b8fd977f71c..73e153f9b449 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -1614,4 +1614,5 @@ EXPORT_SYMBOL(vringh_need_notify_iotlb);
 
 #endif
 
+MODULE_DESCRIPTION("host side of a virtio ring");
 MODULE_LICENSE("GPL");

---
base-commit: 7f094f0e3866f83ca705519b1e8f5a7d6ecce232
change-id: 20240516-md-vringh-c43803ae0ba4

[PATCH] tracing: Fix trace_pid_list_free() kernel-doc

[Jeff Johnson]

make C=1 reports:

kernel/trace/pid_list.c:458: warning: Function parameter or struct member 
'pid_list' not described in 'trace_pid_list_free'

Add the missing parameter to the trace_pid_list_free() kernel-doc.

Signed-off-by: Jeff Johnson 
---
 kernel/trace/pid_list.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/pid_list.c b/kernel/trace/pid_list.c
index 95106d02b32d..19b271a12c99 100644
--- a/kernel/trace/pid_list.c
+++ b/kernel/trace/pid_list.c
@@ -451,6 +451,7 @@ struct trace_pid_list *trace_pid_list_alloc(void)
 
 /**
  * trace_pid_list_free - Frees an allocated pid_list.
+ * @pid_list: The pid list to free.
  *
  * Frees the memory for a pid_list that was allocated.
  */

---
base-commit: dd5a440a31fae6e459c0d627162825505361
change-id: 20240506-trace_pid_list_free-kdoc-e2bf15be84ee

Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()

[Jeff Johnson]

On 2/23/2024 9:56 AM, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" 
> 
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.9 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
> 
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.

Just curious if this could be done piecemeal by first changing the
macros to be variadic macros which allows you to ignore the extra
argument. The callers could then be modified in their separate trees.
And then once all the callers have be merged, the macros could be
changed to no longer be variadic.

Re: [net-next PATCH v3 3/3] net: phy: add support for PHY package MMD read/write

[Jeff Johnson]

On 12/5/2023 10:14 AM, Russell King (Oracle) wrote:
> On Tue, Dec 05, 2023 at 09:44:05AM -0800, Jeff Johnson wrote:
>> So in my experience a function prototype IS the function definition, and
>> the actual function is just the implementation of that definition.
>>
>> But that thinking obviously isn't shared by others.
> 
> Interestingly, the view that a function prototype is a function
> definition does not seem to be shared by w3school, Microsoft, IBM,
> and many more.
> 
> If we look at the C99 standard, then 6.9.1 Function definitions gives
> the syntax as including a compound-statement, which is defined as
> requiring the curley braces and contents. Therefore, a function
> definition as defined by the C standard includes its body.
> 

Note I was speaking in terms of functional languages in general, not C
specifically. Perhaps I should have used the term "specification"
instead of "definition" (which would align with the Ada terminology).

Having worked with closed-source systems, especially VxWorks, for many
years (where the header files contain all the documentation), it just
seems strange to embed the documentation in the .c files.

/jeff

Re: [net-next PATCH v3 3/3] net: phy: add support for PHY package MMD read/write

[Jeff Johnson]

On 12/5/2023 8:11 AM, Russell King (Oracle) wrote:
> On Tue, Dec 05, 2023 at 07:29:12AM -0800, Jakub Kicinski wrote:
>> On Tue, 5 Dec 2023 15:10:50 + Russell King (Oracle) wrote:
>>> I've raised this before in other subsystems, and it's suggested that
>>> it's better to have it in the .c file. I guess the reason is that it's
>>> more obvious that the function is documented when modifying it, so
>>> there's a higher probability that the kdoc will get updated when the
>>> function is altered.
>>
>> Plus I think people using IDEs (i.e. not me) may use the "jump to
>> definition" functionality, to find the doc? 
>>
>> TBH I thought putting kdoc in the C source was documented in the coding
>> style, but I can't find any mention of it now.
> 
> Well, in Documentation/doc-guide/kernel-doc.rst:
> 
>   The function and type kernel-doc comments should be placed just before
>   the function or type being described in order to maximise the chance
>   that somebody changing the code will also change the documentation.
> 
> That implies (but not explicitly) that it should be at the function
> definition site, since "changing the code" is used as an argument as
> I did in my previous email.
> 
> Secondly, this document goes on to give an example of running
> scripts/kernel-doc on a .c file.
> 
> Thirdly, there are seven references in this document of kernel-doc
> in .c files, and only one for kernel-doc in a .h file. So this suggests
> that "it will be in a .c file" isn't a rule (it can't be because of
> documenting structures!)
> 
> So let's not get hung up on whether it should be in .c or .h because I
> think that isn't relevant. Instead, I think it's about "it should be at
> the definition site" - that being a structure definition or a function
> definition, and not at a function prototype.
> 
> The only exception I can think of is the style I've used in
> linux/phylink.h for the _method_ definitions which look like function
> prototypes - that's just a work-around because one can't kernel-doc
> the structure-of-function-pointers and document the function parameters
> without jumping through that hoop, and it would be silly to document
> the methods in some random driver!
> 

The Linux Kernel philosophy of documenting functions instead of
prototypes has always bothered me since I'm "old school" and am
ingrained with the software engineering philosophy that you document
interfaces, not implementations. This was reinforced early in my career
by working on multiple projects in different programming languages using
processes outlined in DOD-STD-2167A, and for some projects, especially
ones written in Ada, the header files were the design and the documentation.

This philosophy was further enforced when working with closed source
projects (Windows, IOS, VxWorks) where all the documentation was
contained in shared header files.

So in my experience a function prototype IS the function definition, and
the actual function is just the implementation of that definition.

But that thinking obviously isn't shared by others.

/jeff

[PATCH v2 2/2] kbuild: handle excessively long argument lists

[Jeff Johnson]

From: Mahesh Kumar Kalikot Veetil 

Modules with a large number of compilation units may be
exceeding AR and LD command argument list. Handle this gracefully by
writing the long argument list in a file. The command line options
read from file are inserted in place of the original @file option.

The usage is well documented at
https://www.gnu.org/software/make/manual/html_node/File-Function.html

Signed-off-by: Mahesh Kumar Kalikot Veetil 
Signed-off-by: Jeff Johnson 
---

Changes in v2:
  - Remove spurious endif
  
scripts/Makefile.build | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 252b7d2..787dca2 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -425,7 +425,10 @@ $(obj)/lib.a: $(lib-y) FORCE
 # module is turned into a multi object module, $^ will contain header file
 # dependencies recorded in the .*.cmd file.
 quiet_cmd_link_multi-m = LD [M]  $@
-  cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
+  cmd_link_multi-m =   \
+   $(file >$@.in,$(filter %.o,$^)) \
+   $(LD) $(ld_flags) -r -o $@ @$@.in;  \
+   rm -f $@.in
 
 $(multi-used-m): FORCE
$(call if_changed,link_multi-m)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/2] kbuild: handle excessively long argument lists

[Jeff Johnson]

From: Mahesh Kumar Kalikot Veetil 

Modules with a large number of compilation units may be
exceeding AR and LD command argument list. Handle this gracefully by
writing the long argument list in a file. The command line options
read from file are inserted in place of the original @file option.

The usage is well documented at
https://www.gnu.org/software/make/manual/html_node/File-Function.html

Signed-off-by: Mahesh Kumar Kalikot Veetil 
Signed-off-by: Jeff Johnson 
---
 scripts/Makefile.build | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 252b7d2..d5ef345 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -425,7 +425,11 @@ $(obj)/lib.a: $(lib-y) FORCE
 # module is turned into a multi object module, $^ will contain header file
 # dependencies recorded in the .*.cmd file.
 quiet_cmd_link_multi-m = LD [M]  $@
-  cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
+  cmd_link_multi-m =   \
+   $(file >$@.in,$(filter %.o,$^)) \
+   $(LD) $(ld_flags) -r -o $@ @$@.in;  \
+   rm -f $@.in
+endif
 
 $(multi-used-m): FORCE
$(call if_changed,link_multi-m)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 1/2] kbuild: simplify cmd_mod

[Jeff Johnson]

From: Mahesh Kumar Kalikot Veetil 

Modules with a large number of compilation units can exceed execv
argument list resulting in E2BIG (Argument list too long) error.

Fix this by replacing shell 'echo > file' into a more native
$(file op filename[,text]) option.

Signed-off-by: Mahesh Kumar Kalikot Veetil 
Signed-off-by: Jeff Johnson 
---
 scripts/Makefile.build | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 4c058f1..252b7d2 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -279,10 +279,11 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) 
$(objtool_dep) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)
 
-cmd_mod = { \
-   echo $(if $($*-objs)$($*-y)$($*-m), $(addprefix $(obj)/, $($*-objs) 
$($*-y) $($*-m)), $(@:.mod=.o)); \
-   $(undefined_syms) echo; \
-   } > $@
+cmd_mod = $(file >$@,\
+   $(if $($*-objs)$($*-y)$($*-m), \
+   $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)), \
+   $(@:.mod=.o))) \
+   $(undefined_syms) echo >> $@
 
 $(obj)/%.mod: $(obj)/%.o FORCE
$(call if_changed,mod)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2] net: enable wireless core features with LEGACY_WEXT_ALLCONFIG

[Jeff Johnson]

On 2019-09-09 08:44, Johannes Berg wrote:

Also, you probably know this, but in this particular case you really
should just get rid of your wext dependencies


This.

Particularly for one out-of-tree driver with which I'm intimately 
familiar there has been considerable recent work to make all WEXT code 
correctly conditional, and nothing in the Android support should be 
reliant upon WEXT.

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Jeff Johnson]

On 2018-05-17 04:32, Ramon Fried wrote:

From: Eyal Ilsar 

...

+int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
+   struct ieee80211_vif *vif, void *ptt_msg, 
size_t len,
+   void **ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
+   int ret = 0;
+
+   mutex_lock(>hal_mutex);
+   p_msg_body = kmalloc(
+   sizeof(struct wcn36xx_hal_process_ptt_msg_req_msg) + len,
+   GFP_ATOMIC);


NULL check required?


+   INIT_HAL_PTT_MSG(p_msg_body, len);
+

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Jeff Johnson]

On 2018-05-17 04:32, Ramon Fried wrote:

From: Eyal Ilsar 

...

+int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
+   struct ieee80211_vif *vif, void *ptt_msg, 
size_t len,
+   void **ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
+   int ret = 0;
+
+   mutex_lock(>hal_mutex);
+   p_msg_body = kmalloc(
+   sizeof(struct wcn36xx_hal_process_ptt_msg_req_msg) + len,
+   GFP_ATOMIC);


NULL check required?


+   INIT_HAL_PTT_MSG(p_msg_body, len);
+

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Jeff Johnson]

On 2018-05-17 04:32, Ramon Fried wrote:

From: Eyal Ilsar 

...

+static int wcn36xx_smd_process_ptt_msg_rsp(void *buf, size_t len,
+  void **p_ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_rsp_msg *rsp;
+   int ret = 0;


why initialize 'ret' when you immediately overwrite?


+   ret = wcn36xx_smd_rsp_status_check(buf, len);

...

+   if (rsp->header.len > 0) {
+   *p_ptt_rsp_msg = kmalloc(rsp->header.len, GFP_ATOMIC);


NULL check required?


+   memcpy(*p_ptt_rsp_msg, rsp->ptt_msg, rsp->header.len);
+   }
+   return ret;
+}
+
+int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
+   struct ieee80211_vif *vif, void *ptt_msg, 
size_t len,
+   void **ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
+   int ret = 0;


why initialize 'ret' when it is always overwritten before use?


+   ret = wcn36xx_smd_send_and_wait(wcn, p_msg_body->header.len);

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Jeff Johnson]

On 2018-05-17 04:32, Ramon Fried wrote:

From: Eyal Ilsar 

...

+static int wcn36xx_smd_process_ptt_msg_rsp(void *buf, size_t len,
+  void **p_ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_rsp_msg *rsp;
+   int ret = 0;


why initialize 'ret' when you immediately overwrite?


+   ret = wcn36xx_smd_rsp_status_check(buf, len);

...

+   if (rsp->header.len > 0) {
+   *p_ptt_rsp_msg = kmalloc(rsp->header.len, GFP_ATOMIC);


NULL check required?


+   memcpy(*p_ptt_rsp_msg, rsp->ptt_msg, rsp->header.len);
+   }
+   return ret;
+}
+
+int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
+   struct ieee80211_vif *vif, void *ptt_msg, 
size_t len,
+   void **ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
+   int ret = 0;


why initialize 'ret' when it is always overwritten before use?


+   ret = wcn36xx_smd_send_and_wait(wcn, p_msg_body->header.len);

2.6 diskstats and 4096 sector block devices?

[Jeff Johnson]

Greetings,

I am analyzing output in /proc/diskstats for block devices that have a 
4096B physical and logical sector size. 4KN or 4K-native disk drives.


8  33 sdc1 1289 0 10312 5893 3258606 53 1668254992 16804002 31 
583987 16807101


Fields 3 and 7 are sectors read and written respectively. For operations 
on a 4KN device should fields 3 and 7 be interpreted as number of 4K 
sectors written (total transferred = N * 8) or is each 4K sector 
operation reflected as 8 ticks to N?


I am expecting 1MB transfers to the device and using 'delta[field7] / 
delta[field5]' getting 128 which would point to diskstats handling 
everything in terms of 512B operations.


To multiply by 8 or not to multiply by 8, that is the question.. :-)

Thanks,

--Jeff

--
Jeff Johnson
Co-Founder
Aeon Computing

jeff.johnson "dot" aeoncomputing.com
www.aeoncomputing.com

4170 Morena Boulevard, Suite D - San Diego, CA 92117

High-performance Computing / Lustre Filesystems / Scale-out Storage

2.6 diskstats and 4096 sector block devices?

[Jeff Johnson]

Greetings,

I am analyzing output in /proc/diskstats for block devices that have a 
4096B physical and logical sector size. 4KN or 4K-native disk drives.


8  33 sdc1 1289 0 10312 5893 3258606 53 1668254992 16804002 31 
583987 16807101


Fields 3 and 7 are sectors read and written respectively. For operations 
on a 4KN device should fields 3 and 7 be interpreted as number of 4K 
sectors written (total transferred = N * 8) or is each 4K sector 
operation reflected as 8 ticks to N?


I am expecting 1MB transfers to the device and using 'delta[field7] / 
delta[field5]' getting 128 which would point to diskstats handling 
everything in terms of 512B operations.


To multiply by 8 or not to multiply by 8, that is the question.. :-)

Thanks,

--Jeff

--
Jeff Johnson
Co-Founder
Aeon Computing

jeff.johnson "dot" aeoncomputing.com
www.aeoncomputing.com

4170 Morena Boulevard, Suite D - San Diego, CA 92117

High-performance Computing / Lustre Filesystems / Scale-out Storage