* [PATCH RFC] nvmet: fix tmpfs-based backstores support
@ 2021-11-17 15:24 Maurizio Lombardi
2021-11-18 1:59 ` Chaitanya Kulkarni
2021-11-19 17:02 ` Christoph Hellwig
0 siblings, 2 replies; 8+ messages in thread
From: Maurizio Lombardi @ 2021-11-17 15:24 UTC (permalink / raw)
To: kch; +Cc: linux-nvme, sagi, hch
commit 50a909db36f2 ("nvmet: use IOCB_NOWAIT for file-ns buffered I/O")
optimized the read command behaviour when buffered file-ns
configurations are used.
It issues the read commands with the IOCB_NOWAIT flag set to try
to access the data from the cache.
However, the patch was buggy because both read and write commands are
issued with IOCB_NOWAIT, this broke tmpfs-based backstores because tmpfs
doesn't accept write commands with IOCB_NOWAIT.
Fix this bug by restricting IOCB_NOWAIT to read commands only.
Fixes: 50a909db36f2 ("nvmet: use IOCB_NOWAIT for file-ns buffered I/O")
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
drivers/nvme/target/io-cmd-file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 6aa30f30b572..1d9b41190583 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -266,6 +266,7 @@ static void nvmet_file_execute_rw(struct nvmet_req *req)
if (req->ns->buffered_io) {
if (likely(!req->f.mpool_alloc) &&
+ req->cmd->rw.opcode == nvme_cmd_read &&
nvmet_file_execute_io(req, IOCB_NOWAIT))
return;
nvmet_file_submit_buffered_io(req);
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] nvmet: fix tmpfs-based backstores support
2021-11-17 15:24 [PATCH RFC] nvmet: fix tmpfs-based backstores support Maurizio Lombardi
@ 2021-11-18 1:59 ` Chaitanya Kulkarni
2021-11-18 8:29 ` Maurizio Lombardi
2021-11-19 17:02 ` Christoph Hellwig
1 sibling, 1 reply; 8+ messages in thread
From: Chaitanya Kulkarni @ 2021-11-18 1:59 UTC (permalink / raw)
To: Maurizio Lombardi, Chaitanya Kulkarni; +Cc: linux-nvme, sagi, hch
On 11/17/21 07:24, Maurizio Lombardi wrote:
> External email: Use caution opening links or attachments
>
>
> commit 50a909db36f2 ("nvmet: use IOCB_NOWAIT for file-ns buffered I/O")
> optimized the read command behaviour when buffered file-ns
s/behaviour/behavior
> configurations are used.
> It issues the read commands with the IOCB_NOWAIT flag set to try
> to access the data from the cache.
>
> However, the patch was buggy because both read and write commands are
> issued with IOCB_NOWAIT, this broke tmpfs-based backstores because tmpfs
> doesn't accept write commands with IOCB_NOWAIT.
>
> Fix this bug by restricting IOCB_NOWAIT to read commands only.
>
> Fixes: 50a909db36f2 ("nvmet: use IOCB_NOWAIT for file-ns buffered I/O")
>
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
We will need blktests for this.
Also, can you share performance numbers with this patch and
the errors that you are getting without this patch ?
> ---
> drivers/nvme/target/io-cmd-file.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
> index 6aa30f30b572..1d9b41190583 100644
> --- a/drivers/nvme/target/io-cmd-file.c
> +++ b/drivers/nvme/target/io-cmd-file.c
> @@ -266,6 +266,7 @@ static void nvmet_file_execute_rw(struct nvmet_req *req)
>
> if (req->ns->buffered_io) {
> if (likely(!req->f.mpool_alloc) &&
> + req->cmd->rw.opcode == nvme_cmd_read &&
> nvmet_file_execute_io(req, IOCB_NOWAIT))
> return;
> nvmet_file_submit_buffered_io(req);
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] nvmet: fix tmpfs-based backstores support
2021-11-18 1:59 ` Chaitanya Kulkarni
@ 2021-11-18 8:29 ` Maurizio Lombardi
2021-11-18 8:41 ` Chaitanya Kulkarni
2021-11-18 8:42 ` Chaitanya Kulkarni
0 siblings, 2 replies; 8+ messages in thread
From: Maurizio Lombardi @ 2021-11-18 8:29 UTC (permalink / raw)
To: Chaitanya Kulkarni; +Cc: linux-nvme, sagi, hch
On Thu, Nov 18, 2021 at 01:59:29AM +0000, Chaitanya Kulkarni wrote:
> We will need blktests for this.
>
> Also, can you share performance numbers with this patch and
> the errors that you are getting without this patch ?
Ok, I will try to run blktests and provide some performance numbers.
Without this patch, writes get lost:
[root@kvm-02-guest03 ~]# nvme connect -t tcp -a 10.37.153.75 -n test-nqn -s 4080
[root@kvm-02-guest03 ~]# dd if=/dev/urandom of=/dev/nvme0n1 bs=1M count=1
1+0 records in
1+0 records out
1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0604551 s, 17.3 MB/s
[root@kvm-02-guest03 ~]# hexdump -C /dev/nvme0n1
00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
06400000
dmesg output:
[81489.687548] blk_update_request: critical target error, dev nvme0c0n1, sector 0 op 0x1:(WRITE) flags 0x2000800 phys_seg 256 prio class 0
[81489.691139] buffer_io_error: 246 callbacks suppressed
[81489.691141] Buffer I/O error on dev nvme0n1, logical block 0, lost async page write
[81489.695013] Buffer I/O error on dev nvme0n1, logical block 1, lost async page write
[81489.697183] Buffer I/O error on dev nvme0n1, logical block 2, lost async page write
[81489.699239] Buffer I/O error on dev nvme0n1, logical block 3, lost async page write
[81489.701297] Buffer I/O error on dev nvme0n1, logical block 4, lost async page write
[81489.703374] Buffer I/O error on dev nvme0n1, logical block 5, lost async page write
[81489.705459] Buffer I/O error on dev nvme0n1, logical block 6, lost async page write
[81489.707511] Buffer I/O error on dev nvme0n1, logical block 7, lost async page write
[81489.709438] Buffer I/O error on dev nvme0n1, logical block 8, lost async page write
[81489.711364] Buffer I/O error on dev nvme0n1, logical block 9, lost async page write
Maurizio
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] nvmet: fix tmpfs-based backstores support
2021-11-18 8:29 ` Maurizio Lombardi
@ 2021-11-18 8:41 ` Chaitanya Kulkarni
2021-11-18 8:42 ` Chaitanya Kulkarni
1 sibling, 0 replies; 8+ messages in thread
From: Chaitanya Kulkarni @ 2021-11-18 8:41 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: linux-nvme, sagi, hch
On 11/18/2021 12:29 AM, Maurizio Lombardi wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, Nov 18, 2021 at 01:59:29AM +0000, Chaitanya Kulkarni wrote:
>> We will need blktests for this.
>>
>> Also, can you share performance numbers with this patch and
>> the errors that you are getting without this patch ?
>
> Ok, I will try to run blktests and provide some performance numbers.
>
> Without this patch, writes get lost:
>
> [root@kvm-02-guest03 ~]# nvme connect -t tcp -a 10.37.153.75 -n test-nqn -s 4080
> [root@kvm-02-guest03 ~]# dd if=/dev/urandom of=/dev/nvme0n1 bs=1M count=1
> 1+0 records in
> 1+0 records out
> 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0604551 s, 17.3 MB/s
> [root@kvm-02-guest03 ~]# hexdump -C /dev/nvme0n1
> 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
> *
> 06400000
>
> dmesg output:
>
> [81489.687548] blk_update_request: critical target error, dev nvme0c0n1, sector 0 op 0x1:(WRITE) flags 0x2000800 phys_seg 256 prio class 0
> [81489.691139] buffer_io_error: 246 callbacks suppressed
> [81489.691141] Buffer I/O error on dev nvme0n1, logical block 0, lost async page write
> [81489.695013] Buffer I/O error on dev nvme0n1, logical block 1, lost async page write
> [81489.697183] Buffer I/O error on dev nvme0n1, logical block 2, lost async page write
> [81489.699239] Buffer I/O error on dev nvme0n1, logical block 3, lost async page write
> [81489.701297] Buffer I/O error on dev nvme0n1, logical block 4, lost async page write
> [81489.703374] Buffer I/O error on dev nvme0n1, logical block 5, lost async page write
> [81489.705459] Buffer I/O error on dev nvme0n1, logical block 6, lost async page write
> [81489.707511] Buffer I/O error on dev nvme0n1, logical block 7, lost async page write
> [81489.709438] Buffer I/O error on dev nvme0n1, logical block 8, lost async page write
> [81489.711364] Buffer I/O error on dev nvme0n1, logical block 9, lost async page write
>
> Maurizio
>
Thanks for the prompt reply and sharing this ..
Please send a formal-patch (non-RFC) with the testing done.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] nvmet: fix tmpfs-based backstores support
2021-11-18 8:29 ` Maurizio Lombardi
2021-11-18 8:41 ` Chaitanya Kulkarni
@ 2021-11-18 8:42 ` Chaitanya Kulkarni
2021-11-18 9:16 ` Maurizio Lombardi
1 sibling, 1 reply; 8+ messages in thread
From: Chaitanya Kulkarni @ 2021-11-18 8:42 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: linux-nvme, sagi, hch
On 11/18/2021 12:29 AM, Maurizio Lombardi wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, Nov 18, 2021 at 01:59:29AM +0000, Chaitanya Kulkarni wrote:
>> We will need blktests for this.
>>
One more thing you need to add a new block tests with the
configuration that you have used to find this problem so it
will get tested every release.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] nvmet: fix tmpfs-based backstores support
2021-11-18 8:42 ` Chaitanya Kulkarni
@ 2021-11-18 9:16 ` Maurizio Lombardi
0 siblings, 0 replies; 8+ messages in thread
From: Maurizio Lombardi @ 2021-11-18 9:16 UTC (permalink / raw)
To: Chaitanya Kulkarni; +Cc: linux-nvme, sagi, hch
On Thu, Nov 18, 2021 at 08:42:25AM +0000, Chaitanya Kulkarni wrote:
> One more thing you need to add a new block tests with the
> configuration that you have used to find this problem so it
> will get tested every release.
>
Ok, will do.
Maurizio
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] nvmet: fix tmpfs-based backstores support
2021-11-17 15:24 [PATCH RFC] nvmet: fix tmpfs-based backstores support Maurizio Lombardi
2021-11-18 1:59 ` Chaitanya Kulkarni
@ 2021-11-19 17:02 ` Christoph Hellwig
2021-11-21 14:25 ` Maurizio Lombardi
1 sibling, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2021-11-19 17:02 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: kch, linux-nvme, sagi, hch
The proper fix is to check if the file has FMODE_NOWAIT is set before
using FMODE_NOWAIT for reads or writes, not to blindly disable it for
reads.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] nvmet: fix tmpfs-based backstores support
2021-11-19 17:02 ` Christoph Hellwig
@ 2021-11-21 14:25 ` Maurizio Lombardi
0 siblings, 0 replies; 8+ messages in thread
From: Maurizio Lombardi @ 2021-11-21 14:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: kch, linux-nvme, sagi
On Fri, Nov 19, 2021 at 06:02:33PM +0100, Christoph Hellwig wrote:
> The proper fix is to check if the file has FMODE_NOWAIT is set before
> using FMODE_NOWAIT for reads or writes, not to blindly disable it for
> reads.
>
Ah thanks for the info!
Now I see that filesystems that support NOWAIT set the FMODE_NOWAIT
flag in their open() functions.
So the correct patch should be the following:
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 1dd1a0fe2e81..bf3f440c98d5 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -266,6 +266,7 @@ static void nvmet_file_execute_rw(struct nvmet_req *req)
if (req->ns->buffered_io) {
if (likely(!req->f.mpool_alloc) &&
+ (req->ns->file->f_mode & FMODE_NOWAIT) &&
nvmet_file_execute_io(req, IOCB_NOWAIT))
return;
nvmet_file_submit_buffered_io(req);
I'm going to test it.
Maurizio
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-11-21 14:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 15:24 [PATCH RFC] nvmet: fix tmpfs-based backstores support Maurizio Lombardi
2021-11-18 1:59 ` Chaitanya Kulkarni
2021-11-18 8:29 ` Maurizio Lombardi
2021-11-18 8:41 ` Chaitanya Kulkarni
2021-11-18 8:42 ` Chaitanya Kulkarni
2021-11-18 9:16 ` Maurizio Lombardi
2021-11-19 17:02 ` Christoph Hellwig
2021-11-21 14:25 ` Maurizio Lombardi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.