xfs
[Top] [All Lists]

Re: [PATCH v3 00/10] xfs_ioc_bulkstat code refactoring and consolidation

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH v3 00/10] xfs_ioc_bulkstat code refactoring and consolidation
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Fri, 01 Aug 2014 14:09:12 +0800
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <53D9F67F.9060609@xxxxxxxxxx>
References: <538D92B6.5050402@xxxxxxxxxx> <20140729232748.GI26465@dastard> <53D9F67F.9060609@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0
On 07/31/2014 15:55 PM, Jeff Liu wrote:
> Hi Dave,
> 
> On 07/30/2014 07:27 AM, Dave Chinner wrote:
>> On Tue, Jun 03, 2014 at 05:17:42PM +0800, Jeff Liu wrote:
>>> Hi folk,
>>>
>>> This is the revised patches for xfs_ioc_bulkstat consolidation and code
>>> refactoring. As per Christoph's comments, I'm not include the per AG
>>> inumber patch in this series given that I don't actually introduce the
>>> relevant inumbers interface now. Similar to that reason, I also dropped
>>> the per AG bulkstat patch, it would be included in parallel quota check
>>> series.
>>>
>>>
>>> v3->v2:
>>> - one major bug fix is at xfs_bulkstat_ag_ichunk() regarding the user buffer
>>>   pointer operations, it should be defined as a pointer-to-pointer since it
>>>   would be updated inside xfs_bulkstat_ag_ichunk().
>>>
>>> - separate xfs_inumber consolidate patch into two patches, the first one
>>>   fix the formater function return value and consolidate the codes, another
>>>   one does the actual logic changes for better error handling.
>>>
>>> - Add a separate patch to get rid of the redundant user buffer count
>>>   checks at xfs_bulkstat()
>>>
>>> - fixed agino calculation issue at xfs_bulkstat_grab_ichunk().
>>>
>>> v2: http://oss.sgi.com/archives/xfs/2014-04/msg00554.html
>>> v1: http://oss.sgi.com/archives/xfs/2013-12/msg00901.html
>>>
>>>
>>> Any comments are welcome!
>>
>> Hi Jeff, I ported this to the current dev tree based on the
>> xfs-libxfs-restructure branch, and I keep seeing fsstress failing
>> with memory corruption after random bulkstat ioctls. I see regular
>> failures with generic/013, generic/068, xfs/167 and the other
>> fstress tests also randomly fail. The typical failure is glibc
>> detected memory heap corruption on freeing the bulkstat structure
>> after the ioctl:
>>
>> generic/068 42s ...*** Error in `./ltp/fsstress': double free or corruption 
>> (!prev): 0x00007f0224000b70 ***
>> ======= Backtrace: =========
> 
> Sorry for my late response and thanks for help porting this patch series.
> 
> Now I can reproduce this issue frequently with generic/013, will try to fix
> it ASAP.

Hi Dave,

Could you please check the patch below for this issue?


From: Jie Liu <jeff.liu@xxxxxxxxxx>
Subject: xfs: always updating acp elements at xfs_bulkstat_ag_ichunk

After processing the inodes in chunk, we should always updating the
last inode number and the left user buffer info no matter anything
wrong while formatting an inode to the user buffer. Otherwise, the
related info might be updated improperly at xfs_bulkstat() for the
next round of inode processing.

Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
---
 fs/xfs/xfs_itable.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 9959a05..f71be9c 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -328,11 +328,9 @@ xfs_bulkstat_ag_ichunk(
                lastino = ino;
        }
 
-       if (!error) {
-               acp->ac_lastino = lastino;
-               acp->ac_ubleft = ubleft;
-               acp->ac_ubelem = ubelem;
-       }
+       acp->ac_lastino = lastino;
+       acp->ac_ubleft = ubleft;
+       acp->ac_ubelem = ubelem;
 
        return error;
 }
-- 
1.8.3.2



Cheers,
-Jeff

<Prev in Thread] Current Thread [Next in Thread>