Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Tue, 26 Sep 2023 15:37:18 +0530
From:   Nitesh Jagadeesh Shetty <nj.shetty@samsung.com>
To:     Jinyoung Choi <j-young.choi@samsung.com>
Cc:     Jens Axboe <axboe@kernel.dk>, Jonathan Corbet <corbet@lwn.net>,
        Alasdair Kergon <agk@redhat.com>,
        Mike Snitzer <snitzer@kernel.org>,
        "dm-devel@redhat.com" <dm-devel@redhat.com>,
        Keith Busch <kbusch@kernel.org>,
        Christoph Hellwig <hch@lst.de>,
        Sagi Grimberg <sagi@grimberg.me>,
        Chaitanya Kulkarni <kch@nvidia.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <brauner@kernel.org>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "nitheshshetty@gmail.com" <nitheshshetty@gmail.com>,
        "anuj1072538@gmail.com" <anuj1072538@gmail.com>,
        SSDR Gost Dev <gost.dev@samsung.com>,
        "mcgrof@kernel.org" <mcgrof@kernel.org>,
        Vincent Kang Fu <vincent.fu@samsung.com>,
        Anuj Gupta <anuj20.g@samsung.com>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v16 04/12] block: add emulation for copy
Message-ID: <20230926100718.wcptispc2zhfi5eh@green245>
MIME-Version: 1.0
In-Reply-To: <20230922130815epcms2p631fc5fc5ebe634cc948fef1992f83a38@epcms2p6>
User-Agent: NeoMutt/20171215
Content-Type: multipart/mixed;
        boundary="----LAXCzQHOdZ.PJY8ISgkP8Kui-li3V03FA_f8OYW0UVQARYhI=_21324_"
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20230920080756.11919-5-nj.shetty@samsung.com>
        <20230920080756.11919-1-nj.shetty@samsung.com>
        <CGME20230920081458epcas5p3a3e12d8b5661b5d6f4420316630b02e1@epcms2p6>
        <20230922130815epcms2p631fc5fc5ebe634cc948fef1992f83a38@epcms2p6>
Precedence: bulk

------LAXCzQHOdZ.PJY8ISgkP8Kui-li3V03FA_f8OYW0UVQARYhI=_21324_
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Transfer-Encoding: 8bit
Content-Disposition: inline

>> +                write_bio->bi_iter.bi_size = chunk;
>> +                ret = submit_bio_wait(write_bio);
>> +                kfree(write_bio);
>
>blk_mq_map_bio_put(write_bio) ?
>or bio_uninit(write_bio); kfree(write_bio)?
>
>hmm...
>It continuously allocates and releases memory for bio,
>Why don't you just allocate and reuse bio outside the loop?
>

Agree, we will update this in next version.

>> +                if (ret)
>> +                        break;
>> +
>> +                pos_in += chunk;
>> +                pos_out += chunk;
>> +        }
>> +        cio->status = ret;
>> +        kvfree(emulation_io->buf);
>> +        kfree(emulation_io);
>
>I have not usually seen an implementation that releases memory for
>itself while performing a worker. ( I don't know what's right. :) )
>
The worker is already executing at this point.
We think releasing the reference after it starts executing should not
be an issue, and it didn't come-up in any of our testing too.

>Since blkdev_copy_emulation() allocates memory for the emulation
>and waits for it to be completed, wouldn't it be better to proceed
>with the memory release for it in the same context?
>
>That is, IMO, wouldn't it be better to free the memory related to
>emulation in blkdev_copy_wait_io_completion()?
>

Above mentioned design works for synchronous IOs. But for asynchronous
IOs emulation job is punted to worker and submitter task returns.
Submitter doesn't wait for emulation to complete and memory is freed
later by worker.

Thank you,
Nitesh Shetty

------LAXCzQHOdZ.PJY8ISgkP8Kui-li3V03FA_f8OYW0UVQARYhI=_21324_
Content-Type: text/plain; charset="utf-8"


------LAXCzQHOdZ.PJY8ISgkP8Kui-li3V03FA_f8OYW0UVQARYhI=_21324_--