All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
@ 2014-05-30 14:59 Neil Horman
  2014-06-02  8:35 ` Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Neil Horman @ 2014-05-30 14:59 UTC (permalink / raw)
  To: fcoe-devel; +Cc: Neil Horman, linux-scsi, Robert Love, Vasu Dev

debugfs caught this:
WARNING: at lib/debugobjects.c:260 debug_print_object+0x83/0xa0()
ODEBUG: free active (active state 0) object type: work_struct
hint: fc_scsi_scan_rport+0x0/0xd0 [scsi_transport_fc]
 CPU: 1 PID: 184 Comm: kworker/1:1 Tainted: G        W
--------------   3.10.0-123.el7.x86_64.debug #1
Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
Workqueue: fc_wq_5 fc_rport_final_delete [scsi_transport_fc]
Call Trace:
[<ffffffff8169efec>] dump_stack+0x19/0x1b
[<ffffffff8106cbd1>] warn_slowpath_common+0x61/0x80
[<ffffffff8106cc4c>] warn_slowpath_fmt+0x5c/0x80
[<ffffffff8133e003>] debug_print_object+0x83/0xa0
[<ffffffffa04e2f40>] ? fc_parse_wwn+0x100/0x100

[<ffffffff8133f23b>] debug_check_no_obj_freed+0x22b/0x270
[<ffffffffa04e127e>] ? fc_rport_dev_release+0x1e/0x30
[<ffffffff811db3e9>] kfree+0xd9/0x2d0
[<ffffffffa04e127e>] fc_rport_dev_release+0x1e/0x30
[<ffffffff81428032>] device_release+0x32/0xa0
[<ffffffff8132701e>] kobject_release+0x7e/0x1b0
[<ffffffff81326ed8>] kobject_put+0x28/0x60
[<ffffffff81428397>] put_device+0x17/0x20
[<ffffffffa04e5025>] fc_rport_final_delete+0x165/0x210
[<ffffffff810959b0>] process_one_work+0x220/0x710
[<ffffffff81095944>] ? process_one_work+0x1b4/0x710
[<ffffffff81095fbb>] worker_thread+0x11b/0x3a0
[<ffffffff81095ea0>] ? process_one_work+0x710/0x710
[<ffffffff8109e0cd>] kthread+0xed/0x100
[<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
[<ffffffff816b2fec>] ret_from_fork+0x7c/0xb0
[<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80

Seems to be because the scan_work work_struct might be active when the housing
fc_rport struct gets freed.  Ensure that we cancel it prior to freeing the rport

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: linux-scsi@vger.kernel.org
CC: Robert Love <robert.w.love@intel.com>
CC: Vasu Dev <vasu.dev@intel.com>
---
 drivers/scsi/scsi_transport_fc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 4628fd5..5bd552c 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2548,6 +2548,7 @@ fc_rport_final_delete(struct work_struct *work)
 			fc_flush_devloss(shost);
 		if (!cancel_delayed_work(&rport->dev_loss_work))
 			fc_flush_devloss(shost);
+		cancel_work_sync(&rport->scan_work);
 		spin_lock_irqsave(shost->host_lock, flags);
 		rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
 	}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
  2014-05-30 14:59 [PATCH] fc: ensure scan_work isn't active when freeing fc_rport Neil Horman
@ 2014-06-02  8:35 ` Christoph Hellwig
  2014-06-02  9:28 ` Hannes Reinecke
       [not found] ` <1401461974-7879-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
  2 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2014-06-02  8:35 UTC (permalink / raw)
  To: Neil Horman; +Cc: fcoe-devel, linux-scsi, Robert Love, Vasu Dev

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
  2014-05-30 14:59 [PATCH] fc: ensure scan_work isn't active when freeing fc_rport Neil Horman
  2014-06-02  8:35 ` Christoph Hellwig
@ 2014-06-02  9:28 ` Hannes Reinecke
       [not found] ` <1401461974-7879-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
  2 siblings, 0 replies; 11+ messages in thread
From: Hannes Reinecke @ 2014-06-02  9:28 UTC (permalink / raw)
  To: Neil Horman, fcoe-devel; +Cc: linux-scsi, Robert Love, Vasu Dev

On 05/30/2014 04:59 PM, Neil Horman wrote:
> debugfs caught this:
> WARNING: at lib/debugobjects.c:260 debug_print_object+0x83/0xa0()
> ODEBUG: free active (active state 0) object type: work_struct
> hint: fc_scsi_scan_rport+0x0/0xd0 [scsi_transport_fc]
>   CPU: 1 PID: 184 Comm: kworker/1:1 Tainted: G        W
> --------------   3.10.0-123.el7.x86_64.debug #1
> Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
> Workqueue: fc_wq_5 fc_rport_final_delete [scsi_transport_fc]
> Call Trace:
> [<ffffffff8169efec>] dump_stack+0x19/0x1b
> [<ffffffff8106cbd1>] warn_slowpath_common+0x61/0x80
> [<ffffffff8106cc4c>] warn_slowpath_fmt+0x5c/0x80
> [<ffffffff8133e003>] debug_print_object+0x83/0xa0
> [<ffffffffa04e2f40>] ? fc_parse_wwn+0x100/0x100
>
> [<ffffffff8133f23b>] debug_check_no_obj_freed+0x22b/0x270
> [<ffffffffa04e127e>] ? fc_rport_dev_release+0x1e/0x30
> [<ffffffff811db3e9>] kfree+0xd9/0x2d0
> [<ffffffffa04e127e>] fc_rport_dev_release+0x1e/0x30
> [<ffffffff81428032>] device_release+0x32/0xa0
> [<ffffffff8132701e>] kobject_release+0x7e/0x1b0
> [<ffffffff81326ed8>] kobject_put+0x28/0x60
> [<ffffffff81428397>] put_device+0x17/0x20
> [<ffffffffa04e5025>] fc_rport_final_delete+0x165/0x210
> [<ffffffff810959b0>] process_one_work+0x220/0x710
> [<ffffffff81095944>] ? process_one_work+0x1b4/0x710
> [<ffffffff81095fbb>] worker_thread+0x11b/0x3a0
> [<ffffffff81095ea0>] ? process_one_work+0x710/0x710
> [<ffffffff8109e0cd>] kthread+0xed/0x100
> [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> [<ffffffff816b2fec>] ret_from_fork+0x7c/0xb0
> [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
>
> Seems to be because the scan_work work_struct might be active when the housing
> fc_rport struct gets freed.  Ensure that we cancel it prior to freeing the rport
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: linux-scsi@vger.kernel.org
> CC: Robert Love <robert.w.love@intel.com>
> CC: Vasu Dev <vasu.dev@intel.com>
> ---
>   drivers/scsi/scsi_transport_fc.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 4628fd5..5bd552c 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -2548,6 +2548,7 @@ fc_rport_final_delete(struct work_struct *work)
>   			fc_flush_devloss(shost);
>   		if (!cancel_delayed_work(&rport->dev_loss_work))
>   			fc_flush_devloss(shost);
> +		cancel_work_sync(&rport->scan_work);
>   		spin_lock_irqsave(shost->host_lock, flags);
>   		rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
>   	}
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
       [not found] ` <1401461974-7879-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
@ 2014-06-02 23:22   ` Vasu Dev
  2014-06-06 20:54     ` Neil Horman
  2014-06-16 11:15     ` [Open-FCoE] " Neil Horman
  0 siblings, 2 replies; 11+ messages in thread
From: Vasu Dev @ 2014-06-02 23:22 UTC (permalink / raw)
  To: Neil Horman
  Cc: fcoe-devel-s9riP+hp16TNLxjTenLetw, linux-scsi-u79uwXL29TY76Z2rM5mHXA

On Fri, 2014-05-30 at 10:59 -0400, Neil Horman wrote:
> debugfs caught this:
> WARNING: at lib/debugobjects.c:260 debug_print_object+0x83/0xa0()
> ODEBUG: free active (active state 0) object type: work_struct
> hint: fc_scsi_scan_rport+0x0/0xd0 [scsi_transport_fc]
>  CPU: 1 PID: 184 Comm: kworker/1:1 Tainted: G        W
> --------------   3.10.0-123.el7.x86_64.debug #1
> Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
> Workqueue: fc_wq_5 fc_rport_final_delete [scsi_transport_fc]
> Call Trace:
> [<ffffffff8169efec>] dump_stack+0x19/0x1b
> [<ffffffff8106cbd1>] warn_slowpath_common+0x61/0x80
> [<ffffffff8106cc4c>] warn_slowpath_fmt+0x5c/0x80
> [<ffffffff8133e003>] debug_print_object+0x83/0xa0
> [<ffffffffa04e2f40>] ? fc_parse_wwn+0x100/0x100
> 
> [<ffffffff8133f23b>] debug_check_no_obj_freed+0x22b/0x270
> [<ffffffffa04e127e>] ? fc_rport_dev_release+0x1e/0x30
> [<ffffffff811db3e9>] kfree+0xd9/0x2d0
> [<ffffffffa04e127e>] fc_rport_dev_release+0x1e/0x30
> [<ffffffff81428032>] device_release+0x32/0xa0
> [<ffffffff8132701e>] kobject_release+0x7e/0x1b0
> [<ffffffff81326ed8>] kobject_put+0x28/0x60
> [<ffffffff81428397>] put_device+0x17/0x20
> [<ffffffffa04e5025>] fc_rport_final_delete+0x165/0x210
> [<ffffffff810959b0>] process_one_work+0x220/0x710
> [<ffffffff81095944>] ? process_one_work+0x1b4/0x710
> [<ffffffff81095fbb>] worker_thread+0x11b/0x3a0
> [<ffffffff81095ea0>] ? process_one_work+0x710/0x710
> [<ffffffff8109e0cd>] kthread+0xed/0x100
> [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> [<ffffffff816b2fec>] ret_from_fork+0x7c/0xb0
> [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> 
> Seems to be because the scan_work work_struct might be active when the housing
> fc_rport struct gets freed.  Ensure that we cancel it prior to freeing the rport
> 
> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> CC: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: Robert Love <robert.w.love-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> CC: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/scsi/scsi_transport_fc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 4628fd5..5bd552c 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -2548,6 +2548,7 @@ fc_rport_final_delete(struct work_struct *work)
>  			fc_flush_devloss(shost);
>  		if (!cancel_delayed_work(&rport->dev_loss_work))
>  			fc_flush_devloss(shost);
> +		cancel_work_sync(&rport->scan_work);

Make sense to ensure pending work canceled, adding James Smart for his
ACK as transport FC class author. 


Reviewed-by: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 


>  		spin_lock_irqsave(shost->host_lock, flags);
>  		rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
>  	}

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
  2014-06-02 23:22   ` Vasu Dev
@ 2014-06-06 20:54     ` Neil Horman
       [not found]       ` <20140606205409.GD2543-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org>
  2014-06-16 11:15     ` [Open-FCoE] " Neil Horman
  1 sibling, 1 reply; 11+ messages in thread
From: Neil Horman @ 2014-06-06 20:54 UTC (permalink / raw)
  To: Vasu Dev
  Cc: fcoe-devel-s9riP+hp16TNLxjTenLetw, linux-scsi-u79uwXL29TY76Z2rM5mHXA

On Mon, Jun 02, 2014 at 04:22:50PM -0700, Vasu Dev wrote:
> On Fri, 2014-05-30 at 10:59 -0400, Neil Horman wrote:
> > debugfs caught this:
> > WARNING: at lib/debugobjects.c:260 debug_print_object+0x83/0xa0()
> > ODEBUG: free active (active state 0) object type: work_struct
> > hint: fc_scsi_scan_rport+0x0/0xd0 [scsi_transport_fc]
> >  CPU: 1 PID: 184 Comm: kworker/1:1 Tainted: G        W
> > --------------   3.10.0-123.el7.x86_64.debug #1
> > Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
> > Workqueue: fc_wq_5 fc_rport_final_delete [scsi_transport_fc]
> > Call Trace:
> > [<ffffffff8169efec>] dump_stack+0x19/0x1b
> > [<ffffffff8106cbd1>] warn_slowpath_common+0x61/0x80
> > [<ffffffff8106cc4c>] warn_slowpath_fmt+0x5c/0x80
> > [<ffffffff8133e003>] debug_print_object+0x83/0xa0
> > [<ffffffffa04e2f40>] ? fc_parse_wwn+0x100/0x100
> > 
> > [<ffffffff8133f23b>] debug_check_no_obj_freed+0x22b/0x270
> > [<ffffffffa04e127e>] ? fc_rport_dev_release+0x1e/0x30
> > [<ffffffff811db3e9>] kfree+0xd9/0x2d0
> > [<ffffffffa04e127e>] fc_rport_dev_release+0x1e/0x30
> > [<ffffffff81428032>] device_release+0x32/0xa0
> > [<ffffffff8132701e>] kobject_release+0x7e/0x1b0
> > [<ffffffff81326ed8>] kobject_put+0x28/0x60
> > [<ffffffff81428397>] put_device+0x17/0x20
> > [<ffffffffa04e5025>] fc_rport_final_delete+0x165/0x210
> > [<ffffffff810959b0>] process_one_work+0x220/0x710
> > [<ffffffff81095944>] ? process_one_work+0x1b4/0x710
> > [<ffffffff81095fbb>] worker_thread+0x11b/0x3a0
> > [<ffffffff81095ea0>] ? process_one_work+0x710/0x710
> > [<ffffffff8109e0cd>] kthread+0xed/0x100
> > [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> > [<ffffffff816b2fec>] ret_from_fork+0x7c/0xb0
> > [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> > 
> > Seems to be because the scan_work work_struct might be active when the housing
> > fc_rport struct gets freed.  Ensure that we cancel it prior to freeing the rport
> > 
> > Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> > CC: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > CC: Robert Love <robert.w.love-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > CC: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> >  drivers/scsi/scsi_transport_fc.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> > index 4628fd5..5bd552c 100644
> > --- a/drivers/scsi/scsi_transport_fc.c
> > +++ b/drivers/scsi/scsi_transport_fc.c
> > @@ -2548,6 +2548,7 @@ fc_rport_final_delete(struct work_struct *work)
> >  			fc_flush_devloss(shost);
> >  		if (!cancel_delayed_work(&rport->dev_loss_work))
> >  			fc_flush_devloss(shost);
> > +		cancel_work_sync(&rport->scan_work);
> 
> Make sense to ensure pending work canceled, adding James Smart for his
> ACK as transport FC class author. 
> 
> 
> Reviewed-by: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 
> 
Ping on this, Something just occured to me.  I was thinking (perhaps
erroneously) that this would go through the FCoE tree, but I don't see that
you've setup a tree yet vasu (and Rob's has been idle for 6 months).  Whats the
plan for this (and future) fcoe patchs.  Will you have a tree, or will we send
this through Christophs new scsi tree perhaps?

Neil

> 
> >  		spin_lock_irqsave(shost->host_lock, flags);
> >  		rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
> >  	}
> 
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
       [not found]       ` <20140606205409.GD2543-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org>
@ 2014-06-09 18:09         ` Vasu Dev
  2014-06-09 19:16           ` [Open-FCoE] " Neil Horman
  0 siblings, 1 reply; 11+ messages in thread
From: Vasu Dev @ 2014-06-09 18:09 UTC (permalink / raw)
  To: Neil Horman
  Cc: fcoe-devel-s9riP+hp16TNLxjTenLetw, linux-scsi-u79uwXL29TY76Z2rM5mHXA

On Fri, 2014-06-06 at 16:54 -0400, Neil Horman wrote:
> On Mon, Jun 02, 2014 at 04:22:50PM -0700, Vasu Dev wrote:
> > On Fri, 2014-05-30 at 10:59 -0400, Neil Horman wrote:
> > > debugfs caught this:
> > > WARNING: at lib/debugobjects.c:260 debug_print_object+0x83/0xa0()
> > > ODEBUG: free active (active state 0) object type: work_struct
> > > hint: fc_scsi_scan_rport+0x0/0xd0 [scsi_transport_fc]
> > >  CPU: 1 PID: 184 Comm: kworker/1:1 Tainted: G        W
> > > --------------   3.10.0-123.el7.x86_64.debug #1
> > > Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
> > > Workqueue: fc_wq_5 fc_rport_final_delete [scsi_transport_fc]
> > > Call Trace:
> > > [<ffffffff8169efec>] dump_stack+0x19/0x1b
> > > [<ffffffff8106cbd1>] warn_slowpath_common+0x61/0x80
> > > [<ffffffff8106cc4c>] warn_slowpath_fmt+0x5c/0x80
> > > [<ffffffff8133e003>] debug_print_object+0x83/0xa0
> > > [<ffffffffa04e2f40>] ? fc_parse_wwn+0x100/0x100
> > > 
> > > [<ffffffff8133f23b>] debug_check_no_obj_freed+0x22b/0x270
> > > [<ffffffffa04e127e>] ? fc_rport_dev_release+0x1e/0x30
> > > [<ffffffff811db3e9>] kfree+0xd9/0x2d0
> > > [<ffffffffa04e127e>] fc_rport_dev_release+0x1e/0x30
> > > [<ffffffff81428032>] device_release+0x32/0xa0
> > > [<ffffffff8132701e>] kobject_release+0x7e/0x1b0
> > > [<ffffffff81326ed8>] kobject_put+0x28/0x60
> > > [<ffffffff81428397>] put_device+0x17/0x20
> > > [<ffffffffa04e5025>] fc_rport_final_delete+0x165/0x210
> > > [<ffffffff810959b0>] process_one_work+0x220/0x710
> > > [<ffffffff81095944>] ? process_one_work+0x1b4/0x710
> > > [<ffffffff81095fbb>] worker_thread+0x11b/0x3a0
> > > [<ffffffff81095ea0>] ? process_one_work+0x710/0x710
> > > [<ffffffff8109e0cd>] kthread+0xed/0x100
> > > [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> > > [<ffffffff816b2fec>] ret_from_fork+0x7c/0xb0
> > > [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> > > 
> > > Seems to be because the scan_work work_struct might be active when the housing
> > > fc_rport struct gets freed.  Ensure that we cancel it prior to freeing the rport
> > > 
> > > Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> > > CC: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > CC: Robert Love <robert.w.love-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > CC: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > ---
> > >  drivers/scsi/scsi_transport_fc.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> > > index 4628fd5..5bd552c 100644
> > > --- a/drivers/scsi/scsi_transport_fc.c
> > > +++ b/drivers/scsi/scsi_transport_fc.c
> > > @@ -2548,6 +2548,7 @@ fc_rport_final_delete(struct work_struct *work)
> > >  			fc_flush_devloss(shost);
> > >  		if (!cancel_delayed_work(&rport->dev_loss_work))
> > >  			fc_flush_devloss(shost);
> > > +		cancel_work_sync(&rport->scan_work);
> > 
> > Make sense to ensure pending work canceled, adding James Smart for his
> > ACK as transport FC class author. 
> > 
> > 
> > Reviewed-by: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 
> > 
> Ping on this, Something just occured to me.  I was thinking (perhaps
> erroneously) that this would go through the FCoE tree, but I don't see that
> you've setup a tree yet vasu (and Rob's has been idle for 6 months).  Whats the
> plan for this (and future) fcoe patchs.  Will you have a tree, or will we send
> this through Christophs new scsi tree perhaps?
> 

Thanks Neil for bringing this, I and Rob also had off list discussion on
this just last week. 

Given fcoe is quite mature now and its patches volume is very low, so
getting its kernel patches directly to scsi subsystem should work fine
and should be okay with James or Christophs to pull into scsi subsystem
directly once I've my non-author signoff ACK there as described in this
announcement at http://marc.info/?l=linux-scsi&m=140050839729415&w=2 

If no alternate suggestion or objection to this then I'll formally
announce this on fcoe mailing list.

However for any huges patches series bomb or RFCs, I'll request fcoe
developers to send patches against scsi tree at fcoe devel list first
and then if needed I can roll them up.

Thanks,
Vasu

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Open-FCoE] [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
  2014-06-09 18:09         ` Vasu Dev
@ 2014-06-09 19:16           ` Neil Horman
  2014-06-10 11:38             ` Christoph Hellwig
  0 siblings, 1 reply; 11+ messages in thread
From: Neil Horman @ 2014-06-09 19:16 UTC (permalink / raw)
  To: Vasu Dev; +Cc: fcoe-devel, linux-scsi, james.smart, hch

On Mon, Jun 09, 2014 at 11:09:43AM -0700, Vasu Dev wrote:
> On Fri, 2014-06-06 at 16:54 -0400, Neil Horman wrote:
> > On Mon, Jun 02, 2014 at 04:22:50PM -0700, Vasu Dev wrote:
> > > On Fri, 2014-05-30 at 10:59 -0400, Neil Horman wrote:
> > > > debugfs caught this:
> > > > WARNING: at lib/debugobjects.c:260 debug_print_object+0x83/0xa0()
> > > > ODEBUG: free active (active state 0) object type: work_struct
> > > > hint: fc_scsi_scan_rport+0x0/0xd0 [scsi_transport_fc]
> > > >  CPU: 1 PID: 184 Comm: kworker/1:1 Tainted: G        W
> > > > --------------   3.10.0-123.el7.x86_64.debug #1
> > > > Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
> > > > Workqueue: fc_wq_5 fc_rport_final_delete [scsi_transport_fc]
> > > > Call Trace:
> > > > [<ffffffff8169efec>] dump_stack+0x19/0x1b
> > > > [<ffffffff8106cbd1>] warn_slowpath_common+0x61/0x80
> > > > [<ffffffff8106cc4c>] warn_slowpath_fmt+0x5c/0x80
> > > > [<ffffffff8133e003>] debug_print_object+0x83/0xa0
> > > > [<ffffffffa04e2f40>] ? fc_parse_wwn+0x100/0x100
> > > > 
> > > > [<ffffffff8133f23b>] debug_check_no_obj_freed+0x22b/0x270
> > > > [<ffffffffa04e127e>] ? fc_rport_dev_release+0x1e/0x30
> > > > [<ffffffff811db3e9>] kfree+0xd9/0x2d0
> > > > [<ffffffffa04e127e>] fc_rport_dev_release+0x1e/0x30
> > > > [<ffffffff81428032>] device_release+0x32/0xa0
> > > > [<ffffffff8132701e>] kobject_release+0x7e/0x1b0
> > > > [<ffffffff81326ed8>] kobject_put+0x28/0x60
> > > > [<ffffffff81428397>] put_device+0x17/0x20
> > > > [<ffffffffa04e5025>] fc_rport_final_delete+0x165/0x210
> > > > [<ffffffff810959b0>] process_one_work+0x220/0x710
> > > > [<ffffffff81095944>] ? process_one_work+0x1b4/0x710
> > > > [<ffffffff81095fbb>] worker_thread+0x11b/0x3a0
> > > > [<ffffffff81095ea0>] ? process_one_work+0x710/0x710
> > > > [<ffffffff8109e0cd>] kthread+0xed/0x100
> > > > [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> > > > [<ffffffff816b2fec>] ret_from_fork+0x7c/0xb0
> > > > [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> > > > 
> > > > Seems to be because the scan_work work_struct might be active when the housing
> > > > fc_rport struct gets freed.  Ensure that we cancel it prior to freeing the rport
> > > > 
> > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > CC: linux-scsi@vger.kernel.org
> > > > CC: Robert Love <robert.w.love@intel.com>
> > > > CC: Vasu Dev <vasu.dev@intel.com>
> > > > ---
> > > >  drivers/scsi/scsi_transport_fc.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> > > > index 4628fd5..5bd552c 100644
> > > > --- a/drivers/scsi/scsi_transport_fc.c
> > > > +++ b/drivers/scsi/scsi_transport_fc.c
> > > > @@ -2548,6 +2548,7 @@ fc_rport_final_delete(struct work_struct *work)
> > > >  			fc_flush_devloss(shost);
> > > >  		if (!cancel_delayed_work(&rport->dev_loss_work))
> > > >  			fc_flush_devloss(shost);
> > > > +		cancel_work_sync(&rport->scan_work);
> > > 
> > > Make sense to ensure pending work canceled, adding James Smart for his
> > > ACK as transport FC class author. 
> > > 
> > > 
> > > Reviewed-by: Vasu Dev <vasu.dev@intel.com> 
> > > 
> > Ping on this, Something just occured to me.  I was thinking (perhaps
> > erroneously) that this would go through the FCoE tree, but I don't see that
> > you've setup a tree yet vasu (and Rob's has been idle for 6 months).  Whats the
> > plan for this (and future) fcoe patchs.  Will you have a tree, or will we send
> > this through Christophs new scsi tree perhaps?
> > 
> 
> Thanks Neil for bringing this, I and Rob also had off list discussion on
> this just last week. 
> 
> Given fcoe is quite mature now and its patches volume is very low, so
> getting its kernel patches directly to scsi subsystem should work fine
> and should be okay with James or Christophs to pull into scsi subsystem
> directly once I've my non-author signoff ACK there as described in this
> announcement at http://marc.info/?l=linux-scsi&m=140050839729415&w=2 
> 
> If no alternate suggestion or objection to this then I'll formally
> announce this on fcoe mailing list.
> 
> However for any huges patches series bomb or RFCs, I'll request fcoe
> developers to send patches against scsi tree at fcoe devel list first
> and then if needed I can roll them up.
> 
> Thanks,
> Vasu
> 
Copy that Vasu, Christoph, is that ok with you?

Neil

> 
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Open-FCoE] [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
  2014-06-09 19:16           ` [Open-FCoE] " Neil Horman
@ 2014-06-10 11:38             ` Christoph Hellwig
       [not found]               ` <20140610113841.GC26195-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2014-06-10 11:38 UTC (permalink / raw)
  To: Neil Horman; +Cc: Vasu Dev, fcoe-devel, linux-scsi, james.smart, hch

On Mon, Jun 09, 2014 at 03:16:37PM -0400, Neil Horman wrote:
> > Given fcoe is quite mature now and its patches volume is very low, so
> > getting its kernel patches directly to scsi subsystem should work fine
> > and should be okay with James or Christophs to pull into scsi subsystem
> > directly once I've my non-author signoff ACK there as described in this
> > announcement at http://marc.info/?l=linux-scsi&m=140050839729415&w=2 
> > 
> > If no alternate suggestion or objection to this then I'll formally
> > announce this on fcoe mailing list.
> > 
> > However for any huges patches series bomb or RFCs, I'll request fcoe
> > developers to send patches against scsi tree at fcoe devel list first
> > and then if needed I can roll them up.
> > 
> > Thanks,
> > Vasu
> > 
> Copy that Vasu, Christoph, is that ok with you?

That's fine with me.  It would help greatly if you could make sure
all the paches get a review or two very quickly so I can just pick them
up immediately after reviewing them.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
       [not found]               ` <20140610113841.GC26195-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2014-06-10 15:49                 ` Neil Horman
  0 siblings, 0 replies; 11+ messages in thread
From: Neil Horman @ 2014-06-10 15:49 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: fcoe-devel-s9riP+hp16TNLxjTenLetw, linux-scsi-u79uwXL29TY76Z2rM5mHXA

On Tue, Jun 10, 2014 at 04:38:41AM -0700, Christoph Hellwig wrote:
> On Mon, Jun 09, 2014 at 03:16:37PM -0400, Neil Horman wrote:
> > > Given fcoe is quite mature now and its patches volume is very low, so
> > > getting its kernel patches directly to scsi subsystem should work fine
> > > and should be okay with James or Christophs to pull into scsi subsystem
> > > directly once I've my non-author signoff ACK there as described in this
> > > announcement at http://marc.info/?l=linux-scsi&m=140050839729415&w=2 
> > > 
> > > If no alternate suggestion or objection to this then I'll formally
> > > announce this on fcoe mailing list.
> > > 
> > > However for any huges patches series bomb or RFCs, I'll request fcoe
> > > developers to send patches against scsi tree at fcoe devel list first
> > > and then if needed I can roll them up.
> > > 
> > > Thanks,
> > > Vasu
> > > 
> > Copy that Vasu, Christoph, is that ok with you?
> 
> That's fine with me.  It would help greatly if you could make sure
> all the paches get a review or two very quickly so I can just pick them
> up immediately after reviewing them.
> 
> 
Roger that, Vasu has already acked this.  I thought there was a second, but I'm
not sure, my mailbox is a bit messed up at the moment.

Sorry for not cc-ing you on this sooner, I thought it was going to go through
the FCoE tree initially

Best
Neil

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Open-FCoE] [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
  2014-06-02 23:22   ` Vasu Dev
  2014-06-06 20:54     ` Neil Horman
@ 2014-06-16 11:15     ` Neil Horman
  1 sibling, 0 replies; 11+ messages in thread
From: Neil Horman @ 2014-06-16 11:15 UTC (permalink / raw)
  To: Vasu Dev; +Cc: fcoe-devel, linux-scsi, james.smart

On Mon, Jun 02, 2014 at 04:22:50PM -0700, Vasu Dev wrote:
> On Fri, 2014-05-30 at 10:59 -0400, Neil Horman wrote:
> > debugfs caught this:
> > WARNING: at lib/debugobjects.c:260 debug_print_object+0x83/0xa0()
> > ODEBUG: free active (active state 0) object type: work_struct
> > hint: fc_scsi_scan_rport+0x0/0xd0 [scsi_transport_fc]
> >  CPU: 1 PID: 184 Comm: kworker/1:1 Tainted: G        W
> > --------------   3.10.0-123.el7.x86_64.debug #1
> > Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
> > Workqueue: fc_wq_5 fc_rport_final_delete [scsi_transport_fc]
> > Call Trace:
> > [<ffffffff8169efec>] dump_stack+0x19/0x1b
> > [<ffffffff8106cbd1>] warn_slowpath_common+0x61/0x80
> > [<ffffffff8106cc4c>] warn_slowpath_fmt+0x5c/0x80
> > [<ffffffff8133e003>] debug_print_object+0x83/0xa0
> > [<ffffffffa04e2f40>] ? fc_parse_wwn+0x100/0x100
> > 
> > [<ffffffff8133f23b>] debug_check_no_obj_freed+0x22b/0x270
> > [<ffffffffa04e127e>] ? fc_rport_dev_release+0x1e/0x30
> > [<ffffffff811db3e9>] kfree+0xd9/0x2d0
> > [<ffffffffa04e127e>] fc_rport_dev_release+0x1e/0x30
> > [<ffffffff81428032>] device_release+0x32/0xa0
> > [<ffffffff8132701e>] kobject_release+0x7e/0x1b0
> > [<ffffffff81326ed8>] kobject_put+0x28/0x60
> > [<ffffffff81428397>] put_device+0x17/0x20
> > [<ffffffffa04e5025>] fc_rport_final_delete+0x165/0x210
> > [<ffffffff810959b0>] process_one_work+0x220/0x710
> > [<ffffffff81095944>] ? process_one_work+0x1b4/0x710
> > [<ffffffff81095fbb>] worker_thread+0x11b/0x3a0
> > [<ffffffff81095ea0>] ? process_one_work+0x710/0x710
> > [<ffffffff8109e0cd>] kthread+0xed/0x100
> > [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> > [<ffffffff816b2fec>] ret_from_fork+0x7c/0xb0
> > [<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
> > 
> > Seems to be because the scan_work work_struct might be active when the housing
> > fc_rport struct gets freed.  Ensure that we cancel it prior to freeing the rport
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: linux-scsi@vger.kernel.org
> > CC: Robert Love <robert.w.love@intel.com>
> > CC: Vasu Dev <vasu.dev@intel.com>
> > ---
> >  drivers/scsi/scsi_transport_fc.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> > index 4628fd5..5bd552c 100644
> > --- a/drivers/scsi/scsi_transport_fc.c
> > +++ b/drivers/scsi/scsi_transport_fc.c
> > @@ -2548,6 +2548,7 @@ fc_rport_final_delete(struct work_struct *work)
> >  			fc_flush_devloss(shost);
> >  		if (!cancel_delayed_work(&rport->dev_loss_work))
> >  			fc_flush_devloss(shost);
> > +		cancel_work_sync(&rport->scan_work);
> 
> Make sense to ensure pending work canceled, adding James Smart for his
> ACK as transport FC class author. 
> 
> 
> Reviewed-by: Vasu Dev <vasu.dev@intel.com> 
> 
> 
> >  		spin_lock_irqsave(shost->host_lock, flags);
> >  		rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
> >  	}
> 
> 
> 
Ping James, I beleve Christoph is still waiting on a review from you here.

Neil


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] fc: ensure scan_work isn't active when freeing fc_rport
@ 2014-06-23 14:40 Neil Horman
  0 siblings, 0 replies; 11+ messages in thread
From: Neil Horman @ 2014-06-23 14:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Horman, James E.J. Bottomley, Christoph Hellwig, linux-scsi,
	Robert Love, Vasu Dev

debugfs caught this:
WARNING: at lib/debugobjects.c:260 debug_print_object+0x83/0xa0()
ODEBUG: free active (active state 0) object type: work_struct
hint: fc_scsi_scan_rport+0x0/0xd0 [scsi_transport_fc]
 CPU: 1 PID: 184 Comm: kworker/1:1 Tainted: G        W
--------------   3.10.0-123.el7.x86_64.debug #1
Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
Workqueue: fc_wq_5 fc_rport_final_delete [scsi_transport_fc]
Call Trace:
[<ffffffff8169efec>] dump_stack+0x19/0x1b
[<ffffffff8106cbd1>] warn_slowpath_common+0x61/0x80
[<ffffffff8106cc4c>] warn_slowpath_fmt+0x5c/0x80
[<ffffffff8133e003>] debug_print_object+0x83/0xa0
[<ffffffffa04e2f40>] ? fc_parse_wwn+0x100/0x100

[<ffffffff8133f23b>] debug_check_no_obj_freed+0x22b/0x270
[<ffffffffa04e127e>] ? fc_rport_dev_release+0x1e/0x30
[<ffffffff811db3e9>] kfree+0xd9/0x2d0
[<ffffffffa04e127e>] fc_rport_dev_release+0x1e/0x30
[<ffffffff81428032>] device_release+0x32/0xa0
[<ffffffff8132701e>] kobject_release+0x7e/0x1b0
[<ffffffff81326ed8>] kobject_put+0x28/0x60
[<ffffffff81428397>] put_device+0x17/0x20
[<ffffffffa04e5025>] fc_rport_final_delete+0x165/0x210
[<ffffffff810959b0>] process_one_work+0x220/0x710
[<ffffffff81095944>] ? process_one_work+0x1b4/0x710
[<ffffffff81095fbb>] worker_thread+0x11b/0x3a0
[<ffffffff81095ea0>] ? process_one_work+0x710/0x710
[<ffffffff8109e0cd>] kthread+0xed/0x100
[<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80
[<ffffffff816b2fec>] ret_from_fork+0x7c/0xb0
[<ffffffff8109dfe0>] ? insert_kthread_work+0x80/0x80

Seems to be because the scan_work work_struct might be active when the housing
fc_rport struct gets freed.  Ensure that we cancel it prior to freeing the rport

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: "James E.J. Bottomley" <JBottomley@parallels.com>
CC: Christoph Hellwig <hch@infradead.org>
CC: linux-scsi@vger.kernel.org
CC: Robert Love <robert.w.love@intel.com>
CC: Vasu Dev <vasu.dev@intel.com>
---
 drivers/scsi/scsi_transport_fc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 4628fd5..5bd552c 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2548,6 +2548,7 @@ fc_rport_final_delete(struct work_struct *work)
 			fc_flush_devloss(shost);
 		if (!cancel_delayed_work(&rport->dev_loss_work))
 			fc_flush_devloss(shost);
+		cancel_work_sync(&rport->scan_work);
 		spin_lock_irqsave(shost->host_lock, flags);
 		rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
 	}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-06-23 14:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-30 14:59 [PATCH] fc: ensure scan_work isn't active when freeing fc_rport Neil Horman
2014-06-02  8:35 ` Christoph Hellwig
2014-06-02  9:28 ` Hannes Reinecke
     [not found] ` <1401461974-7879-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
2014-06-02 23:22   ` Vasu Dev
2014-06-06 20:54     ` Neil Horman
     [not found]       ` <20140606205409.GD2543-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org>
2014-06-09 18:09         ` Vasu Dev
2014-06-09 19:16           ` [Open-FCoE] " Neil Horman
2014-06-10 11:38             ` Christoph Hellwig
     [not found]               ` <20140610113841.GC26195-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-06-10 15:49                 ` Neil Horman
2014-06-16 11:15     ` [Open-FCoE] " Neil Horman
2014-06-23 14:40 Neil Horman

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.