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. > > > That makes sense, but then locking ops should be setting a > > flag indicating that this is locking op. I.e. in fuse_setlk(): > > > > args.blocking_lock = true; > > > > And this should be verified when the reply with the positive error comes back. > > So this args.blocking_lock, goes to server as well? Or this is something > internal to fuse client so that client can decide whether ->error=1 is > a valid response or not. IOW, client is trying to do verification > whether server should have generated ->error=1 or not for this specific > request. Right, it's for the client. Thanks, Miklos