* [PATCH 0/2] fuse, dax: Couple of fixes for fuse dax support
@ 2020-09-01 14:26 ` Vivek Goyal
0 siblings, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2020-09-01 14:26 UTC (permalink / raw)
To: linux-fsdevel, virtio-fs, miklos; +Cc: vgoyal, stefanha, dgilbert
Hi Miklos,
I am testing fuse dax branch now. To begin with here are couple of
simple fixes to make sure I/O is going through dax path.
Either you can roll these fixes into existing patches or apply on
top.
I ran blogbench workload and some fio mmap jobs and these seem to be
running fine after these fixes.
Thanks
Vivek
Vivek Goyal (2):
fuse, dax: Use correct config option CONFIG_FUSE_DAX
fuse, dax: Save dax device in fuse_conn_dax
fs/fuse/dax.c | 1 +
fs/fuse/file.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
--
2.25.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Virtio-fs] [PATCH 1/2] fuse, dax: Use correct config option CONFIG_FUSE_DAX
2020-09-01 14:26 ` Vivek Goyal
@ 2020-09-01 14:26 ` Vivek Goyal
-1 siblings, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2020-09-01 14:26 UTC (permalink / raw)
To: linux-fsdevel, virtio-fs, miklos; +Cc: vgoyal
Correct config option to use is CONFIG_FUSE_DAX and not CONFIG_VIRTIO_FS_DAX.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/fuse/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 7b53ae43860c..380577159c6e 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3480,6 +3480,6 @@ void fuse_init_file_inode(struct inode *inode)
init_waitqueue_head(&fi->page_waitq);
fi->writepages = RB_ROOT;
- if (IS_ENABLED(CONFIG_VIRTIO_FS_DAX))
+ if (IS_ENABLED(CONFIG_FUSE_DAX))
fuse_dax_inode_init(inode);
}
--
2.25.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/2] fuse, dax: Use correct config option CONFIG_FUSE_DAX
@ 2020-09-01 14:26 ` Vivek Goyal
0 siblings, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2020-09-01 14:26 UTC (permalink / raw)
To: linux-fsdevel, virtio-fs, miklos; +Cc: vgoyal, stefanha, dgilbert
Correct config option to use is CONFIG_FUSE_DAX and not CONFIG_VIRTIO_FS_DAX.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/fuse/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 7b53ae43860c..380577159c6e 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3480,6 +3480,6 @@ void fuse_init_file_inode(struct inode *inode)
init_waitqueue_head(&fi->page_waitq);
fi->writepages = RB_ROOT;
- if (IS_ENABLED(CONFIG_VIRTIO_FS_DAX))
+ if (IS_ENABLED(CONFIG_FUSE_DAX))
fuse_dax_inode_init(inode);
}
--
2.25.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Virtio-fs] [PATCH 2/2] fuse, dax: Save dax device in fuse_conn_dax
2020-09-01 14:26 ` Vivek Goyal
@ 2020-09-01 14:26 ` Vivek Goyal
-1 siblings, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2020-09-01 14:26 UTC (permalink / raw)
To: linux-fsdevel, virtio-fs, miklos; +Cc: vgoyal
This is needed later in fuse_iomap_begin to fill iomap->dax_dev. Otherwise
dax generic code (dax_iomap_rw()) returns -EOPNOTSUPP.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/fuse/dax.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index c7099eec17b4..aa89173234c1 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1296,6 +1296,7 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev)
fcd->nr_free_ranges = nr_ranges;
fcd->nr_ranges = nr_ranges;
+ fcd->dev = dax_dev;
fc->dax = fcd;
return 0;
--
2.25.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] fuse, dax: Save dax device in fuse_conn_dax
@ 2020-09-01 14:26 ` Vivek Goyal
0 siblings, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2020-09-01 14:26 UTC (permalink / raw)
To: linux-fsdevel, virtio-fs, miklos; +Cc: vgoyal, stefanha, dgilbert
This is needed later in fuse_iomap_begin to fill iomap->dax_dev. Otherwise
dax generic code (dax_iomap_rw()) returns -EOPNOTSUPP.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/fuse/dax.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index c7099eec17b4..aa89173234c1 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1296,6 +1296,7 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev)
fcd->nr_free_ranges = nr_ranges;
fcd->nr_ranges = nr_ranges;
+ fcd->dev = dax_dev;
fc->dax = fcd;
return 0;
--
2.25.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Virtio-fs] [PATCH 0/2] fuse, dax: Couple of fixes for fuse dax support
2020-09-01 14:26 ` Vivek Goyal
@ 2020-09-04 9:45 ` Miklos Szeredi
-1 siblings, 0 replies; 10+ messages in thread
From: Miklos Szeredi @ 2020-09-04 9:45 UTC (permalink / raw)
To: Vivek Goyal; +Cc: linux-fsdevel, virtio-fs-list
On Tue, Sep 1, 2020 at 4:26 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> Hi Miklos,
>
> I am testing fuse dax branch now. To begin with here are couple of
> simple fixes to make sure I/O is going through dax path.
>
> Either you can roll these fixes into existing patches or apply on
> top.
>
> I ran blogbench workload and some fio mmap jobs and these seem to be
> running fine after these fixes.
Thanks for testing and fixing.
Pushed a rerolled series to #for-next. Would be good if you cour retest.
There's one checkpatch warning I'm unsure about:
| WARNING: Using vsprintf specifier '%px' potentially exposes the
kernel memory layout, if you don't really need the address please
consider using '%p'.
| #173: FILE: fs/fuse/virtio_fs.c:812:
| + dev_dbg(&vdev->dev, "%s: window kaddr 0x%px phys_addr 0x%llx
len 0x%llx\n",
| + __func__, fs->window_kaddr, cache_reg.addr, cache_reg.len);
|
| total: 0 errors, 1 warnings, 175 lines checked
|
| NOTE: For some of the reported defects, checkpatch may be able to
| mechanically convert to the typical style using --fix or --fix-inplace.
|
| patches/virtio_fs-dax-set-up-virtio_fs-dax_device.patch has style
problems, please review.
Do you think that the kernel address in the debug output is necessary?
Thanks,
Miklos
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] fuse, dax: Couple of fixes for fuse dax support
@ 2020-09-04 9:45 ` Miklos Szeredi
0 siblings, 0 replies; 10+ messages in thread
From: Miklos Szeredi @ 2020-09-04 9:45 UTC (permalink / raw)
To: Vivek Goyal
Cc: linux-fsdevel, virtio-fs-list, Stefan Hajnoczi,
Dr. David Alan Gilbert
On Tue, Sep 1, 2020 at 4:26 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> Hi Miklos,
>
> I am testing fuse dax branch now. To begin with here are couple of
> simple fixes to make sure I/O is going through dax path.
>
> Either you can roll these fixes into existing patches or apply on
> top.
>
> I ran blogbench workload and some fio mmap jobs and these seem to be
> running fine after these fixes.
Thanks for testing and fixing.
Pushed a rerolled series to #for-next. Would be good if you cour retest.
There's one checkpatch warning I'm unsure about:
| WARNING: Using vsprintf specifier '%px' potentially exposes the
kernel memory layout, if you don't really need the address please
consider using '%p'.
| #173: FILE: fs/fuse/virtio_fs.c:812:
| + dev_dbg(&vdev->dev, "%s: window kaddr 0x%px phys_addr 0x%llx
len 0x%llx\n",
| + __func__, fs->window_kaddr, cache_reg.addr, cache_reg.len);
|
| total: 0 errors, 1 warnings, 175 lines checked
|
| NOTE: For some of the reported defects, checkpatch may be able to
| mechanically convert to the typical style using --fix or --fix-inplace.
|
| patches/virtio_fs-dax-set-up-virtio_fs-dax_device.patch has style
problems, please review.
Do you think that the kernel address in the debug output is necessary?
Thanks,
Miklos
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Virtio-fs] [PATCH 0/2] fuse, dax: Couple of fixes for fuse dax support
2020-09-04 9:45 ` Miklos Szeredi
@ 2020-09-04 13:05 ` Vivek Goyal
-1 siblings, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2020-09-04 13:05 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, virtio-fs-list
On Fri, Sep 04, 2020 at 11:45:53AM +0200, Miklos Szeredi wrote:
> On Tue, Sep 1, 2020 at 4:26 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > Hi Miklos,
> >
> > I am testing fuse dax branch now. To begin with here are couple of
> > simple fixes to make sure I/O is going through dax path.
> >
> > Either you can roll these fixes into existing patches or apply on
> > top.
> >
> > I ran blogbench workload and some fio mmap jobs and these seem to be
> > running fine after these fixes.
>
> Thanks for testing and fixing.
>
> Pushed a rerolled series to #for-next. Would be good if you cour retest.
Thanks. Will test again next week.
>
> There's one checkpatch warning I'm unsure about:
>
> | WARNING: Using vsprintf specifier '%px' potentially exposes the
> kernel memory layout, if you don't really need the address please
> consider using '%p'.
> | #173: FILE: fs/fuse/virtio_fs.c:812:
> | + dev_dbg(&vdev->dev, "%s: window kaddr 0x%px phys_addr 0x%llx
> len 0x%llx\n",
> | + __func__, fs->window_kaddr, cache_reg.addr, cache_reg.len);
> |
> | total: 0 errors, 1 warnings, 175 lines checked
> |
> | NOTE: For some of the reported defects, checkpatch may be able to
> | mechanically convert to the typical style using --fix or --fix-inplace.
> |
> | patches/virtio_fs-dax-set-up-virtio_fs-dax_device.patch has style
> problems, please review.
>
> Do you think that the kernel address in the debug output is necessary?
It is more of a nice to have thing. Was more useful in initial
development. Now things have stablized, so I don't use it that much
anymore.
So I am fine converting %px to %p. I am not sure though how people
practically make use of %p output for debugging. IIUC, that's a
hash of actual value.
This debug output atleast tells us that a certain virtio device
provided a cache window and driver mapped it.
Thanks
Vivek
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] fuse, dax: Couple of fixes for fuse dax support
@ 2020-09-04 13:05 ` Vivek Goyal
0 siblings, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2020-09-04 13:05 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, virtio-fs-list, Stefan Hajnoczi,
Dr. David Alan Gilbert
On Fri, Sep 04, 2020 at 11:45:53AM +0200, Miklos Szeredi wrote:
> On Tue, Sep 1, 2020 at 4:26 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > Hi Miklos,
> >
> > I am testing fuse dax branch now. To begin with here are couple of
> > simple fixes to make sure I/O is going through dax path.
> >
> > Either you can roll these fixes into existing patches or apply on
> > top.
> >
> > I ran blogbench workload and some fio mmap jobs and these seem to be
> > running fine after these fixes.
>
> Thanks for testing and fixing.
>
> Pushed a rerolled series to #for-next. Would be good if you cour retest.
Thanks. Will test again next week.
>
> There's one checkpatch warning I'm unsure about:
>
> | WARNING: Using vsprintf specifier '%px' potentially exposes the
> kernel memory layout, if you don't really need the address please
> consider using '%p'.
> | #173: FILE: fs/fuse/virtio_fs.c:812:
> | + dev_dbg(&vdev->dev, "%s: window kaddr 0x%px phys_addr 0x%llx
> len 0x%llx\n",
> | + __func__, fs->window_kaddr, cache_reg.addr, cache_reg.len);
> |
> | total: 0 errors, 1 warnings, 175 lines checked
> |
> | NOTE: For some of the reported defects, checkpatch may be able to
> | mechanically convert to the typical style using --fix or --fix-inplace.
> |
> | patches/virtio_fs-dax-set-up-virtio_fs-dax_device.patch has style
> problems, please review.
>
> Do you think that the kernel address in the debug output is necessary?
It is more of a nice to have thing. Was more useful in initial
development. Now things have stablized, so I don't use it that much
anymore.
So I am fine converting %px to %p. I am not sure though how people
practically make use of %p output for debugging. IIUC, that's a
hash of actual value.
This debug output atleast tells us that a certain virtio device
provided a cache window and driver mapped it.
Thanks
Vivek
^ permalink raw reply [flat|nested] 10+ messages in thread