1. Patchew
  2. linux
  3. View series

[PATCH] scsi: libsas: Add error handling in the sas_register_phys()

Chaohai Chen posted 1 patch 2 weeks ago 
drivers/scsi/libsas/sas_phy.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
[PATCH] scsi: libsas: Add error handling in the sas_register_phys()
Posted by Chaohai Chen 2 weeks ago 
If an error is triggered in the loop process, we need to roll back
the resources that have already been requested.

Signed-off-by: Chaohai Chen <wdhh6@aliyun.com>
---
 drivers/scsi/libsas/sas_phy.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_phy.c b/drivers/scsi/libsas/sas_phy.c
index 635835c28ecd..455118c7e889 100644
--- a/drivers/scsi/libsas/sas_phy.c
+++ b/drivers/scsi/libsas/sas_phy.c
@@ -116,6 +116,7 @@ static void sas_phye_shutdown(struct work_struct *work)
 int sas_register_phys(struct sas_ha_struct *sas_ha)
 {
 	int i;
+	int ret = 0;
 
 	/* Now register the phys. */
 	for (i = 0; i < sas_ha->num_phys; i++) {
@@ -132,8 +133,10 @@ int sas_register_phys(struct sas_ha_struct *sas_ha)
 		phy->frame_rcvd_size = 0;
 
 		phy->phy = sas_phy_alloc(&sas_ha->shost->shost_gendev, i);
-		if (!phy->phy)
-			return -ENOMEM;
+		if (!phy->phy) {
+			ret = -ENOMEM;
+			goto fail;
+		}
 
 		phy->phy->identify.initiator_port_protocols =
 			phy->iproto;
@@ -146,10 +149,22 @@ int sas_register_phys(struct sas_ha_struct *sas_ha)
 		phy->phy->maximum_linkrate = SAS_LINK_RATE_UNKNOWN;
 		phy->phy->negotiated_linkrate = SAS_LINK_RATE_UNKNOWN;
 
-		sas_phy_add(phy->phy);
+		ret = sas_phy_add(phy->phy);
+		if (ret) {
+			sas_phy_free(phy->phy);
+			goto fail;
+		}
 	}
 
 	return 0;
+fail:
+	for (i--; i >= 0; i--) {
+		struct asd_sas_phy *phy = sas_ha->sas_phy[i];
+
+		sas_phy_delete(phy->phy);
+		sas_phy_free(phy);
+	}
+	return ret;
 }
 
 const work_func_t sas_phy_event_fns[PHY_NUM_EVENTS] = {
-- 
2.43.7
Re: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
Posted by kernel test robot 1 week, 6 days ago 
Hi Chaohai,

kernel test robot noticed the following build errors:

[auto build test ERROR on jejb-scsi/for-next]
[also build test ERROR on mkp-scsi/for-next linus/master v6.18 next-20251205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chaohai-Chen/scsi-libsas-Add-error-handling-in-the-sas_register_phys/20251205-174712
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
patch link:    https://lore.kernel.org/r/20251205080252.1020028-1-wdhh6%40aliyun.com
patch subject: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
config: hexagon-randconfig-001-20251206 (https://download.01.org/0day-ci/archive/20251206/202512061547.zszMg0HR-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251206/202512061547.zszMg0HR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512061547.zszMg0HR-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/scsi/libsas/sas_phy.c:165:16: error: incompatible pointer types passing 'struct asd_sas_phy *' to parameter of type 'struct sas_phy *' [-Werror,-Wincompatible-pointer-types]
                   sas_phy_free(phy);
                                ^~~
   include/scsi/scsi_transport_sas.h:192:42: note: passing argument to parameter here
   extern void sas_phy_free(struct sas_phy *);
                                            ^
   1 error generated.


vim +165 drivers/scsi/libsas/sas_phy.c

   115	
   116	int sas_register_phys(struct sas_ha_struct *sas_ha)
   117	{
   118		int i;
   119		int ret = 0;
   120	
   121		/* Now register the phys. */
   122		for (i = 0; i < sas_ha->num_phys; i++) {
   123			struct asd_sas_phy *phy = sas_ha->sas_phy[i];
   124	
   125			phy->error = 0;
   126			atomic_set(&phy->event_nr, 0);
   127			INIT_LIST_HEAD(&phy->port_phy_el);
   128	
   129			phy->port = NULL;
   130			phy->ha = sas_ha;
   131			spin_lock_init(&phy->frame_rcvd_lock);
   132			spin_lock_init(&phy->sas_prim_lock);
   133			phy->frame_rcvd_size = 0;
   134	
   135			phy->phy = sas_phy_alloc(&sas_ha->shost->shost_gendev, i);
   136			if (!phy->phy) {
   137				ret = -ENOMEM;
   138				goto fail;
   139			}
   140	
   141			phy->phy->identify.initiator_port_protocols =
   142				phy->iproto;
   143			phy->phy->identify.target_port_protocols = phy->tproto;
   144			phy->phy->identify.sas_address = SAS_ADDR(sas_ha->sas_addr);
   145			phy->phy->identify.phy_identifier = i;
   146			phy->phy->minimum_linkrate_hw = SAS_LINK_RATE_UNKNOWN;
   147			phy->phy->maximum_linkrate_hw = SAS_LINK_RATE_UNKNOWN;
   148			phy->phy->minimum_linkrate = SAS_LINK_RATE_UNKNOWN;
   149			phy->phy->maximum_linkrate = SAS_LINK_RATE_UNKNOWN;
   150			phy->phy->negotiated_linkrate = SAS_LINK_RATE_UNKNOWN;
   151	
   152			ret = sas_phy_add(phy->phy);
   153			if (ret) {
   154				sas_phy_free(phy->phy);
   155				goto fail;
   156			}
   157		}
   158	
   159		return 0;
   160	fail:
   161		for (i--; i >= 0; i--) {
   162			struct asd_sas_phy *phy = sas_ha->sas_phy[i];
   163	
   164			sas_phy_delete(phy->phy);
 > 165			sas_phy_free(phy);
   166		}
   167		return ret;
   168	}
   169	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
Posted by kernel test robot 1 week, 6 days ago 
Hi Chaohai,

kernel test robot noticed the following build errors:

[auto build test ERROR on jejb-scsi/for-next]
[also build test ERROR on mkp-scsi/for-next linus/master v6.18 next-20251205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chaohai-Chen/scsi-libsas-Add-error-handling-in-the-sas_register_phys/20251205-174712
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
patch link:    https://lore.kernel.org/r/20251205080252.1020028-1-wdhh6%40aliyun.com
patch subject: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
config: parisc-randconfig-002-20251206 (https://download.01.org/0day-ci/archive/20251206/202512061205.azqGXDZt-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251206/202512061205.azqGXDZt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512061205.azqGXDZt-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/scsi/libsas/sas_phy.c: In function 'sas_register_phys':
>> drivers/scsi/libsas/sas_phy.c:165:30: error: passing argument 1 of 'sas_phy_free' from incompatible pointer type [-Wincompatible-pointer-types]
     165 |                 sas_phy_free(phy);
         |                              ^~~
         |                              |
         |                              struct asd_sas_phy *
   In file included from drivers/scsi/libsas/sas_internal.h:14,
                    from drivers/scsi/libsas/sas_phy.c:9:
   include/scsi/scsi_transport_sas.h:192:26: note: expected 'struct sas_phy *' but argument is of type 'struct asd_sas_phy *'
     192 | extern void sas_phy_free(struct sas_phy *);
         |                          ^~~~~~~~~~~~~~~~


vim +/sas_phy_free +165 drivers/scsi/libsas/sas_phy.c

   115	
   116	int sas_register_phys(struct sas_ha_struct *sas_ha)
   117	{
   118		int i;
   119		int ret = 0;
   120	
   121		/* Now register the phys. */
   122		for (i = 0; i < sas_ha->num_phys; i++) {
   123			struct asd_sas_phy *phy = sas_ha->sas_phy[i];
   124	
   125			phy->error = 0;
   126			atomic_set(&phy->event_nr, 0);
   127			INIT_LIST_HEAD(&phy->port_phy_el);
   128	
   129			phy->port = NULL;
   130			phy->ha = sas_ha;
   131			spin_lock_init(&phy->frame_rcvd_lock);
   132			spin_lock_init(&phy->sas_prim_lock);
   133			phy->frame_rcvd_size = 0;
   134	
   135			phy->phy = sas_phy_alloc(&sas_ha->shost->shost_gendev, i);
   136			if (!phy->phy) {
   137				ret = -ENOMEM;
   138				goto fail;
   139			}
   140	
   141			phy->phy->identify.initiator_port_protocols =
   142				phy->iproto;
   143			phy->phy->identify.target_port_protocols = phy->tproto;
   144			phy->phy->identify.sas_address = SAS_ADDR(sas_ha->sas_addr);
   145			phy->phy->identify.phy_identifier = i;
   146			phy->phy->minimum_linkrate_hw = SAS_LINK_RATE_UNKNOWN;
   147			phy->phy->maximum_linkrate_hw = SAS_LINK_RATE_UNKNOWN;
   148			phy->phy->minimum_linkrate = SAS_LINK_RATE_UNKNOWN;
   149			phy->phy->maximum_linkrate = SAS_LINK_RATE_UNKNOWN;
   150			phy->phy->negotiated_linkrate = SAS_LINK_RATE_UNKNOWN;
   151	
   152			ret = sas_phy_add(phy->phy);
   153			if (ret) {
   154				sas_phy_free(phy->phy);
   155				goto fail;
   156			}
   157		}
   158	
   159		return 0;
   160	fail:
   161		for (i--; i >= 0; i--) {
   162			struct asd_sas_phy *phy = sas_ha->sas_phy[i];
   163	
   164			sas_phy_delete(phy->phy);
 > 165			sas_phy_free(phy);
   166		}
   167		return ret;
   168	}
   169	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
Posted by Jason Yan 2 weeks ago 
在 2025/12/5 16:02, Chaohai Chen 写道:
> If an error is triggered in the loop process, we need to roll back
> the resources that have already been requested.
> 
> Signed-off-by: Chaohai Chen <wdhh6@aliyun.com>
> ---
>   drivers/scsi/libsas/sas_phy.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_phy.c b/drivers/scsi/libsas/sas_phy.c
> index 635835c28ecd..455118c7e889 100644
> --- a/drivers/scsi/libsas/sas_phy.c
> +++ b/drivers/scsi/libsas/sas_phy.c
> @@ -116,6 +116,7 @@ static void sas_phye_shutdown(struct work_struct *work)
>   int sas_register_phys(struct sas_ha_struct *sas_ha)
>   {
>   	int i;
> +	int ret = 0;
>   
>   	/* Now register the phys. */
>   	for (i = 0; i < sas_ha->num_phys; i++) {
> @@ -132,8 +133,10 @@ int sas_register_phys(struct sas_ha_struct *sas_ha)
>   		phy->frame_rcvd_size = 0;
>   
>   		phy->phy = sas_phy_alloc(&sas_ha->shost->shost_gendev, i);
> -		if (!phy->phy)
> -			return -ENOMEM;
> +		if (!phy->phy) {
> +			ret = -ENOMEM;
> +			goto fail;
> +		}
>   
>   		phy->phy->identify.initiator_port_protocols =
>   			phy->iproto;
> @@ -146,10 +149,22 @@ int sas_register_phys(struct sas_ha_struct *sas_ha)
>   		phy->phy->maximum_linkrate = SAS_LINK_RATE_UNKNOWN;
>   		phy->phy->negotiated_linkrate = SAS_LINK_RATE_UNKNOWN;
>   
> -		sas_phy_add(phy->phy);
> +		ret = sas_phy_add(phy->phy);
> +		if (ret) {
> +			sas_phy_free(phy->phy);
> +			goto fail;
> +		}
>   	}
>   
>   	return 0;
> +fail:
> +	for (i--; i >= 0; i--) {
> +		struct asd_sas_phy *phy = sas_ha->sas_phy[i];
> +
> +		sas_phy_delete(phy->phy);
> +		sas_phy_free(phy);
> +	}
> +	return ret;
>   }
>   
>   const work_func_t sas_phy_event_fns[PHY_NUM_EVENTS] = {

Since you are at it, you should also complete Undo_phys label in 
sas_register_ha().

Thanks,
祝一切顺利
Re: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
Posted by Damien Le Moal 2 weeks ago 
On 12/5/25 17:02, Chaohai Chen wrote:
> If an error is triggered in the loop process, we need to roll back
> the resources that have already been requested.
> 
> Signed-off-by: Chaohai Chen <wdhh6@aliyun.com>
> ---
>  drivers/scsi/libsas/sas_phy.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_phy.c b/drivers/scsi/libsas/sas_phy.c
> index 635835c28ecd..455118c7e889 100644
> --- a/drivers/scsi/libsas/sas_phy.c
> +++ b/drivers/scsi/libsas/sas_phy.c
> @@ -116,6 +116,7 @@ static void sas_phye_shutdown(struct work_struct *work)
>  int sas_register_phys(struct sas_ha_struct *sas_ha)
>  {
>  	int i;
> +	int ret = 0;

"= 0" assignment does not seem to be needed here.

>  
>  	/* Now register the phys. */
>  	for (i = 0; i < sas_ha->num_phys; i++) {
> @@ -132,8 +133,10 @@ int sas_register_phys(struct sas_ha_struct *sas_ha)
>  		phy->frame_rcvd_size = 0;
>  
>  		phy->phy = sas_phy_alloc(&sas_ha->shost->shost_gendev, i);
> -		if (!phy->phy)
> -			return -ENOMEM;
> +		if (!phy->phy) {
> +			ret = -ENOMEM;
> +			goto fail;

Please call the label using a descriptive name. E.g. "goto delete_phy;"

> +		}
>  
>  		phy->phy->identify.initiator_port_protocols =
>  			phy->iproto;
> @@ -146,10 +149,22 @@ int sas_register_phys(struct sas_ha_struct *sas_ha)
>  		phy->phy->maximum_linkrate = SAS_LINK_RATE_UNKNOWN;
>  		phy->phy->negotiated_linkrate = SAS_LINK_RATE_UNKNOWN;
>  
> -		sas_phy_add(phy->phy);
> +		ret = sas_phy_add(phy->phy);
> +		if (ret) {
> +			sas_phy_free(phy->phy);
> +			goto fail;
> +		}
>  	}
>  
>  	return 0;
> +fail:
> +	for (i--; i >= 0; i--) {
> +		struct asd_sas_phy *phy = sas_ha->sas_phy[i];
> +
> +		sas_phy_delete(phy->phy);
> +		sas_phy_free(phy);
> +	}
> +	return ret;
>  }
>  
>  const work_func_t sas_phy_event_fns[PHY_NUM_EVENTS] = {


-- 
Damien Le Moal
Western Digital Research

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.