From 59019ae40597dc3986688f81c734625323e83d93 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Thu, 1 Dec 2011 03:36:15 +0000 Subject: [PATCH] Use get/put_unaligned() instead of open coding these such that the compiler can generate better code. As an example, the get_unaligned_be24() function used in the implementation of READ_6, WRITE_6 and VERIFY_6 together with "& 0x1f0000" is inlined by the compiler and is translated as follows on an x86_64 system (2031616 equals 0x1f0000): movl 0(%r13), %r11d bswapl %r11d andl $2031616, %r11d Also eliminate a conditional branch instruction from get_trans_len_1_256(). BSD-Signed-off-by: Bart Van Assche git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@3943 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/include/scst.h | 14 ++++ scst/src/dev_handlers/scst_vdisk.c | 102 ++++++----------------------- scst/src/scst_lib.c | 68 ++++++------------- scst/src/scst_targ.c | 5 +- 4 files changed, 57 insertions(+), 132 deletions(-) diff --git a/scst/include/scst.h b/scst/include/scst.h index 5daaa8fcb..81637d92c 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -34,6 +34,7 @@ #ifdef CONFIG_SCST_MEASURE_LATENCY #include #endif +#include /* #define CONFIG_SCST_PROC */ @@ -4189,6 +4190,19 @@ if (!(condition)) { \ finish_wait(&(wq), &__wait); \ } +/* Only use get_unaligned_be24() if reading p - 1 is allowed. */ +static inline uint32_t get_unaligned_be24(const uint8_t *const p) +{ + return get_unaligned_be32(p - 1) & 0xffffffU; +} + +static inline void put_unaligned_be24(const uint32_t v, uint8_t *const p) +{ + p[0] = v >> 16; + p[1] = v >> 8; + p[2] = v >> 0; +} + #ifndef CONFIG_SCST_PROC /* diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index dcce52b3e..864f71181 100644 --- a/scst/src/dev_handlers/scst_vdisk.c +++ b/scst/src/dev_handlers/scst_vdisk.c @@ -93,7 +93,6 @@ static struct scst_trace_log vdisk_local_trace_tbl[] = { #define SP 0x01 /* save pages */ #define PS 0x80 /* parameter saveable */ -#define BYTE 8 #define DEF_DISK_BLOCKSIZE_SHIFT 9 #define DEF_DISK_BLOCKSIZE (1 << DEF_DISK_BLOCKSIZE_SHIFT) #define DEF_CDROM_BLOCKSIZE_SHIFT 11 @@ -988,9 +987,7 @@ static int vdisk_do_job(struct scst_cmd *cmd) case READ_6: case WRITE_6: case VERIFY_6: - lba_start = (((cdb[1] & 0x1f) << (BYTE * 2)) + - (cdb[2] << (BYTE * 1)) + - (cdb[3] << (BYTE * 0))); + lba_start = get_unaligned_be24(&cdb[1]) & 0x1f0000U; data_len = cmd->bufflen; break; case READ_10: @@ -1001,33 +998,19 @@ static int vdisk_do_job(struct scst_cmd *cmd) case WRITE_VERIFY: case WRITE_VERIFY_12: case VERIFY_12: - lba_start |= ((u64)cdb[2]) << 24; - lba_start |= ((u64)cdb[3]) << 16; - lba_start |= ((u64)cdb[4]) << 8; - lba_start |= ((u64)cdb[5]); + lba_start = get_unaligned_be32(&cdb[2]); data_len = cmd->bufflen; break; case READ_16: case WRITE_16: case WRITE_VERIFY_16: case VERIFY_16: - lba_start |= ((u64)cdb[2]) << 56; - lba_start |= ((u64)cdb[3]) << 48; - lba_start |= ((u64)cdb[4]) << 40; - lba_start |= ((u64)cdb[5]) << 32; - lba_start |= ((u64)cdb[6]) << 24; - lba_start |= ((u64)cdb[7]) << 16; - lba_start |= ((u64)cdb[8]) << 8; - lba_start |= ((u64)cdb[9]); + lba_start = get_unaligned_be64(&cdb[2]); data_len = cmd->bufflen; break; case SYNCHRONIZE_CACHE: - lba_start |= ((u64)cdb[2]) << 24; - lba_start |= ((u64)cdb[3]) << 16; - lba_start |= ((u64)cdb[4]) << 8; - lba_start |= ((u64)cdb[5]); - data_len = ((cdb[7] << (BYTE * 1)) + (cdb[8] << (BYTE * 0))) - << virt_dev->block_shift; + lba_start = get_unaligned_be32(&cdb[2]); + data_len = get_unaligned_be16(&cdb[7]) << virt_dev->block_shift; if (data_len == 0) data_len = virt_dev->file_size - ((loff_t)lba_start << virt_dev->block_shift); @@ -1329,10 +1312,10 @@ static void vdisk_exec_unmap(struct scst_cmd *cmd, struct scst_vdisk_thr *thr) inode = fd->f_dentry->d_inode; - total_len = cmd->cdb[7] << 8 | cmd->cdb[8]; /* length */ + total_len = get_unaligned_be16(&cmd->cdb[7]); /* length */ offset = 8; - descriptor_len = address[2] << 8 | address[3]; + descriptor_len = get_unaligned_be16(&address[2]); TRACE_DBG("total_len %d, descriptor_len %d", total_len, descriptor_len); @@ -1627,8 +1610,7 @@ static void vdisk_exec_inquiry(struct scst_cmd *cmd) num += buf[num + 3]; resp_len = num; - buf[2] = (resp_len >> 8) & 0xFF; - buf[3] = resp_len & 0xFF; + put_unaligned_be16(resp_len, &buf[2]); resp_len += 4; } else if ((0xB0 == cmd->cdb[2]) && (virt_dev->dev->type == TYPE_DISK)) { @@ -1924,10 +1906,8 @@ static int vdisk_format_pg(unsigned char *p, int pcontrol, 0, 0, 0, 0, 0x40, 0, 0, 0}; memcpy(p, format_pg, sizeof(format_pg)); - p[10] = (DEF_SECTORS >> 8) & 0xff; - p[11] = DEF_SECTORS & 0xff; - p[12] = (virt_dev->block_size >> 8) & 0xff; - p[13] = virt_dev->block_size & 0xff; + put_unaligned_be16(DEF_SECTORS, &p[10]); + put_unaligned_be16(virt_dev->block_size, &p[12]); if (1 == pcontrol) memset(p + 2, 0, sizeof(format_pg) - 2); return sizeof(format_pg); @@ -2076,22 +2056,10 @@ static void vdisk_exec_mode_sense(struct scst_cmd *cmd) if (!dbd) { /* Create block descriptor */ buf[offset - 1] = 0x08; /* block descriptor length */ - if (nblocks >> 32) { - buf[offset + 0] = 0xFF; - buf[offset + 1] = 0xFF; - buf[offset + 2] = 0xFF; - buf[offset + 3] = 0xFF; - } else { - /* num blks */ - buf[offset + 0] = (nblocks >> (BYTE * 3)) & 0xFF; - buf[offset + 1] = (nblocks >> (BYTE * 2)) & 0xFF; - buf[offset + 2] = (nblocks >> (BYTE * 1)) & 0xFF; - buf[offset + 3] = (nblocks >> (BYTE * 0)) & 0xFF; - } + put_unaligned_be32(nblocks >> 32 ? 0xffffffffU : nblocks, + &buf[offset]); buf[offset + 4] = 0; /* density code */ - buf[offset + 5] = (blocksize >> (BYTE * 2)) & 0xFF;/* blklen */ - buf[offset + 6] = (blocksize >> (BYTE * 1)) & 0xFF; - buf[offset + 7] = (blocksize >> (BYTE * 0)) & 0xFF; + put_unaligned_be24(blocksize, &buf[offset + 5]); /* blklen */ offset += 8; /* increment offset */ } @@ -2140,10 +2108,8 @@ static void vdisk_exec_mode_sense(struct scst_cmd *cmd) if (msense_6) buf[0] = offset - 1; - else { - buf[0] = ((offset - 2) >> 8) & 0xff; - buf[1] = (offset - 2) & 0xff; - } + else + put_unaligned_be16(offset - 2, &buf[0]); if (offset > length) offset = length; @@ -2353,21 +2319,9 @@ static void vdisk_exec_read_capacity(struct scst_cmd *cmd) * issues a READ_CAPACITY(16) so we can return the TPE bit. By * returning 0xFFFFFFFF we do that. */ - if (nblocks >> 32 || virt_dev->thin_provisioned) { - buffer[0] = 0xFF; - buffer[1] = 0xFF; - buffer[2] = 0xFF; - buffer[3] = 0xFF; - } else { - buffer[0] = ((nblocks - 1) >> (BYTE * 3)) & 0xFF; - buffer[1] = ((nblocks - 1) >> (BYTE * 2)) & 0xFF; - buffer[2] = ((nblocks - 1) >> (BYTE * 1)) & 0xFF; - buffer[3] = ((nblocks - 1) >> (BYTE * 0)) & 0xFF; - } - buffer[4] = (blocksize >> (BYTE * 3)) & 0xFF; - buffer[5] = (blocksize >> (BYTE * 2)) & 0xFF; - buffer[6] = (blocksize >> (BYTE * 1)) & 0xFF; - buffer[7] = (blocksize >> (BYTE * 0)) & 0xFF; + put_unaligned_be32(nblocks >> 32 || virt_dev->thin_provisioned ? + 0xffffffffU : nblocks - 1, &buffer[0]); + put_unaligned_be32(blocksize, &buffer[4]); length = scst_get_buf_full(cmd, &address); if (unlikely(length <= 0)) { @@ -2420,19 +2374,8 @@ static void vdisk_exec_read_capacity16(struct scst_cmd *cmd) memset(buffer, 0, sizeof(buffer)); - buffer[0] = nblocks >> 56; - buffer[1] = (nblocks >> 48) & 0xFF; - buffer[2] = (nblocks >> 40) & 0xFF; - buffer[3] = (nblocks >> 32) & 0xFF; - buffer[4] = (nblocks >> 24) & 0xFF; - buffer[5] = (nblocks >> 16) & 0xFF; - buffer[6] = (nblocks >> 8) & 0xFF; - buffer[7] = nblocks & 0xFF; - - buffer[8] = (blocksize >> (BYTE * 3)) & 0xFF; - buffer[9] = (blocksize >> (BYTE * 2)) & 0xFF; - buffer[10] = (blocksize >> (BYTE * 1)) & 0xFF; - buffer[11] = (blocksize >> (BYTE * 0)) & 0xFF; + put_unaligned_be64(nblocks, &buffer[0]); + put_unaligned_be32(blocksize, &buffer[8]); switch (blocksize) { case 512: @@ -2609,10 +2552,7 @@ static void vdisk_exec_read_toc(struct scst_cmd *cmd) /* Track Number */ buffer[off+2] = 0xAA; /* Track Start Address */ - buffer[off+4] = (nblocks >> (BYTE * 3)) & 0xFF; - buffer[off+5] = (nblocks >> (BYTE * 2)) & 0xFF; - buffer[off+6] = (nblocks >> (BYTE * 1)) & 0xFF; - buffer[off+7] = (nblocks >> (BYTE * 0)) & 0xFF; + put_unaligned_be32(nblocks, &buffer[off + 4]); off += 8; } diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 266f902d2..527346085 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -5045,12 +5045,9 @@ static int get_trans_len_single(struct scst_cmd *cmd, uint8_t off) static int get_trans_len_read_pos(struct scst_cmd *cmd, uint8_t off) { - uint8_t *p = (uint8_t *)cmd->cdb + off; int res = 0; - cmd->bufflen = 0; - cmd->bufflen |= ((u32)p[0]) << 8; - cmd->bufflen |= ((u32)p[1]); + cmd->bufflen = get_unaligned_be16(cmd->cdb + off); switch (cmd->cdb[1] & 0x1f) { case 0: @@ -5111,12 +5108,7 @@ static int get_trans_len_start_stop(struct scst_cmd *cmd, uint8_t off) static int get_trans_len_3_read_elem_stat(struct scst_cmd *cmd, uint8_t off) { - const uint8_t *p = cmd->cdb + off; - - cmd->bufflen = 0; - cmd->bufflen |= ((u32)p[0]) << 16; - cmd->bufflen |= ((u32)p[1]) << 8; - cmd->bufflen |= ((u32)p[2]); + cmd->bufflen = get_unaligned_be24(cmd->cdb + off); if ((cmd->cdb[6] & 0x2) == 0x2) cmd->op_flags |= SCST_REG_RESERVE_ALLOWED | @@ -5132,45 +5124,37 @@ static int get_trans_len_1(struct scst_cmd *cmd, uint8_t off) static int get_trans_len_1_256(struct scst_cmd *cmd, uint8_t off) { - cmd->bufflen = (u32)cmd->cdb[off]; - if (cmd->bufflen == 0) - cmd->bufflen = 256; + /* + * From the READ(6) specification: a TRANSFER LENGTH field set to zero + * specifies that 256 logical blocks shall be read. + * + * Note: while the C standard specifies that the behavior of a + * computation with signed integers that overflows is undefined, the + * same standard guarantees that the result of a computation with + * unsigned integers that cannot be represented will yield the value + * is reduced modulo the largest value that can be represented by the + * resulting type. + */ + cmd->bufflen = (u8)(cmd->cdb[off] - 1) + 1; return 0; } static int get_trans_len_2(struct scst_cmd *cmd, uint8_t off) { - const uint8_t *p = cmd->cdb + off; - - cmd->bufflen = 0; - cmd->bufflen |= ((u32)p[0]) << 8; - cmd->bufflen |= ((u32)p[1]); - + cmd->bufflen = get_unaligned_be16(cmd->cdb + off); return 0; } static int get_trans_len_3(struct scst_cmd *cmd, uint8_t off) { - const uint8_t *p = cmd->cdb + off; - - cmd->bufflen = 0; - cmd->bufflen |= ((u32)p[0]) << 16; - cmd->bufflen |= ((u32)p[1]) << 8; - cmd->bufflen |= ((u32)p[2]); + cmd->bufflen = get_unaligned_be24(cmd->cdb + off); return 0; } static int get_trans_len_4(struct scst_cmd *cmd, uint8_t off) { - const uint8_t *p = cmd->cdb + off; - - cmd->bufflen = 0; - cmd->bufflen |= ((u32)p[0]) << 24; - cmd->bufflen |= ((u32)p[1]) << 16; - cmd->bufflen |= ((u32)p[2]) << 8; - cmd->bufflen |= ((u32)p[3]); - + cmd->bufflen = get_unaligned_be32(cmd->cdb + off); return 0; } @@ -5182,14 +5166,8 @@ static int get_trans_len_none(struct scst_cmd *cmd, uint8_t off) static int get_bidi_trans_len_2(struct scst_cmd *cmd, uint8_t off) { - const uint8_t *p = cmd->cdb + off; - - cmd->bufflen = 0; - cmd->bufflen |= ((u32)p[0]) << 8; - cmd->bufflen |= ((u32)p[1]); - + cmd->bufflen = get_unaligned_be16(cmd->cdb + off); cmd->out_bufflen = cmd->bufflen; - return 0; } @@ -5738,9 +5716,7 @@ int scst_block_generic_dev_done(struct scst_cmd *cmd, goto out; } - sector_size = - ((buffer[4] << 24) | (buffer[5] << 16) | - (buffer[6] << 8) | (buffer[7] << 0)); + sector_size = get_unaligned_be32(&buffer[4]); scst_put_buf_full(cmd, buffer); if (sector_size != 0) sh = scst_calc_block_shift(sector_size); @@ -5808,8 +5784,7 @@ int scst_tape_generic_dev_done(struct scst_cmd *cmd, TRACE_DBG("%s", "MODE_SENSE"); if ((cmd->cdb[2] & 0xC0) == 0) { if (buffer[3] == 8) { - bs = (buffer[9] << 16) | - (buffer[10] << 8) | buffer[11]; + bs = get_unaligned_be24(&buffer[9]); set_block_size(cmd, bs); } } @@ -5817,8 +5792,7 @@ int scst_tape_generic_dev_done(struct scst_cmd *cmd, case MODE_SELECT: TRACE_DBG("%s", "MODE_SELECT"); if (buffer[3] == 8) { - bs = (buffer[9] << 16) | (buffer[10] << 8) | - (buffer[11]); + bs = get_unaligned_be24(&buffer[9]); set_block_size(cmd, bs); } break; diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index ad56b8b75..dcac3f67b 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -1734,10 +1734,7 @@ inc_dev_cnt: /* Set the response header */ dev_cnt *= 8; - buffer[0] = (dev_cnt >> 24) & 0xff; - buffer[1] = (dev_cnt >> 16) & 0xff; - buffer[2] = (dev_cnt >> 8) & 0xff; - buffer[3] = dev_cnt & 0xff; + put_unaligned_be32(dev_cnt, buffer); scst_put_buf_full(cmd, buffer);