[virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

v2:
 * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages

This patch series adds the notification queue to the VIRTIO specification.
This new virtqueue carries device->driver FUSE notify messages.  They are
currently unused but will be necessary for file locking, which can block for an
unbounded amount of time and therefore needs a asynchronous completion event
instead of a request/response buffer that consumes space in the request
virtqueue until the operation completes.

Patch 1 corrects an oversight I noticed: the file system device was not added
to the Conformance chapter.

Stefan Hajnoczi (2):
  virtio-fs: add file system device to Conformance chapter
  virtio-fs: add notification queue

 conformance.tex | 23 ++++++++++++++++
 virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 84 insertions(+), 10 deletions(-)

-- 
2.29.2

[virtio-dev] [PATCH v2 1/2] virtio-fs: add file system device to Conformance chapter

[Stefan Hajnoczi]

The file system device is not listed in the Conformance chapter. Fix
this.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 conformance.tex | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/conformance.tex b/conformance.tex
index 17c390d..9a7fe0b 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -25,6 +25,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Driver Conformance / Input Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / Crypto Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / Socket Driver Conformance},
+\ref{sec:Conformance / Driver Conformance / File System Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / RPMB Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / IOMMU Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / Sound Driver Conformance}
@@ -47,6 +48,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Device Conformance / Input Device Conformance}, 
 \ref{sec:Conformance / Device Conformance / Crypto Device Conformance}, 
 \ref{sec:Conformance / Device Conformance / Socket Device Conformance}, 
+\ref{sec:Conformance / Device Conformance / File System Device Conformance},
 \ref{sec:Conformance / Device Conformance / RPMB Device Conformance},
 \ref{sec:Conformance / Device Conformance / IOMMU Device Conformance},
 \ref{sec:Conformance / Device Conformance / Sound Device Conformance}
@@ -218,6 +220,16 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Device Events}
 \end{itemize}
 
+\conformance{\subsection}{File System Driver Conformance}\label{sec:Conformance / Driver Conformance / File System Driver Conformance}
+
+A file system driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / File System Device / Device configuration layout}
+\item \ref{drivernormative:Device Types / File System Device / Device Operation / Device Operation: High Priority Queue}
+\item \ref{drivernormative:Device Types / File System Device / Device Operation / Device Operation: DAX Window}
+\end{itemize}
+
 \conformance{\subsection}{RPMB Driver Conformance}\label{sec:Conformance / Driver Conformance / RPMB Driver Conformance}
 
 A RPMB driver MUST conform to the following normative statements:
@@ -437,6 +449,16 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Device Types / Socket Device / Device Operation / Receive and Transmit}
 \end{itemize}
 
+\conformance{\subsection}{File System Device Conformance}\label{sec:Conformance / Device Conformance / File System Device Conformance}
+
+A file system device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / File System Device / Device configuration layout}
+\item \ref{devicenormative:Device Types / File System Device / Device Operation / Device Operation: High Priority Queue}
+\item \ref{devicenormative:Device Types / File System Device / Device Operation / Device Operation: DAX Window}
+\end{itemize}
+
 \conformance{\subsection}{RPMB Device Conformance}\label{sec:Conformance / Device Conformance / RPMB Device Conformance}
 
 An RPMB device MUST conform to the following normative statements:
-- 
2.29.2

[virtio-dev] [PATCH v2 2/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

The FUSE protocol allows the file server (device) to initiate
communication with the client (driver) using FUSE notify messages.
Normally only the client can initiate communication. This feature is
used to report asynchronous events that are not related to an in-flight
request.

This patch adds a notification queue that works like an rx queue in
other VIRTIO device types. The device can emit FUSE notify messages by
using a buffer from this queue.

This mechanism was designed by Vivek Goyal <vgoyal@redhat.com>.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 conformance.tex |  1 +
 virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 9a7fe0b..8c2f511 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -227,6 +227,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \begin{itemize}
 \item \ref{drivernormative:Device Types / File System Device / Device configuration layout}
 \item \ref{drivernormative:Device Types / File System Device / Device Operation / Device Operation: High Priority Queue}
+\item \ref{drivernormative:Device Types / File System Device / Device Operation / Device Operation: Notification Queue}
 \item \ref{drivernormative:Device Types / File System Device / Device Operation / Device Operation: DAX Window}
 \end{itemize}
 
diff --git a/virtio-fs.tex b/virtio-fs.tex
index 158d066..c995748 100644
--- a/virtio-fs.tex
+++ b/virtio-fs.tex
@@ -25,24 +25,33 @@ \subsection{Virtqueues}\label{sec:Device Types / File System Device / Virtqueues
 
 \begin{description}
 \item[0] hiprio
-\item[1\ldots n] request queues
+\item[1] notification queue
+\item[2\ldots n] request queues
 \end{description}
 
+The notification queue only exists if VIRTIO_FS_F_NOTIFICATION is set.
+
 \subsection{Feature bits}\label{sec:Device Types / File System Device / Feature bits}
 
-There are currently no feature bits defined.
+\begin{description}
+\item[VIRTIO_FS_F_NOTIFICATION (0)] Device has support for FUSE notify
+    messages.  The notification queue is virtqueue 1.
+\end{description}
 
 \subsection{Device configuration layout}\label{sec:Device Types / File System Device / Device configuration layout}
 
-All fields of this configuration are always available.
-
 \begin{lstlisting}
 struct virtio_fs_config {
         char tag[36];
         le32 num_request_queues;
+        le32 notify_buf_size;
 };
 \end{lstlisting}
 
+The \field{tag} and \field{num_request_queues} fields are always available.
+The \field{notify_buf_size} field is only available when
+VIRTIO_FS_F_NOTIFICATION is set.
+
 \begin{description}
 \item[\field{tag}] is the name associated with this file system.  The tag is
     encoded in UTF-8 and padded with NUL bytes if shorter than the
@@ -53,6 +62,8 @@ \subsection{Device configuration layout}\label{sec:Device Types / File System De
     there are no ordering guarantees between requests made available on
     different queues.  Use of multiple queues is intended to increase
     performance.
+\item[\field{notify_buf_size}] is the minimum number of bytes required for each
+    buffer in the notification queue.
 \end{description}
 
 \drivernormative{\subsubsection}{Device configuration layout}{Device Types / File System Device / Device configuration layout}
@@ -65,13 +76,20 @@ \subsection{Device configuration layout}\label{sec:Device Types / File System De
 
 The device MUST set \field{num_request_queues} to 1 or greater.
 
+The device MUST set \field{notify_buf_size} to be large enough to hold any of
+the FUSE notify messages that this device emits.
+
 \subsection{Device Initialization}\label{Device Types / File System Device / Device Initialization}
 
-On initialization the driver first discovers the device's virtqueues.  The FUSE
-session is started by sending a FUSE\_INIT request as defined by the FUSE
-protocol on one request virtqueue.  All virtqueues provide access to the same
-FUSE session and therefore only one FUSE\_INIT request is required regardless
-of the number of available virtqueues.
+On initialization the driver first discovers the device's virtqueues.
+
+The driver populates the notification queue with buffers for receiving FUSE
+notify messages if VIRTIO_FS_F_NOTIFICATION is set.
+
+The FUSE session is started by sending a FUSE\_INIT request as defined by the
+FUSE protocol on one request virtqueue.  All virtqueues provide access to the
+same FUSE session and therefore only one FUSE\_INIT request is required
+regardless of the number of available virtqueues.
 
 \subsection{Device Operation}\label{sec:Device Types / File System Device / Device Operation}
 
@@ -88,7 +106,8 @@ \subsection{Device Operation}\label{sec:Device Types / File System Device / Devi
       full.
 \end{itemize}
 
-Note that FUSE notification requests are not supported.
+FUSE notify messages are received on the notification queue if
+VIRTIO_FS_F_NOTIFICATION is set.
 
 \subsubsection{Device Operation: Request Queues}\label{sec:Device Types / File System Device / Device Operation / Device Operation: Request Queues}
 
@@ -179,6 +198,38 @@ \subsubsection{Device Operation: High Priority Queue}\label{sec:Device Types / F
 
 The driver MUST anticipate that request queues are processed concurrently with the hiprio queue.
 
+\subsubsection{Device Operation: Notification Queue}\label{sec:Device Types / File System Device / Device Operation / Device Operation: Notification Queue}
+
+The notification queue is populated with buffers by the driver and these
+buffers are used by the device to emit FUSE notify messages.  Notification
+queue buffer layout is as follows:
+
+\begin{lstlisting}
+struct virtio_fs_notify {
+        // Device-writable part
+        struct fuse_out_header out_hdr;
+        char outarg[notify_buf_size - sizeof(struct fuse_out_header)];
+};
+\end{lstlisting}
+
+\field{outarg} contains the FUSE notify message payload that depends on the
+type of notification being emitted.
+
+If the driver provides notification queue buffers at a slower rate than the
+device emits FUSE notify messages then the virtqueue will eventually become
+empty.  The behavior in response to an empty virtqueue depends on the FUSE
+notify message type:
+\begin{itemize}
+\item FUSE\_NOTIFY\_LOCK messages are delivered when buffers become available again. When the device runs out of resources new lock requests fail with ENOLCK.
+\end{itemize}
+
+\drivernormative{\paragraph}{Device Operation: Notification Queue}{Device Types / File System Device / Device Operation / Device Operation: Notification Queue}
+
+The driver MUST provide buffers of at least \field{notify_buf_size} bytes.
+
+The driver SHOULD replenish notification queue buffers sufficiently quickly so
+that there is always at least one available buffer.
+
 \subsubsection{Device Operation: DAX Window}\label{sec:Device Types / File System Device / Device Operation / Device Operation: DAX Window}
 
 FUSE\_READ and FUSE\_WRITE requests transfer file contents between the
-- 
2.29.2

[virtio-dev] Re: [PATCH v2 0/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> v2:
>  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> 
> This patch series adds the notification queue to the VIRTIO specification.
> This new virtqueue carries device->driver FUSE notify messages.  They are
> currently unused but will be necessary for file locking, which can block for an
> unbounded amount of time and therefore needs a asynchronous completion event
> instead of a request/response buffer that consumes space in the request
> virtqueue until the operation completes.
> 
> Patch 1 corrects an oversight I noticed: the file system device was not added
> to the Conformance chapter.
> 
> Stefan Hajnoczi (2):
>   virtio-fs: add file system device to Conformance chapter
>   virtio-fs: add notification queue
> 
>  conformance.tex | 23 ++++++++++++++++
>  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 84 insertions(+), 10 deletions(-)

Ping?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> v2:
>  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> 
> This patch series adds the notification queue to the VIRTIO specification.
> This new virtqueue carries device->driver FUSE notify messages.  They are
> currently unused but will be necessary for file locking, which can block for an
> unbounded amount of time and therefore needs a asynchronous completion event
> instead of a request/response buffer that consumes space in the request
> virtqueue until the operation completes.
> 
> Patch 1 corrects an oversight I noticed: the file system device was not added
> to the Conformance chapter.
> 
> Stefan Hajnoczi (2):
>   virtio-fs: add file system device to Conformance chapter
>   virtio-fs: add notification queue
> 
>  conformance.tex | 23 ++++++++++++++++
>  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 84 insertions(+), 10 deletions(-)

Reminder to anyone who needs the virtio-fs notification queue: please
review this series.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> v2:
>  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> 
> This patch series adds the notification queue to the VIRTIO specification.
> This new virtqueue carries device->driver FUSE notify messages.  They are
> currently unused but will be necessary for file locking, which can block for an
> unbounded amount of time and therefore needs a asynchronous completion event
> instead of a request/response buffer that consumes space in the request
> virtqueue until the operation completes.
> 
> Patch 1 corrects an oversight I noticed: the file system device was not added
> to the Conformance chapter.
> 
> Stefan Hajnoczi (2):
>   virtio-fs: add file system device to Conformance chapter
>   virtio-fs: add notification queue
> 
>  conformance.tex | 23 ++++++++++++++++
>  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 84 insertions(+), 10 deletions(-)

Reminder to anyone who needs the virtio-fs notification queue: please
review this series.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Liu Bo]

Hi Stefan,

On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > v2:
> >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > 
> > This patch series adds the notification queue to the VIRTIO specification.
> > This new virtqueue carries device->driver FUSE notify messages.  They are
> > currently unused but will be necessary for file locking, which can block for an
> > unbounded amount of time and therefore needs a asynchronous completion event
> > instead of a request/response buffer that consumes space in the request
> > virtqueue until the operation completes.
> > 
> > Patch 1 corrects an oversight I noticed: the file system device was not added
> > to the Conformance chapter.
> > 
> > Stefan Hajnoczi (2):
> >   virtio-fs: add file system device to Conformance chapter
> >   virtio-fs: add notification queue
> > 
> >  conformance.tex | 23 ++++++++++++++++
> >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> >  2 files changed, 84 insertions(+), 10 deletions(-)
> 
> Reminder to anyone who needs the virtio-fs notification queue: please
> review this series.
>

Besides using notification queue to provide posix lock support, I've
also managed to invalidate dentry/inode's cache with notification
queue, it worked well.

I've read the patches and it looks good to me, so

Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>

(Somehow I couldn't find the original patches in my mailbox, but I've
read both at
https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg07062.html
https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg07222.html
)

thanks,
liubo

> Stefan



> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://listman.redhat.com/mailman/listinfo/virtio-fs

[virtio-dev] Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]

On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> Hi Stefan,
> 
> On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > v2:
> > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > 
> > > This patch series adds the notification queue to the VIRTIO specification.
> > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > currently unused but will be necessary for file locking, which can block for an
> > > unbounded amount of time and therefore needs a asynchronous completion event
> > > instead of a request/response buffer that consumes space in the request
> > > virtqueue until the operation completes.
> > > 
> > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > to the Conformance chapter.
> > > 
> > > Stefan Hajnoczi (2):
> > >   virtio-fs: add file system device to Conformance chapter
> > >   virtio-fs: add notification queue
> > > 
> > >  conformance.tex | 23 ++++++++++++++++
> > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > 
> > Reminder to anyone who needs the virtio-fs notification queue: please
> > review this series.
> >
> 
> Besides using notification queue to provide posix lock support, I've
> also managed to invalidate dentry/inode's cache with notification
> queue, it worked well.

Thank you!

Are you using dentry/inode cache invalidation to reduce the number of
file descriptors that virtiofsd needs to hold open, or are you using it
because the file system is shared by multiple systems and you want to
stronger cache coherency?

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]

On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> Hi Stefan,
> 
> On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > v2:
> > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > 
> > > This patch series adds the notification queue to the VIRTIO specification.
> > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > currently unused but will be necessary for file locking, which can block for an
> > > unbounded amount of time and therefore needs a asynchronous completion event
> > > instead of a request/response buffer that consumes space in the request
> > > virtqueue until the operation completes.
> > > 
> > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > to the Conformance chapter.
> > > 
> > > Stefan Hajnoczi (2):
> > >   virtio-fs: add file system device to Conformance chapter
> > >   virtio-fs: add notification queue
> > > 
> > >  conformance.tex | 23 ++++++++++++++++
> > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > 
> > Reminder to anyone who needs the virtio-fs notification queue: please
> > review this series.
> >
> 
> Besides using notification queue to provide posix lock support, I've
> also managed to invalidate dentry/inode's cache with notification
> queue, it worked well.

Thank you!

Are you using dentry/inode cache invalidation to reduce the number of
file descriptors that virtiofsd needs to hold open, or are you using it
because the file system is shared by multiple systems and you want to
stronger cache coherency?

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Liu Bo]

On Thu, May 13, 2021 at 02:44:42PM +0100, Stefan Hajnoczi wrote:
> On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> > Hi Stefan,
> > 
> > On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > > v2:
> > > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > > 
> > > > This patch series adds the notification queue to the VIRTIO specification.
> > > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > > currently unused but will be necessary for file locking, which can block for an
> > > > unbounded amount of time and therefore needs a asynchronous completion event
> > > > instead of a request/response buffer that consumes space in the request
> > > > virtqueue until the operation completes.
> > > > 
> > > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > > to the Conformance chapter.
> > > > 
> > > > Stefan Hajnoczi (2):
> > > >   virtio-fs: add file system device to Conformance chapter
> > > >   virtio-fs: add notification queue
> > > > 
> > > >  conformance.tex | 23 ++++++++++++++++
> > > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > > 
> > > Reminder to anyone who needs the virtio-fs notification queue: please
> > > review this series.
> > >
> > 
> > Besides using notification queue to provide posix lock support, I've
> > also managed to invalidate dentry/inode's cache with notification
> > queue, it worked well.
> 
> Thank you!
> 
> Are you using dentry/inode cache invalidation to reduce the number of
> file descriptors that virtiofsd needs to hold open, or are you using it
> because the file system is shared by multiple systems and you want to
> stronger cache coherency?
>

The former one is one of the problems I've come across, but I worked
around it by set_rlimit'ing a large enough number of fds.

My scenario is that

a) a bind mount point was shared as a sub-directory of virtiofs's shared directory,

b) and I needed to umount the bind mount point on the host side but I
was not able to because virtiofsd enables cache=always,

So basically it was used as a preciser "drop_cache".

Besides that, I'm going to do some experiements with notification
queue to give a warm up for fuse's metadata in order to have less
metadata requests.  Although this may be done with ebpf as well, the
idea with notification queue seems more straightforward.

thanks,
liubo

[virtio-dev] Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3442 bytes --]

On Fri, May 14, 2021 at 01:50:06AM +0800, Liu Bo wrote:
> On Thu, May 13, 2021 at 02:44:42PM +0100, Stefan Hajnoczi wrote:
> > On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> > > Hi Stefan,
> > > 
> > > On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > > > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > > > v2:
> > > > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > > > 
> > > > > This patch series adds the notification queue to the VIRTIO specification.
> > > > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > > > currently unused but will be necessary for file locking, which can block for an
> > > > > unbounded amount of time and therefore needs a asynchronous completion event
> > > > > instead of a request/response buffer that consumes space in the request
> > > > > virtqueue until the operation completes.
> > > > > 
> > > > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > > > to the Conformance chapter.
> > > > > 
> > > > > Stefan Hajnoczi (2):
> > > > >   virtio-fs: add file system device to Conformance chapter
> > > > >   virtio-fs: add notification queue
> > > > > 
> > > > >  conformance.tex | 23 ++++++++++++++++
> > > > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > > > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > > > 
> > > > Reminder to anyone who needs the virtio-fs notification queue: please
> > > > review this series.
> > > >
> > > 
> > > Besides using notification queue to provide posix lock support, I've
> > > also managed to invalidate dentry/inode's cache with notification
> > > queue, it worked well.
> > 
> > Thank you!
> > 
> > Are you using dentry/inode cache invalidation to reduce the number of
> > file descriptors that virtiofsd needs to hold open, or are you using it
> > because the file system is shared by multiple systems and you want to
> > stronger cache coherency?
> >
> 
> The former one is one of the problems I've come across, but I worked
> around it by set_rlimit'ing a large enough number of fds.
> 
> My scenario is that
> 
> a) a bind mount point was shared as a sub-directory of virtiofs's shared directory,
> 
> b) and I needed to umount the bind mount point on the host side but I
> was not able to because virtiofsd enables cache=always,
> 
> So basically it was used as a preciser "drop_cache".
> 
> Besides that, I'm going to do some experiements with notification
> queue to give a warm up for fuse's metadata in order to have less
> metadata requests.  Although this may be done with ebpf as well, the
> idea with notification queue seems more straightforward.

Thanks for sharing. I was curious because I shared the NFSv4
FH4_NOEXPIRE_WITH_OPEN approach in a previous virtio-fs bi-weekly call:
https://www.rfc-editor.org/rfc/rfc7530.html#section-4.2.3

This approach does not involve the notification queue. It's lazy in the
sense that the client is unaware which inode have expired until it
accesses them. An advantage is that no host<->guest communication is
required (fewer CPU cycles), but the disadvantage is that the client may
have expired inode/dentry cache structures allocated so it will end up
throwing them away but was unable to use the memory for something else
in the meantime.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3442 bytes --]

On Fri, May 14, 2021 at 01:50:06AM +0800, Liu Bo wrote:
> On Thu, May 13, 2021 at 02:44:42PM +0100, Stefan Hajnoczi wrote:
> > On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> > > Hi Stefan,
> > > 
> > > On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > > > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > > > v2:
> > > > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > > > 
> > > > > This patch series adds the notification queue to the VIRTIO specification.
> > > > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > > > currently unused but will be necessary for file locking, which can block for an
> > > > > unbounded amount of time and therefore needs a asynchronous completion event
> > > > > instead of a request/response buffer that consumes space in the request
> > > > > virtqueue until the operation completes.
> > > > > 
> > > > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > > > to the Conformance chapter.
> > > > > 
> > > > > Stefan Hajnoczi (2):
> > > > >   virtio-fs: add file system device to Conformance chapter
> > > > >   virtio-fs: add notification queue
> > > > > 
> > > > >  conformance.tex | 23 ++++++++++++++++
> > > > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > > > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > > > 
> > > > Reminder to anyone who needs the virtio-fs notification queue: please
> > > > review this series.
> > > >
> > > 
> > > Besides using notification queue to provide posix lock support, I've
> > > also managed to invalidate dentry/inode's cache with notification
> > > queue, it worked well.
> > 
> > Thank you!
> > 
> > Are you using dentry/inode cache invalidation to reduce the number of
> > file descriptors that virtiofsd needs to hold open, or are you using it
> > because the file system is shared by multiple systems and you want to
> > stronger cache coherency?
> >
> 
> The former one is one of the problems I've come across, but I worked
> around it by set_rlimit'ing a large enough number of fds.
> 
> My scenario is that
> 
> a) a bind mount point was shared as a sub-directory of virtiofs's shared directory,
> 
> b) and I needed to umount the bind mount point on the host side but I
> was not able to because virtiofsd enables cache=always,
> 
> So basically it was used as a preciser "drop_cache".
> 
> Besides that, I'm going to do some experiements with notification
> queue to give a warm up for fuse's metadata in order to have less
> metadata requests.  Although this may be done with ebpf as well, the
> idea with notification queue seems more straightforward.

Thanks for sharing. I was curious because I shared the NFSv4
FH4_NOEXPIRE_WITH_OPEN approach in a previous virtio-fs bi-weekly call:
https://www.rfc-editor.org/rfc/rfc7530.html#section-4.2.3

This approach does not involve the notification queue. It's lazy in the
sense that the client is unaware which inode have expired until it
accesses them. An advantage is that no host<->guest communication is
required (fewer CPU cycles), but the disadvantage is that the client may
have expired inode/dentry cache structures allocated so it will end up
throwing them away but was unable to use the memory for something else
in the meantime.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[virtio-dev] Re: [PATCH v2 2/2] virtio-fs: add notification queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 10517 bytes --]

On Fri, May 14, 2021 at 12:33:20PM -0400, Vivek Goyal wrote:
> On Mon, Feb 15, 2021 at 09:54:10AM +0000, Stefan Hajnoczi wrote:
> > The FUSE protocol allows the file server (device) to initiate
> > communication with the client (driver) using FUSE notify messages.
> > Normally only the client can initiate communication. This feature is
> > used to report asynchronous events that are not related to an in-flight
> > request.
> > 
> > This patch adds a notification queue that works like an rx queue in
> > other VIRTIO device types. The device can emit FUSE notify messages by
> > using a buffer from this queue.
> > 
> > This mechanism was designed by Vivek Goyal <vgoyal@redhat.com>.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  conformance.tex |  1 +
> >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> >  2 files changed, 62 insertions(+), 10 deletions(-)
> > 
> > diff --git a/conformance.tex b/conformance.tex
> > index 9a7fe0b..8c2f511 100644
> > --- a/conformance.tex
> > +++ b/conformance.tex
> > @@ -227,6 +227,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \begin{itemize}
> >  \item \ref{drivernormative:Device Types / File System Device / Device configuration layout}
> >  \item \ref{drivernormative:Device Types / File System Device / Device Operation / Device Operation: High Priority Queue}
> > +\item \ref{drivernormative:Device Types / File System Device / Device Operation / Device Operation: Notification Queue}
> >  \item \ref{drivernormative:Device Types / File System Device / Device Operation / Device Operation: DAX Window}
> >  \end{itemize}
> >  
> > diff --git a/virtio-fs.tex b/virtio-fs.tex
> > index 158d066..c995748 100644
> > --- a/virtio-fs.tex
> > +++ b/virtio-fs.tex
> > @@ -25,24 +25,33 @@ \subsection{Virtqueues}\label{sec:Device Types / File System Device / Virtqueues
> >  
> >  \begin{description}
> >  \item[0] hiprio
> > -\item[1\ldots n] request queues
> > +\item[1] notification queue
> > +\item[2\ldots n] request queues
> >  \end{description}
> >  
> > +The notification queue only exists if VIRTIO_FS_F_NOTIFICATION is set.
> 
> Hi Stefan,
> 
> Say device is new enough that it sets Feature bit, VIRTIO_FS_F_NOTIFICATION
> but driver is old and does not understand this feature. I am assuming for
> these cases protocol allows for feature negotiation and if driver does not
> acknowledge VIRTIO_FS_F_NOTIFICATION feature, then queue at index 1, will
> be a request queue and not notification queue?

Yes, exactly. The virtqueue layout is only decided when feature
negotiation completes. If the driver did not enable
VIRTIO_FS_F_NOTIFICATION then the bit will be cleared and virtqueues
will use the traditional layout without a notification queue.

> > +
> >  \subsection{Feature bits}\label{sec:Device Types / File System Device / Feature bits}
> >  
> > -There are currently no feature bits defined.
> > +\begin{description}
> > +\item[VIRTIO_FS_F_NOTIFICATION (0)] Device has support for FUSE notify
> > +    messages.  The notification queue is virtqueue 1.
> > +\end{description}
> >  
> >  \subsection{Device configuration layout}\label{sec:Device Types / File System Device / Device configuration layout}
> >  
> > -All fields of this configuration are always available.
> > -
> >  \begin{lstlisting}
> >  struct virtio_fs_config {
> >          char tag[36];
> >          le32 num_request_queues;
> > +        le32 notify_buf_size;
> >  };
> >  \end{lstlisting}
> >  
> > +The \field{tag} and \field{num_request_queues} fields are always available.
> > +The \field{notify_buf_size} field is only available when
> > +VIRTIO_FS_F_NOTIFICATION is set.
> > +
> >  \begin{description}
> >  \item[\field{tag}] is the name associated with this file system.  The tag is
> >      encoded in UTF-8 and padded with NUL bytes if shorter than the
> > @@ -53,6 +62,8 @@ \subsection{Device configuration layout}\label{sec:Device Types / File System De
> >      there are no ordering guarantees between requests made available on
> >      different queues.  Use of multiple queues is intended to increase
> >      performance.
> > +\item[\field{notify_buf_size}] is the minimum number of bytes required for each
> > +    buffer in the notification queue.
> >  \end{description}
> >  
> >  \drivernormative{\subsubsection}{Device configuration layout}{Device Types / File System Device / Device configuration layout}
> > @@ -65,13 +76,20 @@ \subsection{Device configuration layout}\label{sec:Device Types / File System De
> >  
> >  The device MUST set \field{num_request_queues} to 1 or greater.
> >  
> > +The device MUST set \field{notify_buf_size} to be large enough to hold any of
> > +the FUSE notify messages that this device emits.
> > +
> >  \subsection{Device Initialization}\label{Device Types / File System Device / Device Initialization}
> >  
> > -On initialization the driver first discovers the device's virtqueues.  The FUSE
> > -session is started by sending a FUSE\_INIT request as defined by the FUSE
> > -protocol on one request virtqueue.  All virtqueues provide access to the same
> > -FUSE session and therefore only one FUSE\_INIT request is required regardless
> > -of the number of available virtqueues.
> > +On initialization the driver first discovers the device's virtqueues.
> > +
> > +The driver populates the notification queue with buffers for receiving FUSE
> > +notify messages if VIRTIO_FS_F_NOTIFICATION is set.
> > +
> > +The FUSE session is started by sending a FUSE\_INIT request as defined by the
> > +FUSE protocol on one request virtqueue.  All virtqueues provide access to the
> > +same FUSE session and therefore only one FUSE\_INIT request is required
> > +regardless of the number of available virtqueues.
> >  
> >  \subsection{Device Operation}\label{sec:Device Types / File System Device / Device Operation}
> >  
> > @@ -88,7 +106,8 @@ \subsection{Device Operation}\label{sec:Device Types / File System Device / Devi
> >        full.
> >  \end{itemize}
> >  
> > -Note that FUSE notification requests are not supported.
> > +FUSE notify messages are received on the notification queue if
> > +VIRTIO_FS_F_NOTIFICATION is set.
> >  
> >  \subsubsection{Device Operation: Request Queues}\label{sec:Device Types / File System Device / Device Operation / Device Operation: Request Queues}
> >  
> > @@ -179,6 +198,38 @@ \subsubsection{Device Operation: High Priority Queue}\label{sec:Device Types / F
> >  
> >  The driver MUST anticipate that request queues are processed concurrently with the hiprio queue.
> >  
> > +\subsubsection{Device Operation: Notification Queue}\label{sec:Device Types / File System Device / Device Operation / Device Operation: Notification Queue}
> > +
> > +The notification queue is populated with buffers by the driver and these
> > +buffers are used by the device to emit FUSE notify messages.  Notification
> > +queue buffer layout is as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_fs_notify {
> > +        // Device-writable part
> > +        struct fuse_out_header out_hdr;
> > +        char outarg[notify_buf_size - sizeof(struct fuse_out_header)];
> > +};
> > +\end{lstlisting}
> > +
> > +\field{outarg} contains the FUSE notify message payload that depends on the
> > +type of notification being emitted.
> > +
> > +If the driver provides notification queue buffers at a slower rate than the
> > +device emits FUSE notify messages then the virtqueue will eventually become
> > +empty.  The behavior in response to an empty virtqueue depends on the FUSE
> > +notify message type:
> 
> So we are right now definiting the behavior of only one notification
> message, FUSE_NOTIFY_LOCK. Others will be filled later? Or it will be
> left to device implementation.

Others will be added to the spec when their use with virtiofs is defined.

> > +\begin{itemize}
> > +\item FUSE\_NOTIFY\_LOCK messages are delivered when buffers become available again. When the device runs out of resources new lock requests fail with ENOLCK.
> 
> So device should check for available notification buffer every time a lock
> request comes in to make sure atleast one buffer is available. But it is
> possible that 5 requests come and only 1 notification buffer is available
> and they all will succeed. And by the time lock is available, 4
> notifications will still have to wait. 
> 
> IOW, we probably should define the notion of buffering of events in 
> device and deliver to driver when notification buffers are available.
> And leave it to device when should it start returning ENOLOCK. Some
> devices might decide to buffer X number of notifications and if number
> of buffered notifications cross X, then start returning ENOLOCK.

I'll reword this part because that's exactly what I meant: the device
has internal resources for blocking FUSE_NOTIFY_LOCK messages. These are
internal resources, they are separate from the empty/full state of the
notification queue. The device will refuse to accept new blocking lock
requests with ENOLCK if it runs out of internal resources.

> I guess we have to return ENOLOCK only for waiting lock messages
> (F_SETLKW). For non-blocking locks, we will immediately return either with
> lock held or with error EACCESS or EAGAIN.
> 
> > +\end{itemize}
> > +
> > +\drivernormative{\paragraph}{Device Operation: Notification Queue}{Device Types / File System Device / Device Operation / Device Operation: Notification Queue}
> > +
> > +The driver MUST provide buffers of at least \field{notify_buf_size} bytes.
> > +
> > +The driver SHOULD replenish notification queue buffers sufficiently quickly so
> > +that there is always at least one available buffer.
> 
> I am wondering how can driver ensure that there is always atleast one
> available buffer. Its possible driver is still processing all
> notifications and leaving a brief interval where notification queue
> is empty. IOW, dirvers can try but can't guarantee this, I guess.

Yes, that is why SHOULD is used instead of MUST. It's a best-effort
thing and the behavior for running out of resources is defined above
(FUSE_NOTIFY_LOCK have device internal resources associated with them so
they can wait for new notification queue buffers to become available
without dropping notifications).

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Vivek Goyal]

On Thu, May 13, 2021 at 02:44:42PM +0100, Stefan Hajnoczi wrote:
> On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> > Hi Stefan,
> > 
> > On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > > v2:
> > > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > > 
> > > > This patch series adds the notification queue to the VIRTIO specification.
> > > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > > currently unused but will be necessary for file locking, which can block for an
> > > > unbounded amount of time and therefore needs a asynchronous completion event
> > > > instead of a request/response buffer that consumes space in the request
> > > > virtqueue until the operation completes.
> > > > 
> > > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > > to the Conformance chapter.
> > > > 
> > > > Stefan Hajnoczi (2):
> > > >   virtio-fs: add file system device to Conformance chapter
> > > >   virtio-fs: add notification queue
> > > > 
> > > >  conformance.tex | 23 ++++++++++++++++
> > > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > > 
> > > Reminder to anyone who needs the virtio-fs notification queue: please
> > > review this series.
> > >
> > 
> > Besides using notification queue to provide posix lock support, I've
> > also managed to invalidate dentry/inode's cache with notification
> > queue, it worked well.
> 
> Thank you!
> 
> Are you using dentry/inode cache invalidation to reduce the number of
> file descriptors that virtiofsd needs to hold open,

Using notifications to invalidate client's dentry/inode cache sounds
interesting. Will like to see an actual implementation though. Various
races w.r.t opening the same file for which server is trying to invalidate
the dentry/inode might be little tricky.

Thanks
Vivek

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Vivek Goyal]

On Fri, May 14, 2021 at 01:50:06AM +0800, Liu Bo wrote:
> On Thu, May 13, 2021 at 02:44:42PM +0100, Stefan Hajnoczi wrote:
> > On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> > > Hi Stefan,
> > > 
> > > On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > > > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > > > v2:
> > > > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > > > 
> > > > > This patch series adds the notification queue to the VIRTIO specification.
> > > > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > > > currently unused but will be necessary for file locking, which can block for an
> > > > > unbounded amount of time and therefore needs a asynchronous completion event
> > > > > instead of a request/response buffer that consumes space in the request
> > > > > virtqueue until the operation completes.
> > > > > 
> > > > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > > > to the Conformance chapter.
> > > > > 
> > > > > Stefan Hajnoczi (2):
> > > > >   virtio-fs: add file system device to Conformance chapter
> > > > >   virtio-fs: add notification queue
> > > > > 
> > > > >  conformance.tex | 23 ++++++++++++++++
> > > > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > > > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > > > 
> > > > Reminder to anyone who needs the virtio-fs notification queue: please
> > > > review this series.
> > > >
> > > 
> > > Besides using notification queue to provide posix lock support, I've
> > > also managed to invalidate dentry/inode's cache with notification
> > > queue, it worked well.
> > 
> > Thank you!
> > 
> > Are you using dentry/inode cache invalidation to reduce the number of
> > file descriptors that virtiofsd needs to hold open, or are you using it
> > because the file system is shared by multiple systems and you want to
> > stronger cache coherency?
> >
> 
> The former one is one of the problems I've come across, but I worked
> around it by set_rlimit'ing a large enough number of fds.
> 
> My scenario is that
> 
> a) a bind mount point was shared as a sub-directory of virtiofs's shared directory,
> 
> b) and I needed to umount the bind mount point on the host side but I
> was not able to because virtiofsd enables cache=always,
> 
> So basically it was used as a preciser "drop_cache".
> 
> Besides that, I'm going to do some experiements with notification
> queue to give a warm up for fuse's metadata in order to have less
> metadata requests.  Although this may be done with ebpf as well, the
> idea with notification queue seems more straightforward.

Interesting idea. Pre-filling caches. This probably is more useful only
with cache=always. Cache=auto will invalidate metadata anyway after 1
second. Also it requires one to know which metadata workload is likely
to exercise otherwise you will consume more memory.

Thanks
Vivek

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Liu Bo]

On Mon, May 17, 2021 at 08:48:29AM -0400, Vivek Goyal wrote:
> On Thu, May 13, 2021 at 02:44:42PM +0100, Stefan Hajnoczi wrote:
> > On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> > > Hi Stefan,
> > > 
> > > On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > > > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > > > v2:
> > > > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > > > 
> > > > > This patch series adds the notification queue to the VIRTIO specification.
> > > > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > > > currently unused but will be necessary for file locking, which can block for an
> > > > > unbounded amount of time and therefore needs a asynchronous completion event
> > > > > instead of a request/response buffer that consumes space in the request
> > > > > virtqueue until the operation completes.
> > > > > 
> > > > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > > > to the Conformance chapter.
> > > > > 
> > > > > Stefan Hajnoczi (2):
> > > > >   virtio-fs: add file system device to Conformance chapter
> > > > >   virtio-fs: add notification queue
> > > > > 
> > > > >  conformance.tex | 23 ++++++++++++++++
> > > > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > > > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > > > 
> > > > Reminder to anyone who needs the virtio-fs notification queue: please
> > > > review this series.
> > > >
> > > 
> > > Besides using notification queue to provide posix lock support, I've
> > > also managed to invalidate dentry/inode's cache with notification
> > > queue, it worked well.
> > 
> > Thank you!
> > 
> > Are you using dentry/inode cache invalidation to reduce the number of
> > file descriptors that virtiofsd needs to hold open,
> 
> Using notifications to invalidate client's dentry/inode cache sounds
> interesting. Will like to see an actual implementation though. Various
> races w.r.t opening the same file for which server is trying to invalidate
> the dentry/inode might be little tricky.
>

Right, to avoid the race, the invalidation should only be done when
both host and guest are under control.

IIRC, on virtiofsd, I did it in a very hacky/experimental way,
firstly, I defined a unlinked file that one can only access it by
looking up /proc/{virtiofsd's pid}/fd and write the relative file path
to it, and then I made use of signal SIGUSR2 to read the path from it
and send an invalidation request over notification queue.

> Thanks
> Vivek

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Vivek Goyal]

On Wed, May 19, 2021 at 02:46:14AM +0800, Liu Bo wrote:
> On Mon, May 17, 2021 at 08:48:29AM -0400, Vivek Goyal wrote:
> > On Thu, May 13, 2021 at 02:44:42PM +0100, Stefan Hajnoczi wrote:
> > > On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> > > > Hi Stefan,
> > > > 
> > > > On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > > > > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > > > > v2:
> > > > > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > > > > 
> > > > > > This patch series adds the notification queue to the VIRTIO specification.
> > > > > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > > > > currently unused but will be necessary for file locking, which can block for an
> > > > > > unbounded amount of time and therefore needs a asynchronous completion event
> > > > > > instead of a request/response buffer that consumes space in the request
> > > > > > virtqueue until the operation completes.
> > > > > > 
> > > > > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > > > > to the Conformance chapter.
> > > > > > 
> > > > > > Stefan Hajnoczi (2):
> > > > > >   virtio-fs: add file system device to Conformance chapter
> > > > > >   virtio-fs: add notification queue
> > > > > > 
> > > > > >  conformance.tex | 23 ++++++++++++++++
> > > > > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > > > > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > > > > 
> > > > > Reminder to anyone who needs the virtio-fs notification queue: please
> > > > > review this series.
> > > > >
> > > > 
> > > > Besides using notification queue to provide posix lock support, I've
> > > > also managed to invalidate dentry/inode's cache with notification
> > > > queue, it worked well.
> > > 
> > > Thank you!
> > > 
> > > Are you using dentry/inode cache invalidation to reduce the number of
> > > file descriptors that virtiofsd needs to hold open,
> > 
> > Using notifications to invalidate client's dentry/inode cache sounds
> > interesting. Will like to see an actual implementation though. Various
> > races w.r.t opening the same file for which server is trying to invalidate
> > the dentry/inode might be little tricky.
> >
> 
> Right, to avoid the race, the invalidation should only be done when
> both host and guest are under control.
> 
> IIRC, on virtiofsd, I did it in a very hacky/experimental way,
> firstly, I defined a unlinked file that one can only access it by
> looking up /proc/{virtiofsd's pid}/fd and write the relative file path
> to it,

Was it a file opened with open flag "O_TMPFILE"?

And contents of this file is path of the mount point which you want
to unmount?

> and then I made use of signal SIGUSR2 to read the path from it
> and send an invalidation request over notification queue.

Ok.

Thanks
Vivek

Re: [Virtio-fs] [virtio-dev] [PATCH v2 0/2] virtio-fs: add notification queue

[Liu Bo]

On Tue, May 18, 2021 at 04:42:08PM -0400, Vivek Goyal wrote:
> On Wed, May 19, 2021 at 02:46:14AM +0800, Liu Bo wrote:
> > On Mon, May 17, 2021 at 08:48:29AM -0400, Vivek Goyal wrote:
> > > On Thu, May 13, 2021 at 02:44:42PM +0100, Stefan Hajnoczi wrote:
> > > > On Thu, May 13, 2021 at 06:36:37AM +0800, Liu Bo wrote:
> > > > > Hi Stefan,
> > > > > 
> > > > > On Tue, May 11, 2021 at 09:22:24AM +0100, Stefan Hajnoczi wrote:
> > > > > > On Mon, Feb 15, 2021 at 09:54:08AM +0000, Stefan Hajnoczi wrote:
> > > > > > > v2:
> > > > > > >  * Document empty virtqueue behavior for FUSE_NOTIFY_LOCK messages
> > > > > > > 
> > > > > > > This patch series adds the notification queue to the VIRTIO specification.
> > > > > > > This new virtqueue carries device->driver FUSE notify messages.  They are
> > > > > > > currently unused but will be necessary for file locking, which can block for an
> > > > > > > unbounded amount of time and therefore needs a asynchronous completion event
> > > > > > > instead of a request/response buffer that consumes space in the request
> > > > > > > virtqueue until the operation completes.
> > > > > > > 
> > > > > > > Patch 1 corrects an oversight I noticed: the file system device was not added
> > > > > > > to the Conformance chapter.
> > > > > > > 
> > > > > > > Stefan Hajnoczi (2):
> > > > > > >   virtio-fs: add file system device to Conformance chapter
> > > > > > >   virtio-fs: add notification queue
> > > > > > > 
> > > > > > >  conformance.tex | 23 ++++++++++++++++
> > > > > > >  virtio-fs.tex   | 71 ++++++++++++++++++++++++++++++++++++++++++-------
> > > > > > >  2 files changed, 84 insertions(+), 10 deletions(-)
> > > > > > 
> > > > > > Reminder to anyone who needs the virtio-fs notification queue: please
> > > > > > review this series.
> > > > > >
> > > > > 
> > > > > Besides using notification queue to provide posix lock support, I've
> > > > > also managed to invalidate dentry/inode's cache with notification
> > > > > queue, it worked well.
> > > > 
> > > > Thank you!
> > > > 
> > > > Are you using dentry/inode cache invalidation to reduce the number of
> > > > file descriptors that virtiofsd needs to hold open,
> > > 
> > > Using notifications to invalidate client's dentry/inode cache sounds
> > > interesting. Will like to see an actual implementation though. Various
> > > races w.r.t opening the same file for which server is trying to invalidate
> > > the dentry/inode might be little tricky.
> > >
> > 
> > Right, to avoid the race, the invalidation should only be done when
> > both host and guest are under control.
> > 
> > IIRC, on virtiofsd, I did it in a very hacky/experimental way,
> > firstly, I defined a unlinked file that one can only access it by
> > looking up /proc/{virtiofsd's pid}/fd and write the relative file path
> > to it,
> 
> Was it a file opened with open flag "O_TMPFILE"?
>
> And contents of this file is path of the mount point which you want
> to unmount?
>

Yes and yes.

thanks,
liubo

> > and then I made use of signal SIGUSR2 to read the path from it
> > and send an invalidation request over notification queue.
> 
> Ok.
> 
> Thanks
> Vivek