Skip to contentSkip to conversation

fusion: fibre channel: don't sync cache if remote port deleted

PATCH In review Michael Reed 2 participants 3 messages Jul 31, 2006 – Aug 1, 2006

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 4/4 fusion: fibre channel: don't sync cache if remote port deleted

mptscsih_slave_destroy issues an unconditional synchronize cache. It is not necessary for fibre channel devices for which the rport has been deleted. This patch creates a new function, mptfc_slave_destroy(), which implements the appropriate test.

Additionally, Eric's review of the original version of this patch pointed me at some unnecessary tests of the rport dd_data field. I've removed one and made two others bugs.

drivers/message/fusion/mptscsih.h +2 -0
@@ -101,3 +101,5 @@ extern void mptscsih_timer_expired(unsig
 extern int mptscsih_TMHandler(MPT_SCSI_HOST *hd, u8 type, u8 channel, u8 target, u8 lun, int ctx2abort, ulong timeout);
 extern int mptscsih_raid_id_to_num(MPT_SCSI_HOST *hd, uint physdiskid);
 extern int mptscsih_is_phys_disk(MPT_ADAPTER *ioc, int id);
+extern void mptscsih_synchronize_cache(MPT_SCSI_HOST *hd, VirtDevice *vdevice);
+extern void mptscsih_search_running_cmds(MPT_SCSI_HOST *hd, VirtDevice *vdevice);
drivers/message/fusion/mptscsih.c +5 -3
@@ -140,7 +140,7 @@ static void	mptscsih_setTargetNegoParms(
 static int	mptscsih_writeIOCPage4(MPT_SCSI_HOST *hd, int target_id, int bus);
 int		mptscsih_scandv_complete(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf, MPT_FRAME_HDR *r);
 static int	mptscsih_do_cmd(MPT_SCSI_HOST *hd, INTERNAL_CMD *iocmd);
-static void	mptscsih_synchronize_cache(MPT_SCSI_HOST *hd, VirtDevice *vdevice);
+void		mptscsih_synchronize_cache(MPT_SCSI_HOST *hd, VirtDevice *vdevice);
 
 void 		mptscsih_remove(struct pci_dev *);
 void 		mptscsih_shutdown(struct pci_dev *);
@@ -936,7 +936,7 @@ mptscsih_flush_running_cmds(MPT_SCSI_HOS
  *
  *	Called from slave_destroy.
  */
-static void
+void
 mptscsih_search_running_cmds(MPT_SCSI_HOST *hd, VirtDevice *vdevice)
 {
 	SCSIIORequest_t	*mf = NULL;
@@ -3436,7 +3436,7 @@ mptscsih_do_cmd(MPT_SCSI_HOST *hd, INTER
  *	MUST be single-threaded.
  *
  */
-static void
+void
 mptscsih_synchronize_cache(MPT_SCSI_HOST *hd, VirtDevice *vdevice)
 {
 	INTERNAL_CMD		 iocmd;
@@ -3460,6 +3460,8 @@ mptscsih_synchronize_cache(MPT_SCSI_HOST
 		mptscsih_do_cmd(hd, &iocmd);
 }
 
+EXPORT_SYMBOL(mptscsih_search_running_cmds);
+EXPORT_SYMBOL(mptscsih_synchronize_cache);
 EXPORT_SYMBOL(mptscsih_remove);
 EXPORT_SYMBOL(mptscsih_shutdown);
 #ifdef CONFIG_PM
drivers/message/fusion/mptfc.c +48 -24

123 lines not on this page · raw message

#1 lore raw
Quoted 1 line · Michael Reed

mptscsih_slave_destroy issues an unconditional synchronize cache.

A) mptscsih_slave_destroy shouldn't be doing this ... it should be relying on the functionality in the mid-layer.

Quoted 3 lines

It is not necessary for fibre channel devices for which the rport has been deleted. This patch creates a new function, mptfc_slave_destroy(), which implements the appropriate test.

So, this is really code that shouldn't be in a driver, it should be higher up.

The other three patches look fine.

James

#2 lore raw

The mid-layer appears to be inconsistent with regard to sync cache. I agree the sync should not be in the lldd. It's been in fusion for a long time and I'm just fixing the implementation for fibre channel. This isn't a new feature, just a fix to an existing one.

I refer you to two threads which discussed the issue.

http://marc.theaimsgroup.com/?l=linux-scsi&m=115386616817044&w=2
http://marc.theaimsgroup.com/?l=linux-scsi&m=115266599732078&w=2

Mike

Quoted 17 lines · James Bottomley
Quoted 1 line · Michael Reed

mptscsih_slave_destroy issues an unconditional synchronize cache.

A) mptscsih_slave_destroy shouldn't be doing this ... it should be relying on the functionality in the mid-layer.

Quoted 3 lines

It is not necessary for fibre channel devices for which the rport has been deleted. This patch creates a new function, mptfc_slave_destroy(), which implements the appropriate test.

So, this is really code that shouldn't be in a driver, it should be higher up.

The other three patches look fine.

James