| Message ID | 20170817073844.1351-1-jtulak@redhat.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Thu, Aug 17, 2017 at 09:38:44AM +0200, Jan Tulak wrote: > The man page is missing description of these options. > Add it. Just remove the options. They are redundant as they cannot be different from the values set by the "-s" sector size options. They aren't documented, so just remove them from mkfs. Cheers, Dave.
On Thu, Aug 17, 2017 at 10:35 AM, Dave Chinner <david@fromorbit.com> wrote: > On Thu, Aug 17, 2017 at 09:38:44AM +0200, Jan Tulak wrote: >> The man page is missing description of these options. >> Add it. > > Just remove the options. They are redundant as they cannot be > different from the values set by the "-s" sector size options. > They aren't documented, so just remove them from mkfs. > The code doesn't look like that. From what it does: -d sectsize: will set data section sector size -l sectsize: will set log section sector size (and as I see now, this is not in man page as well) -s size: will set both data and log sector size Does the removal of -d sectsize really make a sense? In any case, the man page needs to be updated to reflect the code. Cheers, Jan -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 17, 2017 at 11:45:34AM +0200, Jan Tulak wrote: > On Thu, Aug 17, 2017 at 10:35 AM, Dave Chinner <david@fromorbit.com> wrote: > > On Thu, Aug 17, 2017 at 09:38:44AM +0200, Jan Tulak wrote: > >> The man page is missing description of these options. > >> Add it. > > > > Just remove the options. They are redundant as they cannot be > > different from the values set by the "-s" sector size options. > > They aren't documented, so just remove them from mkfs. > > > > The code doesn't look like that. From what it does: > > -d sectsize: will set data section sector size > -l sectsize: will set log section sector size (and as I see now, this > is not in man page as well) > -s size: will set both data and log sector size If you just look at the option parsing, then it appears that way. But go an look at the code that validates and uses these options. It will only take a log sector size specification for external logs, other wise it will use the global sector size. Which is the same as the data section sector size. Hence for internal log filesystems, "-l sectsize" is ignored, and "-s size" and "-d sectsize" set exactly the same variable. And for external logs, having "-s size" override the "-l sectsize" is completely wrong, but that's what it does.... IOWs, the way the sector size parsing and validation is done is an utter mess - it's not surprising that nobody can work out what it does exactly because the code is even more messy and hard to understand than the CLI interface.... Cheers, Dave.
On Thu, Aug 17, 2017 at 1:22 PM, Dave Chinner <david@fromorbit.com> wrote: > On Thu, Aug 17, 2017 at 11:45:34AM +0200, Jan Tulak wrote: >> On Thu, Aug 17, 2017 at 10:35 AM, Dave Chinner <david@fromorbit.com> wrote: >> > On Thu, Aug 17, 2017 at 09:38:44AM +0200, Jan Tulak wrote: >> >> The man page is missing description of these options. >> >> Add it. >> > >> > Just remove the options. They are redundant as they cannot be >> > different from the values set by the "-s" sector size options. >> > They aren't documented, so just remove them from mkfs. >> > >> >> The code doesn't look like that. From what it does: >> >> -d sectsize: will set data section sector size >> -l sectsize: will set log section sector size (and as I see now, this >> is not in man page as well) >> -s size: will set both data and log sector size > > If you just look at the option parsing, then it appears that way. > > But go an look at the code that validates and uses these options. > It will only take a log sector size specification for external logs, > other wise it will use the global sector size. Which is the same as > the data section sector size. > > Hence for internal log filesystems, "-l sectsize" is ignored, and > "-s size" and "-d sectsize" set exactly the same variable. And for > external logs, having "-s size" override the "-l sectsize" is > completely wrong, but that's what it does.... > > IOWs, the way the sector size parsing and validation is done is an > utter mess - it's not surprising that nobody can work out what it > does exactly because the code is even more messy and hard to > understand than the CLI interface.... > OK, I agree now that -d sectsize should be removed and -s size should not override log sector size. Cheers, Jan -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/17/17 6:22 AM, Dave Chinner wrote: > On Thu, Aug 17, 2017 at 11:45:34AM +0200, Jan Tulak wrote: >> On Thu, Aug 17, 2017 at 10:35 AM, Dave Chinner <david@fromorbit.com> wrote: >>> On Thu, Aug 17, 2017 at 09:38:44AM +0200, Jan Tulak wrote: >>>> The man page is missing description of these options. >>>> Add it. >>> >>> Just remove the options. They are redundant as they cannot be >>> different from the values set by the "-s" sector size options. >>> They aren't documented, so just remove them from mkfs. >>> >> >> The code doesn't look like that. From what it does: >> >> -d sectsize: will set data section sector size >> -l sectsize: will set log section sector size (and as I see now, this >> is not in man page as well) >> -s size: will set both data and log sector size > > If you just look at the option parsing, then it appears that way. > > But go an look at the code that validates and uses these options. > It will only take a log sector size specification for external logs, > other wise it will use the global sector size. Which is the same as > the data section sector size. > > Hence for internal log filesystems, "-l sectsize" is ignored, and > "-s size" and "-d sectsize" set exactly the same variable. And for > external logs, having "-s size" override the "-l sectsize" is > completely wrong, but that's what it does.... Is it even safe/legit to have different sector sizes specified for log vs data? -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 17, 2017 at 6:01 PM, Eric Sandeen <sandeen@sandeen.net> wrote: > On 8/17/17 6:22 AM, Dave Chinner wrote: >> On Thu, Aug 17, 2017 at 11:45:34AM +0200, Jan Tulak wrote: >>> On Thu, Aug 17, 2017 at 10:35 AM, Dave Chinner <david@fromorbit.com> wrote: >>>> On Thu, Aug 17, 2017 at 09:38:44AM +0200, Jan Tulak wrote: >>>>> The man page is missing description of these options. >>>>> Add it. >>>> >>>> Just remove the options. They are redundant as they cannot be >>>> different from the values set by the "-s" sector size options. >>>> They aren't documented, so just remove them from mkfs. >>>> >>> >>> The code doesn't look like that. From what it does: >>> >>> -d sectsize: will set data section sector size >>> -l sectsize: will set log section sector size (and as I see now, this >>> is not in man page as well) >>> -s size: will set both data and log sector size >> >> If you just look at the option parsing, then it appears that way. >> >> But go an look at the code that validates and uses these options. >> It will only take a log sector size specification for external logs, >> other wise it will use the global sector size. Which is the same as >> the data section sector size. >> >> Hence for internal log filesystems, "-l sectsize" is ignored, and >> "-s size" and "-d sectsize" set exactly the same variable. And for >> external logs, having "-s size" override the "-l sectsize" is >> completely wrong, but that's what it does.... > > Is it even safe/legit to have different sector sizes specified > for log vs data? > From my understanding, if the log is external (on another device), it is completely ok. Unless there is some hiccup somewhere deep in the code... -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/17/17 2:50 PM, Jan Tulak wrote: > On Thu, Aug 17, 2017 at 6:01 PM, Eric Sandeen <sandeen@sandeen.net> wrote: >> On 8/17/17 6:22 AM, Dave Chinner wrote: >>> On Thu, Aug 17, 2017 at 11:45:34AM +0200, Jan Tulak wrote: >>>> On Thu, Aug 17, 2017 at 10:35 AM, Dave Chinner <david@fromorbit.com> wrote: >>>>> On Thu, Aug 17, 2017 at 09:38:44AM +0200, Jan Tulak wrote: >>>>>> The man page is missing description of these options. >>>>>> Add it. >>>>> >>>>> Just remove the options. They are redundant as they cannot be >>>>> different from the values set by the "-s" sector size options. >>>>> They aren't documented, so just remove them from mkfs. >>>>> >>>> >>>> The code doesn't look like that. From what it does: >>>> >>>> -d sectsize: will set data section sector size >>>> -l sectsize: will set log section sector size (and as I see now, this >>>> is not in man page as well) >>>> -s size: will set both data and log sector size >>> >>> If you just look at the option parsing, then it appears that way. >>> >>> But go an look at the code that validates and uses these options. >>> It will only take a log sector size specification for external logs, >>> other wise it will use the global sector size. Which is the same as >>> the data section sector size. >>> >>> Hence for internal log filesystems, "-l sectsize" is ignored, and >>> "-s size" and "-d sectsize" set exactly the same variable. And for >>> external logs, having "-s size" override the "-l sectsize" is >>> completely wrong, but that's what it does.... >> >> Is it even safe/legit to have different sector sizes specified >> for log vs data? >> > > From my understanding, if the log is external (on another device), it > is completely ok. Unless there is some hiccup somewhere deep in the > code... It can be specified, but I wonder if it's safe to have i.e. a 4k log sector replaying onto a 512 byte sector data section, or vice versa. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 17, 2017 at 02:54:30PM -0500, Eric Sandeen wrote: > On 8/17/17 2:50 PM, Jan Tulak wrote: > > On Thu, Aug 17, 2017 at 6:01 PM, Eric Sandeen <sandeen@sandeen.net> wrote: > >> On 8/17/17 6:22 AM, Dave Chinner wrote: > >>> On Thu, Aug 17, 2017 at 11:45:34AM +0200, Jan Tulak wrote: > >>>> On Thu, Aug 17, 2017 at 10:35 AM, Dave Chinner <david@fromorbit.com> wrote: > >>>>> On Thu, Aug 17, 2017 at 09:38:44AM +0200, Jan Tulak wrote: > >>>>>> The man page is missing description of these options. > >>>>>> Add it. > >>>>> > >>>>> Just remove the options. They are redundant as they cannot be > >>>>> different from the values set by the "-s" sector size options. > >>>>> They aren't documented, so just remove them from mkfs. > >>>>> > >>>> > >>>> The code doesn't look like that. From what it does: > >>>> > >>>> -d sectsize: will set data section sector size > >>>> -l sectsize: will set log section sector size (and as I see now, this > >>>> is not in man page as well) > >>>> -s size: will set both data and log sector size > >>> > >>> If you just look at the option parsing, then it appears that way. > >>> > >>> But go an look at the code that validates and uses these options. > >>> It will only take a log sector size specification for external logs, > >>> other wise it will use the global sector size. Which is the same as > >>> the data section sector size. > >>> > >>> Hence for internal log filesystems, "-l sectsize" is ignored, and > >>> "-s size" and "-d sectsize" set exactly the same variable. And for > >>> external logs, having "-s size" override the "-l sectsize" is > >>> completely wrong, but that's what it does.... > >> > >> Is it even safe/legit to have different sector sizes specified > >> for log vs data? > >> > > > > From my understanding, if the log is external (on another device), it > > is completely ok. Unless there is some hiccup somewhere deep in the > > code... > > It can be specified, but I wonder if it's safe to have i.e. a 4k log > sector replaying onto a 512 byte sector data section, or vice versa. For external logs, that's fine. The log format is independent of the size of the objects being written to it. The sector size determines the smallest log write that can be done and hence the minimum log record size and the padding boundaries. It doesn't affect the format of the data that is being stored in the log records. For internal logs? They should always end up using the same sector size as the data device they reside on as both the data and the log have the same requirements for atomic sector writes... Cheers, Dave.
diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8 index b2bc223b..8ee64660 100644 --- a/man/man8/mkfs.xfs.8 +++ b/man/man8/mkfs.xfs.8 @@ -297,6 +297,24 @@ suboption is used only to make a filesystem image. If the .I value is omitted then 1 is assumed. .TP +.BI sectsize= value +This is used to specify the sector size for data section. The size is specified +as a value in bytes. The minimum value is 512 bytes; the maximum is 32768 +(32 KiB). The sector size must be a power of 2 and cannot be made larger than +the filesystem block size. The default size is 512 bytes. This option is +mutually exclusive with +.B \-d sectlog +option. +.TP +.BI sectlog= value +This is used to specify the sector size for data section. The size is specified +as a base two logarithm. The minimum value is 512 bytes; the maximum is 32768 +(32 KiB). The sector size must be a power of 2 and cannot be made larger than +the filesystem block size. The default size is 512 bytes. This option is +mutually exclusive with +.B \-d sectsize +option. +.TP .BI size= value This is used to specify the size of the data section. This suboption is required if
The man page is missing description of these options. Add it. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- man/man8/mkfs.xfs.8 | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)