On Wed, May 19, 2021 at 08:13:09AM -0700, Bart Van Assche wrote:
> 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).
So you don't need this "if (dev->use_fast_reg)" check.
Thanks
>
> Thanks,
>
> Bart.
>
>
