On 2023/04/16 3:40, Jeff Layton wrote: > On Sun, 2023-04-16 at 02:11 +0900, Tetsuo Handa wrote: >> On 2023/04/16 1:13, Chuck Lever III wrote: >>>> On Apr 15, 2023, at 7:07 AM, Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote: >>>> >>>> Since GFP_KERNEL is GFP_NOFS | __GFP_FS, usage like GFP_KERNEL | GFP_NOFS >>>> does not make sense. Drop __GFP_FS flag in order to avoid deadlock. >>> >>> The server side threads run in process context. GFP_KERNEL >>> is safe to use here -- as Jeff said, this code is not in >>> the server's reclaim path. Plenty of other call sites in >>> the NFS server code use GFP_KERNEL. >> >> GFP_KERNEL memory allocation calls filesystem's shrinker functions >> because of __GFP_FS flag. My understanding is >> >> Whether this code is in memory reclaim path or not is irrelevant. >> Whether memory reclaim path might hold lock or not is relevant. >> >> . Therefore, question is, does nfsd hold i_rwsem during memory reclaim path? >> > > No. At the time of these allocations, the i_rwsem is not held. Excuse me? nfsd_getxattr()/nfsd_listxattr() _are_ holding i_rwsem via inode_lock_shared(inode) before kvmalloc(GFP_KERNEL | GFP_NOFS) allocation. That's why /* * We're holding i_rwsem - use GFP_NOFS. */ is explicitly there in nfsd_listxattr() side. If memory reclaim path (directly or indirectly via locking dependency) involves inode_lock_shared(inode)/inode_lock(inode), it is not safe to use __GFP_FS flag.
