* [Virtio-fs] False error message when DAX cache is disabled
@ 2019-10-24 9:05 misono.tomohiro
2019-10-24 17:13 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 4+ messages in thread
From: misono.tomohiro @ 2019-10-24 9:05 UTC (permalink / raw)
To: virtio-fs
Hi,
I'm testing virtiofs on ARM enviroment.
Since current ARM does not support DAX, I use cache-size=0 for vhost-user-fs-pci parameter.
This results in the following error message during unmount:
```
vhost_user_fs_slave_unmap: unmap when DAX cache not present
lo_destroy: unmap during destroy failed
```
This is because lo_destroy() always calls fuse_virtio_unmap().
So, I want to fix this by not calling fuse_virtio_unmap() when cache is not used,
but is there any good way for virtiofsd to know vhost-user's cahce-size?
Thanks.
Misono
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Virtio-fs] False error message when DAX cache is disabled
2019-10-24 9:05 [Virtio-fs] False error message when DAX cache is disabled misono.tomohiro
@ 2019-10-24 17:13 ` Dr. David Alan Gilbert
2019-10-25 10:00 ` misono.tomohiro
0 siblings, 1 reply; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2019-10-24 17:13 UTC (permalink / raw)
To: misono.tomohiro; +Cc: virtio-fs
* misono.tomohiro@fujitsu.com (misono.tomohiro@fujitsu.com) wrote:
> Hi,
Hi Misono,
> I'm testing virtiofs on ARM enviroment.
> Since current ARM does not support DAX, I use cache-size=0 for vhost-user-fs-pci parameter.
>
> This results in the following error message during unmount:
> ```
> vhost_user_fs_slave_unmap: unmap when DAX cache not present
> lo_destroy: unmap during destroy failed
> ```
Thanks for reporting this,
> This is because lo_destroy() always calls fuse_virtio_unmap().
> So, I want to fix this by not calling fuse_virtio_unmap() when cache is not used,
> but is there any good way for virtiofsd to know vhost-user's cahce-size?
Unfortunately not; that's why I used the special ~0 as a 'unmap all'.
It feels easier to fix the qemu side to be happy unmapping everything
when everything is actually nothing :-)
Does the following (build tested only) patch fix it:
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index 6cacdb24b0..07a04d9b21 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -92,10 +92,6 @@ uint64_t vhost_user_fs_slave_unmap(struct vhost_dev *dev, VhostUserFSSlaveMsg *s
return -1;
}
size_t cache_size = fs->conf.cache_size;
- if (!cache_size) {
- fprintf(stderr, "%s: unmap when DAX cache not present\n", __func__);
- return (uint64_t)-1;
- }
void *cache_host = memory_region_get_ram_ptr(&fs->cache);
unsigned int i;
@@ -106,13 +102,19 @@ uint64_t vhost_user_fs_slave_unmap(struct vhost_dev *dev, VhostUserFSSlaveMsg *s
*/
for (i = 0; i < VHOST_USER_FS_SLAVE_ENTRIES; i++) {
void *ptr;
+ if (sm->len[i] == ~(uint64_t)0) {
+ /* Special case meaning the whole arena */
+ sm->len[i] = cache_size;
+ }
+
if (sm->len[i] == 0) {
continue;
}
- if (sm->len[i] == ~(uint64_t)0) {
- /* Special case meaning the whole arena */
- sm->len[i] = cache_size;
+ if (!cache_size) {
+ fprintf(stderr, "%s: unmap when DAX cache not present\n", __func__);
+ res = -1;
+ break;
}
if ((sm->c_offset[i] + sm->len[i]) < sm->len[i] ||
> Thanks.
> Misono
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Virtio-fs] False error message when DAX cache is disabled
2019-10-24 17:13 ` Dr. David Alan Gilbert
@ 2019-10-25 10:00 ` misono.tomohiro
2019-10-25 10:41 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 4+ messages in thread
From: misono.tomohiro @ 2019-10-25 10:00 UTC (permalink / raw)
To: 'Dr. David Alan Gilbert'; +Cc: virtio-fs
> * misono.tomohiro@fujitsu.com (misono.tomohiro@fujitsu.com) wrote:
> > Hi,
>
> Hi Misono,
>
> > I'm testing virtiofs on ARM enviroment.
> > Since current ARM does not support DAX, I use cache-size=0 for vhost-user-fs-pci parameter.
> >
> > This results in the following error message during unmount:
> > ```
> > vhost_user_fs_slave_unmap: unmap when DAX cache not present
> > lo_destroy: unmap during destroy failed ```
>
> Thanks for reporting this,
>
> > This is because lo_destroy() always calls fuse_virtio_unmap().
> > So, I want to fix this by not calling fuse_virtio_unmap() when cache
> > is not used, but is there any good way for virtiofsd to know vhost-user's cahce-size?
>
> Unfortunately not; that's why I used the special ~0 as a 'unmap all'.
> It feels easier to fix the qemu side to be happy unmapping everything when everything is actually nothing :-)
>
> Does the following (build tested only) patch fix it:
>
> diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c index 6cacdb24b0..07a04d9b21 100644
> --- a/hw/virtio/vhost-user-fs.c
> +++ b/hw/virtio/vhost-user-fs.c
> @@ -92,10 +92,6 @@ uint64_t vhost_user_fs_slave_unmap(struct vhost_dev *dev, VhostUserFSSlaveMsg *s
> return -1;
> }
> size_t cache_size = fs->conf.cache_size;
> - if (!cache_size) {
> - fprintf(stderr, "%s: unmap when DAX cache not present\n", __func__);
> - return (uint64_t)-1;
> - }
> void *cache_host = memory_region_get_ram_ptr(&fs->cache);
>
> unsigned int i;
> @@ -106,13 +102,19 @@ uint64_t vhost_user_fs_slave_unmap(struct vhost_dev *dev, VhostUserFSSlaveMsg *s
> */
> for (i = 0; i < VHOST_USER_FS_SLAVE_ENTRIES; i++) {
> void *ptr;
> + if (sm->len[i] == ~(uint64_t)0) {
> + /* Special case meaning the whole arena */
> + sm->len[i] = cache_size;
> + }
> +
> if (sm->len[i] == 0) {
> continue;
> }
>
> - if (sm->len[i] == ~(uint64_t)0) {
> - /* Special case meaning the whole arena */
> - sm->len[i] = cache_size;
> + if (!cache_size) {
> + fprintf(stderr, "%s: unmap when DAX cache not present\n", __func__);
> + res = -1;
> + break;
> }
>
> if ((sm->c_offset[i] + sm->len[i]) < sm->len[i] ||
>
Thanks for the patch, but I got qemu crash during umount:
qemu/memory.c:2187: memory_region_get_ram_ptr: Assertion `mr->ram_block' failed.
Obviously we don't setup fs->cache when cache_size is zero...
So, maybe we can just do something like this?
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index 6cacdb24b0..459dce5bde 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -93,6 +93,14 @@ uint64_t vhost_user_fs_slave_unmap(struct vhost_dev *dev, VhostUserFSSlaveMsg *s
}
size_t cache_size = fs->conf.cache_size;
if (!cache_size) {
+ /*
+ * Since dax cache is disabled, there should be no unmap request.
+ * Howerver we still receives whole range unmap request during umount
+ * for cleanup. Ignore it.
+ */
+ if (sm->len[0] == ~(uint64_t)0)
+ return 0;
+
fprintf(stderr, "%s: unmap when DAX cache not present\n", __func__);
return (uint64_t)-1;
}
Thanks,
Misono
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Virtio-fs] False error message when DAX cache is disabled
2019-10-25 10:00 ` misono.tomohiro
@ 2019-10-25 10:41 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2019-10-25 10:41 UTC (permalink / raw)
To: misono.tomohiro; +Cc: virtio-fs
* misono.tomohiro@fujitsu.com (misono.tomohiro@fujitsu.com) wrote:
> > * misono.tomohiro@fujitsu.com (misono.tomohiro@fujitsu.com) wrote:
> > > Hi,
> >
> > Hi Misono,
> >
> > > I'm testing virtiofs on ARM enviroment.
> > > Since current ARM does not support DAX, I use cache-size=0 for vhost-user-fs-pci parameter.
> > >
> > > This results in the following error message during unmount:
> > > ```
> > > vhost_user_fs_slave_unmap: unmap when DAX cache not present
> > > lo_destroy: unmap during destroy failed ```
> >
> > Thanks for reporting this,
> >
> > > This is because lo_destroy() always calls fuse_virtio_unmap().
> > > So, I want to fix this by not calling fuse_virtio_unmap() when cache
> > > is not used, but is there any good way for virtiofsd to know vhost-user's cahce-size?
> >
> > Unfortunately not; that's why I used the special ~0 as a 'unmap all'.
> > It feels easier to fix the qemu side to be happy unmapping everything when everything is actually nothing :-)
> >
> > Does the following (build tested only) patch fix it:
> >
> > diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c index 6cacdb24b0..07a04d9b21 100644
> > --- a/hw/virtio/vhost-user-fs.c
> > +++ b/hw/virtio/vhost-user-fs.c
> > @@ -92,10 +92,6 @@ uint64_t vhost_user_fs_slave_unmap(struct vhost_dev *dev, VhostUserFSSlaveMsg *s
> > return -1;
> > }
> > size_t cache_size = fs->conf.cache_size;
> > - if (!cache_size) {
> > - fprintf(stderr, "%s: unmap when DAX cache not present\n", __func__);
> > - return (uint64_t)-1;
> > - }
> > void *cache_host = memory_region_get_ram_ptr(&fs->cache);
> >
> > unsigned int i;
> > @@ -106,13 +102,19 @@ uint64_t vhost_user_fs_slave_unmap(struct vhost_dev *dev, VhostUserFSSlaveMsg *s
> > */
> > for (i = 0; i < VHOST_USER_FS_SLAVE_ENTRIES; i++) {
> > void *ptr;
> > + if (sm->len[i] == ~(uint64_t)0) {
> > + /* Special case meaning the whole arena */
> > + sm->len[i] = cache_size;
> > + }
> > +
> > if (sm->len[i] == 0) {
> > continue;
> > }
> >
> > - if (sm->len[i] == ~(uint64_t)0) {
> > - /* Special case meaning the whole arena */
> > - sm->len[i] = cache_size;
> > + if (!cache_size) {
> > + fprintf(stderr, "%s: unmap when DAX cache not present\n", __func__);
> > + res = -1;
> > + break;
> > }
> >
> > if ((sm->c_offset[i] + sm->len[i]) < sm->len[i] ||
> >
>
> Thanks for the patch, but I got qemu crash during umount:
> qemu/memory.c:2187: memory_region_get_ram_ptr: Assertion `mr->ram_block' failed.
Oops yes, I missed that.
> Obviously we don't setup fs->cache when cache_size is zero...
> So, maybe we can just do something like this?
>
> diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
> index 6cacdb24b0..459dce5bde 100644
> --- a/hw/virtio/vhost-user-fs.c
> +++ b/hw/virtio/vhost-user-fs.c
> @@ -93,6 +93,14 @@ uint64_t vhost_user_fs_slave_unmap(struct vhost_dev *dev, VhostUserFSSlaveMsg *s
> }
> size_t cache_size = fs->conf.cache_size;
> if (!cache_size) {
> + /*
> + * Since dax cache is disabled, there should be no unmap request.
> + * Howerver we still receives whole range unmap request during umount
> + * for cleanup. Ignore it.
> + */
> + if (sm->len[0] == ~(uint64_t)0)
> + return 0;
> +
> fprintf(stderr, "%s: unmap when DAX cache not present\n", __func__);
> return (uint64_t)-1;
> }
Yes, that's simpler; I'll take that - thank you!
Dave
> Thanks,
> Misono
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-25 10:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 9:05 [Virtio-fs] False error message when DAX cache is disabled misono.tomohiro
2019-10-24 17:13 ` Dr. David Alan Gilbert
2019-10-25 10:00 ` misono.tomohiro
2019-10-25 10:41 ` Dr. David Alan Gilbert
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.