Re: [PATCH][SCSI]: Save some bytes in struct scsi_target
On Mon, Dec 11, 2006 at 08:52:22PM -0700, Matthew Wilcox wrote: > On Tue, Dec 12, 2006 at 01:17:18AM -0200, Arnaldo Carvalho de Melo wrote: > > }; /* size: 368, cachelines: 12 */ > > }; /* size: 364, cachelines: 12 */ > > Saving space is always good ;-) > > > - unsigned intcreate:1; /* signal that it needs to be added */ > > + charscsi_level; > > + unsigned char create:1; /* signal that it needs to be added */ > > unsigned intpdt_1f_for_no_lun; /* PDT = 0x1f */ > > /* means no lun present */ > > > > - charscsi_level; > > However, pdt_1f_for_no_lun is really only one bit, saving another 4 bytes. > > > struct execute_work ew; > > enum scsi_target_state state; > > enums are a bit of a pain. Even though scsi_target_state uses only two > values, it's represented as an int. Unless you're on arm-eabi, when > it'll use less. And even then, it won't use less than a byte, as it has > to be addressable. I wonder if we can turn scsi_target_state into a > bit. That'll save another 8 bytes total. I guess we could use: enum scsi_target_state state:1; And make the enum entries start with 0 and not 1 as is today, no? With that we get down to: }; /* size: 356, cachelines: 12 */ /* last cacheline: 4 bytes */ Anything else to save these 4 bytes and get down to 11 cachelines per scsi_target instance? Following patch is on top of the previous one. Signed-off-by: Arnaldo Carvalho de Melo <[EMAIL PROTECTED]> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index ab245fc..772f834 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -157,7 +157,7 @@ #define scmd_printk(prefix, scmd, fmt, a dev_printk(prefix, &(scmd)->device->sdev_gendev, fmt, ##a) enum scsi_target_state { - STARGET_RUNNING = 1, + STARGET_RUNNING = 0, STARGET_DEL, }; @@ -176,12 +176,12 @@ struct scsi_target { unsigned intid; /* target id ... replace * scsi_device.id eventually */ charscsi_level; + enum scsi_target_state state:1; unsigned char create:1; /* signal that it needs to be added */ - unsigned intpdt_1f_for_no_lun; /* PDT = 0x1f */ + unsigned char pdt_1f_for_no_lun:1;/* PDT = 0x1f */ /* means no lun present */ struct execute_work ew; - enum scsi_target_state state; void*hostdata; /* available to low-level driver */ unsigned long starget_data[0]; /* for the transport */ /* starget_data must be the last element */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][SCSI]: Save some bytes in struct scsi_target
On Mon, Dec 11, 2006 at 08:52:22PM -0700, Matthew Wilcox wrote: On Tue, Dec 12, 2006 at 01:17:18AM -0200, Arnaldo Carvalho de Melo wrote: }; /* size: 368, cachelines: 12 */ }; /* size: 364, cachelines: 12 */ Saving space is always good ;-) - unsigned intcreate:1; /* signal that it needs to be added */ + charscsi_level; + unsigned char create:1; /* signal that it needs to be added */ unsigned intpdt_1f_for_no_lun; /* PDT = 0x1f */ /* means no lun present */ - charscsi_level; However, pdt_1f_for_no_lun is really only one bit, saving another 4 bytes. struct execute_work ew; enum scsi_target_state state; enums are a bit of a pain. Even though scsi_target_state uses only two values, it's represented as an int. Unless you're on arm-eabi, when it'll use less. And even then, it won't use less than a byte, as it has to be addressable. I wonder if we can turn scsi_target_state into a bit. That'll save another 8 bytes total. I guess we could use: enum scsi_target_state state:1; And make the enum entries start with 0 and not 1 as is today, no? With that we get down to: }; /* size: 356, cachelines: 12 */ /* last cacheline: 4 bytes */ Anything else to save these 4 bytes and get down to 11 cachelines per scsi_target instance? Following patch is on top of the previous one. Signed-off-by: Arnaldo Carvalho de Melo [EMAIL PROTECTED] diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index ab245fc..772f834 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -157,7 +157,7 @@ #define scmd_printk(prefix, scmd, fmt, a dev_printk(prefix, (scmd)-device-sdev_gendev, fmt, ##a) enum scsi_target_state { - STARGET_RUNNING = 1, + STARGET_RUNNING = 0, STARGET_DEL, }; @@ -176,12 +176,12 @@ struct scsi_target { unsigned intid; /* target id ... replace * scsi_device.id eventually */ charscsi_level; + enum scsi_target_state state:1; unsigned char create:1; /* signal that it needs to be added */ - unsigned intpdt_1f_for_no_lun; /* PDT = 0x1f */ + unsigned char pdt_1f_for_no_lun:1;/* PDT = 0x1f */ /* means no lun present */ struct execute_work ew; - enum scsi_target_state state; void*hostdata; /* available to low-level driver */ unsigned long starget_data[0]; /* for the transport */ /* starget_data must be the last element */ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][SCSI]: Save some bytes in struct scsi_target
On Tue, Dec 12, 2006 at 01:17:18AM -0200, Arnaldo Carvalho de Melo wrote: > }; /* size: 368, cachelines: 12 */ > }; /* size: 364, cachelines: 12 */ Saving space is always good ;-) > - unsigned intcreate:1; /* signal that it needs to be added */ > + charscsi_level; > + unsigned char create:1; /* signal that it needs to be added */ > unsigned intpdt_1f_for_no_lun; /* PDT = 0x1f */ > /* means no lun present */ > > - charscsi_level; However, pdt_1f_for_no_lun is really only one bit, saving another 4 bytes. > struct execute_work ew; > enum scsi_target_state state; enums are a bit of a pain. Even though scsi_target_state uses only two values, it's represented as an int. Unless you're on arm-eabi, when it'll use less. And even then, it won't use less than a byte, as it has to be addressable. I wonder if we can turn scsi_target_state into a bit. That'll save another 8 bytes total. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH][SCSI]: Save some bytes in struct scsi_target
Before: [EMAIL PROTECTED] kpahole-2.6]$ pahole --cacheline 32 /tmp/scsi.o.before scsi_target /* include/scsi/scsi_device.h:86 */ struct scsi_target { struct scsi_device * starget_sdev_user;/* 0 4 */ struct list_head siblings; /* 4 8 */ struct list_head devices; /*12 8 */ struct device dev; /*20 300 */ /* --- cacheline 10 boundary (320 bytes) --- */ unsigned int reap_ref; /* 320 4 */ unsigned int channel; /* 324 4 */ unsigned int id; /* 328 4 */ unsigned int create:1; /* 332 4 */ /* XXX 31 bits hole, try to pack */ unsigned int pdt_1f_for_no_lun;/* 336 4 */ char scsi_level; /* 340 1 */ /* XXX 3 bytes hole, try to pack */ struct execute_workew; /* 34416 */ /* --- cacheline 11 boundary (352 bytes) was 8 bytes ago --- */ enum scsi_target_state state;/* 360 4 */ void * hostdata; /* 364 4 */ long unsigned int starget_data[0]; /* 368 0 */ }; /* size: 368, cachelines: 12 */ /* sum members: 365, holes: 1, sum holes: 3 */ /* bit holes: 1, sum bit holes: 31 bits */ /* last cacheline: 16 bytes */ After: [EMAIL PROTECTED] kpahole-2.6]$ pahole --cacheline 32 drivers/scsi/scsi.o scsi_target /* include/scsi/scsi_device.h:86 */ struct scsi_target { struct scsi_device * starget_sdev_user;/* 0 4 */ struct list_head siblings; /* 4 8 */ struct list_head devices; /*12 8 */ struct device dev; /*20 300 */ /* --- cacheline 10 boundary (320 bytes) --- */ unsigned int reap_ref; /* 320 4 */ unsigned int channel; /* 324 4 */ unsigned int id; /* 328 4 */ char scsi_level; /* 332 1 */ unsigned char create:1; /* 333 1 */ /* XXX 7 bits hole, try to pack */ /* XXX 2 bytes hole, try to pack */ unsigned int pdt_1f_for_no_lun;/* 336 4 */ struct execute_workew; /* 34016 */ /* --- cacheline 11 boundary (352 bytes) was 4 bytes ago --- */ enum scsi_target_state state;/* 356 4 */ void * hostdata; /* 360 4 */ long unsigned int starget_data[0]; /* 364 0 */ }; /* size: 364, cachelines: 12 */ /* sum members: 362, holes: 1, sum holes: 2 */ /* bit holes: 1, sum bit holes: 7 bits */ /* last cacheline: 12 bytes */ [EMAIL PROTECTED] kpahole-2.6]$ codiff -V /tmp/scsi.o.before drivers/scsi/scsi.o drivers/scsi/scsi.c: struct scsi_target | -4 create:1; from: unsigned int /* 332(31)4(1) */ to: unsigned char /* 333(7) 1(1) */ scsi_level; from: char /* 340(0) 1(0) */ to: char /* 332(0) 1(0) */ 1 struct changed Signed-off-by: Arnaldo Carvalho de Melo <[EMAIL PROTECTED]> --- scsi_device.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index ebf31b1..ab245fc 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -175,11 +175,11 @@ struct scsi_target { unsigned intchannel; unsigned intid; /* target id ... replace * scsi_device.id eventually */ - unsigned intcreate:1; /* signal that it needs to be added */ + charscsi_level; + unsigned char create:1; /* signal that it needs to be added */ unsigned intpdt_1f_for_no_lun; /* PDT = 0x1f */ /* means no lun present */ - charscsi_level; struct execute_work ew; enum scsi_target_state state; void*hostdata; /* available to low-level driver */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH][SCSI]: Save some bytes in struct scsi_target
Before: [EMAIL PROTECTED] kpahole-2.6]$ pahole --cacheline 32 /tmp/scsi.o.before scsi_target /* include/scsi/scsi_device.h:86 */ struct scsi_target { struct scsi_device * starget_sdev_user;/* 0 4 */ struct list_head siblings; /* 4 8 */ struct list_head devices; /*12 8 */ struct device dev; /*20 300 */ /* --- cacheline 10 boundary (320 bytes) --- */ unsigned int reap_ref; /* 320 4 */ unsigned int channel; /* 324 4 */ unsigned int id; /* 328 4 */ unsigned int create:1; /* 332 4 */ /* XXX 31 bits hole, try to pack */ unsigned int pdt_1f_for_no_lun;/* 336 4 */ char scsi_level; /* 340 1 */ /* XXX 3 bytes hole, try to pack */ struct execute_workew; /* 34416 */ /* --- cacheline 11 boundary (352 bytes) was 8 bytes ago --- */ enum scsi_target_state state;/* 360 4 */ void * hostdata; /* 364 4 */ long unsigned int starget_data[0]; /* 368 0 */ }; /* size: 368, cachelines: 12 */ /* sum members: 365, holes: 1, sum holes: 3 */ /* bit holes: 1, sum bit holes: 31 bits */ /* last cacheline: 16 bytes */ After: [EMAIL PROTECTED] kpahole-2.6]$ pahole --cacheline 32 drivers/scsi/scsi.o scsi_target /* include/scsi/scsi_device.h:86 */ struct scsi_target { struct scsi_device * starget_sdev_user;/* 0 4 */ struct list_head siblings; /* 4 8 */ struct list_head devices; /*12 8 */ struct device dev; /*20 300 */ /* --- cacheline 10 boundary (320 bytes) --- */ unsigned int reap_ref; /* 320 4 */ unsigned int channel; /* 324 4 */ unsigned int id; /* 328 4 */ char scsi_level; /* 332 1 */ unsigned char create:1; /* 333 1 */ /* XXX 7 bits hole, try to pack */ /* XXX 2 bytes hole, try to pack */ unsigned int pdt_1f_for_no_lun;/* 336 4 */ struct execute_workew; /* 34016 */ /* --- cacheline 11 boundary (352 bytes) was 4 bytes ago --- */ enum scsi_target_state state;/* 356 4 */ void * hostdata; /* 360 4 */ long unsigned int starget_data[0]; /* 364 0 */ }; /* size: 364, cachelines: 12 */ /* sum members: 362, holes: 1, sum holes: 2 */ /* bit holes: 1, sum bit holes: 7 bits */ /* last cacheline: 12 bytes */ [EMAIL PROTECTED] kpahole-2.6]$ codiff -V /tmp/scsi.o.before drivers/scsi/scsi.o drivers/scsi/scsi.c: struct scsi_target | -4 create:1; from: unsigned int /* 332(31)4(1) */ to: unsigned char /* 333(7) 1(1) */ scsi_level; from: char /* 340(0) 1(0) */ to: char /* 332(0) 1(0) */ SNIP offset changes 1 struct changed Signed-off-by: Arnaldo Carvalho de Melo [EMAIL PROTECTED] --- scsi_device.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index ebf31b1..ab245fc 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -175,11 +175,11 @@ struct scsi_target { unsigned intchannel; unsigned intid; /* target id ... replace * scsi_device.id eventually */ - unsigned intcreate:1; /* signal that it needs to be added */ + charscsi_level; + unsigned char create:1; /* signal that it needs to be added */ unsigned intpdt_1f_for_no_lun; /* PDT = 0x1f */ /* means no lun present */ - charscsi_level; struct execute_work ew; enum scsi_target_state state; void*hostdata; /* available to low-level driver */ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][SCSI]: Save some bytes in struct scsi_target
On Tue, Dec 12, 2006 at 01:17:18AM -0200, Arnaldo Carvalho de Melo wrote: }; /* size: 368, cachelines: 12 */ }; /* size: 364, cachelines: 12 */ Saving space is always good ;-) - unsigned intcreate:1; /* signal that it needs to be added */ + charscsi_level; + unsigned char create:1; /* signal that it needs to be added */ unsigned intpdt_1f_for_no_lun; /* PDT = 0x1f */ /* means no lun present */ - charscsi_level; However, pdt_1f_for_no_lun is really only one bit, saving another 4 bytes. struct execute_work ew; enum scsi_target_state state; enums are a bit of a pain. Even though scsi_target_state uses only two values, it's represented as an int. Unless you're on arm-eabi, when it'll use less. And even then, it won't use less than a byte, as it has to be addressable. I wonder if we can turn scsi_target_state into a bit. That'll save another 8 bytes total. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/