* [PATCH v2] scsi: libsas: cleanup sas_form_port()
@ 2022-02-28 9:48 Damien Le Moal
2022-02-28 10:08 ` John Garry
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Damien Le Moal @ 2022-02-28 9:48 UTC (permalink / raw)
To: linux-scsi, Martin K . Petersen, John Garry, Dan Carpenter, Jason Yan
Sparse throws a warning about context imbalance ("different lock
contexts for basic block") in sas_form_port() as it gets confused with
the fact that a port is locked within one of the 2 search loop and
unlocked afterward outside of the search loops once the phy is added to
the port. Since this code is not easy to follow, improve it by factoring
out the code adding the phy to the port once the port is locked into the
helper function sas_form_port_add_phy(). This helper can then be called
directly within the port search loops, avoiding confusion and clearing
the sparse warning.
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
Changes from v1:
* Renamed helper function to sas_form_port_add_phy()
drivers/scsi/libsas/sas_port.c | 73 +++++++++++++++++++---------------
1 file changed, 42 insertions(+), 31 deletions(-)
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index 67b429dcf1ff..11599c0e3fc3 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -67,6 +67,34 @@ static void sas_resume_port(struct asd_sas_phy *phy)
sas_discover_event(port, DISCE_RESUME);
}
+static void sas_form_port_add_phy(struct asd_sas_port *port,
+ struct asd_sas_phy *phy, bool wideport)
+{
+ list_add_tail(&phy->port_phy_el, &port->phy_list);
+ sas_phy_set_target(phy, port->port_dev);
+ phy->port = port;
+ port->num_phys++;
+ port->phy_mask |= (1U << phy->id);
+
+ if (wideport)
+ pr_debug("phy%d matched wide port%d\n", phy->id,
+ port->id);
+ else
+ memcpy(port->sas_addr, phy->sas_addr, SAS_ADDR_SIZE);
+
+ if (*(u64 *)port->attached_sas_addr == 0) {
+ port->class = phy->class;
+ memcpy(port->attached_sas_addr, phy->attached_sas_addr,
+ SAS_ADDR_SIZE);
+ port->iproto = phy->iproto;
+ port->tproto = phy->tproto;
+ port->oob_mode = phy->oob_mode;
+ port->linkrate = phy->linkrate;
+ } else {
+ port->linkrate = max(port->linkrate, phy->linkrate);
+ }
+}
+
/**
* sas_form_port - add this phy to a port
* @phy: the phy of interest
@@ -79,7 +107,7 @@ static void sas_form_port(struct asd_sas_phy *phy)
int i;
struct sas_ha_struct *sas_ha = phy->ha;
struct asd_sas_port *port = phy->port;
- struct domain_device *port_dev;
+ struct domain_device *port_dev = NULL;
struct sas_internal *si =
to_sas_internal(sas_ha->core.shost->transportt);
unsigned long flags;
@@ -110,8 +138,9 @@ static void sas_form_port(struct asd_sas_phy *phy)
if (*(u64 *) port->sas_addr &&
phy_is_wideport_member(port, phy) && port->num_phys > 0) {
/* wide port */
- pr_debug("phy%d matched wide port%d\n", phy->id,
- port->id);
+ port_dev = port->port_dev;
+ sas_form_port_add_phy(port, phy, true);
+ spin_unlock(&port->phy_list_lock);
break;
}
spin_unlock(&port->phy_list_lock);
@@ -122,40 +151,22 @@ static void sas_form_port(struct asd_sas_phy *phy)
port = sas_ha->sas_port[i];
spin_lock(&port->phy_list_lock);
if (*(u64 *)port->sas_addr == 0
- && port->num_phys == 0) {
- memcpy(port->sas_addr, phy->sas_addr,
- SAS_ADDR_SIZE);
+ && port->num_phys == 0) {
+ port_dev = port->port_dev;
+ sas_form_port_add_phy(port, phy, false);
+ spin_unlock(&port->phy_list_lock);
break;
}
spin_unlock(&port->phy_list_lock);
}
- }
- if (i >= sas_ha->num_phys) {
- pr_err("%s: couldn't find a free port, bug?\n", __func__);
- spin_unlock_irqrestore(&sas_ha->phy_port_lock, flags);
- return;
+ if (i >= sas_ha->num_phys) {
+ pr_err("%s: couldn't find a free port, bug?\n",
+ __func__);
+ spin_unlock_irqrestore(&sas_ha->phy_port_lock, flags);
+ return;
+ }
}
-
- /* add the phy to the port */
- port_dev = port->port_dev;
- list_add_tail(&phy->port_phy_el, &port->phy_list);
- sas_phy_set_target(phy, port_dev);
- phy->port = port;
- port->num_phys++;
- port->phy_mask |= (1U << phy->id);
-
- if (*(u64 *)port->attached_sas_addr == 0) {
- port->class = phy->class;
- memcpy(port->attached_sas_addr, phy->attached_sas_addr,
- SAS_ADDR_SIZE);
- port->iproto = phy->iproto;
- port->tproto = phy->tproto;
- port->oob_mode = phy->oob_mode;
- port->linkrate = phy->linkrate;
- } else
- port->linkrate = max(port->linkrate, phy->linkrate);
- spin_unlock(&port->phy_list_lock);
spin_unlock_irqrestore(&sas_ha->phy_port_lock, flags);
if (!port->port) {
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] scsi: libsas: cleanup sas_form_port()
2022-02-28 9:48 [PATCH v2] scsi: libsas: cleanup sas_form_port() Damien Le Moal
@ 2022-02-28 10:08 ` John Garry
2022-03-02 4:09 ` Martin K. Petersen
2022-03-09 4:14 ` Martin K. Petersen
2 siblings, 0 replies; 5+ messages in thread
From: John Garry @ 2022-02-28 10:08 UTC (permalink / raw)
To: Damien Le Moal, linux-scsi, Martin K . Petersen, Dan Carpenter,
Jason Yan
On 28/02/2022 09:48, Damien Le Moal wrote:
> Sparse throws a warning about context imbalance ("different lock
> contexts for basic block") in sas_form_port() as it gets confused with
> the fact that a port is locked within one of the 2 search loop and
> unlocked afterward outside of the search loops once the phy is added to
> the port. Since this code is not easy to follow, improve it by factoring
> out the code adding the phy to the port once the port is locked into the
> helper function sas_form_port_add_phy(). This helper can then be called
> directly within the port search loops, avoiding confusion and clearing
> the sparse warning.
>
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: John Garry <john.garry@huawei.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] scsi: libsas: cleanup sas_form_port()
2022-02-28 9:48 [PATCH v2] scsi: libsas: cleanup sas_form_port() Damien Le Moal
2022-02-28 10:08 ` John Garry
@ 2022-03-02 4:09 ` Martin K. Petersen
2022-03-09 4:14 ` Martin K. Petersen
2 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2022-03-02 4:09 UTC (permalink / raw)
To: Damien Le Moal
Cc: linux-scsi, Martin K . Petersen, John Garry, Dan Carpenter, Jason Yan
Damien,
> Sparse throws a warning about context imbalance ("different lock
> contexts for basic block") in sas_form_port() as it gets confused with
> the fact that a port is locked within one of the 2 search loop and
> unlocked afterward outside of the search loops once the phy is added
> to the port. Since this code is not easy to follow, improve it by
> factoring out the code adding the phy to the port once the port is
> locked into the helper function sas_form_port_add_phy(). This helper
> can then be called directly within the port search loops, avoiding
> confusion and clearing the sparse warning.
Applied to 5.18/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] scsi: libsas: cleanup sas_form_port()
2022-02-28 9:48 [PATCH v2] scsi: libsas: cleanup sas_form_port() Damien Le Moal
2022-02-28 10:08 ` John Garry
2022-03-02 4:09 ` Martin K. Petersen
@ 2022-03-09 4:14 ` Martin K. Petersen
2 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2022-03-09 4:14 UTC (permalink / raw)
To: John Garry, linux-scsi, Damien Le Moal, Jason Yan, Dan Carpenter
Cc: Martin K . Petersen
On Mon, 28 Feb 2022 18:48:57 +0900, Damien Le Moal wrote:
> Sparse throws a warning about context imbalance ("different lock
> contexts for basic block") in sas_form_port() as it gets confused with
> the fact that a port is locked within one of the 2 search loop and
> unlocked afterward outside of the search loops once the phy is added to
> the port. Since this code is not easy to follow, improve it by factoring
> out the code adding the phy to the port once the port is locked into the
> helper function sas_form_port_add_phy(). This helper can then be called
> directly within the port search loops, avoiding confusion and clearing
> the sparse warning.
>
> [...]
Applied to 5.18/scsi-queue, thanks!
[1/1] scsi: libsas: cleanup sas_form_port()
https://git.kernel.org/mkp/scsi/c/32698c955295
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] scsi: libsas: cleanup sas_form_port()
@ 2022-02-28 20:20 kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-28 20:20 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 9959 bytes --]
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220228094857.557329-1-damien.lemoal@opensource.wdc.com>
References: <20220228094857.557329-1-damien.lemoal@opensource.wdc.com>
TO: Damien Le Moal <damien.lemoal@opensource.wdc.com>
TO: linux-scsi(a)vger.kernel.org
TO: "Martin K . Petersen" <martin.petersen@oracle.com>
TO: John Garry <john.garry@huawei.com>
TO: Dan Carpenter <error27@gmail.com>
TO: Jason Yan <yanaijie@huawei.com>
Hi Damien,
I love your patch! Perhaps something to improve:
[auto build test WARNING on jejb-scsi/for-next]
[also build test WARNING on mkp-scsi/for-next v5.17-rc6 next-20220225]
[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]
url: https://github.com/0day-ci/linux/commits/Damien-Le-Moal/scsi-libsas-cleanup-sas_form_port/20220228-175040
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:::::: branch date: 10 hours ago
:::::: commit date: 10 hours ago
config: x86_64-randconfig-m001-20220228 (https://download.01.org/0day-ci/archive/20220301/202203010428.5Gka1xt4-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/scsi/libsas/sas_port.c:172 sas_form_port() error: we previously assumed 'port' could be null (see line 115)
vim +/port +172 drivers/scsi/libsas/sas_port.c
2da00b1cacb2885 Damien Le Moal 2022-02-28 97
2908d778ab3e244 James Bottomley 2006-08-29 98 /**
121246ae93a1ef3 Bart Van Assche 2018-02-22 99 * sas_form_port - add this phy to a port
2908d778ab3e244 James Bottomley 2006-08-29 100 * @phy: the phy of interest
2908d778ab3e244 James Bottomley 2006-08-29 101 *
2908d778ab3e244 James Bottomley 2006-08-29 102 * This function adds this phy to an existing port, thus creating a wide
2908d778ab3e244 James Bottomley 2006-08-29 103 * port, or it creates a port and adds the phy to the port.
2908d778ab3e244 James Bottomley 2006-08-29 104 */
2908d778ab3e244 James Bottomley 2006-08-29 105 static void sas_form_port(struct asd_sas_phy *phy)
2908d778ab3e244 James Bottomley 2006-08-29 106 {
2908d778ab3e244 James Bottomley 2006-08-29 107 int i;
2908d778ab3e244 James Bottomley 2006-08-29 108 struct sas_ha_struct *sas_ha = phy->ha;
2908d778ab3e244 James Bottomley 2006-08-29 109 struct asd_sas_port *port = phy->port;
2da00b1cacb2885 Damien Le Moal 2022-02-28 110 struct domain_device *port_dev = NULL;
2908d778ab3e244 James Bottomley 2006-08-29 111 struct sas_internal *si =
2908d778ab3e244 James Bottomley 2006-08-29 112 to_sas_internal(sas_ha->core.shost->transportt);
980fa2f9d64b9be Darrick J. Wong 2007-01-11 113 unsigned long flags;
2908d778ab3e244 James Bottomley 2006-08-29 114
2908d778ab3e244 James Bottomley 2006-08-29 @115 if (port) {
00f0254ed9b1916 Dan Williams 2010-10-01 116 if (!phy_is_wideport_member(port, phy))
90f1e10d08bad84 Dan Williams 2011-05-24 117 sas_deform_port(phy, 0);
303694eeee5eaca Dan Williams 2012-06-21 118 else if (phy->suspended) {
303694eeee5eaca Dan Williams 2012-06-21 119 phy->suspended = 0;
303694eeee5eaca Dan Williams 2012-06-21 120 sas_resume_port(phy);
303694eeee5eaca Dan Williams 2012-06-21 121
303694eeee5eaca Dan Williams 2012-06-21 122 /* phy came back, try to cancel the timeout */
303694eeee5eaca Dan Williams 2012-06-21 123 wake_up(&sas_ha->eh_wait_q);
303694eeee5eaca Dan Williams 2012-06-21 124 return;
303694eeee5eaca Dan Williams 2012-06-21 125 } else {
15ba7806c316ce8 John Garry 2018-11-15 126 pr_info("%s: phy%d belongs to port%d already(%d)!\n",
cadbd4a5e36dde7 Harvey Harrison 2008-07-03 127 __func__, phy->id, phy->port->id,
2908d778ab3e244 James Bottomley 2006-08-29 128 phy->port->num_phys);
2908d778ab3e244 James Bottomley 2006-08-29 129 return;
2908d778ab3e244 James Bottomley 2006-08-29 130 }
2908d778ab3e244 James Bottomley 2006-08-29 131 }
2908d778ab3e244 James Bottomley 2006-08-29 132
5381837f125cc62 Tom Peng 2009-07-01 133 /* see if the phy should be part of a wide port */
980fa2f9d64b9be Darrick J. Wong 2007-01-11 134 spin_lock_irqsave(&sas_ha->phy_port_lock, flags);
2908d778ab3e244 James Bottomley 2006-08-29 135 for (i = 0; i < sas_ha->num_phys; i++) {
2908d778ab3e244 James Bottomley 2006-08-29 136 port = sas_ha->sas_port[i];
2908d778ab3e244 James Bottomley 2006-08-29 137 spin_lock(&port->phy_list_lock);
2908d778ab3e244 James Bottomley 2006-08-29 138 if (*(u64 *) port->sas_addr &&
00f0254ed9b1916 Dan Williams 2010-10-01 139 phy_is_wideport_member(port, phy) && port->num_phys > 0) {
2908d778ab3e244 James Bottomley 2006-08-29 140 /* wide port */
2da00b1cacb2885 Damien Le Moal 2022-02-28 141 port_dev = port->port_dev;
2da00b1cacb2885 Damien Le Moal 2022-02-28 142 sas_form_port_add_phy(port, phy, true);
2da00b1cacb2885 Damien Le Moal 2022-02-28 143 spin_unlock(&port->phy_list_lock);
2908d778ab3e244 James Bottomley 2006-08-29 144 break;
5381837f125cc62 Tom Peng 2009-07-01 145 }
5381837f125cc62 Tom Peng 2009-07-01 146 spin_unlock(&port->phy_list_lock);
5381837f125cc62 Tom Peng 2009-07-01 147 }
5381837f125cc62 Tom Peng 2009-07-01 148 /* The phy does not match any existing port, create a new one */
5381837f125cc62 Tom Peng 2009-07-01 149 if (i == sas_ha->num_phys) {
5381837f125cc62 Tom Peng 2009-07-01 150 for (i = 0; i < sas_ha->num_phys; i++) {
5381837f125cc62 Tom Peng 2009-07-01 151 port = sas_ha->sas_port[i];
5381837f125cc62 Tom Peng 2009-07-01 152 spin_lock(&port->phy_list_lock);
5381837f125cc62 Tom Peng 2009-07-01 153 if (*(u64 *)port->sas_addr == 0
5381837f125cc62 Tom Peng 2009-07-01 154 && port->num_phys == 0) {
2da00b1cacb2885 Damien Le Moal 2022-02-28 155 port_dev = port->port_dev;
2da00b1cacb2885 Damien Le Moal 2022-02-28 156 sas_form_port_add_phy(port, phy, false);
2da00b1cacb2885 Damien Le Moal 2022-02-28 157 spin_unlock(&port->phy_list_lock);
2908d778ab3e244 James Bottomley 2006-08-29 158 break;
2908d778ab3e244 James Bottomley 2006-08-29 159 }
2908d778ab3e244 James Bottomley 2006-08-29 160 spin_unlock(&port->phy_list_lock);
2908d778ab3e244 James Bottomley 2006-08-29 161 }
2908d778ab3e244 James Bottomley 2006-08-29 162
2908d778ab3e244 James Bottomley 2006-08-29 163 if (i >= sas_ha->num_phys) {
2da00b1cacb2885 Damien Le Moal 2022-02-28 164 pr_err("%s: couldn't find a free port, bug?\n",
2da00b1cacb2885 Damien Le Moal 2022-02-28 165 __func__);
980fa2f9d64b9be Darrick J. Wong 2007-01-11 166 spin_unlock_irqrestore(&sas_ha->phy_port_lock, flags);
2908d778ab3e244 James Bottomley 2006-08-29 167 return;
2908d778ab3e244 James Bottomley 2006-08-29 168 }
2da00b1cacb2885 Damien Le Moal 2022-02-28 169 }
980fa2f9d64b9be Darrick J. Wong 2007-01-11 170 spin_unlock_irqrestore(&sas_ha->phy_port_lock, flags);
2908d778ab3e244 James Bottomley 2006-08-29 171
2908d778ab3e244 James Bottomley 2006-08-29 @172 if (!port->port) {
b4698d88585e23d Dan Williams 2012-04-19 173 port->port = sas_port_alloc(phy->phy->dev.parent, port->id);
2908d778ab3e244 James Bottomley 2006-08-29 174 BUG_ON(!port->port);
2908d778ab3e244 James Bottomley 2006-08-29 175 sas_port_add(port->port);
2908d778ab3e244 James Bottomley 2006-08-29 176 }
2908d778ab3e244 James Bottomley 2006-08-29 177 sas_port_add_phy(port->port, phy->phy);
2908d778ab3e244 James Bottomley 2006-08-29 178
b3e3d4c618c5b97 John Garry 2019-12-19 179 pr_debug("%s added to %s, phy_mask:0x%x (%016llx)\n",
71610f55fa4db63 Kay Sievers 2008-12-03 180 dev_name(&phy->phy->dev), dev_name(&port->port->dev),
a29c05153630b2c James Bottomley 2008-02-23 181 port->phy_mask,
a29c05153630b2c James Bottomley 2008-02-23 182 SAS_ADDR(port->attached_sas_addr));
a29c05153630b2c James Bottomley 2008-02-23 183
085f104a83d5650 John Garry 2019-04-12 184 if (port_dev)
085f104a83d5650 John Garry 2019-04-12 185 port_dev->pathways = port->num_phys;
2908d778ab3e244 James Bottomley 2006-08-29 186
2908d778ab3e244 James Bottomley 2006-08-29 187 /* Tell the LLDD about this port formation. */
2908d778ab3e244 James Bottomley 2006-08-29 188 if (si->dft->lldd_port_formed)
2908d778ab3e244 James Bottomley 2006-08-29 189 si->dft->lldd_port_formed(phy);
2908d778ab3e244 James Bottomley 2006-08-29 190
2908d778ab3e244 James Bottomley 2006-08-29 191 sas_discover_event(phy->port, DISCE_DISCOVER_DOMAIN);
085f104a83d5650 John Garry 2019-04-12 192 /* Only insert a revalidate event after initial discovery */
924a3541eab0d28 John Garry 2019-06-10 193 if (port_dev && dev_is_expander(port_dev->dev_type)) {
085f104a83d5650 John Garry 2019-04-12 194 struct expander_device *ex_dev = &port_dev->ex_dev;
085f104a83d5650 John Garry 2019-04-12 195
085f104a83d5650 John Garry 2019-04-12 196 ex_dev->ex_change_count = -1;
085f104a83d5650 John Garry 2019-04-12 197 sas_discover_event(port, DISCE_REVALIDATE_DOMAIN);
085f104a83d5650 John Garry 2019-04-12 198 }
517e5153d242cb2 Jason Yan 2017-12-08 199 flush_workqueue(sas_ha->disco_q);
2908d778ab3e244 James Bottomley 2006-08-29 200 }
2908d778ab3e244 James Bottomley 2006-08-29 201
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-09 4:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-28 9:48 [PATCH v2] scsi: libsas: cleanup sas_form_port() Damien Le Moal
2022-02-28 10:08 ` John Garry
2022-03-02 4:09 ` Martin K. Petersen
2022-03-09 4:14 ` Martin K. Petersen
2022-02-28 20:20 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.