* Re: [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*()
2024-11-10 12:58 [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*() Karol Przybylski
@ 2024-11-10 15:52 ` kernel test robot
2024-11-11 3:48 ` kernel test robot
2024-11-11 8:04 ` Hans Verkuil
2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2024-11-10 15:52 UTC (permalink / raw)
To: Karol Przybylski, hverkuil, mchehab
Cc: oe-kbuild-all, Karol Przybylski, linux-media, linux-kernel, skhan
Hi Karol,
kernel test robot noticed the following build errors:
[auto build test ERROR on linuxtv-media-stage/master]
[also build test ERROR on linus/master v6.12-rc6 next-20241108]
[cannot apply to media-tree/master sailus-media-tree/streams sailus-media-tree/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Karol-Przybylski/media-cec-extron-da-hd-4k-plus-add-return-check-for-wait_for_completion/20241110-210018
base: https://git.linuxtv.org/media_stage.git master
patch link: https://lore.kernel.org/r/20241110125814.1899076-1-karprzy7%40gmail.com
patch subject: [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*()
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20241110/202411102341.VuOHMHGh-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241110/202411102341.VuOHMHGh-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411102341.VuOHMHGh-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c: In function 'extron_read_edid':
>> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:563:17: error: 'ret' undeclared (first use in this function); did you mean 'net'?
563 | ret = wait_for_completion_killable_timeout(&extron->edid_completion,
| ^~~
| net
drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:563:17: note: each undeclared identifier is reported only once for each function it appears in
vim +563 drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
540
541 static void extron_read_edid(struct extron_port *port)
542 {
543 struct extron *extron = port->extron;
544 char cmd[10], reply[10];
545 unsigned int idx;
546
547 idx = port->port.port + (port->is_input ? 0 : extron->num_in_ports);
548 snprintf(cmd, sizeof(cmd), "WR%uEDID", idx);
549 snprintf(reply, sizeof(reply), "EdidR%u", idx);
550 if (mutex_lock_interruptible(&extron->edid_lock))
551 return;
552 if (port->read_edid)
553 goto unlock;
554 extron->edid_bytes_read = 0;
555 extron->edid_port = port;
556 port->edid_blocks = 0;
557 if (!port->has_edid)
558 goto no_edid;
559
560 extron->edid_reading = true;
561
562 if (!extron_send_and_wait(extron, port, cmd, reply)) {
> 563 ret = wait_for_completion_killable_timeout(&extron->edid_completion,
564 msecs_to_jiffies(1000));
565 if (ret < 0)
566 goto unlock;
567 }
568 if (port->edid_blocks) {
569 extron_parse_edid(port);
570 port->read_edid = true;
571 if (!port->is_input)
572 v4l2_ctrl_s_ctrl(port->ctrl_tx_edid_present, 1);
573 }
574 no_edid:
575 extron->edid_reading = false;
576 unlock:
577 mutex_unlock(&extron->edid_lock);
578 cancel_delayed_work_sync(&extron->work_update_edid);
579 if (manufacturer_name[0])
580 schedule_delayed_work(&extron->work_update_edid,
581 msecs_to_jiffies(1000));
582 }
583
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*()
2024-11-10 12:58 [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*() Karol Przybylski
2024-11-10 15:52 ` kernel test robot
@ 2024-11-11 3:48 ` kernel test robot
2024-11-11 8:04 ` Hans Verkuil
2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2024-11-11 3:48 UTC (permalink / raw)
To: Karol Przybylski, hverkuil, mchehab
Cc: llvm, oe-kbuild-all, Karol Przybylski, linux-media, linux-kernel, skhan
Hi Karol,
kernel test robot noticed the following build errors:
[auto build test ERROR on linuxtv-media-stage/master]
[also build test ERROR on linus/master v6.12-rc6 next-20241108]
[cannot apply to media-tree/master sailus-media-tree/streams sailus-media-tree/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Karol-Przybylski/media-cec-extron-da-hd-4k-plus-add-return-check-for-wait_for_completion/20241110-210018
base: https://git.linuxtv.org/media_stage.git master
patch link: https://lore.kernel.org/r/20241110125814.1899076-1-karprzy7%40gmail.com
patch subject: [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*()
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20241111/202411111022.sX8ZCJZ8-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241111/202411111022.sX8ZCJZ8-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411111022.sX8ZCJZ8-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:23:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:23:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:23:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:29:
In file included from drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h:13:
In file included from include/media/cec.h:19:
In file included from include/media/rc-core.h:13:
In file included from include/linux/kfifo.h:40:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:563:3: error: use of undeclared identifier 'ret'
563 | ret = wait_for_completion_killable_timeout(&extron->edid_completion,
| ^
drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:565:7: error: use of undeclared identifier 'ret'
565 | if (ret < 0)
| ^
7 warnings and 2 errors generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for MODVERSIONS
Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
Selected by [y]:
- RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]
WARNING: unmet direct dependencies detected for GET_FREE_REGION
Depends on [n]: SPARSEMEM [=n]
Selected by [m]:
- RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
vim +/ret +563 drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
540
541 static void extron_read_edid(struct extron_port *port)
542 {
543 struct extron *extron = port->extron;
544 char cmd[10], reply[10];
545 unsigned int idx;
546
547 idx = port->port.port + (port->is_input ? 0 : extron->num_in_ports);
548 snprintf(cmd, sizeof(cmd), "WR%uEDID", idx);
549 snprintf(reply, sizeof(reply), "EdidR%u", idx);
550 if (mutex_lock_interruptible(&extron->edid_lock))
551 return;
552 if (port->read_edid)
553 goto unlock;
554 extron->edid_bytes_read = 0;
555 extron->edid_port = port;
556 port->edid_blocks = 0;
557 if (!port->has_edid)
558 goto no_edid;
559
560 extron->edid_reading = true;
561
562 if (!extron_send_and_wait(extron, port, cmd, reply)) {
> 563 ret = wait_for_completion_killable_timeout(&extron->edid_completion,
564 msecs_to_jiffies(1000));
565 if (ret < 0)
566 goto unlock;
567 }
568 if (port->edid_blocks) {
569 extron_parse_edid(port);
570 port->read_edid = true;
571 if (!port->is_input)
572 v4l2_ctrl_s_ctrl(port->ctrl_tx_edid_present, 1);
573 }
574 no_edid:
575 extron->edid_reading = false;
576 unlock:
577 mutex_unlock(&extron->edid_lock);
578 cancel_delayed_work_sync(&extron->work_update_edid);
579 if (manufacturer_name[0])
580 schedule_delayed_work(&extron->work_update_edid,
581 msecs_to_jiffies(1000));
582 }
583
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*()
2024-11-10 12:58 [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*() Karol Przybylski
2024-11-10 15:52 ` kernel test robot
2024-11-11 3:48 ` kernel test robot
@ 2024-11-11 8:04 ` Hans Verkuil
2 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2024-11-11 8:04 UTC (permalink / raw)
To: Karol Przybylski, mchehab; +Cc: linux-media, linux-kernel, skhan
On 10/11/2024 13:58, Karol Przybylski wrote:
> According to scheduler/completion.rst, return status of wait_for_completion*()
> function variants should be checked or be accompanied by explanation.
>
> I examined code in extron-da-hd-4k-plus.c and it does look like the return value
> should be checked, but perhaps there is a reason for ignoring it.
There is no need to check the return code in this specific case.
If the wait was successful, then port->edid_blocks will be set, so
effectively that 'if' is the result check of the wait_for_completion
call.
Regards,
Hans
> I drafted a patch for this, but I'm not entirely sure how to approach error handling in this case.
>
> Discovered in coverity, CID1599679
>
> Signed-off-by: Karol Przybylski <karprzy7@gmail.com>
> ---
> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> index cfbfc4c1b2e6..83a790117411 100644
> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> @@ -559,9 +559,12 @@ static void extron_read_edid(struct extron_port *port)
>
> extron->edid_reading = true;
>
> - if (!extron_send_and_wait(extron, port, cmd, reply))
> - wait_for_completion_killable_timeout(&extron->edid_completion,
> + if (!extron_send_and_wait(extron, port, cmd, reply)) {
> + ret = wait_for_completion_killable_timeout(&extron->edid_completion,
> msecs_to_jiffies(1000));
> + if (ret < 0)
> + goto unlock;
> + }
> if (port->edid_blocks) {
> extron_parse_edid(port);
> port->read_edid = true;
^ permalink raw reply [flat|nested] 4+ messages in thread