[PATCH 0/3] debloat aic7xxx and aic79xx drivers
Hi, Following patches debloat drivers/scsi/aic7xxx/*. I also had to add prototypes for ahc_lookup_scb and ahd_lookup_scb to .h files. 1-debloat.patch Deinlines and moves big functions from .h to .c files. Adds prototypes for ahc_lookup_scb and ahd_lookup_scb to .h files. 2-addstatic.patch Adds statics, #ifdefs out huge amount of unused code, adds consts 3-addconst.patch Adds more consts Driver code/data size reductions: Build with debugging on (CONFIG_AIC7XXX_DEBUG_ENABLE=y): textdata bss dec hex filename 310865 499221204 361991 58607 linux-2.6.23.t/drivers/scsi/aic7xxx/built-in.o 22198727541204 225945 37299 linux-2.6.23-aic-3-addconst.t/drivers/scsi/aic7xxx/built-in.o With debugging off: textdata bss dec hex filename 298896 427541172 342822 53b26 linux-2.6.23.tt/drivers/scsi/aic7xxx/built-in.o 21606827541172 219994 35b5a linux-2.6.23-aic-3-addconst.tt/drivers/scsi/aic7xxx/built-in.o make namespacecheck goes from 400+ functions to: drivers/scsi/aic7xxx/aic79xx_core.o ahd_inq ahd_inw ahd_outq ahd_outw drivers/scsi/aic7xxx/aic79xx_osm.o ahd_insb drivers/scsi/aic7xxx/aic7xxx_core.o ahc_inq ahc_outq drivers/scsi/aic7xxx/aic7xxx_osm.o ahc_insb None of these patches touch any logic, code changes are pretty minimal. Compile tested and applies cleanly to 2.6.23. I don't have this hardware anymore and cannot run test these patches. Please apply. Signed-off-by: Denys Vlasenko [EMAIL PROTECTED] -- vda - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] debloat aic7xxx and aic79xx drivers
Adds more consts
Signed-off-by: Denys Vlasenko [EMAIL PROTECTED]
--
vda
diff -urpN linux-2.6.23-aic-2-addstatic/drivers/scsi/aic7xxx/aic79xx.h linux-2.6.23-aic-3-addconst/drivers/scsi/aic7xxx/aic79xx.h
--- linux-2.6.23-aic-2-addstatic/drivers/scsi/aic7xxx/aic79xx.h 2007-10-14 15:05:07.0 +0100
+++ linux-2.6.23-aic-3-addconst/drivers/scsi/aic7xxx/aic79xx.h 2007-10-14 15:21:55.0 +0100
@@ -813,9 +813,9 @@ struct ahd_tmode_tstate {
* to parity errors in each phase table.
*/
struct ahd_phase_table_entry {
-uint8_t phase;
-uint8_t mesg_out; /* Message response to parity errors */
- char *phasemsg;
+ uint8_t phase;
+ uint8_t mesg_out; /* Message response to parity errors */
+ const char *phasemsg;
};
/** Serial EEPROM Format **/
@@ -1328,9 +1328,9 @@ extern const int ahd_num_aic7770_devs;
/**/
/* PCI Front End */
-struct ahd_pci_identity *ahd_find_pci_device(ahd_dev_softc_t);
+const struct ahd_pci_identity *ahd_find_pci_device(ahd_dev_softc_t);
int ahd_pci_config(struct ahd_softc *,
- struct ahd_pci_identity *);
+ const struct ahd_pci_identity *);
int ahd_pci_test_register_access(struct ahd_softc *);
/** SCB and SCB queue management **/
diff -urpN linux-2.6.23-aic-2-addstatic/drivers/scsi/aic7xxx/aic79xx_core.c linux-2.6.23-aic-3-addconst/drivers/scsi/aic7xxx/aic79xx_core.c
--- linux-2.6.23-aic-2-addstatic/drivers/scsi/aic7xxx/aic79xx_core.c 2007-10-14 15:05:07.0 +0100
+++ linux-2.6.23-aic-3-addconst/drivers/scsi/aic7xxx/aic79xx_core.c 2007-10-14 15:21:38.0 +0100
@@ -52,7 +52,7 @@
/* Lookup Tables **/
-static char *ahd_chip_names[] =
+static const char *const ahd_chip_names[] =
{
NONE,
aic7901,
@@ -65,11 +65,11 @@ static const u_int num_chip_names = ARRA
* Hardware error codes.
*/
struct ahd_hard_error_entry {
-uint8_t errno;
- char *errmesg;
+ uint8_t errno;
+ const char *errmesg;
};
-static struct ahd_hard_error_entry ahd_hard_errors[] = {
+static const struct ahd_hard_error_entry ahd_hard_errors[] = {
{ DSCTMOUT, Discard Timer has timed out },
{ ILLOPCODE, Illegal Opcode in sequencer program },
{ SQPARERR, Sequencer Parity Error },
@@ -79,7 +79,7 @@ static struct ahd_hard_error_entry ahd_h
};
static const u_int num_errors = ARRAY_SIZE(ahd_hard_errors);
-static struct ahd_phase_table_entry ahd_phase_table[] =
+static const struct ahd_phase_table_entry ahd_phase_table[] =
{
{ P_DATAOUT, MSG_NOOP, in Data-out phase },
{ P_DATAIN, MSG_INITIATOR_DET_ERR, in Data-in phase },
@@ -213,7 +213,7 @@ static void ahd_dumpseq(struct ahd_soft
#endif
static void ahd_loadseq(struct ahd_softc *ahd);
static int ahd_check_patch(struct ahd_softc *ahd,
- struct patch **start_patch,
+ const struct patch **start_patch,
u_int start_instr, u_int *skip_addr);
static u_int ahd_resolve_seqaddr(struct ahd_softc *ahd,
u_int address);
@@ -254,7 +254,7 @@ static void ahd_freeze_devq(struct ahd_
struct scb *scb);
static void ahd_handle_scb_status(struct ahd_softc *ahd,
struct scb *scb);
-static struct ahd_phase_table_entry* ahd_lookup_phase_entry(int phase);
+static const struct ahd_phase_table_entry* ahd_lookup_phase_entry(int phase);
static void ahd_shutdown(void *arg);
static void ahd_update_coalescing_values(struct ahd_softc *ahd,
u_int timer,
@@ -4337,11 +4337,11 @@ ahd_print_devinfo(struct ahd_softc *ahd,
devinfo-target, devinfo-lun);
}
-static struct ahd_phase_table_entry*
+static const struct ahd_phase_table_entry*
ahd_lookup_phase_entry(int phase)
{
- struct ahd_phase_table_entry *entry;
- struct ahd_phase_table_entry *last_entry;
+ const struct ahd_phase_table_entry *entry;
+ const struct ahd_phase_table_entry *last_entry;
/*
* num_phases doesn't include the default entry which
@@ -9358,7 +9358,7 @@ ahd_loadseq(struct ahd_softc *ahd)
struct cs cs_table[num_critical_sections];
u_int begin_set[num_critical_sections];
u_int end_set[num_critical_sections];
- struct patch *cur_patch;
+ const struct patch *cur_patch;
u_int cs_count;
u_int cur_cs;
u_int i;
@@ -9513,11 +9513,11 @@ ahd_loadseq(struct ahd_softc *ahd)
}
static int
-ahd_check_patch(struct ahd_softc *ahd, struct patch **start_patch,
+ahd_check_patch(struct ahd_softc *ahd, const struct patch **start_patch,
u_int start_instr, u_int *skip_addr)
{
- struct patch *cur_patch;
- struct patch *last_patch;
+ const struct patch *cur_patch;
+ const struct patch *last_patch;
u_int num_patches;
num_patches = ARRAY_SIZE(patches);
@@ -9551,7 +9551,7 @@ ahd_check_patch(struct ahd_softc *ahd, s
static u_int
ahd_resolve_seqaddr(struct ahd_softc *ahd, u_int address
Re: [PATCH 0/3] debloat aic7xxx and aic79xx drivers
Hi Andrew, James, On Monday 15 October 2007 14:53, Gabriel C wrote: Compile tested and applies cleanly to 2.6.23. I don't have this hardware anymore and cannot run test these patches. I can test these patches on an aic7892 controller later on today if you want. Works fine for me tested on : 03:0e.0 SCSI storage controller [0100]: Adaptec AIC-7892P U160/m [9005:008f] (rev 02) I hope this is enough for these patches to be accepted. If it is not, please let me know what do I need to do more. -- vda - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCH] final SCSI pieces for the merge window
On Wednesday 24 October 2007 15:27, Matthew Wilcox wrote: On Wed, Oct 24, 2007 at 09:28:10AM -0400, James Bottomley wrote: OK, so it's no secret that I'm the last of the subsystem maintainers whose day job isn't working on the linux kernel. If you want a full time person, who did you have in mind? I'm willing to take on the role of scsi git-monkey. Alternatively, we could split the scsi maintainer role the same way that Dave and Jeff do for net where Dave handles the core and Jeff handles the drivers. Or we can negotiate some other arrangement. That would be great. Maybe my aic7xxx debloating patches which were submitted four times already (IIRC) will be looked at at last. -- vda - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/lpfc/lpfc_hw.h: Some minor cleanup.
On Tuesday 30 October 2007 10:54, Richard Knutsson wrote:
Signed-off-by: Richard Knutsson [EMAIL PROTECTED]
---
Diffed against linus-git
Checked with script/checkpatch.pl
diff --git a/drivers/scsi/lpfc/lpfc_hw.h b/drivers/scsi/lpfc/lpfc_hw.h
index 451accd..6f56528 100644
--- a/drivers/scsi/lpfc/lpfc_hw.h
+++ b/drivers/scsi/lpfc/lpfc_hw.h
@@ -3158,31 +3158,30 @@ struct lpfc_sli2_slim {
*
* Parameters:
* device : struct pci_dev 's device field
- *
- * return 1 = TRUE
- *0 = FALSE
*/
-static inline int
+static inline bool
lpfc_is_LC_HBA(unsigned short device)
{
- if ((device == PCI_DEVICE_ID_TFLY) ||
- (device == PCI_DEVICE_ID_PFLY) ||
- (device == PCI_DEVICE_ID_LP101) ||
- (device == PCI_DEVICE_ID_BMID) ||
- (device == PCI_DEVICE_ID_BSMB) ||
- (device == PCI_DEVICE_ID_ZMID) ||
- (device == PCI_DEVICE_ID_ZSMB) ||
- (device == PCI_DEVICE_ID_RFLY))
- return 1;
- else
- return 0;
+ switch (device) {
+ case PCI_DEVICE_ID_TFLY:
+ case PCI_DEVICE_ID_PFLY:
+ case PCI_DEVICE_ID_LP101:
+ case PCI_DEVICE_ID_BMID:
+ case PCI_DEVICE_ID_BSMB:
+ case PCI_DEVICE_ID_ZMID:
+ case PCI_DEVICE_ID_ZSMB:
+ case PCI_DEVICE_ID_RFLY:
+ return true;
+ }
+
+ return false;
}
Why is this patch useful? I'd rather do this instead:
-static inline int
+static int
(this function has three callsites, thus de-inlining will
make code smaller)
--
vda
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] debloat aic7xxx and aic79xx drivers by deinlining
On Saturday 25 August 2007 22:57, Arjan van de Ven wrote: On Sat, 25 Aug 2007 22:57:07 +0100 Denys Vlasenko [EMAIL PROTECTED] wrote: Hi, Attached patch deinlines and moves big functions from .h to .c files in drivers/scsi/aic7xxx/*. I also had to add prototypes for ahc_lookup_scb and ahd_lookup_scb to .h files. one question... how many of these can actually be static (or would be if they were in their only-caller-c-file) ? Seeing this patch ignored two or three times already, I tried to make as non-intrusive change as possible, thus maximizing the chances that it will be applied. Come on, this patch saves 60k-90k of text, or 30-40% of driver size. This is ridiculous to have such enormous bloat. In the functions I converted there are no non-static functions which are not declared in a .h file. There are indeed functions which are used only in one .c file: ahc_check_cmdcmpltqueues: aic7xxx_core.c ahc_get_sense_bufaddr: aic7xxx_core.c ahc_hscb_busaddr: aic7xxx_core.c ahc_inq: aic7xxx_core.c ahc_outq: aic7xxx_core.c ahc_pause_bug_fix: aic7xxx_core.c ahc_sg_bus_to_virt: aic7xxx_core.c ahc_sg_virt_to_bus: aic7xxx_core.c ahc_swap_with_next_hscb: aic7xxx_core.c ahc_sync_qoutfifo: aic7xxx_core.c ahc_sync_scb: aic7xxx_core.c ahc_sync_tqinfifo: aic7xxx_core.c ahc_targetcmd_offset: aic7xxx_core.c ahc_update_residual: aic7xxx_core.c ahd_assert_modes: aic79xx_core.c ahd_check_cmdcmpltqueues: aic79xx_core.c ahd_get_hescb_qoff: aic79xx_core.c ahd_get_hnscb_qoff: aic79xx_core.c ahd_get_sdscb_qoff: aic79xx_core.c ahd_get_sescb_qoff: aic79xx_core.c ahd_get_snscb_qoff: aic79xx_core.c ahd_inl_scbram: aic79xx_core.c ahd_inq: aic79xx_core.c ahd_inq_scbram: aic79xx_core.c ahd_outq: aic79xx_core.c ahd_set_hescb_qoff: aic79xx_core.c ahd_set_hnscb_qoff: aic79xx_core.c ahd_set_sdscb_qoff: aic79xx_core.c ahd_set_sescb_qoff: aic79xx_core.c ahd_set_snscb_qoff: aic79xx_core.c ahd_setup_data_scb: aic79xx_core.c ahd_setup_noxfer_scb: aic79xx_core.c ahd_setup_scb_common: aic79xx_core.c ahd_sg_bus_to_virt: aic79xx_core.c ahd_sg_virt_to_bus: aic79xx_core.c ahd_swap_with_next_hscb: aic79xx_core.c ahd_sync_qoutfifo: aic79xx_core.c ahd_sync_scb: aic79xx_core.c ahd_sync_sense: aic79xx_core.c ahd_sync_tqinfifo: aic79xx_core.c ahd_targetcmd_offset: aic79xx_core.c ahd_update_modes: aic79xx_core.c Converting them to static may save additional 3 or 4k, if gcc will be smart enough... I, too, have many questions regarding this driver: why __inline? why u_int?... and who's maintainer, btw? Did you run the find static script or are you waiting for Adrian to do that ;-) $ find -name '*find*static*' $ -- vda - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] debloat aic7xxx and aic79xx drivers by deinlining
On Monday 27 August 2007 03:03, Adrian Bunk wrote: On Sun, Aug 26, 2007 at 04:21:30PM +0100, Denys Vlasenko wrote: On Saturday 25 August 2007 22:57, Arjan van de Ven wrote: ... Did you run the find static script or are you waiting for Adrian to do that ;-) $ find -name '*find*static*' $ make namespacecheck Thanks, nice tool. aic7xxx is kind of not very nice in this regard. See below what I get even on non-patched driver. I am willing to clean it up, but I still would like debloating patch to be accepted. Otherwise I'm left in the dark whether _any_ patches touching aic7xxx are ever looked at, or not. Okay, the list: Externally defined symbols with no external references drivers/scsi/aic7xxx/aic79xx_reg_print.o ahd_abrtbitptr_print ahd_abrtbyteptr_print ahd_accum_print ahd_accum_save_print ahd_ahd_pci_config_base_print ahd_allocfifo_scbptr_print ahd_allones_print ahd_allzeros_print ahd_annexcol_print ahd_annexdat_print ahd_arbctl_print ahd_arg_1_print ahd_arg_2_print ahd_attrptr_print ahd_brdctl_print ahd_brddat_print ahd_brkaddr0_print ahd_brkaddr1_print ahd_businitid_print ahd_bustargid_print ahd_ccscbacnt_print ahd_ccscbaddr_print ahd_ccscbadr_bk_print ahd_ccscbram_print ahd_ccsgaddr_print ahd_ccsgram_print ahd_cdblimit_print ahd_clrerr_print ahd_clrint_print ahd_clrlqiint0_print ahd_clrlqiint1_print ahd_clrlqoint0_print ahd_clrlqoint1_print ahd_clrseqintsrc_print ahd_clrseqintstat_print ahd_clrsint0_print ahd_clrsint1_print ahd_clrsint2_print ahd_clrsint3_print ahd_cmc_rambist_print ahd_cmcpcistat_print ahd_cmcrxmsg0_print ahd_cmcrxmsg1_print ahd_cmcrxmsg2_print ahd_cmcrxmsg3_print ahd_cmcseqbcnt_print ahd_cmcspltstat0_print ahd_cmcspltstat1_print ahd_cmdlenptr_print ahd_cmdptr_print ahd_cmdrsvd0_print ahd_cmds_pending_print ahd_cmdsize_table_print ahd_complete_dma_scb_head_print ahd_complete_dma_scb_tail_print ahd_complete_on_qfreeze_head_print ahd_complete_scb_dmainprog_head_print ahd_complete_scb_head_print ahd_crccontrol_print ahd_curaddr_print ahd_currscb_print ahd_data_count_odd_print ahd_datalenptr_print ahd_dchrxmsg0_print ahd_dchrxmsg1_print ahd_dchrxmsg2_print ahd_dchrxmsg3_print ahd_dchseqbcnt_print ahd_dchspltstat0_print ahd_dchspltstat1_print ahd_df0pcistat_print ahd_df1pcistat_print ahd_dfbcnt_print ahd_dfbkptr_print ahd_dfdat_print ahd_dfdbctl_print ahd_dff_thrsh_print ahd_dfftag_print ahd_dfptrs_print ahd_dfraddr_print ahd_dfscnt_print ahd_dfwaddr_print ahd_dgrpcrci_print ahd_dindex_print ahd_dindir_print ahd_dlcount_print ahd_dmaparams_print ahd_dscommand0_print ahd_dspackctl_print ahd_dspdatactl_print ahd_dspfltrctl_print ahd_dspreqctl_print ahd_dspselect_print ahd_error_print ahd_fairness_print ahd_flagptr_print ahd_flags_print ahd_flexadr_print ahd_flexcnt_print ahd_flexdata_print ahd_flexdmastat_print ahd_function1_print ahd_gsfifo_print ahd_haddr_print ahd_hcnt_print ahd_hcntrl_print ahd_hescb_qoff_print ahd_hnscb_qoff_print ahd_hodmaadr_print ahd_hodmacnt_print ahd_hodmaen_print ahd_idptr_print ahd_initiator_tag_print ahd_int_coalescing_cmdcount_print ahd_int_coalescing_maxcmds_print ahd_int_coalescing_mincmds_print ahd_int_coalescing_timer_print ahd_intvec1_addr_print ahd_intvec2_addr_print ahd_iopdnctl_print ahd_iownid_print ahd_kernel_tqinpos_print ahd_last_msg_print ahd_lastaddr_print ahd_lastscb_print ahd_local_hs_mailbox_print ahd_longjmp_addr_print ahd_lqctl0_print ahd_lqctl1_print ahd_lqctl2_print ahd_lqimode0_print ahd_lqimode1_print ahd_lqin_print ahd_lqistate_print ahd_lqomode0_print ahd_lqomode1_print ahd_lqoscsctl_print ahd_lqostate_print ahd_lqrsvd01_print ahd_lqrsvd16_print ahd_lqrsvd17_print ahd_lunlen_print ahd_lunptr_print ahd_maxcmd2rcv_print ahd_maxcmd_print ahd_maxcmdbytes_print ahd_maxcmdcnt_print ahd_mode_ptr_print ahd_msg_out_print ahd_msipcistat_print ahd_multargid_print ahd_negconopts_print ahd_negoaddr_print ahd_negoffset_print ahd_negperiod_print ahd_negppropts_print ahd_next_queued_scb_addr_print ahd_nextscb_print ahd_none_print ahd_nsenable_print ahd_optionmode_print ahd_os_space_cnt_print ahd_ost_print ahd_ovlyaddr_print ahd_ovlypcistat_print ahd_ovlyrxmsg0_print ahd_ovlyrxmsg1_print ahd_ovlyrxmsg2_print ahd_ovlyrxmsg3_print ahd_ovlyseqbcnt_print ahd_ovlyspltstat0_print ahd_ovlyspltstat1_print ahd_packcrci_print ahd_pcixctl_print
Re: [PATCH] debloat aic7xxx and aic79xx drivers by deinlining
On Tuesday 28 August 2007 16:17, Arjan van de Ven wrote: On Tue, 28 Aug 2007 12:56:34 +010 Denys Vlasenko [EMAIL PROTECTED] wrote: make namespacecheck Thanks, nice tool. aic7xxx is kind of not very nice in this regard. See below what I get even on non-patched driver. I am willing to clean it up, but I still would like debloating patch to be accepted. Fwiw I do like your debloat patch a lot; it's just only half the equation ... if you also do the namespace fixes, I bet the driver debloats even more... Yes, I know, and I am happy to do that too. I just don't know whether patches will be accepted. Why this patch is not commented on by scsi people? Am I sending patches to wrong people/lists? -- vda - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] debloat aic7xxx and aic79xx drivers
On Friday 31 August 2007 14:42, Hannes Reinecke wrote: Jan Engelhardt wrote: On Aug 30 2007 13:02, Matthew Wilcox wrote: Well, you can send it to Linus/Andrew, that will usually upset people and they start commenting on it. Or they don't, and everything is fine. (The default y approach so to speak ;-) The problem is that we don't really have a maintainer for the aic7xyz drivers any more. Volunteers welcome. NOT IT! Take it before someone else does! Well, the semi-official maintainers are James B. and me. So I might as well do it officially. Cool. Thanks to Arjan's insistence, I also took a look at adding statics and unexpectedly discovered yet another 50 kbytes of dead code (I'm not kidding). Attached are three patches which fix that: textdata bss dec hex filename 261433 500181172 312623 4c52f linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o 199654 500181172 250844 3d3dc linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o 184014 213141172 206500 326a4 linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o 20237828501172 206400 32640 linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o 1-debloat.patchdeinlines a lot of functions 2-addstatic.patch adds statics, #ifdefs out huge amount of unused code, adds consts 3-addconst.patch adds more consts make namespacecheck goes from 400+ functions to: drivers/scsi/aic7xxx/aic79xx_core.o ahd_inq ahd_inw ahd_outq ahd_outw drivers/scsi/aic7xxx/aic79xx_osm.o ahd_insb drivers/scsi/aic7xxx/aic7xxx_core.o ahc_inq ahc_outq drivers/scsi/aic7xxx/aic7xxx_osm.o ahc_insb None of these patches actually touch any logic, code changes are pretty minimal. Compile tested and applies cleanly to 2.6.23-rc1, applies with some fuzz to 2.6.23-rc3. Please propagate to mainline. -- vda - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] debloat aic7xxx and aic79xx drivers
On Friday 31 August 2007 16:13, Denys Vlasenko wrote: Attached are three patches which fix that: textdata bss dec hex filename 261433 500181172 312623 4c52f linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o 199654 500181172 250844 3d3dc linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o 184014 213141172 206500 326a4 linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o 20237828501172 206400 32640 linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o 1-debloat.patchdeinlines a lot of functions 2-addstatic.patch adds statics, #ifdefs out huge amount of unused code, adds consts 3-addconst.patch adds more consts -- vda linux-2.6.23-rc1-aic7xxx-1-debloat.patch.bz2 Description: BZip2 compressed data
[PATCH 2/3] debloat aic7xxx and aic79xx drivers
On Friday 31 August 2007 16:15, Denys Vlasenko wrote: On Friday 31 August 2007 16:13, Denys Vlasenko wrote: Attached are three patches which fix that: textdata bss dec hex filename 261433 500181172 312623 4c52f linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o 199654 500181172 250844 3d3dc linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o 184014 213141172 206500 326a4 linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o 20237828501172 206400 32640 linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o 1-debloat.patchdeinlines a lot of functions 2-addstatic.patch adds statics, #ifdefs out huge amount of unused code, adds consts 3-addconst.patch adds more consts -- vda linux-2.6.23-rc1-aic7xxx-2-addstatic.patch.bz2 Description: BZip2 compressed data
[PATCH 3/3] debloat aic7xxx and aic79xx drivers
On Friday 31 August 2007 16:16, Denys Vlasenko wrote: On Friday 31 August 2007 16:15, Denys Vlasenko wrote: On Friday 31 August 2007 16:13, Denys Vlasenko wrote: Attached are three patches which fix that: textdata bss dec hex filename 261433 500181172 312623 4c52f linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o 199654 500181172 250844 3d3dc linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o 184014 213141172 206500 326a4 linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o 20237828501172 206400 32640 linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o 1-debloat.patchdeinlines a lot of functions 2-addstatic.patch adds statics, #ifdefs out huge amount of unused code, adds consts 3-addconst.patch adds more consts -- vda linux-2.6.23-rc1-aic7xxx-3-addconst.patch.bz2 Description: BZip2 compressed data
Re: [PATCH 0/3] debloat aic7xxx and aic79xx drivers
On Friday 31 August 2007 17:27, [EMAIL PROTECTED] wrote: On Fri, 31 Aug 2007 16:13:59 BST, Denys Vlasenko said: textdata bss dec hex filename 261433 500181172 312623 4c52f linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o 199654 500181172 250844 3d3dc linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o 184014 213141172 206500 326a4 linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o 20237828501172 206400 32640 linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o 1-debloat.patchdeinlines a lot of functions 2-addstatic.patch adds statics, #ifdefs out huge amount of unused code, adds consts 3-addconst.patch adds more consts Yowza. Looking at aic1-aic2, it looks like 20K became 'const', and only 3K *wasn't* 'const'? Exactly. There are firmware images/patches and a lot of structures with char* members pointing to text/messages. Btw, aic94xx driver needs the same treatment. -- vda - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] bfa: deinline __bfa_trc, wwn2str and fcid2str
With this .config: http://busybox.net/~vda/kernel_config_ALLYES_Os,
after deinlining these functions have sizes and callsite counts
as follows:
__bfa_trc: 115 bytes, 1494 calls
wwn2str: 108 bytes, 27 calls
fcid2str: 46 bytes, 3 calls
__bfa_trc32 is similar to __bfa_trc, so it should have been
uninlined too. However, it is also unused.
Anil Gurumurthy suggested I can just delete it, I did so.
Change in code size is about 135,000 bytes:
text data bss dec hex filename
91470054 19945080 36421632 147836766 8cfcf5e vmlinux.before
91335078 19944984 36421632 147701694 8cdbfbe vmlinux
Signed-off-by: Denys Vlasenko <dvlas...@redhat.com>
CC: Krishna Gudipati <kgudi...@brocade.com>
CC: Vijaya Mohan Guvva <vmo...@brocade.com>
CC: Anil Gurumurthy <anil.gurumur...@qlogic.com>
CC: James Bottomley <jbottom...@parallels.com>
CC: Fabian Frederick <f...@skynet.be>
CC: Anil Gurumurthy <anil.gurumur...@qlogic.com>
CC: Christoph Hellwig <h...@lst.de>
CC: Guenter Roeck <li...@roeck-us.net>
CC: Ben Hutchings <b...@decadent.org.uk>
CC: James Bottomley <jbottom...@parallels.com>
CC: linux-ker...@vger.kernel.org
CC: linux-scsi@vger.kernel.org
---
drivers/scsi/bfa/bfa_core.c | 45 ++
drivers/scsi/bfa/bfa_cs.h | 67 +++--
2 files changed, 49 insertions(+), 63 deletions(-)
diff --git a/drivers/scsi/bfa/bfa_core.c b/drivers/scsi/bfa/bfa_core.c
index e3f67b0..fdf0f4e 100644
--- a/drivers/scsi/bfa/bfa_core.c
+++ b/drivers/scsi/bfa/bfa_core.c
@@ -90,6 +90,51 @@ static bfa_ioc_mbox_mcfunc_t bfa_mbox_isrs[BFI_MC_MAX] = {
+void
+wwn2str(char *wwn_str, u64 wwn)
+{
+ union {
+ u64 wwn;
+ u8 byte[8];
+ } w;
+
+ w.wwn = wwn;
+ sprintf(wwn_str, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", w.byte[0],
+ w.byte[1], w.byte[2], w.byte[3], w.byte[4], w.byte[5],
+ w.byte[6], w.byte[7]);
+}
+
+void
+fcid2str(char *fcid_str, u32 fcid)
+{
+ union {
+ u32 fcid;
+ u8 byte[4];
+ } f;
+
+ f.fcid = fcid;
+ sprintf(fcid_str, "%02x:%02x:%02x", f.byte[1], f.byte[2], f.byte[3]);
+}
+
+void
+__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data)
+{
+ int tail = trcm->tail;
+ struct bfa_trc_s*trc = >trc[tail];
+
+ if (trcm->stopped)
+ return;
+
+ trc->fileno = (u16) fileno;
+ trc->line = (u16) line;
+ trc->data.u64 = data;
+ trc->timestamp = BFA_TRC_TS(trcm);
+
+ trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
+ if (trcm->tail == trcm->head)
+ trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
+}
+
static void
bfa_com_port_attach(struct bfa_s *bfa)
{
diff --git a/drivers/scsi/bfa/bfa_cs.h b/drivers/scsi/bfa/bfa_cs.h
index 91a8aa3..3f00eab 100644
--- a/drivers/scsi/bfa/bfa_cs.h
+++ b/drivers/scsi/bfa/bfa_cs.h
@@ -107,44 +107,8 @@ bfa_trc_stop(struct bfa_trc_mod_s *trcm)
trcm->stopped = 1;
}
-static inline void
-__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data)
-{
- int tail = trcm->tail;
- struct bfa_trc_s*trc = >trc[tail];
-
- if (trcm->stopped)
- return;
-
- trc->fileno = (u16) fileno;
- trc->line = (u16) line;
- trc->data.u64 = data;
- trc->timestamp = BFA_TRC_TS(trcm);
-
- trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
- if (trcm->tail == trcm->head)
- trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
-}
-
-
-static inline void
-__bfa_trc32(struct bfa_trc_mod_s *trcm, int fileno, int line, u32 data)
-{
- int tail = trcm->tail;
- struct bfa_trc_s *trc = >trc[tail];
-
- if (trcm->stopped)
- return;
-
- trc->fileno = (u16) fileno;
- trc->line = (u16) line;
- trc->data.u32.u32 = data;
- trc->timestamp = BFA_TRC_TS(trcm);
-
- trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
- if (trcm->tail == trcm->head)
- trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
-}
+void
+__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data);
#define bfa_sm_fault(__mod, __event) do {\
bfa_trc(__mod, (((u32)0xDEAD << 16) | __event));\
@@ -324,31 +288,8 @@ bfa_wc_wait(struct bfa_wc_s *wc)
bfa_wc_down(wc);
}
-static inline void
-wwn2str(char *wwn_str, u64 wwn)
-{
- union {
- u64 wwn;
- u8 byte[8];
- } w;
-
- w.wwn = wwn;
- sprintf(wwn_str, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", w.byte[0],
- w.byte[1], w.byte[
[PATCH RESEND] bfa: deinline __bfa_trc() and __bfa_trc32()
__bfa_trc() compiles to 115 bytes of machine code.
With this .config: http://busybox.net/~vda/kernel_config
there are 1494 calls of __bfa_trc().
__bfa_trc32() is very similar, so it is uninlined too.
However, it appears to be unused, therefore this patch
ifdefs it out.
Change in code size is about 130,000 bytes:
text data bss dec hex filename
85975426 22294712 20627456 128897594 7aed23a vmlinux.before
85842882 22294584 20627456 128764922 7accbfa vmlinux
Signed-off-by: Denys Vlasenko <dvlas...@redhat.com>
Acked-by: Anil Gurumurthy <anil.gurumur...@qlogic.com>
CC: Fabian Frederick <f...@skynet.be>
CC: Anil Gurumurthy <anil.gurumur...@qlogic.com>
CC: Christoph Hellwig <h...@lst.de>
CC: Guenter Roeck <li...@roeck-us.net>
CC: Ben Hutchings <b...@decadent.org.uk>
CC: James Bottomley <jbottom...@parallels.com>
CC: linux-ker...@vger.kernel.org
CC: linux-scsi@vger.kernel.org
---
drivers/scsi/bfa/bfa_core.c | 40
drivers/scsi/bfa/bfa_cs.h | 41 -
2 files changed, 44 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/bfa/bfa_core.c b/drivers/scsi/bfa/bfa_core.c
index e3f67b0..3657a00 100644
--- a/drivers/scsi/bfa/bfa_core.c
+++ b/drivers/scsi/bfa/bfa_core.c
@@ -90,6 +90,46 @@ static bfa_ioc_mbox_mcfunc_t bfa_mbox_isrs[BFI_MC_MAX] = {
+void
+__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data)
+{
+ int tail = trcm->tail;
+ struct bfa_trc_s*trc = >trc[tail];
+
+ if (trcm->stopped)
+ return;
+
+ trc->fileno = (u16) fileno;
+ trc->line = (u16) line;
+ trc->data.u64 = data;
+ trc->timestamp = BFA_TRC_TS(trcm);
+
+ trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
+ if (trcm->tail == trcm->head)
+ trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
+}
+
+#if 0 /* UNUSED */
+void
+__bfa_trc32(struct bfa_trc_mod_s *trcm, int fileno, int line, u32 data)
+{
+ int tail = trcm->tail;
+ struct bfa_trc_s *trc = >trc[tail];
+
+ if (trcm->stopped)
+ return;
+
+ trc->fileno = (u16) fileno;
+ trc->line = (u16) line;
+ trc->data.u32.u32 = data;
+ trc->timestamp = BFA_TRC_TS(trcm);
+
+ trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
+ if (trcm->tail == trcm->head)
+ trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
+}
+#endif
+
static void
bfa_com_port_attach(struct bfa_s *bfa)
{
diff --git a/drivers/scsi/bfa/bfa_cs.h b/drivers/scsi/bfa/bfa_cs.h
index 91a8aa3..dd3154e 100644
--- a/drivers/scsi/bfa/bfa_cs.h
+++ b/drivers/scsi/bfa/bfa_cs.h
@@ -107,44 +107,11 @@ bfa_trc_stop(struct bfa_trc_mod_s *trcm)
trcm->stopped = 1;
}
-static inline void
-__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data)
-{
- int tail = trcm->tail;
- struct bfa_trc_s*trc = >trc[tail];
-
- if (trcm->stopped)
- return;
-
- trc->fileno = (u16) fileno;
- trc->line = (u16) line;
- trc->data.u64 = data;
- trc->timestamp = BFA_TRC_TS(trcm);
-
- trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
- if (trcm->tail == trcm->head)
- trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
-}
-
+void
+__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data);
-static inline void
-__bfa_trc32(struct bfa_trc_mod_s *trcm, int fileno, int line, u32 data)
-{
- int tail = trcm->tail;
- struct bfa_trc_s *trc = >trc[tail];
-
- if (trcm->stopped)
- return;
-
- trc->fileno = (u16) fileno;
- trc->line = (u16) line;
- trc->data.u32.u32 = data;
- trc->timestamp = BFA_TRC_TS(trcm);
-
- trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
- if (trcm->tail == trcm->head)
- trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
-}
+void
+__bfa_trc32(struct bfa_trc_mod_s *trcm, int fileno, int line, u32 data);
#define bfa_sm_fault(__mod, __event) do {\
bfa_trc(__mod, (((u32)0xDEAD << 16) | __event));\
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drivers/scsi/fnic/fnic_scsi.c: Deinline fnic_queue_abort_io_req, save 1792 bytes
This function compiles to 511 bytes of machine code.
Abort commands are not time-critical at all.
Signed-off-by: Denys Vlasenko <dvlas...@redhat.com>
CC: James Bottomley <james.bottom...@hansenpartnership.com>
CC: Hiral Patel <hiral...@cisco.com>
CC: Suma Ramars <sram...@cisco.com>
CC: Brian Uchino <buch...@cisco.com>
CC: linux-scsi@vger.kernel.org
CC: linux-ker...@vger.kernel.org
---
drivers/scsi/fnic/fnic_scsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 266b909..0a3edee 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -1435,7 +1435,7 @@ wq_copy_cleanup_scsi_cmd:
}
}
-static inline int fnic_queue_abort_io_req(struct fnic *fnic, int tag,
+static int fnic_queue_abort_io_req(struct fnic *fnic, int tag,
u32 task_req, u8 *fc_lun,
struct fnic_io_req *io_req)
{
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qla2xxx: rewrite code to avoid hitting gcc bug 70646
On 04/15/2016 04:40 PM, James Bottomley wrote:
> On Fri, 2016-04-15 at 12:36 +0200, Denys Vlasenko wrote:
>> More info here:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
>
> This bug is under investigation, so I'd rather not alter code for a gcc
> bug until we know if we can supply options to fix it rather than
> changing code.
Background. The bug exists in gcc for 2 years, but it is rather
hard to trigger, so nobody noticed.
Unfortunately for kernel, these two commits landed in Linus tree
in March 16 and 17:
On 04/13/2016 05:36 AM, Josh Poimboeuf wrote:
> It occurs with the combination of the following two recent commits:
>
> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some
> byteswap operations")
> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")
and now *many* users of qla2x00 and new-ish gcc are going to
very much notice it, as their kernels will start crashing reliably.
The commits can be reverted, sure, but they per se do not contain
anything unusual. They, together with not very typical construct
in qla2x00_get_host_fabric_name, one
which boils down to "swab64p(constant_array_of_8_bytes)",
just happen to nudge gcc in a right way to finally trigger the bug.
So I came with another idea how to forestall the imminent deluge of
qla2x00 oops reports - this patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qla2xxx: rewrite code to avoid hitting gcc bug 70646
On 04/15/2016 09:05 PM, James Bottomley wrote:
> On Fri, 2016-04-15 at 20:56 +0200, Denys Vlasenko wrote:
>> On 04/15/2016 04:40 PM, James Bottomley wrote:
>>> On Fri, 2016-04-15 at 12:36 +0200, Denys Vlasenko wrote:
>>>> More info here:
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
>>>
>>> This bug is under investigation, so I'd rather not alter code for a
>>> gcc
>>> bug until we know if we can supply options to fix it rather than
>>> changing code.
>>
>>
>> Background. The bug exists in gcc for 2 years, but it is rather
>> hard to trigger, so nobody noticed.
>
> We know this ... linux-scsi is on the cc for the other thread on this.
>
>> Unfortunately for kernel, these two commits landed in Linus tree
>> in March 16 and 17:
>>
>>
>> On 04/13/2016 05:36 AM, Josh Poimboeuf wrote:
>>> It occurs with the combination of the following two recent commits:
>>>
>>> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining
>>> of some byteswap operations")
>>> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")
>>
>>
>> and now *many* users of qla2x00 and new-ish gcc are going to
>> very much notice it, as their kernels will start crashing reliably.
>>
>> The commits can be reverted, sure, but they per se do not contain
>> anything unusual. They, together with not very typical construct
>> in qla2x00_get_host_fabric_name, one
>> which boils down to "swab64p(constant_array_of_8_bytes)",
>> just happen to nudge gcc in a right way to finally trigger the bug.
>>
>> So I came with another idea how to forestall the imminent deluge of
>> qla2x00 oops reports - this patch.
>
> There are actually a raft of checkers that run the upstream code which
> aren't seeing any problem; likely because the code is harder to trigger
> than you think. So, lets wait until the resolution of the other thread
> before we panic, especially since we're only at -rc3.
I'm not panicking, James.
By sending a workaround, I just want to make sure that *other people*
won't be forced to fix up a problem which surfaced because of *my* patch.
I'm afraid "harder to trigger than you think" is not true.
It is nearly trivial to trigger it now.
I just tried the following on a freshly installed Fedora 21 machine:
$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
$ cd linux
$ make defconfig
$ sed '/SCSI_FC_ATTRS/d;/SCSI_LOWLEVEL/d' -i .config
$ make oldconfig# answer "yes" to everything
$ nice make -j22
$ objdump -dr drivers/scsi/qla2xxx/qla_attr.o | grep -A10
qla2x00_get_host_fabric_name
1540 :
1540: 55 push %rbp
1541: 48 89 e5mov%rsp,%rbp
1544: 66 66 66 2e 0f 1f 84data32 data32 nopw %cs:0x0(%rax,%rax,1)
154b: 00 00 00 00 00
1550 :
1550: 55 push %rbp
1551: 48 89 e5mov%rsp,%rbp
1554: 53 push %rbx
1555: 48 89 d3mov%rdx,%rbx
See?
I'm sure Fedora 22, 23 and 24 will also do that.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] qla2xxx: rewrite code to avoid hitting gcc bug 70646
More info here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
Signed-off-by: Denys Vlasenko <dvlas...@redhat.com>
CC: Himanshu Madhani <himanshu.madh...@qlogic.com>
CC: James Bottomley <james.bottom...@hansenpartnership.com>
CC: qla2xxx-upstr...@qlogic.com
CC: Josh Poimboeuf <jpoim...@redhat.com>
CC: linux-scsi@vger.kernel.org
CC: linux-ker...@vger.kernel.org
---
drivers/scsi/qla2xxx/qla_attr.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 4dc06a13..2dd9c72 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -2003,9 +2003,14 @@ static void
qla2x00_get_host_fabric_name(struct Scsi_Host *shost)
{
scsi_qla_host_t *vha = shost_priv(shost);
+ /*
+* This can trigger gcc 4.9/5.3 bug.
+* See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
uint8_t node_name[WWN_SIZE] = { 0xFF, 0xFF, 0xFF, 0xFF, \
0xFF, 0xFF, 0xFF, 0xFF};
u64 fabric_name = wwn_to_u64(node_name);
+*/
+ u64 fabric_name = (u64)(s64)-1; /* the same as above */
if (vha->device_flags & SWITCH_FOUND)
fabric_name = wwn_to_u64(vha->fabric_node_name);
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On 04/13/2016 07:10 PM, Josh Poimboeuf wrote:
>> From the disassembly of drivers/scsi/qla2xxx/qla_attr.o:
>>
>> 2f53 :
>> 2f53: 55 push %rbp
>> 2f54: 48 89 e5mov%rsp,%rbp
>>
>> 2f57 :
>> 2f57: 55 push %rbp
>> 2f58: b9 e8 00 00 00 mov$0xe8,%ecx
>> 2f5d: 48 89 e5mov%rsp,%rbp
>> ...
>>
>> Note that qla2x00_get_host_fabric_name() is inexplicably
>> truncated after
>> setting up the frame pointer. It falls through to the next
>> function, which is
>> very wrong.
>
> Wow, that's ... interesting.
>
>
>> I can recreate it with either gcc 5.3.1 or gcc 6.0 on
>> linus/master with
>> the .config from the above link.
>>
>> The call chain which appears to trigger the problem is:
>>
>> qla2x00_get_host_fabric_name()
>> wwn_to_u64()
>> get_unaligned_be64()
>> be64_to_cpup()
>> __be64_to_cpup() <- changed to __always_inline by this
>> patch
>>
>> It occurs with the combination of the following two recent
>> commits:
>>
>> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force
>> inlining of some byteswap operations")
>> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn
>> access")
>>
>> I can confirm that reverting either patch makes the problem go
>> away.
>> I'm planning on opening a gcc bug tomorrow.
>
>
> Note that if CONFIG_OPTIMIZE_INLINING is not set, _all_ "inline"
> keywords are in fact __always_inline, so the bug must be
> triggering
> even without the patch.
Makes sense in theory, but the bug doesn't actually trigger when I
revert the patch and set CONFIG_OPTIMIZE_INLINING=n.
Perhaps even more surprising, it doesn't trigger *with* the patch
and
CONFIG_OPTIMIZE_INLINING=n.
>>>
>>> [ Adding James to CC since this bug affects scsi. ]
>>>
>>> Here's the gcc bug:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
>>>
>>
>>
>> Actually, adding me doesn't help, I've added linux-scsi. The summary
>> is that there's a but in gcc-5.3.1 which is miscompiling qla_attr.c ...
>> this means we're going to have to ask the compiler version of reported
>> crashes.
>
> The bug isn't specific to a compiler version. I've seen it with gcc
> 5.3.1 and gcc 6.0. I haven't tried any older versions. And the gcc bug
> hasn't been resolved (or even investigated) yet.
>
> The bug is triggered by a combination of the above two commits from the
> 4.6 merge window, so presumably we'd need to revert one of them to avoid
> crashes in 4.6.
The bug is indeed in the compiler. 4.9 and all later versions are affected.
gcc bugzilla now has a reproducer. In abridged form:
static inline __attribute__((always_inline)) u64 __swab64p(const u64 *p)
{
return (__builtin_constant_p((u64)(*p)) ? ((u64)( (((u64)(*p) &
(u64)0x00ffULL) << 56) | (((u64)(*p) & (u64)0xff00ULL)
<< 40) | (((u64)(*p) & (u64)0x00ffULL) << 24) | (((u64)(*p) &
(u64)0xff00ULL) << 8) | (((u64)(*p) & (u64)0x00ffULL)
>> 8) | (((u64)(*p) & (u64)0xff00ULL) >> 24) | (((u64)(*p) &
(u64)0x00ffULL) >> 40) | (((u64)(*p) & (u64)0xff00ULL)
>> 56))) : __builtin_bswap64(*p));
}
static inline u64 wwn_to_u64(void *wwn)
{
return __swab64p(wwn);
}
static void qla2x00_get_host_fabric_name(struct Scsi_Host *shost)
{
scsi_qla_host_t *vha = shost_priv(shost);
u8 node_name[8] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
u64 fabric_name = wwn_to_u64(node_name);
if (vha->device_flags & 0x1)
fabric_name = wwn_to_u64(vha->fabric_node_name);
(((struct fc_host_attrs *)(shost)->shost_data)->fabric_name) = fabric_name;
}
Two (or more, there were more before simplification) levels of inlining
are necessary for bug to trigger in this example (folding to one level
makes it go away). "__attribute__((always_inline))" is necessary too.
Since we have lots of __always_inline anyway, this bug has a potential
to miscompile kernels regardless of CONFIG_OPTIMIZE_INLINING setting,
and with or without the patches mentioned above (they just happen
to create a reliable reproducer).
Since it was not detected for two years since gcc 4.9 release,
it must be triggering quite rarely.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: aic7xxx: fix ahc_delay and ahd_delay
They are buggy:
while (usec > 0)
udelay(usec % 1024);
usec -= 1024;
For example, for usec = 100*1024 + 1, old code will udelay(1) 101 times,
i.e. it will be approximately equivalent to udelay(101),
not the expected udelay(102400).
This did not bite because all callers use values far from "pathological" ones,
such as 500 and 1000 - these work fine with buggy code.
This was reported in 2006 but was missed.
Signed-off-by: Denys Vlasenko <dvlas...@redhat.com>
CC: James Bottomley <jbottom...@parallels.com>
CC: Hannes Reinicke <h...@suse.de>
CC: linux-scsi@vger.kernel.org
CC: linux-ker...@vger.kernel.org
---
drivers/scsi/aic7xxx/aic79xx_osm.c | 7 ---
drivers/scsi/aic7xxx/aic7xxx_osm.c | 7 ---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c
b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 2588b8f..e7a7838 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -380,9 +380,10 @@ ahd_delay(long usec)
* multi-millisecond waits. Wait at most
* 1024us per call.
*/
- while (usec > 0) {
- udelay(usec % 1024);
- usec -= 1024;
+ udelay(usec & 1023);
+ usec >>= 10;
+ while (--usec >= 0) {
+ udelay(1024);
}
}
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c
b/drivers/scsi/aic7xxx/aic7xxx_osm.c
index fc6a831..c81798e 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.c
@@ -388,9 +388,10 @@ ahc_delay(long usec)
* multi-millisecond waits. Wait at most
* 1024us per call.
*/
- while (usec > 0) {
- udelay(usec % 1024);
- usec -= 1024;
+ udelay(usec & 1023);
+ usec >>= 10;
+ while (--usec >= 0) {
+ udelay(1024);
}
}
--
2.9.2
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
