On 6/18/15 3:53 AM, Jan Tulak wrote:
>
>
> ----- Original Message -----
>> From: "Eric Sandeen" <sandeen@xxxxxxxxxxx>
>> To: "Jan Tulak" <jtulak@xxxxxxxxxx>, xfs@xxxxxxxxxxx
>> Sent: Wednesday, June 17, 2015 9:43:47 PM
>> Subject: Re: mkfs: a possible bad
>>
>> On 6/17/15 9:28 AM, Jan Tulak wrote:
>>> Hi, I'm looking into mkfs/xfs_mkfs.c and I wonder, is "if (xi.dbsize
>>>> sectorsize)" correct? It is a check for: Warning: the data
>>> subvolume sector size %u is less than the sector size reported by the
>>> device (%u).
>>>
>>> But psectorsize is assigned to sectorsize, not to xi.dbsize, so the
>>> two values seems to be swapped in the condition (and as arguments of
>>> the printf too). I think this gone without noticing because usually,
>>> when creating a partition, the two values are the same. So even if
>>> the condition is wrong, nothing happens. And when -bsize=X is passed,
>>> then it is catched earlier and nothing happens again.
>>>
>>> Only when I apply a patch that changes how mkfs acts when it gets a
>>> file instead of a block device, I start to see the warning, although
>>> physical sector size is 512 and block size is set to 4096. The
>>> numbers are swapped in the warning too...
>>>
>>> I tried to run ./check -g quick and it seems that the change breaks
>>> nothing.
>>>
>>> Cheers, Jan
>>
>> Hohum, xfs_mkfs.c is such spaghetti-code. :) Let me think through
>> this as well:
>>
>> Ok, we init sectorsize to XFS_MIN_SECTORSIZE at the top of main().
>>
>> If we have cmdline args to specify sector size, we reset it there.
>>
>> If not specified, we set it to the physical sector size advertised
>> by the device.
>>
>> And xi.dbsize is set in platform_findsizes, for a device it comes
>> from BLKSSZGET, which gives us the logical sector size of the device.
>>
>> So this test:
>>
>> if (xi.dbsize > sectorsize) {
>> fprintf(stderr, _(
>> "Warning: the data subvolume sector size %u is less than the sector size \n\
>> reported by the device (%u).\n"),
>> sectorsize, xi.dbsize);
>> }
>>
>> is testing whether the logical sector size is greater than the physical
>> sector size - which would indeed be a problem (logical is <= physical)
>
> You write "logical is <= physical", but the text of the warning seems to be telling "it should be logical >= physical, but isn't."
> I initially thought it should be reversed, but now I'm not sure. Maybe the warning could use better words to avoid this confusing?
> In which case, I have to go back to bug hunting and see what is causing the warning to me...
Sorry, I wasn't very clear. The test itself is checking if the device's
logical sector size (the smallest possible IO for the device) is
larger than the filesystem's sector size, which is indeed a problem.
The error message printed does match that, although it states it in
the opposite order.
-Eric
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
