On 07/16/2015 06:26 PM, Matthew R. Ochs wrote:
> +
> +/**
> + * ba_clone() - frees a block from the block allocator
> + * @ba_lun: Block allocator from which to allocate a block.
> + * @to_free: Block to free.
> + *
> + * Return: 0 on success, -1 on failure
> + */
> +static int ba_clone(struct ba_lun *ba_lun, u64 to_clone)
> +{
> + struct ba_lun_info *lun_info =
> + (struct ba_lun_info *)ba_lun->ba_lun_handle;
> +
> + if (validate_alloc(lun_info, to_clone)) {
> + pr_err("%s: AUN %llX is not allocated on lun_id %llX\n",
> + __func__, to_clone, ba_lun->lun_id);
Suggest using dev_err here instead to scope the error to the adapter.
> + return -1;
Might be nice to return a better error to the user in both of the error cases
in this code.
> + }
> +
> + pr_debug("%s: Received a request to clone AUN %llX on lun_id %llX\n",
> + __func__, to_clone, ba_lun->lun_id);
> +
> + if (lun_info->aun_clone_map[to_clone] == MAX_AUN_CLONE_CNT) {
> + pr_err("%s: AUN %llX on lun_id %llX hit max clones already\n",
> + __func__, to_clone, ba_lun->lun_id);
> + return -1;
> + }
> +
> + lun_info->aun_clone_map[to_clone]++;
> +
> + return 0;
> +}
> +
> +
> +/**
> + * init_ba() - initializes and allocates a block allocator
> + * @lun_info: LUN information structure that owns the block allocator.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int init_ba(struct llun_info *lli)
> +{
> + int rc = 0;
> + struct glun_info *gli = lli->parent;
> + struct blka *blka = &gli->blka;
> +
> + memset(blka, 0, sizeof(*blka));
> + mutex_init(&blka->mutex);
> +
> + /* LUN IDs are unique per port, save the index instead */
> + blka->ba_lun.lun_id = lli->lun_index;
> + blka->ba_lun.lsize = gli->max_lba + 1;
> + blka->ba_lun.lba_size = gli->blk_len;
> +
> + blka->ba_lun.au_size = MC_CHUNK_SIZE;
> + blka->nchunk = blka->ba_lun.lsize / MC_CHUNK_SIZE;
> +
> + rc = ba_init(&blka->ba_lun);
> + if (rc) {
> + pr_err("%s: cannot init block_alloc, rc=%d\n", __func__, rc);
> + goto init_ba_exit;
The goto here is unnecessary
> + }
> +
> +init_ba_exit:
> + pr_debug("%s: returning rc=%d lli=%p\n", __func__, rc, lli);
> + return rc;
> +}
> +
> +/**
> + * write_same16() - sends a SCSI WRITE_SAME16 (0) command to specified LUN
> + * @sdev: SCSI device associated with LUN.
> + * @lba: Logical block address to start write same.
> + * @nblks: Number of logical blocks to write same.
> + *
> + * Return: 0 on success, -1 on failure
> + */
> +static int write_same16(struct scsi_device *sdev,
> + u64 lba,
> + u32 nblks)
> +{
> + u8 scsi_cmd[MAX_COMMAND_SIZE];
> + u8 *cmd_buf = NULL;
> + u8 *sense_buf = NULL;
> + int rc = 0;
> + int result = 0;
> + int ws_limit = SISLITE_MAX_WS_BLOCKS;
> + u64 offset = lba;
> + int left = nblks;
> +
> + memset(scsi_cmd, 0, sizeof(scsi_cmd));
> + cmd_buf = kzalloc(CMD_BUFSIZE, GFP_KERNEL);
> + sense_buf = kzalloc(SCSI_SENSE_BUFFERSIZE, GFP_NOIO);
> + if (!cmd_buf || !sense_buf) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + while (left > 0) {
> +
> + scsi_cmd[0] = WRITE_SAME_16;
> + put_unaligned_be64(offset, &scsi_cmd[2]);
> + put_unaligned_be32(ws_limit < left ? ws_limit : left,
> + &scsi_cmd[10]);
> +
> + left -= ws_limit;
> + offset += ws_limit;
> +
> + result = scsi_execute(sdev, scsi_cmd, DMA_TO_DEVICE, cmd_buf,
> + CMD_BUFSIZE, sense_buf,
> + (MC_DISCOVERY_TIMEOUT*HZ), 5, 0, NULL);
5 seconds seems a little on the short side for this command. Perhaps using
sdev->request_queue->rq_timeout would be better?
> +
> + if (result) {
> + pr_err("%s: command failed for offset %lld"
> + " result=0x%x\n", __func__, offset, result);
> + rc = -EIO;
> + goto out;
> + }
> + }
> +
> +out:
You need to free cmd_buf and sense_buf here.
> + pr_debug("%s: returning rc=%d\n", __func__, rc);
> + return rc;
> +}
> +
> +
> +/**
> + * _cxlflash_vlun_resize() - changes the size of a virtual lun
> + * @sdev: SCSI device associated with LUN owning virtual LUN.
> + * @ctxi: Context owning resources.
> + * @resize: Resize ioctl data structure.
> + *
> + * On successful return, the user is informed of the new size (in blocks)
> + * of the virtual lun in last LBA format. When the size of the virtual
> + * lun is zero, the last LBA is reflected as -1.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int _cxlflash_vlun_resize(struct scsi_device *sdev,
> + struct ctx_info *ctxi,
> + struct dk_cxlflash_resize *resize)
> +{
> + struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)sdev->host->hostdata;
> + struct llun_info *lli = sdev->hostdata;
> + struct glun_info *gli = lli->parent;
> + struct afu *afu = cfg->afu;
> + bool unlock_ctx = false;
> +
> + res_hndl_t rhndl = resize->rsrc_handle;
> + u64 new_size;
> + u64 nsectors;
> + u64 ctxid = DECODE_CTXID(resize->context_id),
> + rctxid = resize->context_id;
> +
> + struct sisl_rht_entry *rhte;
> +
> + int rc = 0;
> +
> + /* req_size is always assumed to be in 4k blocks. So we have to convert
> + * it from 4k to chunk size
> + */
> + nsectors = (resize->req_size * CXLFLASH_BLOCK_SIZE) / gli->blk_len;
> + new_size = (nsectors + MC_CHUNK_SIZE - 1) / MC_CHUNK_SIZE;
Use DIV_ROUND_UP instead.
> +
> + pr_debug("%s: ctxid=%llu rhndl=0x%llx, req_size=0x%llx,"
> + "new_size=%llx\n", __func__, ctxid, resize->rsrc_handle,
> + resize->req_size, new_size);
> +
> + if (unlikely(gli->mode != MODE_VIRTUAL)) {
> + pr_err("%s: LUN mode does not support resize! (%d)\n",
> + __func__, gli->mode);
> + rc = -EINVAL;
> + goto out;
> +
> + }
> +
> + if (!ctxi) {
> + ctxi = get_context(cfg, rctxid, lli, CTX_CTRL_ERR_FALLBACK);
> + if (unlikely(!ctxi)) {
> + pr_err("%s: Bad context! (%llu)\n", __func__, ctxid);
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + unlock_ctx = true;
> + }
> +
> + rhte = get_rhte(ctxi, rhndl, lli);
> + if (unlikely(!rhte)) {
> + pr_err("%s: Bad resource handle! (%u)\n", __func__, rhndl);
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + if (new_size > rhte->lxt_cnt)
> + rc = grow_lxt(afu,
> + sdev,
> + ctxid,
> + rhndl,
> + rhte,
> + &new_size);
> + else if (new_size < rhte->lxt_cnt)
> + rc = shrink_lxt(afu,
> + sdev,
> + ctxid,
> + rhndl,
> + rhte,
> + &new_size);
> +
> + resize->hdr.return_flags = 0;
> + resize->last_lba = (new_size * MC_CHUNK_SIZE * gli->blk_len);
> + resize->last_lba /= CXLFLASH_BLOCK_SIZE;
> + resize->last_lba--;
> +
> +out:
> + if (unlock_ctx)
> + mutex_unlock(&ctxi->mutex);
> + pr_debug("%s: resized to %lld returning rc=%d\n",
> + __func__, resize->last_lba, rc);
> + return rc;
> +}
> +
> +int cxlflash_vlun_resize(struct scsi_device *sdev,
> + struct dk_cxlflash_resize *resize)
> +{
> + return _cxlflash_vlun_resize(sdev, NULL, resize);
> +}
> +
> +/**
> + * init_lun_table() - write an entry in the LUN table
> + * @cfg: Internal structure associated with the host.
> + * @lli: Per adapter LUN information structure.
> + *
> + * On successful return, a LUN table entry is created.
> + * At the top for LUNs visible on both ports.
> + * At the bottom for LUNs visible only on one port.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int init_lun_table(struct cxlflash_cfg *cfg, struct llun_info *lli)
> +{
> + u32 chan;
> + int rc = 0;
> + struct afu *afu = cfg->afu;
> +
> + if (lli->in_table)
> + goto out;
> +
> + if (lli->port_sel == BOTH_PORTS) {
> + /*
> + * If this LUN is visible from both ports, we will put
> + * it in the top half of the LUN table.
> + */
> + if ((cfg->promote_lun_index == cfg->last_lun_index[0]) ||
> + (cfg->promote_lun_index == cfg->last_lun_index[1])) {
> + rc = -ENOSPC;
> + goto out;
> + }
> +
> + lli->lun_index = cfg->promote_lun_index;
> + writeq_be(lli->lun_id[0],
> + &afu->afu_map->global.fc_port[0]
> + [cfg->promote_lun_index]);
Would improve readability IMHO if you either put the second parm all on one
line or used a local variable to make it better fit on one line, rather than
line breaking where you did.
> + writeq_be(lli->lun_id[1],
> + &afu->afu_map->global.fc_port[1]
> + [cfg->promote_lun_index]);
> + cfg->promote_lun_index++;
> + pr_debug("%s: Virtual LUN on slot %d id0=%llx, id1=%llx\n",
> + __func__, lli->lun_index, lli->lun_id[0],
> + lli->lun_id[1]);
Suggest changing the pr_debug calls to dev_dbg calls where possible to scope
the messages to an adapter.
> + } else {
> + /*
> + * If this LUN is visible only from one port, we will put
> + * it in the bottom half of the LUN table.
> + */
> + chan = PORT2CHAN(lli->port_sel);
> + if (cfg->promote_lun_index == cfg->last_lun_index[chan]) {
> + rc = -ENOSPC;
> + goto out;
> + }
> +
> + lli->lun_index = cfg->last_lun_index[chan];
> + writeq_be(lli->lun_id[chan],
> + &afu->afu_map->global.fc_port[chan]
> + [cfg->last_lun_index[chan]]);
> + cfg->last_lun_index[chan]--;
> + pr_debug("%s: Virtual LUN on slot %d chan=%d, id=%llx\n",
> + __func__, lli->lun_index, chan, lli->lun_id[chan]);
> + }
> +
> + lli->in_table = true;
> +out:
> + pr_debug("%s: returning rc=%d\n", __func__, rc);
> + return rc;
> +}
> +
--
Brian King
Power Linux I/O
IBM Linux Technology Center
--
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
