Allocate CCBs using UMA instead of malloc(9). CCBs are 1248 bytes
long; using malloc(9) wastes 800 bytes on each one.
The target bits probably should get their own review; I'm not sure
if they are actually required for this to work, though.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 34245 Build 31389: arc lint + arc unit
Event Timeline
| sys/cam/scsi/scsi_target.c | ||
|---|---|---|
| 939 | It seems here was a trick to save memory allocating specifically each CCB type. If you are removing this, then at least previous two lines should be removed also. | |
Comment Actions
I'm out and about today... but I've done this as well and other CAM structures to boot. Let's coordinate.
Comment Actions
There's nothing wrong with this work, but it doesn't solve the real problem that CCBs have grown too large, and that the one-size-fits-all union concept for them doesn't scale. The wasted 800 bytes is not even the biggest problem; a lot more is wasted within the union size of the CCB for regular I/O. This scheme needs to be replaced with variable-sized CCBs at a minimum, and the whole CCB concept needs to be analyzed and re-designed for high IOP workloads. So with that said, I won't block this review, but I urge people to think about these problems and don't stop with just this patch.
Comment Actions
I fully agree about the UMA alone not being enough to fix the big picture; the patch is just the first step. The second step I had planned was to split ccb_getdev; this should make CCBs equal in size to ccb_scsiio. FWIW, on 13.0-CURRENT GENERIC:
(kgdb) p sizeof(struct ccb_scsiio) $1 = 544 (kgdb) p sizeof(struct ccb_getdevlist) $2 = 232 (kgdb) p sizeof(struct ccb_getdev) $3 = 1248 (kgdb) p sizeof(struct ccb_pathinq) $4 = 464 (kgdb) p sizeof(struct ccb_relsim) $5 = 216 (kgdb) p sizeof(struct ccb_setasync) $6 = 224 (kgdb) p sizeof(struct ccb_setdev) $7 = 208 (kgdb) p sizeof(struct ccb_pathstats) $8 = 216 (kgdb) p sizeof(struct ccb_getdevstats) $9 = 248 (kgdb) p sizeof(struct ccb_dev_match) $10 = 304 (kgdb) p sizeof(struct ccb_trans_settings) $11 = 312 (kgdb) p sizeof(struct ccb_calc_geometry) $12 = 224 (kgdb) p sizeof(struct ccb_sim_knob) [0/48] $13 = 328 (kgdb) p sizeof(struct ccb_abort) $14 = 208 (kgdb) p sizeof(struct ccb_resetbus) $15 = 200 (kgdb) p sizeof(struct ccb_resetdev) $16 = 200 (kgdb) p sizeof(struct ccb_termio) $17 = 208 (kgdb) p sizeof(struct ccb_accept_tio) $18 = 480 (kgdb) p sizeof(struct ccb_en_lun) $19 = 208 (kgdb) p sizeof(struct ccb_immed_notify) $20 = 464 (kgdb) p sizeof(struct ccb_notify_ack) $21 = 208 (kgdb) p sizeof(struct ccb_immediate_notify) $22 = 216 (kgdb) p sizeof(struct ccb_notify_acknowledge) $23 = 216 (kgdb) p sizeof(struct ccb_eng_inq) $24 = 216 (kgdb) p sizeof(struct ccb_eng_exec) $25 = 264 (kgdb) p sizeof(struct ccb_smpio) $26 = 240 (kgdb) p sizeof(struct ccb_rescan) $27 = 208 (kgdb) p sizeof(struct ccb_debug) $28 = 208 (kgdb) p sizeof(struct ccb_ataio) $29 = 272 (kgdb) p sizeof(struct ccb_dev_advinfo) $30 = 232 (kgdb) p sizeof(struct ccb_async) $31 = 224 (kgdb) p sizeof(struct ccb_nvmeio) $32 = 304 (kgdb) p sizeof(struct ccb_mmcio) $33 = 320
Comment Actions
Yea, getdev has gotten out of hand... It has all of ATA IDENTIFY, SCSI INQUIRY and MMC data...
And that's not to mention the different types of lists can be on (which I think is driving the ~200byte minimum size).
What's always stopped me from diving in deeply here is compatibility with the past: I never wanted to bite of that issue.
I do like the idea of moving to have more 'wrapped' CCB functions so the actual format is somewhat abstracted, but the kernel/userland interface is still troublesome. Making good progress is tricky... I kinda think that a clean break is needed. What can we do with a fixed-size 128byte redesigned CCB?
https://reviews.freebsd.org/D26873
is what I've done, but your CCB stuff appears to be more complete than what I've done. I did it mostly so I can find CCBs in crash dumps, since finding the data for the kegs is easier than finding them scattered in memory.
Comment Actions
Status update: let's hold off with this; I'm investigating the possibility (or workability, rather) of allocating smaller CCBs for selected few types (scsiio, nvmeio etc), and this would get in the way.