Skip to contentSkip to conversation

aic7xxx: remove excessive inlining

PATCH In review Denis Vlasenko 3 participants 9 messages Aug 13, 2006 – Aug 15, 2006

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

cover aic7xxx: remove excessive inlining

Hello,

This is a resend. I had these patches run-tested at the time of first submission. Now I have no hardware to test them. I think that they are still fine, though.

Basically, patches deinline some functions, mainly those which perform port I/O. They shave off ~55k on AMD64:

# x86_64-pc-linux-gnu-size linux-2.6.17.8_64*/vmlinux
   text    data     bss     dec     hex filename
4632627 1222698  468392 6323717  607e05 linux-2.6.17.8_64/vmlinux
4571327 1223274  468392 6262993  5f90d1 linux-2.6.17.8_64aic/vmlinux
# x86_64-pc-linux-gnu-size linux-2.6.17.8_64*/drivers/scsi/aic7xxx/aic7*xx.o
   text    data     bss     dec     hex filename
 135543   13521     628  149692   248bc linux-2.6.17.8_64/drivers/scsi/aic7xxx/aic79xx.o
 103291   22289     560  126140   1ecbc linux-2.6.17.8_64/drivers/scsi/aic7xxx/aic7xxx.o
  90083   13521     628  104232   19728 linux-2.6.17.8_64aic/drivers/scsi/aic7xxx/aic79xx.o
  87231   22289     560  110080   1ae00 linux-2.6.17.8_64aic/drivers/scsi/aic7xxx/aic7xxx.o
--
vda
#1 lore raw

PATCH 1/4 aic7xxx: remove excessive inlining

Quoted 2 lines · Denis Vlasenko

Basically, patches deinline some functions, mainly those which perform port I/O.

Preparatory patch. s/__inline/inline.
--
vda

#2 lore raw

PATCH 2/4 aic7xxx: remove excessive inlining

Quoted 2 lines · Denis Vlasenko

Basically, patches deinline some functions, mainly those which perform port I/O.

The bulk of deinlining is done here.

Also simplifies ahc_delay() and replaces run-time checks in ahX_pci_read_config() with compile-time ones. --
vda

#3 lore raw

PATCH 3/4 aic7xxx: remove excessive inlining

Quoted 2 lines · Denis Vlasenko

Basically, patches deinline some functions, mainly those which perform port I/O.

ahd_suspend/resume are not used. #ifdef them out.
--
vda

#4 lore raw
Quoted 5 lines · Denis Vlasenko
Quoted 2 lines · Denis Vlasenko

Basically, patches deinline some functions, mainly those which perform port I/O.

ahd_suspend/resume are not used. #ifdef them out.

I forgot to attach the file. :(
--
vda

drivers/scsi/aic7xxx/aic79xx_core.c +3 -1
diff -urpN -U4 linux-2.6.17.8.aic2/drivers/scsi/aic7xxx/aic79xx_core.c linux-2.6.17.8.aic3/drivers/scsi/aic7xxx/aic79xx_core.c
@@ -7681,8 +7681,9 @@ ahd_pause_and_flushwork(struct ahd_softc
 
 	ahd->flags &= ~AHD_ALL_INTERRUPTS;
 }
 
+#ifdef UNUSED
 int
 ahd_suspend(struct ahd_softc *ahd)
 {
 
@@ -7704,16 +7705,17 @@ ahd_resume(struct ahd_softc *ahd)
 	ahd_intr_enable(ahd, TRUE); 
 	ahd_restart(ahd);
 	return (0);
 }
+#endif
 
 /************************** Busy Target Table *********************************/
 /*
  * Set SCBPTR to the SCB that contains the busy
  * table entry for TCL.  Return the offset into
  * the SCB that contains the entry for TCL.
  * saved_scbid is dereferenced and set to the
- * scbid that should be restored once manipualtion
+ * scbid that should be restored once manipulation
  * of the TCL entry is complete.
  */
 static u_int
 ahd_index_busy_tcl(struct ahd_softc *ahd, u_int *saved_scbid, u_int tcl)
#5 lore raw

PATCH 4/4 aic7xxx: remove excessive inlining

In reply to Denis Vlasenko
Quoted 2 lines · Denis Vlasenko

Basically, patches deinline some functions, mainly those which perform port I/O.

Comment says "Read high byte first as some registers increment..." but code doesn't guarantee that, I think:

	return ((ahd_inb(ahd, port+1) << 8) | ahd_inb(ahd, port));

Compiler can reorder it.

Make the order explicit.
--
vda

drivers/scsi/aic7xxx/aic79xx_core.c +3 -1
diff -urpN -U4 linux-2.6.17.8.aic3/drivers/scsi/aic7xxx/aic79xx_core.c linux-2.6.17.8.aic4/drivers/scsi/aic7xxx/aic79xx_core.c
@@ -341,9 +341,11 @@ ahd_inw(struct ahd_softc *ahd, u_int por
 	 * Read high byte first as some registers increment
 	 * or have other side effects when the low byte is
 	 * read.
 	 */
-	return ((ahd_inb(ahd, port+1) << 8) | ahd_inb(ahd, port));
+	uint16_t r = ahd_inb(ahd, port+1) << 8;
+	r |= ahd_inb(ahd, port);
+	return r;
 }
 
 void
 ahd_outw(struct ahd_softc *ahd, u_int port, u_int value)
#6 lore raw
In reply to Denis Vlasenko
Quoted 1 line · On Sun, 13 Aug 2006 14:57:21 +0200 Denis Vlasenko <[email protected]>…

This is a resend.

Please resend ;)

- All these patches had the same Subject:, thus forcing me to invent titles for you.

- The changelogs are weird - think what they'll look like once they hit the git tree. Someone will need to clean those changelogs up, and I'd prefer that it not be me.

- Missing Signed-off-by:'s.

http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt is here to help.

Thanks.

#7 lore raw
Quoted 10 lines · On Mon, 14 Aug 2006 16:14:34 -0700 Andrew Morton wrote:
Quoted 1 line · On Sun, 13 Aug 2006 14:57:21 +0200 Denis Vlasenko <[email protected]>…

This is a resend.

Please resend ;)

- All these patches had the same Subject:, thus forcing me to invent titles for you.

Quoted 8 lines

- The changelogs are weird - think what they'll look like once they hit the git tree. Someone will need to clean those changelogs up, and I'd prefer that it not be me.

- Missing Signed-off-by:'s.

http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt is here to help.

Don't *zip them.

---
~Randy

#8 lore raw
Quoted 16 lines · Randy.Dunlap
Quoted 14 lines · On Mon, 14 Aug 2006 16:14:34 -0700 Andrew Morton wrote:
Quoted 1 line · On Sun, 13 Aug 2006 14:57:21 +0200 Denis Vlasenko <[email protected]>…

This is a resend.

Please resend ;)

- All these patches had the same Subject:, thus forcing me to invent titles for you.

- The changelogs are weird - think what they'll look like once they hit the git tree. Someone will need to clean those changelogs up, and I'd prefer that it not be me.

Quoted 6 lines
Quoted 4 lines

- Missing Signed-off-by:'s.

http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt is here to help.

Don't *zip them.

Ok, resending to Andrew off-list.
--
vda