[0/2] fuse, dax: Couple of fixes for fuse dax support

Message

Vivek Goyal Sept. 1, 2020, 2:26 p.m. UTC

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(-)

Comments

Miklos Szeredi Sept. 4, 2020, 9:45 a.m. UTC | #1

On Tue, Sep 1, 2020 at 4:26 PM Vivek Goyal <[e-mail]> 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

Vivek Goyal Sept. 4, 2020, 1:05 p.m. UTC | #2

On Fri, Sep 04, 2020 at 11:45:53AM +0200, Miklos Szeredi wrote:
> On Tue, Sep 1, 2020 at 4:26 PM Vivek Goyal <[e-mail]> 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