Re: [PATCH] xfs_mdrestore: initialize sb prior to xfs_sb_from_disk()

[Eric Sandeen <sandeen@sandeen.net>]

On 6/5/14, 9:53 PM, Eric Sandeen wrote:
> On 6/5/14, 8:42 PM, Dave Chinner wrote:
>> On Thu, Jun 05, 2014 at 07:00:02PM -0500, Eric Sandeen wrote:
>>> On 6/5/14, 6:56 PM, Dave Chinner wrote:
>>>> On Thu, Jun 05, 2014 at 06:09:16PM -0500, Eric Sandeen wrote:
>>>
>>> ...
>>>
>>>>> But ... should we maybe just do this once and for all in
>>>>> xfs_sb_from_disk?  I'm not sure leaving it up to every
>>>>> caller is a good idea, unless somebody ahs a reason to
>>>>> pre-populate some fields - I can't imagine why that would
>>>>> be, though...
>>>>
>>>> We don't ever read in the CRC field into the in-memory structures
>>>> because it has no meaning in memory. Simiarly, we don't ever write
>>>> the CRC field from the in-core structure because we always
>>>> re-calculate it in the IO path if CRCs are configured. That is
>>>> consistent behaviour across the entire code-base.

But as you say in userspace, this
 
	libxfs_sb_to_disk(buf, sbp, XFS_SB_ALL_BITS)

*does* write it.

kernelspace does the same here:

0 xfs_fsops.c xfs_growfs_data_private 520 xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, XFS_SB_ALL_BITS);

>>> <snip stuff>
>>>
>>>> Perhaps we should move the memset() to within xfs_sb_from_disk()
>>>> to make this explicit?
>>>
>>> Yes, that's what I meant by "this" in "do this once and for all" - 
>>> sorry, that wasn't clear.  memset(0) in xfs_sb_from_disk().
>>
>> I didn't read it clearly. my fault.
>>
>>> Yeah, the more I think about it, the more I think that's probably
>>> the obviously correct thing to do.
> 
> Actually, a memset() seems like overkill - every field except
> sb_crc is explicitly filled in in the function.
> 
> Maybe better to just set sb_crc to 0, with a comment as to why?
> I think I'll whip that one up.

But now I realize we do sometimes write the in-memory value to
disk, as seen above.

Backing up - shouldn't we just go ahead and read/write it from/to
disk just like every other field, at least in the cases where we
are writing all, and when we are reading (which always reads all)?

-Eric


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs