diff mbox series

[-next] NFSv4: Use GFP_ATOMIC under spin lock in _pnfs_grab_empty_layout()

Message ID 20200428071932.69976-1-weiyongjun1@huawei.com (mailing list archive)
State New, archived
Headers show
Series [-next] NFSv4: Use GFP_ATOMIC under spin lock in _pnfs_grab_empty_layout() | expand

Commit Message

Wei Yongjun April 28, 2020, 7:19 a.m. UTC
A spin lock is taken here so we should use GFP_ATOMIC.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 fs/nfs/pnfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Trond Myklebust April 28, 2020, 5:41 p.m. UTC | #1
On Tue, 2020-04-28 at 07:19 +0000, Wei Yongjun wrote:
> A spin lock is taken here so we should use GFP_ATOMIC.
> 
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  fs/nfs/pnfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index dd2e14f5875d..d84c1b7b71d2 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -2170,7 +2170,7 @@ _pnfs_grab_empty_layout(struct inode *ino,
> struct nfs_open_context *ctx)
>  	struct pnfs_layout_hdr *lo;
>  
>  	spin_lock(&ino->i_lock);
> -	lo = pnfs_find_alloc_layout(ino, ctx, GFP_KERNEL);
> +	lo = pnfs_find_alloc_layout(ino, ctx, GFP_ATOMIC);
>  	if (!lo)
>  		goto out_unlock;
>  	if (!test_bit(NFS_LAYOUT_INVALID_STID, &lo->plh_flags))
> 
> 
> 

NACK. There is no allocation under the spinlock.
Dan Carpenter April 28, 2020, 6:04 p.m. UTC | #2
On Tue, Apr 28, 2020 at 07:19:32AM +0000, Wei Yongjun wrote:
> A spin lock is taken here so we should use GFP_ATOMIC.
> 
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  fs/nfs/pnfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index dd2e14f5875d..d84c1b7b71d2 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -2170,7 +2170,7 @@ _pnfs_grab_empty_layout(struct inode *ino, struct nfs_open_context *ctx)
>  	struct pnfs_layout_hdr *lo;
>  
>  	spin_lock(&ino->i_lock);
                   ^^^
> -	lo = pnfs_find_alloc_layout(ino, ctx, GFP_KERNEL);
> +	lo = pnfs_find_alloc_layout(ino, ctx, GFP_ATOMIC);
                                    ^^^
It releases the lock before allocating.  It's annotated.

regards,
dan carpenter
Wei Yongjun April 29, 2020, 1:03 a.m. UTC | #3
On 2020/4/29 2:04, Dan Carpenter wrote:
> On Tue, Apr 28, 2020 at 07:19:32AM +0000, Wei Yongjun wrote:
>> A spin lock is taken here so we should use GFP_ATOMIC.
>>
>> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>> ---
>>  fs/nfs/pnfs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index dd2e14f5875d..d84c1b7b71d2 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -2170,7 +2170,7 @@ _pnfs_grab_empty_layout(struct inode *ino, struct nfs_open_context *ctx)
>>  	struct pnfs_layout_hdr *lo;
>>  
>>  	spin_lock(&ino->i_lock);
>                    ^^^
>> -	lo = pnfs_find_alloc_layout(ino, ctx, GFP_KERNEL);
>> +	lo = pnfs_find_alloc_layout(ino, ctx, GFP_ATOMIC);
>                                     ^^^
> It releases the lock before allocating.  It's annotated.
> 

Got it, thanks.

regards,
Wei Yongjun
diff mbox series

Patch

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index dd2e14f5875d..d84c1b7b71d2 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -2170,7 +2170,7 @@  _pnfs_grab_empty_layout(struct inode *ino, struct nfs_open_context *ctx)
 	struct pnfs_layout_hdr *lo;
 
 	spin_lock(&ino->i_lock);
-	lo = pnfs_find_alloc_layout(ino, ctx, GFP_KERNEL);
+	lo = pnfs_find_alloc_layout(ino, ctx, GFP_ATOMIC);
 	if (!lo)
 		goto out_unlock;
 	if (!test_bit(NFS_LAYOUT_INVALID_STID, &lo->plh_flags))