@kib is there anything else you'd like me to do for this PR, short of rewriting everything so the vnode lock isn't held while waiting for the daemon?

In D55230#1269768, @asomers wrote:

@kib is there anything else you'd like me to do for this PR, short of rewriting everything so the vnode lock isn't held while waiting for the daemon?

We talked about avoiding lock upgrade. I am fine with that done after this change is committed.

In D55230#1269852, @kib wrote:

In D55230#1269768, @asomers wrote:

@kib is there anything else you'd like me to do for this PR, short of rewriting everything so the vnode lock isn't held while waiting for the daemon?

We talked about avoiding lock upgrade. I am fine with that done after this change is committed.

D55595 should help with the vnode_pager_setsize(), in particular, it should allow to avoid the lock upgrade.

In D55230#1271915, @kib wrote:

D55595 should help with the vnode_pager_setsize(), in particular, it should allow to avoid the lock upgrade.

I think I can use vn_delay_setsize, except in one case: sometimes fusefs's attribute cache is expired before VOP_READ begins. So it needs to fetch attributes from the server with FUSE_GETATTR _before_ it can send FUSE_READ. If the new attributes indicate that the file's size has changed, then it needs to update the file's size (which includes calling vnode_pager_setsize), _before_ sending FUSE_READ. It does all of that without ever unlocking the vnode. If I use vn_delay_setsize, then the pager won't be updated before the FUSE_READ returns, and bread will be operating on an out-of-date pager. To handle this situation, should I keep the lock upgrade code? Or is it ok for bread to operate on an out-of-date pager?

In D55230#1276542, @asomers wrote:

In D55230#1271915, @kib wrote:

D55595 should help with the vnode_pager_setsize(), in particular, it should allow to avoid the lock upgrade.

I think I can use vn_delay_setsize, except in one case: sometimes fusefs's attribute cache is expired before VOP_READ begins. So it needs to fetch attributes from the server with FUSE_GETATTR _before_ it can send FUSE_READ. If the new attributes indicate that the file's size has changed, then it needs to update the file's size (which includes calling vnode_pager_setsize), _before_ sending FUSE_READ. It does all of that without ever unlocking the vnode. If I use vn_delay_setsize, then the pager won't be updated before the FUSE_READ returns, and bread will be operating on an out-of-date pager. To handle this situation, should I keep the lock upgrade code? Or is it ok for bread to operate on an out-of-date pager?

Am I understand you right: after entering VOP_READ() for fusefs, and having the vnode shared-locked, VOP_READ() might discover that the attributes changed and e.g. the node size increased? So it has to set delayed setsize, but needs to read some block after the current EOF? If yes, this is fine. For instance, nfs operates similarly.

The vnode pager size is effectively used only for one purpose: it prevents the page fault from mapping pages beyond the EOF. For instance, UFS uses pages with negative (but effectively, very high positive) page indexes as the storage for indirect blocks content. And the pager check for EOF exactly prevents mapping of the metadata from the indirect blocks into userspace.

So I believe it should work fine for fusefs.

In D55230#1276682, @kib wrote:

Am I understand you right: after entering VOP_READ() for fusefs, and having the vnode shared-locked, VOP_READ() might discover that the attributes changed and e.g. the node size increased? So it has to set delayed setsize, but needs to read some block after the current EOF? If yes, this is fine. For instance, nfs operates similarly.

The vnode pager size is effectively used only for one purpose: it prevents the page fault from mapping pages beyond the EOF. For instance, UFS uses pages with negative (but effectively, very high positive) page indexes as the storage for indirect blocks content. And the pager check for EOF exactly prevents mapping of the metadata from the indirect blocks into userspace.

So I believe it should work fine for fusefs.

Ok, that's a relief. I've made the requested change, and all existing tests (and one new one) pass. Are there any other tricky requirements for vn_delayed_setsize that I should be aware of? An earlier version of this change used vn_delayed_setsize unconditionally, even if there was already an exclusive vnode lock. But that led to vm_fault_lookup: fault on nofault entry panics that I didn't fully understand.

In D55230#1276905, @asomers wrote:

In D55230#1276682, @kib wrote:

Am I understand you right: after entering VOP_READ() for fusefs, and having the vnode shared-locked, VOP_READ() might discover that the attributes changed and e.g. the node size increased? So it has to set delayed setsize, but needs to read some block after the current EOF? If yes, this is fine. For instance, nfs operates similarly.

The vnode pager size is effectively used only for one purpose: it prevents the page fault from mapping pages beyond the EOF. For instance, UFS uses pages with negative (but effectively, very high positive) page indexes as the storage for indirect blocks content. And the pager check for EOF exactly prevents mapping of the metadata from the indirect blocks into userspace.

So I believe it should work fine for fusefs.

Ok, that's a relief. I've made the requested change, and all existing tests (and one new one) pass. Are there any other tricky requirements for vn_delayed_setsize that I should be aware of? An earlier version of this change used vn_delayed_setsize unconditionally, even if there was already an exclusive vnode lock. But that led to vm_fault_lookup: fault on nofault entry panics that I didn't fully understand.

It is impossible to guess. At least, provide the full kernel log including the panic and backtrace, from the issue.