* Re: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
2025-12-05 8:02 [PATCH] scsi: libsas: Add error handling in the sas_register_phys() Chaohai Chen
@ 2025-12-05 8:27 ` Damien Le Moal
2025-12-05 9:52 ` Jason Yan
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2025-12-05 8:27 UTC (permalink / raw)
To: Chaohai Chen, john.g.garry, yanaijie, James.Bottomley,
martin.petersen
Cc: linux-scsi, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
2025-12-05 8:02 [PATCH] scsi: libsas: Add error handling in the sas_register_phys() Chaohai Chen
2025-12-05 8:27 ` Damien Le Moal
@ 2025-12-05 9:52 ` Jason Yan
2025-12-06 4:32 ` kernel test robot
2025-12-06 7:39 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: Jason Yan @ 2025-12-05 9:52 UTC (permalink / raw)
To: Chaohai Chen, john.g.garry, James.Bottomley, martin.petersen
Cc: dlemoal, linux-scsi, linux-kernel
在 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,
祝一切顺利
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
2025-12-05 8:02 [PATCH] scsi: libsas: Add error handling in the sas_register_phys() Chaohai Chen
2025-12-05 8:27 ` Damien Le Moal
2025-12-05 9:52 ` Jason Yan
@ 2025-12-06 4:32 ` kernel test robot
2025-12-06 7:39 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2025-12-06 4:32 UTC (permalink / raw)
To: Chaohai Chen, john.g.garry, yanaijie, James.Bottomley,
martin.petersen
Cc: oe-kbuild-all, dlemoal, linux-scsi, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] scsi: libsas: Add error handling in the sas_register_phys()
2025-12-05 8:02 [PATCH] scsi: libsas: Add error handling in the sas_register_phys() Chaohai Chen
` (2 preceding siblings ...)
2025-12-06 4:32 ` kernel test robot
@ 2025-12-06 7:39 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2025-12-06 7:39 UTC (permalink / raw)
To: Chaohai Chen, john.g.garry, yanaijie, James.Bottomley,
martin.petersen
Cc: llvm, oe-kbuild-all, dlemoal, linux-scsi, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread