diff mbox

[1/2] hpsa: cleanup sas_phy structures in sysfs when unloading

Message ID 20161121140429.12788-2-mwilck@suse.de (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Martin Wilck Nov. 21, 2016, 2:04 p.m. UTC
When the hpsa module is unloaded using rmmod, dangling
symlinks remain under /sys/class/sas_phy. Fix this by
calling sas_phy_delete() rather than sas_phy_free (which,
according to comments, should not be called for PHYs that
have been set up successfully, anyway).

References: bsc#1010946.
Signed-off-by: Martin Wilck <mwilck@suse.de>
---
 drivers/scsi/hpsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Johannes Thumshirn Nov. 21, 2016, 2:13 p.m. UTC | #1
On Mon, Nov 21, 2016 at 03:04:28PM +0100, Martin Wilck wrote:
> When the hpsa module is unloaded using rmmod, dangling
> symlinks remain under /sys/class/sas_phy. Fix this by
> calling sas_phy_delete() rather than sas_phy_free (which,
> according to comments, should not be called for PHYs that
> have been set up successfully, anyway).
> 
> References: bsc#1010946.

I don't think the SUSE bugzilla tag is of relevance upstream. But for sake of
completeness we could add a 
Link: https://bugzilla.suse.com/show_bug.cgi?id=1010946

> Signed-off-by: Martin Wilck <mwilck@suse.de>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

> ---
>  drivers/scsi/hpsa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index efe2f36..8ec77c3 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -9547,9 +9547,9 @@ static void hpsa_free_sas_phy(struct hpsa_sas_phy *hpsa_sas_phy)
>  	struct sas_phy *phy = hpsa_sas_phy->phy;
>  
>  	sas_port_delete_phy(hpsa_sas_phy->parent_port->port, phy);
> -	sas_phy_free(phy);
>  	if (hpsa_sas_phy->added_to_port)
>  		list_del(&hpsa_sas_phy->phy_list_entry);
> +	sas_phy_delete(phy);
>  	kfree(hpsa_sas_phy);
>  }
>  
> -- 
> 2.10.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Martin Wilck Nov. 21, 2016, 3:13 p.m. UTC | #2
On Mon, 2016-11-21 at 15:13 +0100, Johannes Thumshirn wrote:
> On Mon, Nov 21, 2016 at 03:04:28PM +0100, Martin Wilck wrote:
> > When the hpsa module is unloaded using rmmod, dangling
> > symlinks remain under /sys/class/sas_phy. Fix this by
> > calling sas_phy_delete() rather than sas_phy_free (which,
> > according to comments, should not be called for PHYs that
> > have been set up successfully, anyway).
> > 
> > References: bsc#1010946.
> 
> I don't think the SUSE bugzilla tag is of relevance upstream. But for
> sake of
> completeness we could add a 
> Link: https://bugzilla.suse.com/show_bug.cgi?id=1010946

I am sorry for this mistake.

@Martin, do you want me to re-submit with these references fixed?

I also apologize for the broken cc list of the first series, I hope I
got it right this time.

Regards
Martin


> 
> > Signed-off-by: Martin Wilck <mwilck@suse.de>
> 
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Martin K. Petersen Nov. 22, 2016, 3:47 a.m. UTC | #3
>>>>> "Johannes" == Johannes Thumshirn <jthumshirn@suse.de> writes:

Johannes> I don't think the SUSE bugzilla tag is of relevance upstream.

Nope. I'd rather have really comprehensive patch descriptions.

Johannes> But for sake of completeness we could add a Link:
Johannes> https://bugzilla.suse.com/show_bug.cgi?id=1010946

"You are not authorized to access bug #1010946. To see this bug, you
must first log in to an account with the appropriate permissions."

We'll see what the Microsemi folks think of the patches. If the patches
get acked I can just drop the bsc tag when I apply.
Johannes Thumshirn Nov. 22, 2016, 8:10 a.m. UTC | #4
On Mon, Nov 21, 2016 at 10:47:20PM -0500, Martin K . Petersen wrote:
> >>>>> "Johannes" == Johannes Thumshirn <jthumshirn@suse.de> writes:
> 
> Johannes> I don't think the SUSE bugzilla tag is of relevance upstream.
> 
> Nope. I'd rather have really comprehensive patch descriptions.
> 
> Johannes> But for sake of completeness we could add a Link:
> Johannes> https://bugzilla.suse.com/show_bug.cgi?id=1010946
> 
> "You are not authorized to access bug #1010946. To see this bug, you
> must first log in to an account with the appropriate permissions."

I'm sorry, I always thought that the bugs are visible from the outside if
we don't mark it as closed. I'm sorry, apparently it's only the openSUSE
bugs which are visible.

	Johannes
Don Brace Nov. 29, 2016, 1:52 a.m. UTC | #5
> -----Original Message-----
> From: Martin Wilck [mailto:mwilck@suse.de]
> Sent: Monday, November 21, 2016 8:04 AM
> To: Don Brace
> Cc: dl-esc-Team ESD Storage Dev Support; iss_storagedev@hp.com; linux-
> scsi@vger.kernel.org; JBottomley@odin.com; hch@lst.de; hare@suse.de;
> Martin Wilck
> Subject: [PATCH 1/2] hpsa: cleanup sas_phy structures in sysfs when
> unloading
> 
> EXTERNAL EMAIL
> 
> 
> When the hpsa module is unloaded using rmmod, dangling
> symlinks remain under /sys/class/sas_phy. Fix this by
> calling sas_phy_delete() rather than sas_phy_free (which,
> according to comments, should not be called for PHYs that
> have been set up successfully, anyway).
> 
> References: bsc#1010946.
> Signed-off-by: Martin Wilck <mwilck@suse.de>
> ---
>  drivers/scsi/hpsa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index efe2f36..8ec77c3 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -9547,9 +9547,9 @@ static void hpsa_free_sas_phy(struct hpsa_sas_phy
> *hpsa_sas_phy)
>         struct sas_phy *phy = hpsa_sas_phy->phy;
> 
>         sas_port_delete_phy(hpsa_sas_phy->parent_port->port, phy);
> -       sas_phy_free(phy);
>         if (hpsa_sas_phy->added_to_port)
>                 list_del(&hpsa_sas_phy->phy_list_entry);
> +       sas_phy_delete(phy);
>         kfree(hpsa_sas_phy);
>  }
> 
> --
> 2.10.1

I tried these patches on: 4.9.0-rc7, was this correct?

I got the following stack trace:
[  231.192289] ------------[ cut here ]------------
[  231.214333] WARNING: CPU: 51 PID: 15876 at fs/sysfs/group.c:237 sysfs_remove_group+0x8e/0x90
[  231.254371] sysfs group 'power' not found for kobject '4:0:0:0'
[  231.282637] Modules linked in: ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_security iptable_raw iptable_filter ip_tables sb_edac edac_core x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul iTCO_wdt ghash_clmulni_intel iTCO_vendor_support aesni_intel lrw gf128mul glue_helper ablk_helper cryptd osst pcspkr ch st ioatdma lpc_ich hpilo hpwdt mfd_core dca ipmi_si ipmi_msghandler pcc_cpufreq wmi acpi_cpufreq acpi_power_meter uinput mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops
[  231.620046]  ttm drm crc32c_intel tg3 serio_raw ptp usb_storage i2c_core hpsa(-) pps_core scsi_transport_sas dm_mirror dm_region_hash dm_log dm_mod
[  231.676364] CPU: 51 PID: 15876 Comm: rmmod Not tainted 4.9.0-rc7+ #22
[  231.706769] Hardware name: HP ProLiant DL580 Gen8, BIOS P79 08/18/2016
[  231.737730]  ffffc9002123bbf0 ffffffff815909bd ffffc9002123bc40 0000000000000000
[  231.772474]  ffffc9002123bc30 ffffffff81090901 000000ed00000246 0000000000000000
[  231.807897]  ffffffff81f71560 ffff8820529faea8 ffff88205542ea60 00000000024b0090
[  231.842671] Call Trace:
[  231.854426]  [<ffffffff815909bd>] dump_stack+0x85/0xc8
[  231.878826]  [<ffffffff81090901>] __warn+0xd1/0xf0
[  231.901476]  [<ffffffff8109097f>] warn_slowpath_fmt+0x5f/0x80
[  231.929515]  [<ffffffff8196e5de>] ? mutex_unlock+0xe/0x10
[  231.955067]  [<ffffffff812f1a6a>] ? kernfs_find_and_get_ns+0x4a/0x60
[  231.985095]  [<ffffffff812f56ae>] sysfs_remove_group+0x8e/0x90
[  232.012416]  [<ffffffff816dd1b7>] dpm_sysfs_remove+0x57/0x60
[  232.038904]  [<ffffffff816cf928>] device_del+0x58/0x270
[  232.064056]  [<ffffffff816cfb5a>] device_unregister+0x1a/0x60
[  232.091138]  [<ffffffff8157d470>] bsg_unregister_queue+0x60/0xa0
[  232.119498]  [<ffffffff8170e2ea>] __scsi_remove_device+0xaa/0xd0
[  232.147745]  [<ffffffff8170c369>] scsi_forget_host+0x69/0x70
[  232.174666]  [<ffffffff816ff292>] scsi_remove_host+0x82/0x130
[  232.201804]  [<ffffffffa007cfc3>] hpsa_remove_one+0x93/0x190 [hpsa]
[  232.231329]  [<ffffffff815dd8d9>] pci_device_remove+0x39/0xc0
[  232.258089]  [<ffffffff816d4aca>] __device_release_driver+0x9a/0x150
[  232.288005]  [<ffffffff816d4ca1>] driver_detach+0xc1/0xd0
[  232.313479]  [<ffffffff816d3a98>] bus_remove_driver+0x58/0xd0
[  232.341280]  [<ffffffff816d572c>] driver_unregister+0x2c/0x50
[  232.369272]  [<ffffffff815dbf3a>] pci_unregister_driver+0x2a/0x80
[  232.398723]  [<ffffffffa0085869>] hpsa_cleanup+0x10/0x7a7 [hpsa]
[  232.428094]  [<ffffffff8113571c>] SyS_delete_module+0x1bc/0x220
[  232.456716]  [<ffffffff81003c0c>] do_syscall_64+0x6c/0x200
[  232.483125]  [<ffffffff81971d49>] entry_SYSCALL64_slow_path+0x25/0x25
[  232.514162] ---[ end trace 3c490662736284eb ]---


Thanks,
Don Brace
ESC - Smart Storage
Microsemi Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Martin Wilck Nov. 29, 2016, 9:16 a.m. UTC | #6
Hi Don,

On Tue, 2016-11-29 at 01:52 +0000, Don Brace wrote:
> > -----Original Message-----
> > From: Martin Wilck [mailto:mwilck@suse.de]
> > Sent: Monday, November 21, 2016 8:04 AM
> > To: Don Brace
> > Cc: dl-esc-Team ESD Storage Dev Support; iss_storagedev@hp.com;
> > linux-
> > scsi@vger.kernel.org; JBottomley@odin.com; hch@lst.de; hare@suse.de
> > ;
> > Martin Wilck
> > Subject: [PATCH 1/2] hpsa: cleanup sas_phy structures in sysfs when
> > unloading
> > 
> > EXTERNAL EMAIL
> > 
> > 
> > When the hpsa module is unloaded using rmmod, dangling
> > symlinks remain under /sys/class/sas_phy. Fix this by
> > calling sas_phy_delete() rather than sas_phy_free (which,
> > according to comments, should not be called for PHYs that
> > have been set up successfully, anyway).
> > 
> > References: bsc#1010946.
> > Signed-off-by: Martin Wilck <mwilck@suse.de>
> > ---
> >  drivers/scsi/hpsa.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> > index efe2f36..8ec77c3 100644
> > --- a/drivers/scsi/hpsa.c
> > +++ b/drivers/scsi/hpsa.c
> > @@ -9547,9 +9547,9 @@ static void hpsa_free_sas_phy(struct
> > hpsa_sas_phy
> > *hpsa_sas_phy)
> >         struct sas_phy *phy = hpsa_sas_phy->phy;
> > 
> >         sas_port_delete_phy(hpsa_sas_phy->parent_port->port, phy);
> > -       sas_phy_free(phy);
> >         if (hpsa_sas_phy->added_to_port)
> >                 list_del(&hpsa_sas_phy->phy_list_entry);
> > +       sas_phy_delete(phy);
> >         kfree(hpsa_sas_phy);
> >  }
> > 
> > --
> > 2.10.1
> 
> I tried these patches on: 4.9.0-rc7, was this correct?
> 
> I got the following stack trace:
> [  231.192289] ------------[ cut here ]------------
> [  231.214333] WARNING: CPU: 51 PID: 15876 at fs/sysfs/group.c:237
> sysfs_remove_group+0x8e/0x90
> [  231.254371] sysfs group 'power' not found for kobject '4:0:0:0'

[...]

The stack traces should be gone if you apply the 2nd patch of the
series ("hpsa: destroy sas transport properties before scsi_host").

My testing (done with a SLES12 kernel), without my patches, showed
these traces for the removal of "sas_port" structures. Adding PATCH 1/2
indeed adds more of these warnings (now for "sas_port" *and*
"sas_phy"). But that's not the fault of this patch; it's caused by the
sequence of actions in hpsa_remove_one() and it's fixed in PATCH 2/2.

Regards
Martin

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index efe2f36..8ec77c3 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -9547,9 +9547,9 @@  static void hpsa_free_sas_phy(struct hpsa_sas_phy *hpsa_sas_phy)
 	struct sas_phy *phy = hpsa_sas_phy->phy;
 
 	sas_port_delete_phy(hpsa_sas_phy->parent_port->port, phy);
-	sas_phy_free(phy);
 	if (hpsa_sas_phy->added_to_port)
 		list_del(&hpsa_sas_phy->phy_list_entry);
+	sas_phy_delete(phy);
 	kfree(hpsa_sas_phy);
 }