On Tue, 28 Nov 2023, Al Viro wrote: > On Tue, Nov 28, 2023 at 09:05:21AM +1100, NeilBrown wrote: > > > A simple way to fix this is to treat nfsd threads like normal processes > > for task_work. Thus the pending files are queued for the thread, and > > the same thread finishes the work. > > > > Currently KTHREADs are assumed never to call task_work_run(). With this > > patch that it still the default but it is implemented by storing the > > magic value TASK_WORKS_DISABLED in ->task_works. If a kthread, such as > > nfsd, will call task_work_run() periodically, it sets ->task_works > > to NULL to indicate this. > > > svc_recv(rqstp); > > validate_process_creds(); > > + if (task_work_pending(current)) > > + task_work_run(); > > What locking environment and call chain do you have here? And what happens if > you get something stuck in ->release()? No locking. This is in the top level function of the kthread. A ->release function that waits for an NFS filesystem to flush out data through a filesystem exported by this nfsd might hit problems. But that really requires us nfs-exporting and nfs filesystem which is loop-back mounted. While we do support nfs-reexport and nfs-loop-back mounts, I don't think we make any pretence of supporting a combination. Is that the sort of thing you were think of? > > > > > p->pdeath_signal = 0; > > - p->task_works = NULL; > > + p->task_works = args->kthread ? TASK_WORKS_DISABLED : NULL; > > Umm... why not have them set (by helper in kernel/task_work.c) to > &work_exited? Then the task_work_run parts wouldn't be needed at all... > I hadn't tried to understand what work_exited was for - but now I see that its purpose is precisely to block further work from being queued - exactly what I need. Thanks - I make that change for a v2. I've realised that I'll also need to change the flush_delayed_fput() in fsd_file_close_inode_sync() to task_work_run(). Thanks, NeilBrown
