Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c310695e-4279-b1a7-5c2a-2771cc19aa66@I-love.SAKURA.ne.jp>
Date:   Sun, 16 Apr 2023 08:21:17 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] nfsd: don't use GFP_KERNEL from
 nfsd_getxattr()/nfsd_listxattr()
Content-Language: en-US
To:     Jeff Layton <jlayton@kernel.org>,
        Chuck Lever III <chuck.lever@oracle.com>
Cc:     Frank van der Linden <fllinden@amazon.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
References: <72bf692e-bb6b-c1f2-d1ba-3205ab649b43@I-love.SAKURA.ne.jp>
 <4BC7955B-40E4-4A43-B2D1-2E9302E84337@oracle.com>
 <b014047a-4a70-b38f-c5bb-01bc3c53d6f2@I-love.SAKURA.ne.jp>
 <aee35d52ab19e7e95f69742be8329764db72cbf8.camel@kernel.org>
From:   Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
In-Reply-To: <aee35d52ab19e7e95f69742be8329764db72cbf8.camel@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@I-love.SAKURA.ne.jp> 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.