On Thu, 7 Oct 2021 at 20:32, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > On Thu, Oct 07, 2021 at 08:11:13PM +0200, Miklos Szeredi wrote: > > On Thu, 7 Oct 2021 at 16:22, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > > > > > On Thu, Oct 07, 2021 at 03:45:40PM +0200, Miklos Szeredi wrote: > > > > On Wed, 6 Oct 2021 at 18:13, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > > > > > > > > > On Wed, Oct 06, 2021 at 03:02:36PM +0200, Miklos Szeredi wrote: > > > > > > On Thu, 30 Sept 2021 at 16:39, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > Add a new notification type FUSE_NOTIFY_LOCK. This notification can be > > > > > > > sent by file server to signifiy that a previous locking request has > > > > > > > completed and actual caller should be woken up. > > > > > > > > > > > > Shouldn't this also be generic instead of lock specific? > > > > > > > > > > > > I.e. generic header + original outarg. > > > > > > > > > > Hi Miklos, > > > > > > > > > > I am not sure I understand the idea. Can you please elaborate a bit more. > > > > > > > > > > IIUC, "fuse_out_header + original outarg" is format for responding > > > > > to regular fuse requests. If we use that it will become like responding > > > > > to same request twice. First time we responded with ->error=1 so that > > > > > caller can wait and second time we respond with actual outarg (if > > > > > there is one depending on the type of request). > > > > > > > > > > IOW, this will become more like implementing blocking of request in > > > > > client in a more generic manner. > > > > > > > > > > But outarg, depends on type of request (In case of locking there is > > > > > none). And outarg memory is allocated by driver and filled by server. > > > > > In case of notifications, driver is allocating the memory but it > > > > > does not know what will come in notification and how much memory > > > > > to allocate. So it relies on device telling it how much memory > > > > > to allocate in general so that bunch of pre-defined notification > > > > > types can fit in (fs->notify_buf_size). > > > > > > > > > > I modeled this on the same lines as other fuse notifications where > > > > > server sends notifications with following format. > > > > > > > > > > fuse_out_header + <structure based on notification type> > > > > > > > > > > out_header->unique is 0 for notifications to differentiate notifications > > > > > from request reply. > > > > > > > > > > out_header->error contains the code of actual notification being sent. > > > > > ex. FUSE_NOTIFY_INVAL_INODE or FUSE_NOTIFY_LOCK or FUSE_NOTIFY_DELETE. > > > > > Right now virtiofs supports only one notification type. But in future > > > > > we can introduce more types (to support inotify stuff etc). > > > > > > > > > > In short, I modeled this on existing notion of fuse notifications > > > > > (and not fuse reply). And given notifications are asynchronous, > > > > > we don't know what were original outarg. In fact they might > > > > > be generated not necessarily in response to a request. And that's > > > > > why this notion of defining a type of notification (FUSE_NOTIFY_LOCK) > > > > > and then let driver decide how to handle this notification. > > > > > > > > > > I might have completely misunderstood your suggestion. Please help > > > > > me understand. > > > > > > > > Okay, so we are expecting this mechanism to be only used for blocking > > > > locks. > > > > > > Yes, as of now it is only being used only for blocking locks. So there > > > are two parts to it. > > > > > > A. For a blocking operation, server can reply with error=1, and that's > > > a signal to client to wait for a notification to arrive later. And > > > fuse client will not complete the request and instead will queue it > > > in one of the internal lists. > > > > > > B. Later server will send a fuse notification event (FUSE_NOTIFY_LOCK) > > > when it has acquired the lock. This notification will have unique > > > number of request for which this notification has been generated. > > > Fuse client will search for the request with expected unique number > > > in the list and complete the request. > > > > > > I think part A is generic in the sense it could be used for other > > > kind of blocking requests as well down the line, where server is > > > doing the blocking operation on behalf of client and will send > > > notification later. Part B is very specific to blocking locks though. > > > > I don't really get why B is specific to blocking locks. But anyway... > > we are only implementing it for blocking locks for now. > > Hmm.., I am wondering do I have to make notification specific to > lock. All it is doing is returning an "error" code which signifies > either operation completed successfully or represents error code > if error occurred. > > This probably could be more generic. May be I can call this > notification FUSE_NOTIFY_OP_COMPLETE. This notification is > just signalling that a previously issued request has completed. > Request is identified by notify->unique and result of blocking operation > is in notify->error. So that way it is more generic and can be > used for other kind of operations too (and not just locking). That's exactly what I was thinking. The implementation can remain the same for now... Thanks, Miklos
