Re: [PATCH stable v5.10 0/7] arm64: Default to 32-bit wide ZONE_DMA

[Jing Xiangfeng]

On 2021/3/7 23:24, Greg KH wrote:

On Thu, Mar 04, 2021 at 04:09:28PM +0100, Nicolas Saenz Julienne wrote:

On Thu, 2021-03-04 at 15:17 +0100, Greg KH wrote:

On Thu, Mar 04, 2021 at 03:05:32PM +0100, Nicolas Saenz Julienne wrote:

Hi Greg.

On Thu, 2021-03-04 at 14:46 +0100, Greg KH wrote:

On Wed, Mar 03, 2021 at 03:33:12PM +0800, Jing Xiangfeng wrote:

Using two distinct DMA zones turned out to be problematic. Here's an
attempt go back to a saner default.

What problem does this solve?  How does this fit into the stable kernel
rules?

We changed the way we setup memory zones in arm64 in order to cater for
Raspberry Pi 4's weird DMA constraints: ZONE_DMA spans the lower 1GB of memory
and ZONE_DMA32 the rest of the 32bit address space. Since you can't allocate
memory that crosses zone boundaries, this broke crashkernel allocations on big
machines. This series fixes all this by parsing the HW description and checking
for DMA constrained buses. When not found, the unnecessary zone creation is
skipped.

What kernel/commit caused this "breakage"?

1a8e1cef7603 arm64: use both ZONE_DMA and ZONE_DMA32

Thanks for the info, all now queued up.
There is a fix in 5.11. Please consider applying the following commit to 
5.10.y:


aed5041ef9a3 of: unittest: Fix build on architectures without 
CONFIG_OF_ADDRES


Thanks



greg k-h
.

[PATCH stable v5.10 5/7] arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges

[Jing Xiangfeng]

From: Nicolas Saenz Julienne 

commit 8424ecdde7df99d5426e1a1fd9f0fb36f4183032 upstream

We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)

The DMA layer also needs to be able to allocate memory that is
guaranteed to meet those DMA constraints, for bounce buffering as well
as allocating the backing for consistent mappings. This is why the 1 GB
ZONE_DMA was introduced recently. Unfortunately, it turns out the having
a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes problems with kdump, and
potentially in other places where allocations cannot cross zone
boundaries. Therefore, we should avoid having two separate DMA zones
when possible.

So, with the help of of_dma_get_max_cpu_address() get the topmost
physical address accessible to all DMA masters in system and use that
information to fine-tune ZONE_DMA's size. In the absence of addressing
limited masters ZONE_DMA will span the whole 32-bit address space,
otherwise, in the case of the Raspberry Pi 4 it'll only span the 30-bit
address space, and have ZONE_DMA32 cover the rest of the 32-bit address
space.

Signed-off-by: Nicolas Saenz Julienne 
Link: https://lore.kernel.org/r/20201119175400.9995-6-nsaenzjulie...@suse.de
Signed-off-by: Catalin Marinas 
Cc: 
Signed-off-by: Jing Xiangfeng 
---
 arch/arm64/mm/init.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 7da912bf4222..05a1c2773629 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -42,8 +42,6 @@
 #include 
 #include 
 
-#define ARM64_ZONE_DMA_BITS30
-
 /*
  * We need to be able to catch inadvertent references to memstart_addr
  * that occur (potentially in generic code) before arm64_memblock_init()
@@ -188,9 +186,11 @@ static phys_addr_t __init max_zone_phys(unsigned int 
zone_bits)
 static void __init zone_sizes_init(unsigned long min, unsigned long max)
 {
unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
+   unsigned int __maybe_unused dt_zone_dma_bits;
 
 #ifdef CONFIG_ZONE_DMA
-   zone_dma_bits = ARM64_ZONE_DMA_BITS;
+   dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
+   zone_dma_bits = min(32U, dt_zone_dma_bits);
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
-- 
2.25.1

[PATCH stable v5.10 6/7] arm64: mm: Set ZONE_DMA size based on early IORT scan

[Jing Xiangfeng]

From: Ard Biesheuvel 

commit 2b8652936f0ca9ca2e6c984ae76c7bfcda1b3f22 upstream

We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)

Instructing the DMA layer about these limitations is straight-forward,
even though we had to fix some issues regarding memory limits set in
the IORT for named components, and regarding the handling of ACPI _DMA
methods. However, the DMA layer also needs to be able to allocate
memory that is guaranteed to meet those DMA constraints, for bounce
buffering as well as allocating the backing for consistent mappings.

This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately,
it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes
problems with kdump, and potentially in other places where allocations
cannot cross zone boundaries. Therefore, we should avoid having two
separate DMA zones when possible.

So let's do an early scan of the IORT, and only create the ZONE_DMA
if we encounter any devices that need it. This puts the burden on
the firmware to describe such limitations in the IORT, which may be
redundant (and less precise) if _DMA methods are also being provided.
However, it should be noted that this situation is highly unusual for
arm64 ACPI machines. Also, the DMA subsystem still gives precedence to
the _DMA method if implemented, and so we will not lose the ability to
perform streaming DMA outside the ZONE_DMA if the _DMA method permits
it.

[nsaenz: unified implementation with DT's counterpart]

Signed-off-by: Ard Biesheuvel 
Signed-off-by: Nicolas Saenz Julienne 
Tested-by: Jeremy Linton 
Acked-by: Lorenzo Pieralisi 
Acked-by: Hanjun Guo 
Cc: Jeremy Linton 
Cc: Lorenzo Pieralisi 
Cc: Nicolas Saenz Julienne 
Cc: Rob Herring 
Cc: Christoph Hellwig 
Cc: Robin Murphy 
Cc: Hanjun Guo 
Cc: Sudeep Holla 
Cc: Anshuman Khandual 
Link: https://lore.kernel.org/r/20201119175400.9995-7-nsaenzjulie...@suse.de
Signed-off-by: Catalin Marinas 
Cc: 
Signed-off-by: Jing Xiangfeng 
---
 arch/arm64/mm/init.c  |  5 +++-
 drivers/acpi/arm64/iort.c | 55 +++
 include/linux/acpi_iort.h |  4 +++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 05a1c2773629..b913844ab740 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -186,11 +187,13 @@ static phys_addr_t __init max_zone_phys(unsigned int 
zone_bits)
 static void __init zone_sizes_init(unsigned long min, unsigned long max)
 {
unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
+   unsigned int __maybe_unused acpi_zone_dma_bits;
unsigned int __maybe_unused dt_zone_dma_bits;
 
 #ifdef CONFIG_ZONE_DMA
+   acpi_zone_dma_bits = fls64(acpi_iort_dma_get_max_cpu_address());
dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
-   zone_dma_bits = min(32U, dt_zone_dma_bits);
+   zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits);
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 94f34109695c..2494138a6905 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1730,3 +1730,58 @@ void __init acpi_iort_init(void)
 
iort_init_platform_devices();
 }
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Extract the highest CPU physical address accessible to all DMA masters in
+ * the system. PHYS_ADDR_MAX is returned when no constrained device is found.
+ */
+phys_addr_t __init acpi_iort_dma_get_max_cpu_address(void)
+{
+   phys_addr_t limit = PHYS_ADDR_MAX;
+   struct acpi_iort_node *node, *end;
+   struct acpi_table_iort *iort;
+   acpi_status status;
+   int i;
+
+   if (acpi_disabled)
+   return limit;
+
+   status = acpi_get_table(ACPI_SIG_IORT, 0,
+   (struct acpi_table_header **));
+   if (ACPI_FAILURE(status))
+   return limit;
+
+   node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+   end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+   for (i = 0; i < iort->node_count; i++) {
+   if (node >= end)
+   break;
+
+   switch (node->type) {
+   struct acpi_iort_named_component *ncomp;
+   struct acpi_iort_root_complex *rc;
+   phys_addr_t local_limit;
+
+   case ACPI_IORT_NODE_NAMED_COMPONENT:
+   ncomp = (struct acpi_iort_named_component 

[PATCH stable v5.10 3/7] of/address: Introduce of_dma_get_max_cpu_address()

[Jing Xiangfeng]

From: Nicolas Saenz Julienne 

commit 964db79d6c186cc2ecc6ae46f98eed7e0ea8cf71 upstream

Introduce of_dma_get_max_cpu_address(), which provides the highest CPU
physical address addressable by all DMA masters in the system. It's
specially useful for setting memory zones sizes at early boot time.

Signed-off-by: Nicolas Saenz Julienne 
Reviewed-by: Rob Herring 
Link: https://lore.kernel.org/r/20201119175400.9995-4-nsaenzjulie...@suse.de
Signed-off-by: Catalin Marinas 
Cc: 
Signed-off-by: Jing Xiangfeng 
---
 drivers/of/address.c | 42 ++
 include/linux/of.h   |  7 +++
 2 files changed, 49 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1c3257a2d4e3..73ddf2540f3f 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1024,6 +1024,48 @@ int of_dma_get_range(struct device_node *np, const 
struct bus_dma_region **map)
 }
 #endif /* CONFIG_HAS_DMA */
 
+/**
+ * of_dma_get_max_cpu_address - Gets highest CPU address suitable for DMA
+ * @np: The node to start searching from or NULL to start from the root
+ *
+ * Gets the highest CPU physical address that is addressable by all DMA masters
+ * in the sub-tree pointed by np, or the whole tree if NULL is passed. If no
+ * DMA constrained device is found, it returns PHYS_ADDR_MAX.
+ */
+phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
+{
+   phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
+   struct of_range_parser parser;
+   phys_addr_t subtree_max_addr;
+   struct device_node *child;
+   struct of_range range;
+   const __be32 *ranges;
+   u64 cpu_end = 0;
+   int len;
+
+   if (!np)
+   np = of_root;
+
+   ranges = of_get_property(np, "dma-ranges", );
+   if (ranges && len) {
+   of_dma_range_parser_init(, np);
+   for_each_of_range(, )
+   if (range.cpu_addr + range.size > cpu_end)
+   cpu_end = range.cpu_addr + range.size - 1;
+
+   if (max_cpu_addr > cpu_end)
+   max_cpu_addr = cpu_end;
+   }
+
+   for_each_available_child_of_node(np, child) {
+   subtree_max_addr = of_dma_get_max_cpu_address(child);
+   if (max_cpu_addr > subtree_max_addr)
+   max_cpu_addr = subtree_max_addr;
+   }
+
+   return max_cpu_addr;
+}
+
 /**
  * of_dma_is_coherent - Check if device is coherent
  * @np:device node
diff --git a/include/linux/of.h b/include/linux/of.h
index af655d264f10..0f4e81e6fb23 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id,
   const char *map_name, const char *map_mask_name,
   struct device_node **target, u32 *id_out);
 
+phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
+
 #else /* CONFIG_OF */
 
 static inline void of_core_init(void)
@@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 id,
return -EINVAL;
 }
 
+static inline phys_addr_t of_dma_get_max_cpu_address(struct device_node *np)
+{
+   return PHYS_ADDR_MAX;
+}
+
 #define of_match_ptr(_ptr) NULL
 #define of_match_node(_matches, _node) NULL
 #endif /* CONFIG_OF */
-- 
2.25.1

[PATCH stable v5.10 7/7] mm: Remove examples from enum zone_type comment

[Jing Xiangfeng]

From: Nicolas Saenz Julienne 

commit 04435217f96869ac3a8f055ff68c5237a60bcd7e upstream

We can't really list every setup in common code. On top of that they are
unlikely to stay true for long as things change in the arch trees
independently of this comment.

Suggested-by: Christoph Hellwig 
Signed-off-by: Nicolas Saenz Julienne 
Reviewed-by: Christoph Hellwig 
Link: https://lore.kernel.org/r/20201119175400.9995-8-nsaenzjulie...@suse.de
Signed-off-by: Catalin Marinas 
Cc: 
Signed-off-by: Jing Xiangfeng 
---
 include/linux/mmzone.h | 20 
 1 file changed, 20 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fb3bf696c05e..9d0c454d23cd 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -354,26 +354,6 @@ enum zone_type {
 * DMA mask is assumed when ZONE_DMA32 is defined. Some 64-bit
 * platforms may need both zones as they support peripherals with
 * different DMA addressing limitations.
-*
-* Some examples:
-*
-*  - i386 and x86_64 have a fixed 16M ZONE_DMA and ZONE_DMA32 for the
-*rest of the lower 4G.
-*
-*  - arm only uses ZONE_DMA, the size, up to 4G, may vary depending on
-*the specific device.
-*
-*  - arm64 has a fixed 1G ZONE_DMA and ZONE_DMA32 for the rest of the
-*lower 4G.
-*
-*  - powerpc only uses ZONE_DMA, the size, up to 2G, may vary
-*depending on the specific device.
-*
-*  - s390 uses ZONE_DMA fixed to the lower 2G.
-*
-*  - ia64 and riscv only use ZONE_DMA32.
-*
-*  - parisc uses neither.
 */
 #ifdef CONFIG_ZONE_DMA
ZONE_DMA,
-- 
2.25.1

[PATCH stable v5.10 4/7] of: unittest: Add test for of_dma_get_max_cpu_address()

[Jing Xiangfeng]

From: Nicolas Saenz Julienne 

commit 07d13a1d6120d453c3c1f020578693d072deded5 upstream

Introduce a test for of_dma_get_max_cup_address(), it uses the same DT
data as the rest of dma-ranges unit tests.

Signed-off-by: Nicolas Saenz Julienne 
Reviewed-by: Rob Herring 
Link: https://lore.kernel.org/r/20201119175400.9995-5-nsaenzjulie...@suse.de
Signed-off-by: Catalin Marinas 
Cc: 
Signed-off-by: Jing Xiangfeng 
---
 drivers/of/unittest.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 06cc988faf78..98cc0163301b 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -869,6 +869,23 @@ static void __init of_unittest_changeset(void)
 #endif
 }
 
+static void __init of_unittest_dma_get_max_cpu_address(void)
+{
+   struct device_node *np;
+   phys_addr_t cpu_addr;
+
+   np = of_find_node_by_path("/testcase-data/address-tests");
+   if (!np) {
+   pr_err("missing testcase data\n");
+   return;
+   }
+
+   cpu_addr = of_dma_get_max_cpu_address(np);
+   unittest(cpu_addr == 0x4fff,
+"of_dma_get_max_cpu_address: wrong CPU addr %pad (expecting 
%x)\n",
+_addr, 0x4fff);
+}
+
 static void __init of_unittest_dma_ranges_one(const char *path,
u64 expect_dma_addr, u64 expect_paddr)
 {
@@ -3266,6 +3283,7 @@ static int __init of_unittest(void)
of_unittest_changeset();
of_unittest_parse_interrupts();
of_unittest_parse_interrupts_extended();
+   of_unittest_dma_get_max_cpu_address();
of_unittest_parse_dma_ranges();
of_unittest_pci_dma_ranges();
of_unittest_match_node();
-- 
2.25.1

[PATCH stable v5.10 1/7] arm64: mm: Move reserve_crashkernel() into mem_init()

[Jing Xiangfeng]

From: Nicolas Saenz Julienne 

commit 0a30c53573b07d5561457e41fb0ab046cd857da5 upstream

crashkernel might reserve memory located in ZONE_DMA. We plan to delay
ZONE_DMA's initialization after unflattening the devicetree and ACPI's
boot table initialization, so move it later in the boot process.
Specifically into bootmem_init() since request_standard_resources()
depends on it.

Signed-off-by: Nicolas Saenz Julienne 
Tested-by: Jeremy Linton 
Link: https://lore.kernel.org/r/20201119175400.9995-2-nsaenzjulie...@suse.de
Signed-off-by: Catalin Marinas 
Cc: 
Signed-off-by: Jing Xiangfeng 
---
 arch/arm64/mm/init.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 00576a960f11..686653e33250 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -386,8 +386,6 @@ void __init arm64_memblock_init(void)
else
arm64_dma32_phys_limit = PHYS_MASK + 1;
 
-   reserve_crashkernel();
-
reserve_elfcorehdr();
 
high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
@@ -427,6 +425,12 @@ void __init bootmem_init(void)
sparse_init();
zone_sizes_init(min, max);
 
+   /*
+* request_standard_resources() depends on crashkernel's memory being
+* reserved, so do it here.
+*/
+   reserve_crashkernel();
+
memblock_dump_all();
 }
 
-- 
2.25.1

[PATCH stable v5.10 2/7] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()

[Jing Xiangfeng]

From: Nicolas Saenz Julienne 

commit 9804f8c69b04a39d0ba41d19e6bdc6aa91c19725 upstream

zone_dma_bits's initialization happens earlier that it's actually
needed, in arm64_memblock_init(). So move it into the more suitable
zone_sizes_init().

Signed-off-by: Nicolas Saenz Julienne 
Tested-by: Jeremy Linton 
Link: https://lore.kernel.org/r/20201119175400.9995-3-nsaenzjulie...@suse.de
Signed-off-by: Catalin Marinas 
Cc: 
Signed-off-by: Jing Xiangfeng 
---
 arch/arm64/mm/init.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 686653e33250..7da912bf4222 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -190,6 +190,8 @@ static void __init zone_sizes_init(unsigned long min, 
unsigned long max)
unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
+   zone_dma_bits = ARM64_ZONE_DMA_BITS;
+   arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
 #ifdef CONFIG_ZONE_DMA32
@@ -376,11 +378,6 @@ void __init arm64_memblock_init(void)
 
early_init_fdt_scan_reserved_mem();
 
-   if (IS_ENABLED(CONFIG_ZONE_DMA)) {
-   zone_dma_bits = ARM64_ZONE_DMA_BITS;
-   arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
-   }
-
if (IS_ENABLED(CONFIG_ZONE_DMA32))
arm64_dma32_phys_limit = max_zone_phys(32);
else
-- 
2.25.1

[PATCH stable v5.10 0/7] arm64: Default to 32-bit wide ZONE_DMA

[Jing Xiangfeng]

Using two distinct DMA zones turned out to be problematic. Here's an
attempt go back to a saner default.

Ard Biesheuvel (1):
  arm64: mm: Set ZONE_DMA size based on early IORT scan

Nicolas Saenz Julienne (6):
  arm64: mm: Move reserve_crashkernel() into mem_init()
  arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
  of/address: Introduce of_dma_get_max_cpu_address()
  of: unittest: Add test for of_dma_get_max_cpu_address()
  arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges
  mm: Remove examples from enum zone_type comment

 arch/arm64/mm/init.c  | 22 +---
 drivers/acpi/arm64/iort.c | 55 +++
 drivers/of/address.c  | 42 ++
 drivers/of/unittest.c | 18 +
 include/linux/acpi_iort.h |  4 +++
 include/linux/mmzone.h| 20 --
 include/linux/of.h|  7 +
 7 files changed, 139 insertions(+), 29 deletions(-)

-- 
2.25.1

[PATCH] scsi: storvsc: Fix error return in storvsc_probe()

[Jing Xiangfeng]

Fix to return a error code "-ENOMEM" from the error handling case
instead of 0.

Fixes: 436ad9413353 ("scsi: storvsc: Allow only one remove lun work item to be 
issued per lun")
Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/storvsc_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 0c65fbd41035..ded00a89bfc4 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1994,8 +1994,10 @@ static int storvsc_probe(struct hv_device *device,
alloc_ordered_workqueue("storvsc_error_wq_%d",
WQ_MEM_RECLAIM,
host->host_no);
-   if (!host_dev->handle_error_wq)
+   if (!host_dev->handle_error_wq) {
+   ret = -ENOMEM;
goto err_out2;
+   }
INIT_WORK(_dev->host_scan_work, storvsc_host_scan);
/* Register the HBA and start the scsi bus scan */
ret = scsi_add_host(host, >device);
-- 
2.22.0

[PATCH] memstick: r592: Fix error return in r592_probe()

[Jing Xiangfeng]

Fix to return a error code from the error handling case instead of 0.

Fixes: 926341250102 ("memstick: add driver for Ricoh R5C592 card reader")
Signed-off-by: Jing Xiangfeng 
---
 drivers/memstick/host/r592.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/memstick/host/r592.c b/drivers/memstick/host/r592.c
index dd3a1f3dcc19..d2ef46337191 100644
--- a/drivers/memstick/host/r592.c
+++ b/drivers/memstick/host/r592.c
@@ -759,8 +759,10 @@ static int r592_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
goto error3;
 
dev->mmio = pci_ioremap_bar(pdev, 0);
-   if (!dev->mmio)
+   if (!dev->mmio) {
+   error = -ENOMEM;
goto error4;
+   }
 
dev->irq = pdev->irq;
spin_lock_init(>irq_lock);
@@ -786,12 +788,14 @@ static int r592_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
>dummy_dma_page_physical_address, GFP_KERNEL);
r592_stop_dma(dev , 0);
 
-   if (request_irq(dev->irq, _irq, IRQF_SHARED,
- DRV_NAME, dev))
+   error = request_irq(dev->irq, _irq, IRQF_SHARED,
+ DRV_NAME, dev);
+   if (error)
goto error6;
 
r592_update_card_detect(dev);
-   if (memstick_add_host(host))
+   error = memstick_add_host(host);
+   if (error)
goto error7;
 
message("driver successfully loaded");
-- 
2.22.0

[PATCH] staging: olpc_dcon: Do not call platform_device_unregister() in dcon_probe()

[Jing Xiangfeng]

In dcon_probe(), when platform_device_add() failes to add the device,
it jumps to call platform_device_unregister() to remove the device,
which is unnecessary. So use platform_device_put() instead.

Fixes: 53c43c5ca133 ("Revert "Staging: olpc_dcon: Remove obsolete driver"")
Signed-off-by: Jing Xiangfeng 
---
 drivers/staging/olpc_dcon/olpc_dcon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c 
b/drivers/staging/olpc_dcon/olpc_dcon.c
index a0d6d90f4cc8..e7281212db5b 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon.c
@@ -659,8 +659,9 @@ static int dcon_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
  ecreate:
for (j = 0; j < i; j++)
device_remove_file(_device->dev, _device_files[j]);
+   platform_device_del(dcon_device);
  edev:
-   platform_device_unregister(dcon_device);
+   platform_device_put(dcon_device);
dcon_device = NULL;
  eirq:
free_irq(DCON_IRQ, dcon);
-- 
2.17.1

[PATCH] Bluetooth: btusb: Add the missed release_firmware() in btusb_mtk_setup_firmware()

[Jing Xiangfeng]

btusb_mtk_setup_firmware() misses to call release_firmware() in an error
path. Jump to err_release_fw to fix it.

Fixes: f645125711c8 ("Bluetooth: btusb: fix up firmware download sequence")
Signed-off-by: Jing Xiangfeng 
---
 drivers/bluetooth/btusb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 8d2608ddfd08..14231a5f3474 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3043,7 +3043,7 @@ static int btusb_mtk_setup_firmware(struct hci_dev *hdev, 
const char *fwname)
err = btusb_mtk_hci_wmt_sync(hdev, _params);
if (err < 0) {
bt_dev_err(hdev, "Failed to power on data RAM (%d)", err);
-   return err;
+   goto err_release_fw;
}
 
fw_ptr = fw->data;
-- 
2.17.1

[PATCH] Bluetooth: btmtksdio: Add the missed release_firmware() in mtk_setup_firmware()

[Jing Xiangfeng]

mtk_setup_firmware() misses to call release_firmware() in an error
path. Jump to free_fw to fix it.

Fixes: 737cd06072a7 ("Bluetooth: btmtksdio: fix up firmware download sequence")
Signed-off-by: Jing Xiangfeng 
---
 drivers/bluetooth/btmtksdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index c7ab7a23bd67..c2677ffbd3c0 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -704,7 +704,7 @@ static int mtk_setup_firmware(struct hci_dev *hdev, const 
char *fwname)
err = mtk_hci_wmt_sync(hdev, _params);
if (err < 0) {
bt_dev_err(hdev, "Failed to power on data RAM (%d)", err);
-   return err;
+   goto free_fw;
}
 
fw_ptr = fw->data;
-- 
2.17.1

[PATCH] scsi: aacraid: Correct goto target in aac_resume()

[Jing Xiangfeng]

In current code, it jumps to call pci_disable_device() when
pci_enable_device() failes to initialize device. Add a label
'fail_enable' to fix it.

Fixes: de665f28f788 ("aacraid: Add Power Management support")
Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/aacraid/linit.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index a3aee146537b..13323707 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1943,7 +1943,7 @@ static int aac_resume(struct pci_dev *pdev)
r = pci_enable_device(pdev);
 
if (r)
-   goto fail_device;
+   goto fail_enable;
 
pci_set_master(pdev);
if (aac_acquire_resources(aac))
@@ -1958,9 +1958,10 @@ static int aac_resume(struct pci_dev *pdev)
return 0;
 
 fail_device:
+   pci_disable_device(pdev);
+fail_enable:
printk(KERN_INFO "%s%d: resume failed.\n", aac->name, aac->id);
scsi_host_put(shost);
-   pci_disable_device(pdev);
return -ENODEV;
 }
 #endif
-- 
2.17.1

[PATCH] staging: gasket: interrupt: fix the missed eventfd_ctx_put() in gasket_interrupt.c

[Jing Xiangfeng]

gasket_interrupt_set_eventfd() misses to call eventfd_ctx_put() in an
error path. We check interrupt is valid before calling
eventfd_ctx_fdget() to fix it.

There is the same issue in gasket_interrupt_clear_eventfd(), Add the
missed function call to fix it.

Fixes: 9a69f5087ccc ("drivers/staging: Gasket driver framework + Apex driver")
Signed-off-by: Jing Xiangfeng 
---
 drivers/staging/gasket/gasket_interrupt.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/gasket/gasket_interrupt.c 
b/drivers/staging/gasket/gasket_interrupt.c
index 2d6195f7300e..864342acfd86 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -487,14 +487,16 @@ int gasket_interrupt_system_status(struct gasket_dev 
*gasket_dev)
 int gasket_interrupt_set_eventfd(struct gasket_interrupt_data *interrupt_data,
 int interrupt, int event_fd)
 {
-   struct eventfd_ctx *ctx = eventfd_ctx_fdget(event_fd);
-
-   if (IS_ERR(ctx))
-   return PTR_ERR(ctx);
+   struct eventfd_ctx *ctx;
 
if (interrupt < 0 || interrupt >= interrupt_data->num_interrupts)
return -EINVAL;
 
+   ctx = eventfd_ctx_fdget(event_fd);
+
+   if (IS_ERR(ctx))
+   return PTR_ERR(ctx);
+
interrupt_data->eventfd_ctxs[interrupt] = ctx;
return 0;
 }
@@ -505,6 +507,9 @@ int gasket_interrupt_clear_eventfd(struct 
gasket_interrupt_data *interrupt_data,
if (interrupt < 0 || interrupt >= interrupt_data->num_interrupts)
return -EINVAL;
 
-   interrupt_data->eventfd_ctxs[interrupt] = NULL;
+   if (interrupt_data->eventfd_ctxs[interrupt]) {
+   eventfd_ctx_put(interrupt_data->eventfd_ctxs[interrupt]);
+   interrupt_data->eventfd_ctxs[interrupt] = NULL;
+   }
return 0;
 }
-- 
2.17.1

[PATCH] HID: intel-ish-hid: Remove unnecessary assignment to variable rv

[Jing Xiangfeng]

This assignment to rv is unused in an error path. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/hid/intel-ish-hid/ishtp-hid.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.c 
b/drivers/hid/intel-ish-hid/ishtp-hid.c
index b8aae69ad15d..393bed0abee9 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.c
@@ -211,10 +211,8 @@ int ishtp_hid_probe(unsigned int cur_hid_dev,
struct ishtp_hid_data *hid_data;
 
hid = hid_allocate_device();
-   if (IS_ERR(hid)) {
-   rv = PTR_ERR(hid);
-   return  -ENOMEM;
-   }
+   if (IS_ERR(hid))
+   return PTR_ERR(hid);
 
hid_data = kzalloc(sizeof(*hid_data), GFP_KERNEL);
if (!hid_data) {
-- 
2.17.1

[PATCH v2] mfd: htc-i2cpld: Add the missed i2c_put_adapter() in htcpld_register_chip_i2c()

[Jing Xiangfeng]

htcpld_register_chip_i2c() misses to call i2c_put_adapter() in an error
path. Add the missed function call to fix it.

Fixes: 6048a3dd2371 ("mfd: Add HTCPLD driver")
Signed-off-by: Jing Xiangfeng 
---
v2:
- add the tag Fixes

---
 drivers/mfd/htc-i2cpld.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/htc-i2cpld.c b/drivers/mfd/htc-i2cpld.c
index 247f9849e54a..417b0355d904 100644
--- a/drivers/mfd/htc-i2cpld.c
+++ b/drivers/mfd/htc-i2cpld.c
@@ -346,6 +346,7 @@ static int htcpld_register_chip_i2c(
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
dev_warn(dev, "i2c adapter %d non-functional\n",
 pdata->i2c_adapter_id);
+   i2c_put_adapter(adapter);
return -EINVAL;
}
 
@@ -360,6 +361,7 @@ static int htcpld_register_chip_i2c(
/* I2C device registration failed, contineu with the next */
dev_warn(dev, "Unable to add I2C device for 0x%x\n",
 plat_chip_data->addr);
+   i2c_put_adapter(adapter);
return PTR_ERR(client);
}
 
-- 
2.17.1

Re: [PATCH] mfd: htc-i2cpld: Add the missed i2c_put_adapter() in htcpld_register_chip_i2c()

[Jing Xiangfeng]

On 2020/11/4 14:50, Markus Elfring wrote:

htcpld_register_chip_i2c() misses to call i2c_put_adapter() in an error
path. Add the missed function call to fix it.

Would you like to add the tag “Fixes”?

OK, I'll send a v2 with it.

 Thanks


Regards,
Markus
.

[PATCH] mfd: htc-i2cpld: Add the missed i2c_put_adapter() in htcpld_register_chip_i2c()

[Jing Xiangfeng]

htcpld_register_chip_i2c() misses to call i2c_put_adapter() in an error
path. Add the missed function call to fix it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/mfd/htc-i2cpld.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/htc-i2cpld.c b/drivers/mfd/htc-i2cpld.c
index 247f9849e54a..417b0355d904 100644
--- a/drivers/mfd/htc-i2cpld.c
+++ b/drivers/mfd/htc-i2cpld.c
@@ -346,6 +346,7 @@ static int htcpld_register_chip_i2c(
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
dev_warn(dev, "i2c adapter %d non-functional\n",
 pdata->i2c_adapter_id);
+   i2c_put_adapter(adapter);
return -EINVAL;
}
 
@@ -360,6 +361,7 @@ static int htcpld_register_chip_i2c(
/* I2C device registration failed, contineu with the next */
dev_warn(dev, "Unable to add I2C device for 0x%x\n",
 plat_chip_data->addr);
+   i2c_put_adapter(adapter);
return PTR_ERR(client);
}
 
-- 
2.17.1

[PATCH] scsi: qla4xxx: Remove redundant assignment to variable rval

[Jing Xiangfeng]

The variable rval has been initialized with 'QLA_ERROR'. The assignment
is redundant in an error path. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/qla4xxx/ql4_os.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 676778cbc550..aaccbf71dff5 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -686,7 +686,6 @@ static int qla4xxx_get_chap_by_index(struct scsi_qla_host 
*ha,
 
if (!ha->chap_list) {
ql4_printk(KERN_ERR, ha, "CHAP table cache is empty!\n");
-   rval = QLA_ERROR;
goto exit_get_chap;
}
 
@@ -698,14 +697,12 @@ static int qla4xxx_get_chap_by_index(struct scsi_qla_host 
*ha,
 
if (chap_index > max_chap_entries) {
ql4_printk(KERN_ERR, ha, "Invalid Chap index\n");
-   rval = QLA_ERROR;
goto exit_get_chap;
}
 
*chap_entry = (struct ql4_chap_table *)ha->chap_list + chap_index;
if ((*chap_entry)->cookie !=
 __constant_cpu_to_le16(CHAP_VALID_COOKIE)) {
-   rval = QLA_ERROR;
*chap_entry = NULL;
} else {
rval = QLA_SUCCESS;
-- 
2.17.1

[PATCH] vdpa/mlx5: Fix error return in map_direct_mr()

[Jing Xiangfeng]

Fix to return the variable "err" from the error handling case instead
of "ret".

Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
Signed-off-by: Jing Xiangfeng 
---
 drivers/vdpa/mlx5/core/mr.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index ef1c550f8266..4b6195666c58 100644
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -239,7 +239,6 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, 
struct mlx5_vdpa_direct_mr
u64 paend;
struct scatterlist *sg;
struct device *dma = mvdev->mdev->device;
-   int ret;
 
for (map = vhost_iotlb_itree_first(iotlb, mr->start, mr->end - 1);
 map; map = vhost_iotlb_itree_next(map, start, mr->end - 1)) {
@@ -277,8 +276,8 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, 
struct mlx5_vdpa_direct_mr
 done:
mr->log_size = log_entity_size;
mr->nsg = nsg;
-   ret = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, 
DMA_BIDIRECTIONAL, 0);
-   if (!ret)
+   err = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, 
DMA_BIDIRECTIONAL, 0);
+   if (!err)
goto err_map;
 
err = create_direct_mr(mvdev, mr);
-- 
2.17.1

[PATCH] drm: Add the missed device_unregister() in drm_sysfs_connector_add()

[Jing Xiangfeng]

drm_sysfs_connector_add() misses to call device_unregister() when
sysfs_create_link() fails to create. Add the missed function call
to fix it.

Fixes: e1a29c6c5955 ("drm: Add ddc link in sysfs created by drm_connector")
Signed-off-by: Jing Xiangfeng 
---
 drivers/gpu/drm/drm_sysfs.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index f0336c804639..39e173e10cf7 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -274,6 +274,7 @@ static const struct attribute_group *connector_dev_groups[] 
= {
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
struct drm_device *dev = connector->dev;
+   int ret = 0;
 
if (connector->kdev)
return 0;
@@ -291,10 +292,16 @@ int drm_sysfs_connector_add(struct drm_connector 
*connector)
return PTR_ERR(connector->kdev);
}
 
-   if (connector->ddc)
-   return sysfs_create_link(>kdev->kobj,
+   if (connector->ddc) {
+   ret = sysfs_create_link(>kdev->kobj,
 >ddc->dev.kobj, "ddc");
-   return 0;
+   if (ret) {
+   device_unregister(connector->kdev);
+   connector->kdev = NULL;
+   }
+   }
+
+   return ret;
 }
 
 void drm_sysfs_connector_remove(struct drm_connector *connector)
-- 
2.17.1

[PATCH] ssb: Fix error return in ssb_bus_scan()

[Jing Xiangfeng]

Fix to return error code -EINVAL from the error handling case instead
of 0.

Signed-off-by: Jing Xiangfeng 
---
 drivers/ssb/scan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ssb/scan.c b/drivers/ssb/scan.c
index f49ab1aa2149..4161e5d1f276 100644
--- a/drivers/ssb/scan.c
+++ b/drivers/ssb/scan.c
@@ -325,6 +325,7 @@ int ssb_bus_scan(struct ssb_bus *bus,
if (bus->nr_devices > ARRAY_SIZE(bus->devices)) {
pr_err("More than %d ssb cores found (%d)\n",
   SSB_MAX_NR_CORES, bus->nr_devices);
+   err = -EINVAL;
goto err_unmap;
}
if (bus->bustype == SSB_BUSTYPE_SSB) {
-- 
2.17.1

[PATCH] RDMA/core: Fix error return in _ib_modify_qp()

[Jing Xiangfeng]

Fix to return error code PTR_ERR() from the error handling case instead of
0.

Fixes: 51aab12631dd ("RDMA/core: Get xmit slave for LAG")
Signed-off-by: Jing Xiangfeng 
---
 drivers/infiniband/core/verbs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 307886737646..bf63c7561e8c 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1685,8 +1685,10 @@ static int _ib_modify_qp(struct ib_qp *qp, struct 
ib_qp_attr *attr,
slave = rdma_lag_get_ah_roce_slave(qp->device,
   >ah_attr,
   GFP_KERNEL);
-   if (IS_ERR(slave))
+   if (IS_ERR(slave)) {
+   ret = PTR_ERR(slave);
goto out_av;
+   }
attr->xmit_slave = slave;
}
}
-- 
2.17.1

[PATCH v3] thunderbolt: Add the missed ida_simple_remove() in ring_request_msix()

[Jing Xiangfeng]

ring_request_msix() misses to call ida_simple_remove() in an error path.
Add a label 'err_ida_remove' and jump to it.

Fixes: 046bee1f9ab8 ("thunderbolt: Add MSI-X support")
Signed-off-by: Jing Xiangfeng 
---
 drivers/thunderbolt/nhi.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 5f7489fa1327..a331e52789e3 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -405,12 +405,23 @@ static int ring_request_msix(struct tb_ring *ring, bool 
no_suspend)
 
ring->vector = ret;
 
-   ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector);
-   if (ring->irq < 0)
-   return ring->irq;
+   ret = pci_irq_vector(ring->nhi->pdev, ring->vector);
+   if (ret < 0)
+   goto err_ida_remove;
+
+   ring->irq = ret;
 
irqflags = no_suspend ? IRQF_NO_SUSPEND : 0;
-   return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);
+   ret = request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);
+   if (ret)
+   goto err_ida_remove;
+
+   return 0;
+
+err_ida_remove:
+   ida_simple_remove(>msix_ida, ring->vector);
+
+   return ret;
 }
 
 static void ring_release_msix(struct tb_ring *ring)
-- 
2.17.1

Re: [PATCH v2] thunderbolt: Add the missed ida_simple_remove() in ring_request_msix()

[Jing Xiangfeng]

On 2020/10/14 16:48, Andy Shevchenko wrote:

On Wed, Oct 14, 2020 at 09:46:04AM +0800, Jing Xiangfeng wrote:

ring_request_msix() misses to call ida_simple_remove() in an error path.
Add a label 'err_ida_remove' and jump to it.

...


@@ -406,11 +406,22 @@ static int ring_request_msix(struct tb_ring *ring, bool 
no_suspend)
ring->vector = ret;

^^^


ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector);
-   if (ring->irq < 0)
-   return ring->irq;
+   if (ring->irq < 0) {
+   ret = ring->irq;
+   goto err_ida_remove;
+   }

What about
ret = pci_irq_vector(ring->nhi->pdev, ring->vector);
if (ret < 0)
goto err_ida_remove;

ring->irq = ret;

?

Yeah, I agree. Thanks for your suggestions.



(See also context above)


irqflags = no_suspend ? IRQF_NO_SUSPEND : 0;
-   return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);
+   ret = request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);
+   if (ret)
+   goto err_ida_remove;
+
+   return 0;
+
+err_ida_remove:
+   ida_simple_remove(>msix_ida, ring->vector);
+
+   return ret;
  }
  
  static void ring_release_msix(struct tb_ring *ring)

--
2.17.1

[PATCH v2] thunderbolt: Add the missed ida_simple_remove() in ring_request_msix()

[Jing Xiangfeng]

ring_request_msix() misses to call ida_simple_remove() in an error path.
Add a label 'err_ida_remove' and jump to it.

Fixes: 046bee1f9ab8 ("thunderbolt: Add MSI-X support")
Signed-off-by: Jing Xiangfeng 
---
 drivers/thunderbolt/nhi.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 5f7489fa1327..e066888c4b41 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -406,11 +406,22 @@ static int ring_request_msix(struct tb_ring *ring, bool 
no_suspend)
ring->vector = ret;
 
ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector);
-   if (ring->irq < 0)
-   return ring->irq;
+   if (ring->irq < 0) {
+   ret = ring->irq;
+   goto err_ida_remove;
+   }
 
irqflags = no_suspend ? IRQF_NO_SUSPEND : 0;
-   return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);
+   ret = request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);
+   if (ret)
+   goto err_ida_remove;
+
+   return 0;
+
+err_ida_remove:
+   ida_simple_remove(>msix_ida, ring->vector);
+
+   return ret;
 }
 
 static void ring_release_msix(struct tb_ring *ring)
-- 
2.17.1

Re: [PATCH] thunderbolt: Add the missed ida_simple_remove() in ring_request_msix()

[Jing Xiangfeng]

On 2020/10/13 19:43, Andy Shevchenko wrote:

On Tue, Oct 13, 2020 at 10:45:18AM +0800, Jing Xiangfeng wrote:

ring_request_msix() misses to call ida_simple_remove() in an error path.
Add the missed function call to fix it.

...


ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector);
-   if (ring->irq < 0)
+   if (ring->irq < 0) {
+   ida_simple_remove(>msix_ida, ret);
return ring->irq;
+   }
  
  	irqflags = no_suspend ? IRQF_NO_SUSPEND : 0;

return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);

According to your logic don't you need it here as well?

Hm indeed. Will fix too.

Thanks for your review.

[PATCH] thunderbolt: Add the missed ida_simple_remove() in ring_request_msix()

[Jing Xiangfeng]

ring_request_msix() misses to call ida_simple_remove() in an error path.
Add the missed function call to fix it.

Fixes: 046bee1f9ab8 ("thunderbolt: Add MSI-X support")
Signed-off-by: Jing Xiangfeng 
---
 drivers/thunderbolt/nhi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 5f7489fa1327..7732e7a9942f 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -406,8 +406,10 @@ static int ring_request_msix(struct tb_ring *ring, bool 
no_suspend)
ring->vector = ret;
 
ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector);
-   if (ring->irq < 0)
+   if (ring->irq < 0) {
+   ida_simple_remove(>msix_ida, ret);
return ring->irq;
+   }
 
irqflags = no_suspend ? IRQF_NO_SUSPEND : 0;
return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);
-- 
2.17.1

[PATCH] staging: fieldbus: anybuss: jump to correct label in an error path

[Jing Xiangfeng]

In current code, controller_probe() misses to call ida_simple_remove()
in an error path. Jump to correct label to fix it.

Fixes: 17614978ed34 ("staging: fieldbus: anybus-s: support the Arcx anybus 
controller")
Signed-off-by: Jing Xiangfeng 
---
 drivers/staging/fieldbus/anybuss/arcx-anybus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fieldbus/anybuss/arcx-anybus.c 
b/drivers/staging/fieldbus/anybuss/arcx-anybus.c
index 5b8d0bae9ff3..b5fded15e8a6 100644
--- a/drivers/staging/fieldbus/anybuss/arcx-anybus.c
+++ b/drivers/staging/fieldbus/anybuss/arcx-anybus.c
@@ -293,7 +293,7 @@ static int controller_probe(struct platform_device *pdev)
regulator = devm_regulator_register(dev, _power_desc, );
if (IS_ERR(regulator)) {
err = PTR_ERR(regulator);
-   goto out_reset;
+   goto out_ida;
}
/* make controller info visible to userspace */
cd->class_dev = kzalloc(sizeof(*cd->class_dev), GFP_KERNEL);
-- 
2.17.1

[PATCH] HSI: omap_ssi: Don't jump to free ID in ssi_add_controller()

[Jing Xiangfeng]

In current code, it jumps to ida_simple_remove() when ida_simple_get()
failes to allocate an ID. Just return to fix it.

Fixes: 0fae198988b8 ("HSI: omap_ssi: built omap_ssi and omap_ssi_port into one 
module")
Signed-off-by: Jing Xiangfeng 
---
 drivers/hsi/controllers/omap_ssi_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hsi/controllers/omap_ssi_core.c 
b/drivers/hsi/controllers/omap_ssi_core.c
index fa69b94debd9..7596dc164648 100644
--- a/drivers/hsi/controllers/omap_ssi_core.c
+++ b/drivers/hsi/controllers/omap_ssi_core.c
@@ -355,7 +355,7 @@ static int ssi_add_controller(struct hsi_controller *ssi,
 
err = ida_simple_get(_omap_ssi_ida, 0, 0, GFP_KERNEL);
if (err < 0)
-   goto out_err;
+   return err;
ssi->id = err;
 
ssi->owner = THIS_MODULE;
-- 
2.17.1

[PATCH] ASoC: pxa: remove unnecessary assignment to variable ret

[Jing Xiangfeng]

ret is always zero here in this code path. So remove this assignment.

Signed-off-by: Jing Xiangfeng 
---
 sound/arm/pxa2xx-ac97.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c
index ea8e233150c8..813e608dca82 100644
--- a/sound/arm/pxa2xx-ac97.c
+++ b/sound/arm/pxa2xx-ac97.c
@@ -210,7 +210,6 @@ static int pxa2xx_ac97_pcm_new(struct snd_card *card)
goto out;
 
pxa2xx_ac97_pcm = pcm;
-   ret = 0;
 
  out:
return ret;
-- 
2.17.1

[PATCH] caif_virtio: Remove redundant initialization of variable err

[Jing Xiangfeng]

After commit a8c7687bf216 ("caif_virtio: Check that vringh_config is not
null"), the variable err is being initialized with '-EINVAL' that is
meaningless. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/net/caif/caif_virtio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index 80ea2e913c2b..47a6d62b7511 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -652,7 +652,7 @@ static int cfv_probe(struct virtio_device *vdev)
const char *cfv_netdev_name = "cfvrt";
struct net_device *netdev;
struct cfv_info *cfv;
-   int err = -EINVAL;
+   int err;
 
netdev = alloc_netdev(sizeof(struct cfv_info), cfv_netdev_name,
  NET_NAME_UNKNOWN, cfv_netdev_setup);
-- 
2.17.1

Re: [PATCH] scsi: esas2r: prevent a potential NULL dereference in esas2r_probe()

[Jing Xiangfeng]

On 2020/9/16 5:44, Martin K. Petersen wrote:


Jing,


esas2r_probe() calls scsi_host_put() in an error path. However,
esas2r_log_dev() may hit a potential NULL dereference. So use NUll instead.


Wouldn't it be better to move the scsi_host_put() call after the error
message?


There is already a message before the scsi_host_put() call. It is used 
to record calling function.

[PATCH] thermal: core: add the misse nlmsg_free() for thermal_genl_sampling_temp()

[Jing Xiangfeng]

thermal_genl_sampling_temp() misses to call nlmsg_free() in an error path.
Jump to out_free to fix it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/thermal/thermal_netlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_netlink.c 
b/drivers/thermal/thermal_netlink.c
index af7b2383e8f6..019f4812def6 100644
--- a/drivers/thermal/thermal_netlink.c
+++ b/drivers/thermal/thermal_netlink.c
@@ -78,7 +78,7 @@ int thermal_genl_sampling_temp(int id, int temp)
hdr = genlmsg_put(skb, 0, 0, _gnl_family, 0,
  THERMAL_GENL_SAMPLING_TEMP);
if (!hdr)
-   return -EMSGSIZE;
+   goto out_free;
 
if (nla_put_u32(skb, THERMAL_GENL_ATTR_TZ_ID, id))
goto out_cancel;
@@ -93,6 +93,7 @@ int thermal_genl_sampling_temp(int id, int temp)
return 0;
 out_cancel:
genlmsg_cancel(skb, hdr);
+out_free:
nlmsg_free(skb);
 
return -EMSGSIZE;
-- 
2.26.0.106.g9fadedd

[PATCH] staging: mfd: hi6421-spmi-pmic: Fix error return in hi6421_spmi_pmic_probe()

[Jing Xiangfeng]

Fix to return error code -ENOMEM from the error handling case instead
of 0.

Signed-off-by: Jing Xiangfeng 
---
 drivers/staging/hikey9xx/hi6421-spmi-pmic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/hikey9xx/hi6421-spmi-pmic.c 
b/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
index 64b30d263c8d..4f34a5282970 100644
--- a/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
+++ b/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
@@ -262,8 +262,10 @@ static int hi6421_spmi_pmic_probe(struct spmi_device *pdev)
hi6421_spmi_pmic_irq_prc(pmic);
 
pmic->irqs = devm_kzalloc(dev, HISI_IRQ_NUM * sizeof(int), GFP_KERNEL);
-   if (!pmic->irqs)
+   if (!pmic->irqs) {
+   ret = -ENOMEM;
goto irq_malloc;
+   }
 
pmic->domain = irq_domain_add_simple(np, HISI_IRQ_NUM, 0,
 _spmi_domain_ops, pmic);
-- 
2.26.0.106.g9fadedd

[PATCH] scsi: myrb: remove redundant assignment to variable timeout

[Jing Xiangfeng]

The variable timeout has been initialized with a value '0'. The assignment
before while loop is redundant. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/myrb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
index b2869c5dd7fb..687a808969ac 100644
--- a/drivers/scsi/myrb.c
+++ b/drivers/scsi/myrb.c
@@ -2732,7 +2732,6 @@ static int DAC960_LA_hw_init(struct pci_dev *pdev,
DAC960_LA_disable_intr(base);
DAC960_LA_ack_hw_mbox_status(base);
udelay(1000);
-   timeout = 0;
while (DAC960_LA_init_in_progress(base) &&
   timeout < MYRB_MAILBOX_TIMEOUT) {
if (DAC960_LA_read_error_status(base, ,
-- 
2.17.1

Re: [PATCH] staging: most: don't access hdm_ch before checking it valid

[Jing Xiangfeng]

On 2020/9/28 19:48, Dan Carpenter wrote:

On Mon, Sep 28, 2020 at 06:48:38PM +0800, Jing Xiangfeng wrote:

In try_start_dim_transfer(), pointer hdm_ch is accessed before checking.
This may lead to a potential null pointer dereference. Fix this by
dereferencing hdm_ch after calling BUG_ON().

Signed-off-by: Jing Xiangfeng 
---
  drivers/staging/most/dim2/dim2.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index 509c8012d20b..ccd7cc7545e4 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -148,7 +148,7 @@ void dimcb_on_error(u8 error_id, const char *error_message)
  static int try_start_dim_transfer(struct hdm_channel *hdm_ch)
  {
u16 buf_size;
-   struct list_head *head = _ch->pending_list;

This is not a dereference, it's just pointer math.  In other words:

struct list_head *head = hdm_ch + offsetof(struct hdm_channel, 
pending_list);

Thanks for correcting!



So the commit message is wrong because this cannot lead to a NULL
dereference.  It's better to just delete the BUG_ON().  We don't really
like BUG_ON().  Checkpatch will complain about them.  An Oops gives
basically the same information as a BUG_ON() without completely killing
the kernel so just dereferencing a NULL is preferable.  Finally, we can
see from the callers that "hdm_ch" is never NULL.

regards,
dan carpenter

.

[PATCH] staging: most: don't access hdm_ch before checking it valid

[Jing Xiangfeng]

In try_start_dim_transfer(), pointer hdm_ch is accessed before checking.
This may lead to a potential null pointer dereference. Fix this by
dereferencing hdm_ch after calling BUG_ON().

Signed-off-by: Jing Xiangfeng 
---
 drivers/staging/most/dim2/dim2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index 509c8012d20b..ccd7cc7545e4 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -148,7 +148,7 @@ void dimcb_on_error(u8 error_id, const char *error_message)
 static int try_start_dim_transfer(struct hdm_channel *hdm_ch)
 {
u16 buf_size;
-   struct list_head *head = _ch->pending_list;
+   struct list_head *head;
struct mbo *mbo;
unsigned long flags;
struct dim_ch_state_t st;
@@ -156,6 +156,7 @@ static int try_start_dim_transfer(struct hdm_channel 
*hdm_ch)
BUG_ON(!hdm_ch);
BUG_ON(!hdm_ch->is_initialized);
 
+   head = _ch->pending_list;
spin_lock_irqsave(_lock, flags);
if (list_empty(head)) {
spin_unlock_irqrestore(_lock, flags);
-- 
2.17.1

[PATCH] scsi: bfa: fix error return in bfad_pci_init()

[Jing Xiangfeng]

Fix to return error code -ENODEV from the error handling case instead
of 0.

Fixes: 11ea3824140c ("scsi: bfa: fix calls to dma_set_mask_and_coherent()")
Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/bfa/bfad.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index bc5d84f87d8f..440ef32be048 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -749,6 +749,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 
if (bfad->pci_bar0_kva == NULL) {
printk(KERN_ERR "Fail to map bar0\n");
+   rc = -ENODEV;
goto out_release_region;
}
 
-- 
2.17.1

[PATCH] scsi: snic: Remove unnecessary condition to simplify the code

[Jing Xiangfeng]

ret is always zero or an error in this code path. So the assignment to
ret is redundant, and the code jumping to a label is unneed.
Let's remove them to simplify the code. No functional changes.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/snic/snic_scsi.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
index b3650c989ed4..0c2f31b8ea05 100644
--- a/drivers/scsi/snic/snic_scsi.c
+++ b/drivers/scsi/snic/snic_scsi.c
@@ -1387,10 +1387,6 @@ snic_issue_tm_req(struct snic *snic,
}
 
ret = snic_queue_itmf_req(snic, tmreq, sc, tmf, req_id);
-   if (ret)
-   goto tmreq_err;
-
-   ret = 0;
 
 tmreq_err:
if (ret) {
-- 
2.17.1

[PATCH -next] virtiofs: Move the assignment to ret outside the loop

[Jing Xiangfeng]

There is no need to do the assignment each time. So move the assignment
to ret outside the loop.

Signed-off-by: Jing Xiangfeng 
---
 fs/fuse/dax.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index e394dba08cc4..f18cd7b53ec7 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1259,9 +1259,9 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax 
*fcd)
pr_debug("%s: dax mapped %ld pages. nr_ranges=%ld\n",
__func__, nr_pages, nr_ranges);
 
+   ret = -ENOMEM;
for (i = 0; i < nr_ranges; i++) {
range = kzalloc(sizeof(struct fuse_dax_mapping), GFP_KERNEL);
-   ret = -ENOMEM;
if (!range)
goto out_err;
 
-- 
2.26.0.106.g9fadedd

Re: [PATCH] rapidio: fix the missed put_device() for rio_mport_add_riodev

[Jing Xiangfeng]

On 2020/9/22 16:04, Dan Carpenter wrote:

On Tue, Sep 22, 2020 at 03:25:25PM +0800, Jing Xiangfeng wrote:

rio_mport_add_riodev() misses to call put_device() when the device
already exists. Add the missed function call to fix it.


Looks good.

Reviewed-by: Dan Carpenter 

I notice that rio_mport_del_riodev() has a related bug.

   1802  err = rio_add_device(rdev);
   1803  if (err)
   1804  goto cleanup;
   1805  rio_dev_get(rdev);
 ^
This calls get_device(>dev);

   1806
   1807  return 0;
   1808  cleanup:
   1809  kfree(rdev);
   1810  return err;
   1811  }
   1812
   1813  static int rio_mport_del_riodev(struct mport_cdev_priv *priv, void 
__user *arg)
   1814  {
   1815  struct rio_rdev_info dev_info;
   1816  struct rio_dev *rdev = NULL;
   1817  struct device  *dev;
   1818  struct rio_mport *mport;
   1819  struct rio_net *net;
   1820
   1821  if (copy_from_user(_info, arg, sizeof(dev_info)))
   1822  return -EFAULT;
   1823  dev_info.name[sizeof(dev_info.name) - 1] = '\0';
   1824
   1825  mport = priv->md->mport;
   1826
   1827  /* If device name is specified, removal by name has priority */
   1828  if (strlen(dev_info.name)) {
   1829  dev = bus_find_device_by_name(_bus_type, NULL,
   1830dev_info.name);
   1831  if (dev)
   1832  rdev = to_rio_dev(dev);

This path takes a second get_device(>dev);

   1833  } else {
   1834  do {
   1835  rdev = rio_get_comptag(dev_info.comptag, rdev);
   1836  if (rdev && rdev->dev.parent == >net->dev 
&&
   1837  rdev->destid == dev_info.destid &&
   1838  rdev->hopcount == dev_info.hopcount)
   1839  break;

This path does not call get_device().

 Add  calling rio_dev_get()  in this path? like the following changes:

 static int rio_mport_del_riodev(struct mport_cdev_priv *priv, void 
__user *arg)

rdev = rio_get_comptag(dev_info.comptag, rdev);
if (rdev && rdev->dev.parent == >net->dev &&
rdev->destid == dev_info.destid &&
-   rdev->hopcount == dev_info.hopcount)
+   rdev->hopcount == dev_info.hopcount) {
+   rio_dev_get(rdev);
break;
+   }
} while (rdev);
}



   1840  } while (rdev);
   1841  }
   1842
   1843  if (!rdev) {
   1844  rmcd_debug(RDEV,
   1845  "device name:%s ct:0x%x did:0x%x hc:0x%x not 
found",
   1846  dev_info.name, dev_info.comptag, 
dev_info.destid,
   1847  dev_info.hopcount);
   1848  return -ENODEV;
   1849  }
   1850
   1851  net = rdev->net;
   1852  rio_dev_put(rdev);

This drops a reference.

   1853  rio_del_device(rdev, RIO_DEVICE_SHUTDOWN);

This drops a second reference.  So presumably deleting by component tag
will lead to a use after free.

Indeed,  it  is a mismatched reference.


   1854
   1855  if (list_empty(>devices)) {
   1856  rio_free_net(net);
   1857  mport->net = NULL;
   1858  }
   1859
   1860  return 0;
   1861  }

regards,
dan carpenter
.

[PATCH] udf: Remove redundant initialization of variable ret

[Jing Xiangfeng]

After commit 9293fcfbc181 ("udf: Remove struct ustr as non-needed
intermediate storage"), the variable ret is being initialized with
'-ENOMEM' that is meaningless. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 fs/udf/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 1c42f544096d..c0c8068116c1 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -854,7 +854,7 @@ static int udf_load_pvoldesc(struct super_block *sb, 
sector_t block)
uint8_t *outstr;
struct buffer_head *bh;
uint16_t ident;
-   int ret = -ENOMEM;
+   int ret;
struct timestamp *ts;
 
outstr = kmalloc(128, GFP_NOFS);
-- 
2.17.1

[PATCH] rapidio: fix the missed put_device() for rio_mport_add_riodev

[Jing Xiangfeng]

rio_mport_add_riodev() misses to call put_device() when the device
already exists. Add the missed function call to fix it.

Fixes: e8de370188d0 ("rapidio: add mport char device driver")
Signed-off-by: Jing Xiangfeng 
---
 drivers/rapidio/devices/rio_mport_cdev.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/rapidio/devices/rio_mport_cdev.c 
b/drivers/rapidio/devices/rio_mport_cdev.c
index a30342942e26..829fe2b7c437 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -1679,6 +1679,7 @@ static int rio_mport_add_riodev(struct mport_cdev_priv 
*priv,
struct rio_dev *rdev;
struct rio_switch *rswitch = NULL;
struct rio_mport *mport;
+   struct device *dev;
size_t size;
u32 rval;
u32 swpinfo = 0;
@@ -1693,8 +1694,10 @@ static int rio_mport_add_riodev(struct mport_cdev_priv 
*priv,
rmcd_debug(RDEV, "name:%s ct:0x%x did:0x%x hc:0x%x", dev_info.name,
   dev_info.comptag, dev_info.destid, dev_info.hopcount);
 
-   if (bus_find_device_by_name(_bus_type, NULL, dev_info.name)) {
+   dev = bus_find_device_by_name(_bus_type, NULL, dev_info.name);
+   if (dev) {
rmcd_debug(RDEV, "device %s already exists", dev_info.name);
+   put_device(dev);
return -EEXIST;
}
 
-- 
2.17.1

[PATCH] userns: Remove redundant assignment to variable ret

[Jing Xiangfeng]

After commit d2f007dbe7e4 ("userns: also map extents in the reverse map
to kernel IDs"), the assignment to ret is redundant. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 kernel/user_namespace.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 87804e0371fe..0157ae92e447 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -968,7 +968,6 @@ static ssize_t map_write(struct file *file, const char 
__user *buf,
if (!new_idmap_permitted(file, ns, cap_setid, _map))
goto out;
 
-   ret = -EPERM;
/* Map the lower ids from the parent user namespace to the
 * kernel global id space.
 */
-- 
2.17.1

[PATCH] net: unix: remove redundant assignment to variable 'err'

[Jing Xiangfeng]

After commit 37ab4fa7844a ("net: unix: allow bind to fail on mutex lock"),
the assignment to err is redundant. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 net/unix/af_unix.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 92784e51ee7d..eb82bdc6cf7c 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -878,7 +878,6 @@ static int unix_autobind(struct socket *sock)
if (err)
return err;
 
-   err = 0;
if (u->addr)
goto out;
 
-- 
2.17.1

[PATCH v2] scsi: arcmsr: Remove the superfluous break

[Jing Xiangfeng]

Remove the superfluous break, as there is a 'return' before it.

Fixes: 6b3937227479 ("arcmsr: fix command timeout under heavy load")
Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/arcmsr/arcmsr_hba.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index ec895d0319f0..74add6d247d5 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2699,10 +2699,8 @@ static irqreturn_t arcmsr_interrupt(struct 
AdapterControlBlock *acb)
switch (acb->adapter_type) {
case ACB_ADAPTER_TYPE_A:
return arcmsr_hbaA_handle_isr(acb);
-   break;
case ACB_ADAPTER_TYPE_B:
return arcmsr_hbaB_handle_isr(acb);
-   break;
case ACB_ADAPTER_TYPE_C:
return arcmsr_hbaC_handle_isr(acb);
case ACB_ADAPTER_TYPE_D:
-- 
2.17.1

[PATCH] KVM: PPC: Book3S: Remove redundant initialization of variable ret

[Jing Xiangfeng]

The variable ret is being initialized with '-ENOMEM' that is meaningless.
So remove it.

Signed-off-by: Jing Xiangfeng 
---
 arch/powerpc/kvm/book3s_64_vio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 1a529df0ab44..b277a75cd1be 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -283,7 +283,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
struct kvmppc_spapr_tce_table *siter;
struct mm_struct *mm = kvm->mm;
unsigned long npages, size = args->size;
-   int ret = -ENOMEM;
+   int ret;
 
if (!args->size || args->page_shift < 12 || args->page_shift > 34 ||
(args->offset + args->size > (ULLONG_MAX >> args->page_shift)))
-- 
2.17.1

[PATCH] ARM: OMAP2+: Remove redundant assignment to variable ret

[Jing Xiangfeng]

The variable ret has been initialized with '-ENOMEM'. The assignment
in the if branch is redundant. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 arch/arm/mach-omap2/omap_device.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_device.c 
b/arch/arm/mach-omap2/omap_device.c
index fc7bb2ca1672..f3191704cab9 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -334,10 +334,9 @@ struct omap_device *omap_device_alloc(struct 
platform_device *pdev,
struct omap_hwmod **hwmods;
 
od = kzalloc(sizeof(struct omap_device), GFP_KERNEL);
-   if (!od) {
-   ret = -ENOMEM;
+   if (!od)
goto oda_exit1;
-   }
+
od->hwmods_cnt = oh_cnt;
 
hwmods = kmemdup(ohs, sizeof(struct omap_hwmod *) * oh_cnt, GFP_KERNEL);
-- 
2.17.1

[PATCH] xen: remove redundant initialization of variable ret

[Jing Xiangfeng]

After commit 9f51c05dc41a ("pvcalls-front: Avoid
get_free_pages(GFP_KERNEL) under spinlock"), the variable ret is being
initialized with '-ENOMEM' that is meaningless. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/xen/pvcalls-front.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index 72d725a0ab5c..7984645b5956 100644
--- a/drivers/xen/pvcalls-front.c
+++ b/drivers/xen/pvcalls-front.c
@@ -371,7 +371,7 @@ static int alloc_active_ring(struct sock_mapping *map)
 static int create_active(struct sock_mapping *map, evtchn_port_t *evtchn)
 {
void *bytes;
-   int ret = -ENOMEM, irq = -1, i;
+   int ret, irq = -1, i;
 
*evtchn = 0;
init_waitqueue_head(>active.inflight_conn_req);
-- 
2.17.1

Re: [PATCH] scsi: arcmsr: Remove the superfluous break

[Jing Xiangfeng]

On 2020/9/18 22:56, Gustavo A. R. Silva wrote:

On Fri, Sep 18, 2020 at 05:32:30PM +0800, Jing Xiangfeng wrote:

Remove the superfluous break, as there is a 'return' before it.



Apparently, the change is correct. Please, just add a proper Fixes tag by
yourself this time.


Thanks for comments! I'll resend with the following Fixes tag:

Fixes: 6b3937227479 ("arcmsr: fix command timeout under heavy load")


Thanks
--
Gustavo


Signed-off-by: Jing Xiangfeng 
---
  drivers/scsi/arcmsr/arcmsr_hba.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index ec895d0319f0..74add6d247d5 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2699,10 +2699,8 @@ static irqreturn_t arcmsr_interrupt(struct 
AdapterControlBlock *acb)
switch (acb->adapter_type) {
case ACB_ADAPTER_TYPE_A:
return arcmsr_hbaA_handle_isr(acb);
-   break;
case ACB_ADAPTER_TYPE_B:
return arcmsr_hbaB_handle_isr(acb);
-   break;
case ACB_ADAPTER_TYPE_C:
return arcmsr_hbaC_handle_isr(acb);
case ACB_ADAPTER_TYPE_D:
--
2.17.1


.

[PATCH] scsi: arcmsr: Remove the superfluous break

[Jing Xiangfeng]

Remove the superfluous break, as there is a 'return' before it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/arcmsr/arcmsr_hba.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index ec895d0319f0..74add6d247d5 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2699,10 +2699,8 @@ static irqreturn_t arcmsr_interrupt(struct 
AdapterControlBlock *acb)
switch (acb->adapter_type) {
case ACB_ADAPTER_TYPE_A:
return arcmsr_hbaA_handle_isr(acb);
-   break;
case ACB_ADAPTER_TYPE_B:
return arcmsr_hbaB_handle_isr(acb);
-   break;
case ACB_ADAPTER_TYPE_C:
return arcmsr_hbaC_handle_isr(acb);
case ACB_ADAPTER_TYPE_D:
-- 
2.17.1

[PATCH] scsi: remove redundant initialization of variable ret

[Jing Xiangfeng]

The variable ret is being initialized with '-ENOMEM' that is
meaningless. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/arm/oak.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/arm/oak.c b/drivers/scsi/arm/oak.c
index 7c9d361e91a9..78f33d57c3e8 100644
--- a/drivers/scsi/arm/oak.c
+++ b/drivers/scsi/arm/oak.c
@@ -120,7 +120,7 @@ static struct scsi_host_template oakscsi_template = {
 static int oakscsi_probe(struct expansion_card *ec, const struct ecard_id *id)
 {
struct Scsi_Host *host;
-   int ret = -ENOMEM;
+   int ret;
 
ret = ecard_request_resources(ec);
if (ret)
-- 
2.17.1

[PATCH v2] fbcon: Remove the superfluous break

[Jing Xiangfeng]

Remove the superfluous break, as there is a 'return' before it.

Fixes: bad07ff74c32 ("fbcon: smart blitter usage for scrolling")
Signed-off-by: Jing Xiangfeng 
Reviewed-by: Nick Desaulniers 
Reviewed-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/core/fbcon.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 0b49b0f44edf..623359aadd1e 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1727,7 +1727,6 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int 
t, unsigned int b,
vc->vc_video_erase_char,
vc->vc_size_row * count);
return true;
-   break;
 
case SCROLL_WRAP_MOVE:
if (b - t - count > 3 * vc->vc_rows >> 2) {
@@ -1818,7 +1817,6 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int 
t, unsigned int b,
vc->vc_video_erase_char,
vc->vc_size_row * count);
return true;
-   break;
 
case SCROLL_WRAP_MOVE:
if (b - t - count > 3 * vc->vc_rows >> 2) {
-- 
2.17.1

Re: [PATCH] fbcon: Remove the superfluous break

[Jing Xiangfeng]

On 2020/9/18 2:52, Gustavo A. R. Silva wrote:



On 9/17/20 08:15, Jing Xiangfeng wrote:

Remove the superfuous break, as there is a 'return' before it.

Signed-off-by: Jing Xiangfeng 


Reviewed-by: Gustavo A. R. Silva 

Also, the following Fixes tag should be included in the changelog text:

Fixes: bad07ff74c32 ("fbcon: smart blitter usage for scrolling")


OK, I'll send a v2 with this tag.



Thanks
--
Gustavo


---
  drivers/video/fbdev/core/fbcon.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 0b49b0f44edf..623359aadd1e 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1727,7 +1727,6 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int 
t, unsigned int b,
vc->vc_video_erase_char,
vc->vc_size_row * count);
return true;
-   break;

case SCROLL_WRAP_MOVE:
if (b - t - count > 3 * vc->vc_rows >> 2) {
@@ -1818,7 +1817,6 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int 
t, unsigned int b,
vc->vc_video_erase_char,
vc->vc_size_row * count);
return true;
-   break;

case SCROLL_WRAP_MOVE:
if (b - t - count > 3 * vc->vc_rows >> 2) {


.

Re: [PATCH] fbcon: Remove the superfluous break

[Jing Xiangfeng]

On 2020/9/18 1:36, Nick Desaulniers wrote:

On Thu, Sep 17, 2020 at 6:15 AM Jing Xiangfeng  wrote:


Remove the superfuous break, as there is a 'return' before it.


superfluous (missed "l")


Thanks for correcting!




Signed-off-by: Jing Xiangfeng 


Reviewed-by: Nick Desaulniers 

Thanks for the patch; I audited the rest of the switch statements in
this translation unit; LGTM.


---
  drivers/video/fbdev/core/fbcon.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 0b49b0f44edf..623359aadd1e 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1727,7 +1727,6 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int 
t, unsigned int b,
 vc->vc_video_erase_char,
 vc->vc_size_row * count);
 return true;
-   break;

 case SCROLL_WRAP_MOVE:
 if (b - t - count > 3 * vc->vc_rows >> 2) {
@@ -1818,7 +1817,6 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int 
t, unsigned int b,
 vc->vc_video_erase_char,
 vc->vc_size_row * count);
 return true;
-   break;

 case SCROLL_WRAP_MOVE:
 if (b - t - count > 3 * vc->vc_rows >> 2) {
--
2.17.1

[PATCH] fbcon: Remove the superfluous break

[Jing Xiangfeng]

Remove the superfuous break, as there is a 'return' before it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/video/fbdev/core/fbcon.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 0b49b0f44edf..623359aadd1e 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1727,7 +1727,6 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int 
t, unsigned int b,
vc->vc_video_erase_char,
vc->vc_size_row * count);
return true;
-   break;
 
case SCROLL_WRAP_MOVE:
if (b - t - count > 3 * vc->vc_rows >> 2) {
@@ -1818,7 +1817,6 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int 
t, unsigned int b,
vc->vc_video_erase_char,
vc->vc_size_row * count);
return true;
-   break;
 
case SCROLL_WRAP_MOVE:
if (b - t - count > 3 * vc->vc_rows >> 2) {
-- 
2.17.1

[PATCH] drm/ttm: remove redundant initialization of variable ret

[Jing Xiangfeng]

The variable ret is being initialized with '-ENOMEM' that is
meaningless. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 3437711ddb43..4db87b9b57a8 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -388,7 +388,7 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file 
*persistent_swap_storage)
struct page *from_page;
struct page *to_page;
int i;
-   int ret = -ENOMEM;
+   int ret;
 
BUG_ON(ttm->state != tt_unbound && ttm->state != tt_unpopulated);
BUG_ON(ttm->caching_state != tt_cached);
-- 
2.17.1

[PATCH] scsi: qedf: remove redundant assignment to variable 'rc'

[Jing Xiangfeng]

This assignment is  meaningless, so remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/qedf/qedf_io.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
index acd9774a9387..c04078283121 100644
--- a/drivers/scsi/qedf/qedf_io.c
+++ b/drivers/scsi/qedf/qedf_io.c
@@ -2159,7 +2159,6 @@ int qedf_initiate_cleanup(struct qedf_ioreq *io_req,
/* Sanity check qedf_rport before dereferencing any pointers */
if (!test_bit(QEDF_RPORT_SESSION_READY, >flags)) {
QEDF_ERR(NULL, "tgt not offloaded\n");
-   rc = 1;
return SUCCESS;
}
 
-- 
2.17.1

Re: [PATCH] scsi: esas2r: prevent a potential NULL dereference in esas2r_probe()

[Jing Xiangfeng]

On 2020/9/16 5:44, Martin K. Petersen wrote:


Jing,


esas2r_probe() calls scsi_host_put() in an error path. However,
esas2r_log_dev() may hit a potential NULL dereference. So use NUll instead.


Wouldn't it be better to move the scsi_host_put() call after the error
message?


There is already a message before the scsi_host_put() call. It is used 
to record calling function.

[PATCH] libnvdimm: Fix dereference of pointer ndns before it is null checked

[Jing Xiangfeng]

In current code, the pointer ndns is being dereferenced on the
initialization of pointer parent_uuid before ndns is null check. This
could lead to a potential null pointer dereference. Fix this by
dereferencing ndns after ndns has been null pointer sanity checked.

Signed-off-by: Jing Xiangfeng 
---
 drivers/nvdimm/pfn_devs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 3e11ef8d3f5b..c443994f81f3 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -452,7 +452,7 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
unsigned long align, start_pad;
struct nd_pfn_sb *pfn_sb = nd_pfn->pfn_sb;
struct nd_namespace_common *ndns = nd_pfn->ndns;
-   const u8 *parent_uuid = nd_dev_to_uuid(>dev);
+   const u8 *parent_uuid;
 
if (!pfn_sb || !ndns)
return -ENODEV;
@@ -472,6 +472,7 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
return -ENODEV;
pfn_sb->checksum = cpu_to_le64(checksum);
 
+   parent_uuid = nd_dev_to_uuid(>dev);
if (memcmp(pfn_sb->parent_uuid, parent_uuid, 16) != 0)
return -ENODEV;
 
-- 
2.17.1

[PATCH] ssb: Remove meaningless jump label to simplify the code

[Jing Xiangfeng]

The out jump label has nothing to do. So remove it to simplify the code.

Signed-off-by: Jing Xiangfeng 
---
 drivers/ssb/pci.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c
index 7c3ae52f2b15..dac54041ad8d 100644
--- a/drivers/ssb/pci.c
+++ b/drivers/ssb/pci.c
@@ -1164,17 +1164,12 @@ void ssb_pci_exit(struct ssb_bus *bus)
 int ssb_pci_init(struct ssb_bus *bus)
 {
struct pci_dev *pdev;
-   int err;
 
if (bus->bustype != SSB_BUSTYPE_PCI)
return 0;
 
pdev = bus->host_pci;
mutex_init(>sprom_mutex);
-   err = device_create_file(>dev, _attr_ssb_sprom);
-   if (err)
-   goto out;
 
-out:
-   return err;
+   return device_create_file(>dev, _attr_ssb_sprom);
 }
-- 
2.17.1

[PATCH] scsi: target: remove redundant assignment to variable 'ret'

[Jing Xiangfeng]

The variable ret has been initialized with a value '0'. The assignment
in switch-case is redundant. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/target/iscsi/iscsi_target.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index cd045dc75a58..f5272ac18b16 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4516,7 +4516,6 @@ int iscsit_logout_post_handler(
iscsit_logout_post_handler_closesession(conn);
break;
}
-   ret = 0;
break;
case ISCSI_LOGOUT_REASON_CLOSE_CONNECTION:
if (conn->cid == cmd->logout_cid) {
@@ -4527,7 +4526,6 @@ int iscsit_logout_post_handler(
iscsit_logout_post_handler_samecid(conn);
break;
}
-   ret = 0;
} else {
switch (cmd->logout_response) {
case ISCSI_LOGOUT_SUCCESS:
-- 
2.17.1

[PATCH] sbus: char: Remove meaningless jump label out_free

[Jing Xiangfeng]

After commit 57a4a3d7f756 ("display7seg: Introduce the use of the managed
version of kzalloc"), The out_free jump label has nothing to do but goto
out. So remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/sbus/char/display7seg.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/sbus/char/display7seg.c b/drivers/sbus/char/display7seg.c
index fad936eb845f..00e72b97d0b6 100644
--- a/drivers/sbus/char/display7seg.c
+++ b/drivers/sbus/char/display7seg.c
@@ -186,7 +186,7 @@ static int d7s_probe(struct platform_device *op)
p->regs = of_ioremap(>resource[0], 0, sizeof(u8), "d7s");
if (!p->regs) {
printk(KERN_ERR PFX "Cannot map chip registers\n");
-   goto out_free;
+   goto out;
}
 
err = misc_register(_miscdev);
@@ -228,8 +228,6 @@ static int d7s_probe(struct platform_device *op)
 
 out_iounmap:
of_iounmap(>resource[0], p->regs, sizeof(u8));
-
-out_free:
goto out;
 }
 
-- 
2.17.1

[PATCH] i3c: master: Fix error return in cdns_i3c_master_probe()

[Jing Xiangfeng]

Fix to return negative error code -ENOMEM from the error handling
case instead of 0.

Signed-off-by: Jing Xiangfeng 
---
 drivers/i3c/master/i3c-master-cdns.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master/i3c-master-cdns.c 
b/drivers/i3c/master/i3c-master-cdns.c
index 3fee8bd7fe20..3f2226928fe0 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -1635,8 +1635,10 @@ static int cdns_i3c_master_probe(struct platform_device 
*pdev)
master->ibi.slots = devm_kcalloc(>dev, master->ibi.num_slots,
 sizeof(*master->ibi.slots),
 GFP_KERNEL);
-   if (!master->ibi.slots)
+   if (!master->ibi.slots) {
+   ret = -ENOMEM;
goto err_disable_sysclk;
+   }
 
writel(IBIR_THR(1), master->regs + CMD_IBI_THR_CTRL);
writel(MST_INT_IBIR_THR, master->regs + MST_IER);
-- 
2.17.1

[PATCH] scsi: mvumi: Fix error return in mvumi_io_attach()

[Jing Xiangfeng]

Fix to return error code PTR_ERR() from the error handling case instead
of 0.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/mvumi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
index 8906aceda4c4..0354898d7cac 100644
--- a/drivers/scsi/mvumi.c
+++ b/drivers/scsi/mvumi.c
@@ -2425,6 +2425,7 @@ static int mvumi_io_attach(struct mvumi_hba *mhba)
if (IS_ERR(mhba->dm_thread)) {
dev_err(>pdev->dev,
"failed to create device scan thread\n");
+   ret = PTR_ERR(mhba->dm_thread);
mutex_unlock(>sas_discovery_mutex);
goto fail_create_thread;
}
-- 
2.17.1

Re: [PATCH] drm/mm: prevent a potential null-pointer dereference

[Jing Xiangfeng]

On 2020/9/10 16:58, Christian König wrote:

Am 10.09.20 um 04:38 schrieb Jing Xiangfeng:

The macro 'DECLARE_NEXT_HOLE_ADDR' may hit a potential null-pointer
dereference. So use 'entry' after checking it.


I don't see a potential null-pointer dereference here.

Where should that be?


In current code，the "entry" pointer is checked after entry->rb_hole_addr.
Thanks


Christian.



Fixes: 5fad79fd66ff ("drm/mm: cleanup and improve next_hole_*_addr()")
Signed-off-by: Jing Xiangfeng 
---
  drivers/gpu/drm/drm_mm.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index a4a04d246135..6fcf70f71962 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -392,11 +392,14 @@ first_hole(struct drm_mm *mm,
  #define DECLARE_NEXT_HOLE_ADDR(name, first, last)\
  static struct drm_mm_node *name(struct drm_mm_node *entry, u64
size)\
  {\
-struct rb_node *parent, *node = >rb_hole_addr;\
+struct rb_node *parent, *node;\
  \
-if (!entry || RB_EMPTY_NODE(node))\
+if (!entry)\
  return NULL;\
  \
+node = >rb_hole_addr;\
+if (RB_EMPTY_NODE(node))\
+return NULL;\
  if (usable_hole_addr(node->first, size)) {\
  node = node->first;\
  while (usable_hole_addr(node->last, size))\


.

[PATCH v2] clk: qcom: lpass: Correct goto target in lpass_core_sc7180_probe()

[Jing Xiangfeng]

lpass_core_sc7180_probe() misses to call pm_clk_destroy() and
pm_runtime_disable() in error paths. Correct goto target to fix it.
This issue is found by code inspection.

Fixes: edab812d802d ("clk: qcom: lpass: Add support for LPASS clock controller 
for SC7180")
Signed-off-by: Jing Xiangfeng 
---
 drivers/clk/qcom/lpasscorecc-sc7180.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/lpasscorecc-sc7180.c 
b/drivers/clk/qcom/lpasscorecc-sc7180.c
index d4c1864e1ee9..228d08f5d26f 100644
--- a/drivers/clk/qcom/lpasscorecc-sc7180.c
+++ b/drivers/clk/qcom/lpasscorecc-sc7180.c
@@ -420,17 +420,18 @@ static int lpass_core_sc7180_probe(struct platform_device 
*pdev)
pm_runtime_enable(>dev);
ret = pm_clk_create(>dev);
if (ret)
-   return ret;
+   goto disable_pm_runtime;
 
ret = pm_clk_add(>dev, "iface");
if (ret < 0) {
dev_err(>dev, "failed to acquire iface clock\n");
-   goto disable_pm_runtime;
+   goto destroy_pm_clk;
}
 
+   ret = -EINVAL;
clk_probe = of_device_get_match_data(>dev);
if (!clk_probe)
-   return -EINVAL;
+   goto destroy_pm_clk;
 
ret = clk_probe(pdev);
if (ret)
-- 
2.26.0.106.g9fadedd

Re: [PATCH] clk: qcom: lpass: Correct goto target in lpass_core_sc7180_probe()

[Jing Xiangfeng]

On 2020/9/10 17:03, Stephen Boyd wrote:

Quoting Jing Xiangfeng (2020-08-27 07:16:29)

lpass_core_sc7180_probe() misses to call pm_clk_destroy() and
pm_runtime_disable() in error paths. Correct goto target to fix it.
This issue is found by code inspection.

Signed-off-by: Jing Xiangfeng 
---


HMm.. presumably

Fixes: edab812d802d ("clk: qcom: lpass: Add support for LPASS clock controller for 
SC7180")

should be added?


Ok, I will send a v2 with it.
Thanks

.

[PATCH] drm/mm: prevent a potential null-pointer dereference

[Jing Xiangfeng]

The macro 'DECLARE_NEXT_HOLE_ADDR' may hit a potential null-pointer
dereference. So use 'entry' after checking it.

Fixes: 5fad79fd66ff ("drm/mm: cleanup and improve next_hole_*_addr()")
Signed-off-by: Jing Xiangfeng 
---
 drivers/gpu/drm/drm_mm.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index a4a04d246135..6fcf70f71962 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -392,11 +392,14 @@ first_hole(struct drm_mm *mm,
 #define DECLARE_NEXT_HOLE_ADDR(name, first, last)  \
 static struct drm_mm_node *name(struct drm_mm_node *entry, u64 size)   \
 {  \
-   struct rb_node *parent, *node = >rb_hole_addr;   \
+   struct rb_node *parent, *node;  \
\
-   if (!entry || RB_EMPTY_NODE(node))  \
+   if (!entry) \
return NULL;\
\
+   node = >rb_hole_addr;\
+   if (RB_EMPTY_NODE(node))\
+   return NULL;\
if (usable_hole_addr(node->first, size)) {  \
node = node->first; \
while (usable_hole_addr(node->last, size))  \
-- 
2.17.1

[PATCH] scsi: esas2r: prevent a potential NULL dereference in esas2r_probe()

[Jing Xiangfeng]

esas2r_probe() calls scsi_host_put() in an error path. However,
esas2r_log_dev() may hit a potential NULL dereference. So use NUll instead.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/esas2r/esas2r_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/esas2r/esas2r_main.c 
b/drivers/scsi/esas2r/esas2r_main.c
index 7b49e2e9fcde..7d3fa9dac4ce 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -456,7 +456,7 @@ static int esas2r_probe(struct pci_dev *pcid,
 
scsi_host_put(host);
 
-   esas2r_log_dev(ESAS2R_LOG_INFO, &(host->shost_gendev),
+   esas2r_log_dev(ESAS2R_LOG_INFO, NULL,
   "pci_set_drvdata(%p, NULL) called",
   pcid);
 
-- 
2.17.1

[PATCH] scsi: ibmvfc: Fix error return in ibmvfc_probe()

[Jing Xiangfeng]

Fix to return error code PTR_ERR() from the error handling case instead
of 0.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index ea7c8930592d..70daa0605082 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -4928,6 +4928,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const 
struct vio_device_id *id)
if (IS_ERR(vhost->work_thread)) {
dev_err(dev, "Couldn't create kernel thread: %ld\n",
PTR_ERR(vhost->work_thread));
+   rc = PTR_ERR(vhost->work_thread);
goto free_host_mem;
}
 
-- 
2.17.1

[PATCH] atm: eni: fix the missed pci_disable_device() for eni_init_one()

[Jing Xiangfeng]

eni_init_one() misses to call pci_disable_device() in an error path.
Jump to err_disable to fix it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/atm/eni.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/atm/eni.c b/drivers/atm/eni.c
index 39be444534d0..316a9947541f 100644
--- a/drivers/atm/eni.c
+++ b/drivers/atm/eni.c
@@ -2224,7 +2224,7 @@ static int eni_init_one(struct pci_dev *pci_dev,
 
rc = dma_set_mask_and_coherent(_dev->dev, DMA_BIT_MASK(32));
if (rc < 0)
-   goto out;
+   goto err_disable;
 
rc = -ENOMEM;
eni_dev = kmalloc(sizeof(struct eni_dev), GFP_KERNEL);
-- 
2.17.1

[PATCH] fsi: Fix an IS_ERR() vs NULL check in occ_probe()

[Jing Xiangfeng]

The platform_device_register_full() function returns error pointers,
it never returns NULL.

Signed-off-by: Jing Xiangfeng 
---
 drivers/fsi/fsi-occ.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
index 7da9c81759ac..9eeb856c8905 100644
--- a/drivers/fsi/fsi-occ.c
+++ b/drivers/fsi/fsi-occ.c
@@ -555,7 +555,7 @@ static int occ_probe(struct platform_device *pdev)
 
hwmon_dev_info.id = occ->idx;
hwmon_dev = platform_device_register_full(_dev_info);
-   if (!hwmon_dev)
+   if (IS_ERR(hwmon_dev))
dev_warn(dev, "failed to create hwmon device\n");
 
return 0;
-- 
2.17.1

[PATCH] clk: qcom: lpass: Correct goto target in lpass_core_sc7180_probe()

[Jing Xiangfeng]

lpass_core_sc7180_probe() misses to call pm_clk_destroy() and
pm_runtime_disable() in error paths. Correct goto target to fix it.
This issue is found by code inspection.

Signed-off-by: Jing Xiangfeng 
---
 drivers/clk/qcom/lpasscorecc-sc7180.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/lpasscorecc-sc7180.c 
b/drivers/clk/qcom/lpasscorecc-sc7180.c
index d4c1864e1ee9..228d08f5d26f 100644
--- a/drivers/clk/qcom/lpasscorecc-sc7180.c
+++ b/drivers/clk/qcom/lpasscorecc-sc7180.c
@@ -420,17 +420,18 @@ static int lpass_core_sc7180_probe(struct platform_device 
*pdev)
pm_runtime_enable(>dev);
ret = pm_clk_create(>dev);
if (ret)
-   return ret;
+   goto disable_pm_runtime;
 
ret = pm_clk_add(>dev, "iface");
if (ret < 0) {
dev_err(>dev, "failed to acquire iface clock\n");
-   goto disable_pm_runtime;
+   goto destroy_pm_clk;
}
 
+   ret = -EINVAL;
clk_probe = of_device_get_match_data(>dev);
if (!clk_probe)
-   return -EINVAL;
+   goto destroy_pm_clk;
 
ret = clk_probe(pdev);
if (ret)
-- 
2.26.0.106.g9fadedd

[PATCH] vdpa/mlx5: Remove duplicate include

[Jing Xiangfeng]

Remove linux/mlx5/device.h which is included more than once

Signed-off-by: Jing Xiangfeng 
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 70676a6d1691..b4bdbe062b7c 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -8,7 +8,6 @@
 #include 
 #include 
 #include 
-#include 
 #include "mlx5_vnet.h"
 #include "mlx5_vdpa_ifc.h"
 #include "mlx5_vdpa.h"
-- 
2.26.0.106.g9fadedd

[PATCH] scsi: megaraid: Remove unnecessary assignment to variable ret

[Jing Xiangfeng]

The variable ret is being initialized with 'FAILED'. So we can remove
this assignement.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 0824410f78f8..96d424645b18 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -4700,7 +4700,6 @@ int megasas_reset_target_fusion(struct scsi_cmnd *scmd)
if (atomic_read(>adprecovery) != MEGASAS_HBA_OPERATIONAL) {
dev_err(>pdev->dev, "Controller is not OPERATIONAL,"
"SCSI host:%d\n", instance->host->host_no);
-   ret = FAILED;
return ret;
}
 
@@ -4713,7 +4712,6 @@ int megasas_reset_target_fusion(struct scsi_cmnd *scmd)
}
 
if (!mr_device_priv_data->is_tm_capable) {
-   ret = FAILED;
goto out;
}
 
-- 
2.17.1

[PATCH] USB: usblcd: Remove the superfluous break

[Jing Xiangfeng]

Remove the superfuous break, as there is a 'return' before it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/usb/misc/usblcd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index 61e9e987fe4a..bb546f624a45 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -187,7 +187,6 @@ static long lcd_ioctl(struct file *file, unsigned int cmd, 
unsigned long arg)
break;
default:
return -ENOTTY;
-   break;
}
 
return 0;
-- 
2.17.1

[PATCH v2] scsi: ufs: ti-j721e-ufs: Fix error return in ti_j721e_ufs_probe()

[Jing Xiangfeng]

Fix to return error code PTR_ERR() from the error handling case instead
of 0.

Fixes: 22617e216331 ("scsi: ufs: ti-j721e-ufs: Fix unwinding of pm_runtime 
changes")
Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/ufs/ti-j721e-ufs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/ti-j721e-ufs.c b/drivers/scsi/ufs/ti-j721e-ufs.c
index 46bb905b4d6a..eafe0db98d54 100644
--- a/drivers/scsi/ufs/ti-j721e-ufs.c
+++ b/drivers/scsi/ufs/ti-j721e-ufs.c
@@ -38,6 +38,7 @@ static int ti_j721e_ufs_probe(struct platform_device *pdev)
/* Select MPHY refclk frequency */
clk = devm_clk_get(dev, NULL);
if (IS_ERR(clk)) {
+   ret = PTR_ERR(clk);
dev_err(dev, "Cannot claim MPHY clock.\n");
goto clk_err;
}
-- 
2.17.1

Re: [PATCH] scsi: ufs: ti-j721e-ufs: Fix error return in ti_j721e_ufs_probe()

[Jing Xiangfeng]

Please ignore this patch.
Thanks

On 2020/8/6 14:44, Jing Xiangfeng wrote:

Fix to return error code IS_ERR() from the error handling case instead
of 0.

Fixes: 22617e216331 ("scsi: ufs: ti-j721e-ufs: Fix unwinding of pm_runtime 
changes")
Signed-off-by: Jing Xiangfeng 
---
  drivers/scsi/ufs/ti-j721e-ufs.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/ti-j721e-ufs.c b/drivers/scsi/ufs/ti-j721e-ufs.c
index 46bb905b4d6a..eafe7c08b0c8 100644
--- a/drivers/scsi/ufs/ti-j721e-ufs.c
+++ b/drivers/scsi/ufs/ti-j721e-ufs.c
@@ -38,6 +38,7 @@ static int ti_j721e_ufs_probe(struct platform_device *pdev)
/* Select MPHY refclk frequency */
clk = devm_clk_get(dev, NULL);
if (IS_ERR(clk)) {
+   ret = IS_ERR(clk);
dev_err(dev, "Cannot claim MPHY clock.\n");
goto clk_err;
}

[PATCH] scsi: ufs: ti-j721e-ufs: Fix error return in ti_j721e_ufs_probe()

[Jing Xiangfeng]

Fix to return error code IS_ERR() from the error handling case instead
of 0.

Fixes: 22617e216331 ("scsi: ufs: ti-j721e-ufs: Fix unwinding of pm_runtime 
changes")
Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/ufs/ti-j721e-ufs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/ti-j721e-ufs.c b/drivers/scsi/ufs/ti-j721e-ufs.c
index 46bb905b4d6a..eafe7c08b0c8 100644
--- a/drivers/scsi/ufs/ti-j721e-ufs.c
+++ b/drivers/scsi/ufs/ti-j721e-ufs.c
@@ -38,6 +38,7 @@ static int ti_j721e_ufs_probe(struct platform_device *pdev)
/* Select MPHY refclk frequency */
clk = devm_clk_get(dev, NULL);
if (IS_ERR(clk)) {
+   ret = IS_ERR(clk);
dev_err(dev, "Cannot claim MPHY clock.\n");
goto clk_err;
}
-- 
2.17.1

[PATCH] drm/nouveau/acr: fix a coding style in nvkm_acr_lsfw_load_bl_inst_data_sig()

[Jing Xiangfeng]

This patch performs the following changes:
1. remove a redundant parentheses around the  nvkm_acr_lsfw_add() calls
2. do assignment before this if condition, it is more readable

Signed-off-by: Jing Xiangfeng 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
index 07d1830126ab..5f6006418472 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
@@ -191,7 +191,8 @@ nvkm_acr_lsfw_load_bl_inst_data_sig(struct nvkm_subdev 
*subdev,
u32 *bldata;
int ret;
 
-   if (IS_ERR((lsfw = nvkm_acr_lsfw_add(func, acr, falcon, id
+   lsfw = nvkm_acr_lsfw_add(func, acr, falcon, id);
+   if (IS_ERR(lsfw))
return PTR_ERR(lsfw);
 
ret = nvkm_firmware_load_name(subdev, path, "bl", ver, );
-- 
2.17.1

[PATCH] scsi: lpfc: Add the missed misc_deregister() for lpfc_init()

[Jing Xiangfeng]

lpfc_init() misses to call misc_deregister() in an error path. Add a
label 'unregister' to fix it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/lpfc/lpfc_init.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 6637f84a3d1b..ec40bc91e124 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -13982,17 +13982,18 @@ lpfc_init(void)
printk(KERN_ERR "Could not register lpfcmgmt device, "
"misc_register returned with status %d", error);
 
+   error = -ENOMEM;
lpfc_transport_functions.vport_create = lpfc_vport_create;
lpfc_transport_functions.vport_delete = lpfc_vport_delete;
lpfc_transport_template =
fc_attach_transport(_transport_functions);
if (lpfc_transport_template == NULL)
-   return -ENOMEM;
+   goto unregister;
lpfc_vport_transport_template =
fc_attach_transport(_vport_transport_functions);
if (lpfc_vport_transport_template == NULL) {
fc_release_transport(lpfc_transport_template);
-   return -ENOMEM;
+   goto unregister;
}
lpfc_nvme_cmd_template();
lpfc_nvmet_cmd_template();
@@ -14018,6 +14019,8 @@ lpfc_init(void)
 cpuhp_failure:
fc_release_transport(lpfc_transport_template);
fc_release_transport(lpfc_vport_transport_template);
+unregister:
+   misc_deregister(_mgmt_dev);
 
return error;
 }
-- 
2.17.1

[PATCH v2] clk: ti: clkctrl: fix the missed kfree() for _ti_omap4_clkctrl_setup()

[Jing Xiangfeng]

_ti_omap4_clkctrl_setup() misses to call kfree() in an error path. Jump
to cleanup to fix it.

Fixes: 6c3090520554 ("clk: ti: clkctrl: Fix hidden dependency to node name")
Signed-off-by: Jing Xiangfeng 
---
 drivers/clk/ti/clkctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 864c484bde1b..a562261eb061 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -656,7 +656,7 @@ static void __init _ti_omap4_clkctrl_setup(struct 
device_node *node)
 
hw = kzalloc(sizeof(*hw), GFP_KERNEL);
if (!hw)
-   return;
+   goto cleanup;
 
hw->enable_reg.ptr = provider->base + reg_data->offset;
 
-- 
2.17.1

Re: [PATCH] clk: ti: clkctrl: add the missed kfree() for _ti_omap4_clkctrl_setup()

[Jing Xiangfeng]

On 2020/7/28 9:24, Stephen Boyd wrote:

Quoting Jing Xiangfeng (2020-07-20 05:23:43)

_ti_omap4_clkctrl_setup() misses to call kfree() in an error path. Add
the missed function call to fix it.

Fixes: 6c3090520554 ("clk: ti: clkctrl: Fix hidden dependency to node name")
Signed-off-by: Jing Xiangfeng 
---
  drivers/clk/ti/clkctrl.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 864c484bde1b..868e50132c21 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -655,8 +655,10 @@ static void __init _ti_omap4_clkctrl_setup(struct 
device_node *node)
 }

 hw = kzalloc(sizeof(*hw), GFP_KERNEL);
-   if (!hw)
+   if (!hw) {
+   kfree(clkctrl_name);
 return;
+   }


Why not goto cleanup?


Thanks, I will change it as you suggested.





 hw->enable_reg.ptr = provider->base + reg_data->offset;

--
2.17.1


.

[PATCH] scsi: iscsi: jump to correct label in an error path

[Jing Xiangfeng]

In current code, it jumps to put_host() when scsi_host_lookup()
failes to get host. Jump to correct label to fix it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/scsi/scsi_transport_iscsi.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c 
b/drivers/scsi/scsi_transport_iscsi.c
index 7ae5024..5984596 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3341,7 +3341,7 @@ static int iscsi_new_flashnode(struct iscsi_transport 
*transport,
pr_err("%s could not find host no %u\n",
   __func__, ev->u.new_flashnode.host_no);
err = -ENODEV;
-   goto put_host;
+   goto exit_new_fnode;
}
 
index = transport->new_flashnode(shost, data, len);
@@ -3351,7 +3351,6 @@ static int iscsi_new_flashnode(struct iscsi_transport 
*transport,
else
err = -EIO;
 
-put_host:
scsi_host_put(shost);
 
 exit_new_fnode:
@@ -3376,7 +3375,7 @@ static int iscsi_del_flashnode(struct iscsi_transport 
*transport,
pr_err("%s could not find host no %u\n",
   __func__, ev->u.del_flashnode.host_no);
err = -ENODEV;
-   goto put_host;
+   goto exit_del_fnode;
}
 
idx = ev->u.del_flashnode.flashnode_idx;
@@ -3418,7 +3417,7 @@ static int iscsi_login_flashnode(struct iscsi_transport 
*transport,
pr_err("%s could not find host no %u\n",
   __func__, ev->u.login_flashnode.host_no);
err = -ENODEV;
-   goto put_host;
+   goto exit_login_fnode;
}
 
idx = ev->u.login_flashnode.flashnode_idx;
@@ -3470,7 +3469,7 @@ static int iscsi_logout_flashnode(struct iscsi_transport 
*transport,
pr_err("%s could not find host no %u\n",
   __func__, ev->u.logout_flashnode.host_no);
err = -ENODEV;
-   goto put_host;
+   goto exit_logout_fnode;
}
 
idx = ev->u.logout_flashnode.flashnode_idx;
@@ -3520,7 +3519,7 @@ static int iscsi_logout_flashnode_sid(struct 
iscsi_transport *transport,
pr_err("%s could not find host no %u\n",
   __func__, ev->u.logout_flashnode.host_no);
err = -ENODEV;
-   goto put_host;
+   goto exit_logout_sid;
}
 
session = iscsi_session_lookup(ev->u.logout_flashnode_sid.sid);
-- 
1.8.3.1

[PATCH] ARM: OMAP2+: Fix an IS_ERR() vs NULL check in _get_pwrdm()

[Jing Xiangfeng]

The of_clk_get() function returns error pointers, it never returns NULL.

Fixes: 4ea3711aece4 ("ARM: OMAP2+: omap-iommu.c conversion to ti-sysc")
Signed-off-by: Jing Xiangfeng 
---
 arch/arm/mach-omap2/omap-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
index 54aff33e55e6..bfa5e1b8dba7 100644
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -74,7 +74,7 @@ static struct powerdomain *_get_pwrdm(struct device *dev)
return pwrdm;
 
clk = of_clk_get(dev->of_node->parent, 0);
-   if (!clk) {
+   if (IS_ERR(clk)) {
dev_err(dev, "no fck found\n");
return NULL;
}
-- 
2.17.1

[PATCH] clk: ti: clkctrl: add the missed kfree() for _ti_omap4_clkctrl_setup()

[Jing Xiangfeng]

_ti_omap4_clkctrl_setup() misses to call kfree() in an error path. Add
the missed function call to fix it.

Fixes: 6c3090520554 ("clk: ti: clkctrl: Fix hidden dependency to node name")
Signed-off-by: Jing Xiangfeng 
---
 drivers/clk/ti/clkctrl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 864c484bde1b..868e50132c21 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -655,8 +655,10 @@ static void __init _ti_omap4_clkctrl_setup(struct 
device_node *node)
}
 
hw = kzalloc(sizeof(*hw), GFP_KERNEL);
-   if (!hw)
+   if (!hw) {
+   kfree(clkctrl_name);
return;
+   }
 
hw->enable_reg.ptr = provider->base + reg_data->offset;
 
-- 
2.17.1

[PATCH] ipmi: remve duplicate code in __ipmi_bmc_register()

[Jing Xiangfeng]

__ipmi_bmc_register() jumps to the label 'out_free_my_dev_name' in an
error path. So we can remove duplicate code in the if (rv).

Signed-off-by: Jing Xiangfeng 
---
 drivers/char/ipmi/ipmi_msghandler.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
b/drivers/char/ipmi/ipmi_msghandler.c
index e1b22fe0916c..737c0b6b24ea 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -3080,8 +3080,6 @@ static int __ipmi_bmc_register(struct ipmi_smi *intf,
rv = sysfs_create_link(>pdev.dev.kobj, >si_dev->kobj,
   intf->my_dev_name);
if (rv) {
-   kfree(intf->my_dev_name);
-   intf->my_dev_name = NULL;
dev_err(intf->si_dev, "Unable to create symlink to bmc: %d\n",
rv);
goto out_free_my_dev_name;
-- 
2.17.1

[PATCH] orangefs: remove unnecessary assignment to variable ret

[Jing Xiangfeng]

The variable ret is guaranteed to be 0 in this if (). So we can remove
this assignement.

Signed-off-by: Jing Xiangfeng 
---
 fs/orangefs/orangefs-mod.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/orangefs/orangefs-mod.c b/fs/orangefs/orangefs-mod.c
index 289b648ae196..74a3d6337ef4 100644
--- a/fs/orangefs/orangefs-mod.c
+++ b/fs/orangefs/orangefs-mod.c
@@ -149,7 +149,6 @@ static int __init orangefs_init(void)
pr_info("%s: module version %s loaded\n",
__func__,
ORANGEFS_VERSION);
-   ret = 0;
goto out;
}
 
-- 
2.17.1

[PATCH v2] ASoC: meson: fixes the missed kfree() for axg_card_add_tdm_loopback

[Jing Xiangfeng]

axg_card_add_tdm_loopback() misses to call kfree() in an error path. We
can use devm_kasprintf() to fix the issue, also improve maintainability.
So use it instead.

Fixes: c84836d7f650 ("ASoC: meson: axg-card: use modern dai_link style")
Signed-off-by: Jing Xiangfeng 
---
 sound/soc/meson/axg-card.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/meson/axg-card.c b/sound/soc/meson/axg-card.c
index 89f7f64747cd..47f2d93224fe 100644
--- a/sound/soc/meson/axg-card.c
+++ b/sound/soc/meson/axg-card.c
@@ -116,7 +116,7 @@ static int axg_card_add_tdm_loopback(struct snd_soc_card 
*card,
 
lb = >dai_link[*index + 1];
 
-   lb->name = kasprintf(GFP_KERNEL, "%s-lb", pad->name);
+   lb->name = devm_kasprintf(card->dev, GFP_KERNEL, "%s-lb", pad->name);
if (!lb->name)
return -ENOMEM;
 
-- 
2.17.1

Re: [PATCH] ASoC: meson: add the missed kfree() for axg_card_add_tdm_loopback

[Jing Xiangfeng]

On 2020/7/16 21:29, Jerome Brunet wrote:


On Thu 16 Jul 2020 at 15:25, Jing Xiangfeng  wrote:


axg_card_add_tdm_loopback() misses to call kfree() in an error path. Add
the missed function call to fix it.

Fixes: c84836d7f650 ("ASoC: meson: axg-card: use modern dai_link style")
Signed-off-by: Jing Xiangfeng 


Thanks for fixing this.
Maybe it would be better to use the devm_ variant for the name instead ?


Ok, I'll send a v2 with this change.

Thanks for your review.



---
  sound/soc/meson/axg-card.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/meson/axg-card.c b/sound/soc/meson/axg-card.c
index 89f7f64747cd..6eac22ba8b99 100644
--- a/sound/soc/meson/axg-card.c
+++ b/sound/soc/meson/axg-card.c
@@ -121,8 +121,10 @@ static int axg_card_add_tdm_loopback(struct snd_soc_card 
*card,
return -ENOMEM;

dlc = devm_kzalloc(card->dev, 2 * sizeof(*dlc), GFP_KERNEL);
-   if (!dlc)
+   if (!dlc) {
+   kfree(lb->name);
return -ENOMEM;
+   }

lb->cpus = [0];
lb->codecs = [1];


.

[PATCH] ASoC: meson: add the missed kfree() for axg_card_add_tdm_loopback

[Jing Xiangfeng]

axg_card_add_tdm_loopback() misses to call kfree() in an error path. Add
the missed function call to fix it.

Fixes: c84836d7f650 ("ASoC: meson: axg-card: use modern dai_link style")
Signed-off-by: Jing Xiangfeng 
---
 sound/soc/meson/axg-card.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/meson/axg-card.c b/sound/soc/meson/axg-card.c
index 89f7f64747cd..6eac22ba8b99 100644
--- a/sound/soc/meson/axg-card.c
+++ b/sound/soc/meson/axg-card.c
@@ -121,8 +121,10 @@ static int axg_card_add_tdm_loopback(struct snd_soc_card 
*card,
return -ENOMEM;
 
dlc = devm_kzalloc(card->dev, 2 * sizeof(*dlc), GFP_KERNEL);
-   if (!dlc)
+   if (!dlc) {
+   kfree(lb->name);
return -ENOMEM;
+   }
 
lb->cpus = [0];
lb->codecs = [1];
-- 
2.17.1

Re: [PATCH v2] scsi: fcoe: add missed kfree() in an error path

[Jing Xiangfeng]

On 2020/7/14 1:53, Ewan D. Milne wrote:

See below.

On Thu, 2020-07-09 at 20:05 +0800, Jing Xiangfeng wrote:

fcoe_fdmi_info() misses to call kfree() in an error path.
Add a label 'free_fdmi' and jump to it.

Fixes: f07d46bbc9ba ("fcoe: Fix smatch warning in fcoe_fdmi_info
function")
Signed-off-by: Jing Xiangfeng 
---
  drivers/scsi/fcoe/fcoe.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 25dae9f0b205..a63057a03772 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -830,7 +830,7 @@ static void fcoe_fdmi_info(struct fc_lport
*lport, struct net_device *netdev)
if (rc) {
printk(KERN_INFO "fcoe: Failed to retrieve FDMI
"
"information from netdev.\n");
-   return;
+   goto free_fdmi;
}

snprintf(fc_host_serial_number(lport->host),
@@ -868,6 +868,7 @@ static void fcoe_fdmi_info(struct fc_lport
*lport, struct net_device *netdev)

/* Enable FDMI lport states */
lport->fdmi_enabled = 1;
+free_fdmi:
kfree(fdmi);
} else {
lport->fdmi_enabled = 0;


Normally I would like to see goto labels for error paths outside
conditionals and at the end of the function.


I agree.

 In this case it would

seem to be cleaner to put an else { } clause in the if (rc) above
around the snprintf() calls.


It is ok with me. v1 is also a simpler way.

+Hannes, Which is preferable?

Thanks



-Ewan

.

Re: [PATCH] drm: remove redundant assignment to variable 'ret'

[Jing Xiangfeng]

On 2020/7/15 20:05, Daniel Vetter wrote:

On Wed, Jul 15, 2020 at 03:05:59PM +0800, Jing Xiangfeng wrote:

The variable ret has been assigned the value '-EINVAL'. The assignment
in the if() is redundant. We can remove it.


Nope, that's not correct. Before this assignement ret is guaranteed to be
0.


Before this assignment ret is '-EINVAL'(see commit 45bc3d26c95a: "drm: 
rework SET_MASTER and DROP_MASTER perm handling"). It is set to 0 above

around the drm_drop_master() calls.

Thanks

-Daniel



Signed-off-by: Jing Xiangfeng 
---
  drivers/gpu/drm/drm_auth.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 800ac39f3213..6e1b502f2797 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -299,7 +299,6 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,

if (file_priv->master->lessor != NULL) {
DRM_DEBUG_LEASE("Attempt to drop lessee %d as master\n", 
file_priv->master->lessee_id);
-   ret = -EINVAL;
goto out_unlock;
}

--
2.17.1

[PATCH] drm/nouveau: add the missed kfree() for nouveau_bo_alloc()

[Jing Xiangfeng]

nouveau_bo_alloc() misses to call kfree() in an error path. Add the
missed function call to fix it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index c40f127de3d0..abd80b1a6de3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -276,8 +276,10 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 *size, int 
*align, u32 flags,
break;
}
 
-   if (WARN_ON(pi < 0))
+   if (WARN_ON(pi < 0)) {
+   kfree(nvbo);
return ERR_PTR(-EINVAL);
+   }
 
/* Disable compression if suitable settings couldn't be found. */
if (nvbo->comp && !vmm->page[pi].comp) {
-- 
2.17.1

[PATCH] drm: remove redundant assignment to variable 'ret'

[Jing Xiangfeng]

The variable ret has been assigned the value '-EINVAL'. The assignment
in the if() is redundant. We can remove it.

Signed-off-by: Jing Xiangfeng 
---
 drivers/gpu/drm/drm_auth.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 800ac39f3213..6e1b502f2797 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -299,7 +299,6 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 
if (file_priv->master->lessor != NULL) {
DRM_DEBUG_LEASE("Attempt to drop lessee %d as master\n", 
file_priv->master->lessee_id);
-   ret = -EINVAL;
goto out_unlock;
}
 
-- 
2.17.1

[PATCH] ASoC: Intel: bytcht_es8316: Add missed put_device()

[Jing Xiangfeng]

snd_byt_cht_es8316_mc_probe() misses to call put_device() in an error
path. Add the missed function call to fix it.

Fixes: ba49cf6f8e4a ("ASoC: Intel: bytcht_es8316: Add quirk for inverted jack 
detect")
Signed-off-by: Jing Xiangfeng 
---
 sound/soc/intel/boards/bytcht_es8316.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/bytcht_es8316.c 
b/sound/soc/intel/boards/bytcht_es8316.c
index 9e5fc9430628..ecbc58e8a37f 100644
--- a/sound/soc/intel/boards/bytcht_es8316.c
+++ b/sound/soc/intel/boards/bytcht_es8316.c
@@ -543,8 +543,10 @@ static int snd_byt_cht_es8316_mc_probe(struct 
platform_device *pdev)
 
if (cnt) {
ret = device_add_properties(codec_dev, props);
-   if (ret)
+   if (ret) {
+   put_device(codec_dev);
return ret;
+   }
}
 
devm_acpi_dev_add_driver_gpios(codec_dev, byt_cht_es8316_gpios);
-- 
2.17.1