On Friday, September 8, 2017 10:25:26 PM IST Eric Sandeen wrote:
> On 9/5/17 12:44 AM, Chandan Rajendra wrote:
> > Linux kernel commit 6c6b6f28b3335fd85ec833ee0005d9c9dca6c003 (loop: set
> > physical block size to PAGE_SIZE) now sets PAGE_SIZE as the default
> > physical sector size of loop devices. On ppc64, this causes loop devices
> > to have 64k as the physical sector size.
> >
> > With these changes, mkfs.xfs now prints error messages when filesystem
> > blocksize (4k) is less than underlying device's physical
> > sectorsize (64k). These messages (printed on stderr) now cause several
> > xfstests to fail on ppc64 machine since xfstests' _filter_mkfs() isn't
> > able to filter out stderr.
> >
> > Also, the messages themselves describe a possible sub-optimal setup. But
> > the setup is still usable.
> >
> > Hence this commit removes the calls to fprintf() used to print the
> > messages.
>
> So, it looks like the loop change is getting reverted, right ... still -
>
> Although I suggested this change, I'm rethinking it. I'm not a fan
> of the warning for a default situation; the user can get this warning
> with nothing but a bare mkfs, which is not good IMHO.
>
> (dchinner OTOH thinks we should warn about this suboptimal situation
> in any case - but I really don't think it's mkfs's job to be warning
> about every suboptimal geometry - there are a lot of them out there!)
>
> What I'd now propose is that we change this warning into a failure,
> but only if a too-small block size was actually /specified/, i.e.
> bsflag is set. If we're adjusting sector size based on device geometry
> and /default/ blocksize, I think we should just shut up about it.
>
> i.e. something like:
>
> if ((blocksize < sectorsize) && (blocksize >= ft.lsectorsize)) {
I agree with your changes from a system administrator's perspective. But
without these messages, the sectorsize change for the loop device would
most likely not have been noticed.
> if (bsflag) {
Just FYI, We should also be checking blflag's value.
> fprintf(stderr,
> _("specified blocksize %d cannot be less than device physical sector size %d\n"),
> blocksize, ft.psectorsize);
> usage();
> }
> sectorsize = ft.lsectorsize ? ft.lsectorsize :
> XFS_MIN_SECTORSIZE;
> }
>
> Thoughts?
>
--
chandan
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
