[Devel] [PATCH VZ10] fs/fuse: order FUSE_OPEN reply against FUSE_NOTIFY_INVAL_FILES

[Alexey Kuznetsov]

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. :-)