| Message ID | 1549656647-25115-1-git-send-email-bfields@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Eliminate delegation self-conflicts | expand |
On Fri, 2019-02-08 at 15:10 -0500, J. Bruce Fields wrote: > From: "J. Bruce Fields" <bfields@redhat.com> > > These patches allow NFSv4 clients holding delegations to keep them when > the operation that would break a delegation comes from the same client. > > To do that, we somehow need to pass the identity of the > delegation-breaker down through the VFS. > > This series uses the tgid, a solution suggested by Trond. To do that we > need nfsd tasks to share the same tgid. I do that by extending the > kthread code slightly to allow knfsd to run the kthreadd main loop in a > task of its own, and spawn its server threads off of that task. > > Part of Trond's thinking was that this would work for userspace too. > Delegations are currently only available to knfsd, but Ganesha and Samba > may eventually be interested in a userspace interface (probably a minor > variation on the fcntl F_{GET,SET}LEASE interface). A threaded > userspace server would first resolve conflicts between its own clients, > and then call into the kernel to break any leases acquired by other > processes. That may require some careful locking of the server's own > data structures, but it should work. > > Previously I considered instead adding a new field somewhere in the > struct task. That might require a new system call to expose to user > space. Or we might be able to put this in a keyring, if David Howells > thought that would work. > > Before that I tried passing the identity of the breaker explicitly, but > that looks like it would require passing the new argument around to huge > swaths of the VFS. > > I'm testing this with some a locally modified pynfs; I'll fix that up > and push it out at some point, but pynfs has a number of bugs in this > area. > > I wasn't sure who to ask about the kthread.c changes, so I'm cc'ing a > random assortment of developers in recent changelogs, hope that's OK. > > --b. > > J. Bruce Fields (7): > kthreads: minor kthreadd refactoring > kthreads: Simplify tsk_fork_get_node > kthreads: allow multiple kthreadd's > kthreads: allow cloning threads with different flags > rpc: separate out body of svc_start_kthreads > rpc: move rpc server threads into their own thread group > nfsd: ignore delegation self-conflicts > > fs/locks.c | 39 +++++++++++ > fs/nfsd/nfs4state.c | 61 ++++++++++++++++ > fs/nfsd/state.h | 2 + > fs/nfsd/vfs.c | 32 +++++++-- > include/linux/fs.h | 2 + > include/linux/kthread.h | 21 +++++- > include/linux/sunrpc/svc.h | 1 + > init/init_task.c | 3 + > init/main.c | 4 +- > kernel/fork.c | 4 ++ > kernel/kthread.c | 140 +++++++++++++++++++++++++++---------- > net/sunrpc/svc.c | 83 ++++++++++++++-------- > 12 files changed, 317 insertions(+), 75 deletions(-) > Nice work! I like the basic idea, the changes seem to be well-organized, and the tgid semantics are clear and make sense. Would this preclude us from moving to a workqueue-based model for knfsd later? It's likely to still be worth it, but it'd be good to understand the potential drawbacks. Thanks,
On Sat, Feb 09, 2019 at 07:43:54AM -0500, Jeff Layton wrote: > On Fri, 2019-02-08 at 15:10 -0500, J. Bruce Fields wrote: > > From: "J. Bruce Fields" <bfields@redhat.com> > > > > These patches allow NFSv4 clients holding delegations to keep them when > > the operation that would break a delegation comes from the same client. > > > > To do that, we somehow need to pass the identity of the > > delegation-breaker down through the VFS. > > > > This series uses the tgid, a solution suggested by Trond. To do that we > > need nfsd tasks to share the same tgid. I do that by extending the > > kthread code slightly to allow knfsd to run the kthreadd main loop in a > > task of its own, and spawn its server threads off of that task. ... > Nice work! I like the basic idea, the changes seem to be well-organized, > and the tgid semantics are clear and make sense. > > Would this preclude us from moving to a workqueue-based model for knfsd > later? It's likely to still be worth it, but it'd be good to understand > the potential drawbacks. I was wondering about that too, but I haven't looked into it yet. Workqueues look a lot more complicated than kthreads. --b.
On Mon, Feb 11, 2019 at 10:58:04AM -0500, J. Bruce Fields wrote: > On Sat, Feb 09, 2019 at 07:43:54AM -0500, Jeff Layton wrote: > > On Fri, 2019-02-08 at 15:10 -0500, J. Bruce Fields wrote: > > > From: "J. Bruce Fields" <bfields@redhat.com> > > > > > > These patches allow NFSv4 clients holding delegations to keep them when > > > the operation that would break a delegation comes from the same client. > > > > > > To do that, we somehow need to pass the identity of the > > > delegation-breaker down through the VFS. > > > > > > This series uses the tgid, a solution suggested by Trond. To do that we > > > need nfsd tasks to share the same tgid. I do that by extending the > > > kthread code slightly to allow knfsd to run the kthreadd main loop in a > > > task of its own, and spawn its server threads off of that task. > ... > > Nice work! I like the basic idea, the changes seem to be well-organized, > > and the tgid semantics are clear and make sense. > > > > Would this preclude us from moving to a workqueue-based model for knfsd > > later? It's likely to still be worth it, but it'd be good to understand > > the potential drawbacks. > > I was wondering about that too, but I haven't looked into it yet. > Workqueues look a lot more complicated than kthreads. I spent some time staring, and... I still don't really understand the workqueue code. But if this kthread_group[*] code is acceptable than I can't see why it shouldn't be possible to create a workqueue whose work items are all handled by threads spawned form the same kthread_group. --b. [*] Open to suggestions of better names.
From: "J. Bruce Fields" <bfields@redhat.com> These patches allow NFSv4 clients holding delegations to keep them when the operation that would break a delegation comes from the same client. To do that, we somehow need to pass the identity of the delegation-breaker down through the VFS. This series uses the tgid, a solution suggested by Trond. To do that we need nfsd tasks to share the same tgid. I do that by extending the kthread code slightly to allow knfsd to run the kthreadd main loop in a task of its own, and spawn its server threads off of that task. Part of Trond's thinking was that this would work for userspace too. Delegations are currently only available to knfsd, but Ganesha and Samba may eventually be interested in a userspace interface (probably a minor variation on the fcntl F_{GET,SET}LEASE interface). A threaded userspace server would first resolve conflicts between its own clients, and then call into the kernel to break any leases acquired by other processes. That may require some careful locking of the server's own data structures, but it should work. Previously I considered instead adding a new field somewhere in the struct task. That might require a new system call to expose to user space. Or we might be able to put this in a keyring, if David Howells thought that would work. Before that I tried passing the identity of the breaker explicitly, but that looks like it would require passing the new argument around to huge swaths of the VFS. I'm testing this with some a locally modified pynfs; I'll fix that up and push it out at some point, but pynfs has a number of bugs in this area. I wasn't sure who to ask about the kthread.c changes, so I'm cc'ing a random assortment of developers in recent changelogs, hope that's OK. --b. J. Bruce Fields (7): kthreads: minor kthreadd refactoring kthreads: Simplify tsk_fork_get_node kthreads: allow multiple kthreadd's kthreads: allow cloning threads with different flags rpc: separate out body of svc_start_kthreads rpc: move rpc server threads into their own thread group nfsd: ignore delegation self-conflicts fs/locks.c | 39 +++++++++++ fs/nfsd/nfs4state.c | 61 ++++++++++++++++ fs/nfsd/state.h | 2 + fs/nfsd/vfs.c | 32 +++++++-- include/linux/fs.h | 2 + include/linux/kthread.h | 21 +++++- include/linux/sunrpc/svc.h | 1 + init/init_task.c | 3 + init/main.c | 4 +- kernel/fork.c | 4 ++ kernel/kthread.c | 140 +++++++++++++++++++++++++++---------- net/sunrpc/svc.c | 83 ++++++++++++++-------- 12 files changed, 317 insertions(+), 75 deletions(-)