[PATCH] quota: workaround IO errors on dquot_initialize

[PATCH] quota: workaround IO errors on dquot_initialize

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

On Mon, Oct 11, 2010 at 5:33 PM, Jan Kara <jack@suse.cz> wrote:
> On Sat 09-10-10 23:15:30, Dmitry wrote:
>>
>> I've got following lockup:
>> dquot_disable                              dquot_transfer
>>                                             ->dqget()
>>                                              sb_has_quota_active
>> dqopt->flags &= ~dquot_state_flag(f, cnt)      atomic_inc(dq->dq_count)
>>  ->drop_dquot_ref(sb, cnt);
>>     down_write(dqptr_sem)
>>     inode->i_dquot[cnt] = NULL              ->__dquot_transfer
>> invalidate_dquots(sb, cnt);                  down_write(&dqptr_sem)
>>   ->wait for dq_wait_unused                  inode->i_dquot = new_dquot
>>   /* wait forever */                            ^^^^New quota user^^^^^^
>>
>> Inodes was already cleaned from quotas, and we can not allow new ones.
>> We have to recheck quota state under dqptr_sem and before assignment,
>> as we do it in dquot_initialize()
>  Thanks. Merged into my tree.
BTW, after two days of stress-testing i've got another bug.
Disk space was elapsed, and dqget failed. Fix attached.

>
>                                                                Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>

[-- Attachment #2: 0001-quota-workaround-IO-errors-on-dquot_initialize.patch --]
[-- Type: text/x-diff, Size: 779 bytes --]

From 79d9128c551b3d452cbc788553b70dbf200f2a04 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@gmail.com>
Date: Sun, 10 Oct 2010 00:48:28 +0400
Subject: [PATCH] quota: workaround IO errors on dquot_initialize

Due to IO errors we may not have dquot object.
---
 fs/quota/dquot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 5b262d3..e0f2924 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1394,7 +1394,7 @@ static void __dquot_initialize(struct inode *inode, int type)
 			 * did a write before quota was turned on
 			 */
 			rsv = inode_get_rsv_space(inode);
-			if (unlikely(rsv))
+			if (unlikely(rsv && got[cnt]))
 				dquot_resv_space(inode->i_dquot[cnt], rsv);
 		}
 	}
-- 
1.7.0.4

Re: [PATCH] quota: workaround IO errors on dquot_initialize

[Jan Kara]

> From 79d9128c551b3d452cbc788553b70dbf200f2a04 Mon Sep 17 00:00:00 2001
> From: Dmitry Monakhov <dmonakhov@gmail.com>
> Date: Sun, 10 Oct 2010 00:48:28 +0400
> Subject: [PATCH] quota: workaround IO errors on dquot_initialize
> 
> Due to IO errors we may not have dquot object.
  This can happen also when quotaon() races with __dquot_initialize...

> ---
>  fs/quota/dquot.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 5b262d3..e0f2924 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -1394,7 +1394,7 @@ static void __dquot_initialize(struct inode *inode, int type)
>  			 * did a write before quota was turned on
>  			 */
>  			rsv = inode_get_rsv_space(inode);
> -			if (unlikely(rsv))
> +			if (unlikely(rsv && got[cnt]))
  We should check inode->i_dquot[cnt] instead because got[cnt] just got zeroed
three lines above... In the end I have the following patch:

commit 5ccc70ff4af855a85d7f1d7c843a6eea10693329
Author: Jan Kara <jack@suse.cz>
Date:   Tue Oct 19 00:24:21 2010 +0200

quota: Fix possible oops in __dquot_initialize()
    
When quotaon(8) races with __dquot_initialize() or dqget() fails because
of EIO, ENOSPC, or similar error, we could possibly dereference NULL pointer
in inode->i_dquot[cnt]. Add proper checking.
    
Reported-by: Dmitry Monakhov <dmonakhov@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 1bc38f5..08b1493 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1386,6 +1386,9 @@ static void __dquot_initialize(struct inode *inode, int type)
                /* Avoid races with quotaoff() */
                if (!sb_has_quota_active(sb, cnt))
                        continue;
+               /* We could race with quotaon or dqget() could have failed */
+               if (!got[cnt])
+                       continue;
                if (!inode->i_dquot[cnt]) {
                        inode->i_dquot[cnt] = got[cnt];
                        got[cnt] = NULL;

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR