On 5/19/21 2:09 AM, Leon Romanovsky wrote:
> On Tue, May 11, 2021 at 08:27:52PM -0700, Bart Van Assche wrote:
>> -static void srp_free_req_data(struct srp_target_port *target,
>> - struct srp_rdma_ch *ch)
>> +static int srp_exit_cmd_priv(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
>> {
>> + struct srp_target_port *target = host_to_target(shost);
>> struct srp_device *dev = target->srp_host->srp_dev;
>> struct ib_device *ibdev = dev->dev;
>> - struct srp_request *req;
>> - int i;
>> + struct srp_request *req = scsi_cmd_priv(cmd);
>>
>> - if (!ch->req_ring)
>> - return;
>> -
>> - for (i = 0; i < target->req_ring_size; ++i) {
>> - req = &ch->req_ring[i];
>> - if (dev->use_fast_reg)
>> - kfree(req->fr_list);
>> - if (req->indirect_dma_addr) {
>> - ib_dma_unmap_single(ibdev, req->indirect_dma_addr,
>> - target->indirect_size,
>> - DMA_TO_DEVICE);
>> - }
>> - kfree(req->indirect_desc);
>> + if (dev->use_fast_reg)
>> + kfree(req->fr_list);
>
> Isn't cleaner will be to ensure that fr_list is NULL for !dev->use_fast_reg path?
> In patch #4 https://lore.kernel.org/linux-rdma/20210512032752.16611-5-bvanassche@xxxxxxx
Hi Leon,
I think that per-request private data is zero-initialized and hence that
it is not necessary to clear req->fr_list explicitly. blk_mq_alloc_rqs()
passes __GFP_ZERO to alloc_pages_node(). blk_mq_alloc_rqs() does not
only allocate block layer requests (struct request) but also per-request
private data (set->cmd_size).
Thanks,
Bart.
