Hi Dave,
Thank you for your comments.
I realize that secondary superblocks are needed mostly for repairing a
broken filesystem. However, I don't see that they get updated regularly,
i.e., during normal operation they don't seem to get updated at all. I put a
print in xfs_sb_write_verify, and it gets called only with:
bp->b_bn==XFS_SB_DADDR.
So do I understand correctly (also from comments in
xfs_growfs_data_private), that it is safe to operate a filesystem while
having broken secondary superblocks? For me, it appears to mount properly,
and all the data seems to be there, but xfs_check complains like:
bad sb magic # 0xc2a4baf2 in ag 6144
bad sb version # 0x4b5d in ag 6144
blocks 6144/65536..2192631388 out of range
blocks 6144/65536..2192631388 claimed by block 6144/0
bad sb magic # 0xb20f3079 in ag 6145
bad sb version # 0x6505 in ag 6145
blocks 6145/65536..3530010017 out of range
blocks 6145/65536..3530010017 claimed by block 6145/0
...
Also, if secondary superblocks do not get updated regularly, and there is no
way to ask an operational XFS to update them, then during repair we may not
find a good secondary superblock.
As for the patch, I cannot post a patch against the upstream kernel, because
I am running an older kernel. Unfortunately, I cannot qualify an upstream
patch properly in a reasonable time. Is there a value in posting a patch
against 3.8.13? Otherwise, it's fine by me if somebody else posts it and
takes the credit.
Thanks,
Alex.
-----Original Message-----
From: Dave Chinner
Sent: 02 July, 2014 12:56 AM
To: Alex Lyakas
Cc: xfs@xxxxxxxxxxx
Subject: Re: xfs_growfs_data_private memory leak
On Tue, Jul 01, 2014 at 06:06:38PM +0300, Alex Lyakas wrote:
Greetings,
It appears that if xfs_growfs_data_private fails during the "new AG
headers" loop, it does not free all the per-AG structures for the
new AGs. When XFS is unmounted later, they are not freed as well,
because xfs_growfs_data_private did not update the "sb_agcount"
field, so xfs_free_perag will not free them. This happens on 3.8.13,
but looking at the latest master branch, it seems to have the same
issue.
Code like [1] in xfs_growfs_data, seems to fix the issue.
Why not just do this in the appropriate error stack, like is
done inside xfs_initialize_perag() on error?
for (i = oagcount; i < nagcount; i++) {
pag = radix_tree_delete(&mp->m_perag_tree, index);
kmem_free(pag);
}
(though it might need RCU freeing)
When you have a fix, can you send a proper patch with a sign-off on
it?
A follow-up question: if xfs_grows_data_private fails during the
loop that updates all the secondary superblocks, what is the
consequence? (I am aware that in the latest master branch, the loop
is not broken on first error, but attempts to initialize whatever
possible). When these secondary superblocks will get updated? Is
there a way to force-update them? Otherwise, what can be the
consequence of leaving them not updated?
The consequence is documented in mainline tree - if we don't update
them all, then repair will do the wrong thing. Repair requires a
majority iof identical secondaries to determine if the primary is
correct or out of date. The old behaviour of not updating after the
first error meant that the majority were old superblocks and so at
some time in the future repair could decide your filesystem is
smaller than it really is and hence truncate away the grown section
of the filesystem. i.e. trigger catastrophic, unrecoverable data
loss.
Hence it's far better to write every seconday we can than to leave
a majority in a bad state....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
