Hello! > 1. (Blocking, needs confirmation) The counter is per-fuse_dev, but ordering > must be per-fuse_conn
Not really, ordering is per vstorage-mount threads. This counter is really userspace thing, which we do not want to do in user space because it requires changes in API, including fuse_open protocol request which would be definitely dreastically worse for maintanance. It is useless and not used for non-vstorage fuses, but we are not going to "if" it all over code, our cycles are priceless. How it works: it is changed in context of threads sending fuse responses. For vstorage fuds are bound to vstorage-mount threads 1-1. So that updates are serialized and do not need any additional atomicity. > This only works if the vStorage daemon guarantees that the OPEN reply and > the INVAL_FILES notification for a given inode > always traverse the same /dev/fuse instance. Exactly. And this is not occasional, this is fundamental property, which this solution (and even older code with synchronous inode invalidation) is and was based upon. > 2. (Medium) The atomic create+open path never sets ff->open_ts Hang on. :-) I naively thought this path is not ours, did not even look there ever. Seems, it is not true, I need to check, I do not even understand it for now. Because in this case we are in big trouble, atomic_open path is completely broken, this new kludge would be the most minor flaw, we have lost years of bug fixes. > 3. (Low) write_counter++ is non-atomic See above. It is deliberate. > 4. (Low / verify) fuse_finish_open() can now sleep under inode_lock + > nowrite These flags are definitely not true for vstorage, so this is irrelevant. But interesting, we definitely want to implement atomic O_TRUNC one day. Yet, since I know that fuse_finish_open can be called from a context which is completely unknown for me, I mean atomic_open, I would refrain of further comments until checking what the hell it is and what context it is called from. > 5. (Nit) Comment wording > > dev.c: "Monotonic timestamp matching the daemon's reply/notification send > order" - it's a write counter, not a timestamp; > and it matches the kernel's processing order, which equals the daemon's > send order only on a single fd. Minor, but the > wording currently papers over exactly the assumption in issue #1. Here I rather agree. The term is leftover of previous patch, there were many. :-) _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
