[PATCH v2] nvmet: avoid unnecessary fsync and flush bio

[PATCH v2] nvmet: avoid unnecessary fsync and flush bio

[Guixin Liu]

For none buffered_io file backend and no volatile write cache
block device backend, fsync and flush bio are both unnecessary,
avoid to do that.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
Changes on v2:
- Use bdev_write_cache instead of open code.
- Use NVME_SC_SUCCESS instead of translation, and remove "else".

 drivers/nvme/target/io-cmd-bdev.c | 8 ++++++++
 drivers/nvme/target/io-cmd-file.c | 4 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 2dc1c10..e63fe87 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -337,6 +337,11 @@ static void nvmet_bdev_execute_flush(struct nvmet_req *req)
 	if (!nvmet_check_transfer_len(req, 0))
 		return;
 
+	if (!bdev_write_cache(req->ns->bdev)) {
+		nvmet_req_complete(req, NVME_SC_SUCCESS);
+		return;
+	}
+
 	bio_init(bio, req->ns->bdev, req->inline_bvec,
 		 ARRAY_SIZE(req->inline_bvec), REQ_OP_WRITE | REQ_PREFLUSH);
 	bio->bi_private = req;
@@ -347,6 +352,9 @@ static void nvmet_bdev_execute_flush(struct nvmet_req *req)
 
 u16 nvmet_bdev_flush(struct nvmet_req *req)
 {
+	if (!bdev_write_cache(req->ns->bdev))
+		return 0;
+
 	if (blkdev_issue_flush(req->ns->bdev))
 		return NVME_SC_INTERNAL | NVME_SC_DNR;
 	return 0;
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 64b47e2..fdff026 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -268,7 +268,9 @@ static void nvmet_file_execute_rw(struct nvmet_req *req)
 
 u16 nvmet_file_flush(struct nvmet_req *req)
 {
-	return errno_to_nvme_status(req, vfs_fsync(req->ns->file, 1));
+	if (req->ns->buffered_io)
+		return errno_to_nvme_status(req, vfs_fsync(req->ns->file, 1));
+	return NVME_SC_SUCCESS;
 }
 
 static void nvmet_file_flush_work(struct work_struct *w)
-- 
1.8.3.1

Re: [PATCH v2] nvmet: avoid unnecessary fsync and flush bio

[Chaitanya Kulkarni]

On 7/24/22 23:28, Guixin Liu wrote:
> For none buffered_io file backend and no volatile write cache
> block device backend, fsync and flush bio are both unnecessary,
> avoid to do that.
> 
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>

Looks good to me, I believe you have tested this patch.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH v2] nvmet: avoid unnecessary fsync and flush bio

[Christoph Hellwig]

On Mon, Jul 25, 2022 at 02:28:18PM +0800, Guixin Liu wrote:
>  u16 nvmet_file_flush(struct nvmet_req *req)
>  {
> -	return errno_to_nvme_status(req, vfs_fsync(req->ns->file, 1));
> +	if (req->ns->buffered_io)
> +		return errno_to_nvme_status(req, vfs_fsync(req->ns->file, 1));
> +	return NVME_SC_SUCCESS;

This is broken.  Even direct I/O needs an fsync to commit metadata
changes.

Re: [PATCH v2] nvmet: avoid unnecessary fsync and flush bio

[Guixin Liu]

在 2022/7/26 19:33, Christoph Hellwig 写道:
> On Mon, Jul 25, 2022 at 02:28:18PM +0800, Guixin Liu wrote:
>>   u16 nvmet_file_flush(struct nvmet_req *req)
>>   {
>> -	return errno_to_nvme_status(req, vfs_fsync(req->ns->file, 1));
>> +	if (req->ns->buffered_io)
>> +		return errno_to_nvme_status(req, vfs_fsync(req->ns->file, 1));
>> +	return NVME_SC_SUCCESS;
> This is broken.  Even direct I/O needs an fsync to commit metadata
> changes.

Well, you are right, I will send V3 patch with remove this change.

Thanks,

Guixin Liu

Re: [PATCH v2] nvmet: avoid unnecessary fsync and flush bio

[Chaitanya Kulkarni]

On 7/27/22 01:03, Guixin Liu wrote:
> 
> 在 2022/7/26 19:33, Christoph Hellwig 写道:
>> On Mon, Jul 25, 2022 at 02:28:18PM +0800, Guixin Liu wrote:
>>>   u16 nvmet_file_flush(struct nvmet_req *req)
>>>   {
>>> -    return errno_to_nvme_status(req, vfs_fsync(req->ns->file, 1));
>>> +    if (req->ns->buffered_io)
>>> +        return errno_to_nvme_status(req, vfs_fsync(req->ns->file, 1));
>>> +    return NVME_SC_SUCCESS;
>> This is broken.  Even direct I/O needs an fsync to commit metadata
>> changes.
> 
> Well, you are right, I will send V3 patch with remove this change.
> 
> Thanks,
> 
> Guixin Liu
> 

Please provide the testlog for this patch with V3.

-ck