What is the meaning of XLOG_MIN_RECORD_BSIZE? (Missing a check?)

What is the meaning of XLOG_MIN_RECORD_BSIZE? (Missing a check?)

[Jan Tulak]

[-- Attachment #1.1: Type: text/plain, Size: 650 bytes --]

Hi guys,

what is the meaning of XLOG_MIN_RECORD_BSIZE in libxfs/xfs_log_format.h?
It is not used anywhere. I thought it might be related to -l su/sunit
option, but seeing tests with -l su=4096 (the macro is set to 16k), it
looks more like a forgotten value.

There is no check for a minimal value, so I can do -l su=1 (or su=0). Are
there some caveat​s (other than performance) with such a small value? Can
it be that we are missing a check? Because
XLOG_BIG_RECORD_BSIZE
​ and XLOG_MAX_RECORD_BSIZE are used and the upper bound is limited.​..


​Thanks​.

Cheers,
Jan

-- 
Jan Tulak
jtulak@redhat.com / jan@tulak.me

[-- Attachment #1.2: Type: text/html, Size: 1748 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

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

Re: What is the meaning of XLOG_MIN_RECORD_BSIZE? (Missing a check?)

[Brian Foster]

On Fri, Feb 19, 2016 at 02:34:52PM +0100, Jan Tulak wrote:
> Hi guys,
> 
> what is the meaning of XLOG_MIN_RECORD_BSIZE in libxfs/xfs_log_format.h?
> It is not used anywhere. I thought it might be related to -l su/sunit
> option, but seeing tests with -l su=4096 (the macro is set to 16k), it
> looks more like a forgotten value.
> 

It's the minimum log buffer size allowed in the kernel. It's used in
xfs_super.c at mount time to validate the logbsize option:

        if (mp->m_logbsize != -1 &&
            mp->m_logbsize !=  0 &&
            (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
             mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
             !is_power_of_2(mp->m_logbsize))) {
                xfs_warn(mp,
                        "invalid logbufsize: %d [not 16k,32k,64k,128k or 256k]",
                        mp->m_logbsize);
                return -EINVAL;
        }

I suspect it's not relevant in userspace.

> There is no check for a minimal value, so I can do -l su=1 (or su=0). Are
> there some caveat​s (other than performance) with such a small value? Can
> it be that we are missing a check? Because
> XLOG_BIG_RECORD_BSIZE
> ​ and XLOG_MAX_RECORD_BSIZE are used and the upper bound is limited.​..
> 

On a quick test, it looks like mkfs just ignores certain log stripe unit
values that aren't block aligned. I'd probably expect this to behave
similar to the '-d su' option and complain about invalid input..?

Brian

> 
> ​Thanks​.
> 
> Cheers,
> Jan
> 
> -- 
> Jan Tulak
> jtulak@redhat.com / jan@tulak.me

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

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

Re: What is the meaning of XLOG_MIN_RECORD_BSIZE? (Missing a check?)

[Jan Tulak]

[-- Attachment #1.1: Type: text/plain, Size: 2204 bytes --]

On Fri, Feb 19, 2016 at 4:43 PM, Brian Foster <bfoster@redhat.com> wrote:

> On Fri, Feb 19, 2016 at 02:34:52PM +0100, Jan Tulak wrote:
> > Hi guys,
> >
> > what is the meaning of XLOG_MIN_RECORD_BSIZE in libxfs/xfs_log_format.h?
> > It is not used anywhere. I thought it might be related to -l su/sunit
> > option, but seeing tests with -l su=4096 (the macro is set to 16k), it
> > looks more like a forgotten value.
> >
>
> It's the minimum log buffer size allowed in the kernel. It's used in
> xfs_super.c at mount time to validate the logbsize option:
>
>         if (mp->m_logbsize != -1 &&
>             mp->m_logbsize !=  0 &&
>             (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
>              mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
>              !is_power_of_2(mp->m_logbsize))) {
>                 xfs_warn(mp,
>                         "invalid logbufsize: %d [not 16k,32k,64k,128k or
> 256k]",
>                         mp->m_logbsize);
>                 return -EINVAL;
>         }
>
> I suspect it's not relevant in userspace.
>

This is ok, then. Thank you for pointing me to kernel space, I didn't
realised I should check it there too. :-)
​


>
> > There is no check for a minimal value, so I can do -l su=1 (or su=0). Are
> > there some caveat​s (other than performance) with such a small value? Can
> > it be that we are missing a check? Because
> > XLOG_BIG_RECORD_BSIZE
> > ​ and XLOG_MAX_RECORD_BSIZE are used and the upper bound is limited.​..
> >
>
> On a quick test, it looks like mkfs just ignores certain log stripe unit
> values that aren't block aligned. I'd probably expect this to behave
> similar to the '-d su' option and complain about invalid input..?
>

​Sounds logical and like what I expected and didn't found. I will send a
patch adding this check... the only question is, what should be the minimal
value? Should I check it against block size and forbid smaller sizes?
Aligning a stripe unit with length 1024 on 4096 blocks doesn't looks like a
nice thing. :-)
(And on a quick check, it seems that -d su is doing just that.)

Thanks,
Jan

-- 
Jan Tulak
jtulak@redhat.com / jan@tulak.me

[-- Attachment #1.2: Type: text/html, Size: 4064 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

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

Re: What is the meaning of XLOG_MIN_RECORD_BSIZE? (Missing a check?)

[Brian Foster]

On Fri, Feb 19, 2016 at 05:16:28PM +0100, Jan Tulak wrote:
> On Fri, Feb 19, 2016 at 4:43 PM, Brian Foster <bfoster@redhat.com> wrote:
> 
> > On Fri, Feb 19, 2016 at 02:34:52PM +0100, Jan Tulak wrote:
> > > Hi guys,
> > >
> > > what is the meaning of XLOG_MIN_RECORD_BSIZE in libxfs/xfs_log_format.h?
> > > It is not used anywhere. I thought it might be related to -l su/sunit
> > > option, but seeing tests with -l su=4096 (the macro is set to 16k), it
> > > looks more like a forgotten value.
> > >
> >
> > It's the minimum log buffer size allowed in the kernel. It's used in
> > xfs_super.c at mount time to validate the logbsize option:
> >
> >         if (mp->m_logbsize != -1 &&
> >             mp->m_logbsize !=  0 &&
> >             (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
> >              mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
> >              !is_power_of_2(mp->m_logbsize))) {
> >                 xfs_warn(mp,
> >                         "invalid logbufsize: %d [not 16k,32k,64k,128k or
> > 256k]",
> >                         mp->m_logbsize);
> >                 return -EINVAL;
> >         }
> >
> > I suspect it's not relevant in userspace.
> >
> 
> This is ok, then. Thank you for pointing me to kernel space, I didn't
> realised I should check it there too. :-)
> ​
> 
> 
> >
> > > There is no check for a minimal value, so I can do -l su=1 (or su=0). Are
> > > there some caveat​s (other than performance) with such a small value? Can
> > > it be that we are missing a check? Because
> > > XLOG_BIG_RECORD_BSIZE
> > > ​ and XLOG_MAX_RECORD_BSIZE are used and the upper bound is limited.​..
> > >
> >
> > On a quick test, it looks like mkfs just ignores certain log stripe unit
> > values that aren't block aligned. I'd probably expect this to behave
> > similar to the '-d su' option and complain about invalid input..?
> >
> 
> ​Sounds logical and like what I expected and didn't found. I will send a
> patch adding this check... the only question is, what should be the minimal
> value? Should I check it against block size and forbid smaller sizes?
> Aligning a stripe unit with length 1024 on 4096 blocks doesn't looks like a
> nice thing. :-)
> (And on a quick check, it seems that -d su is doing just that.)
> 

The man page says it must be a multiple of the fsb size. Indeed, '-d su'
complains about anything that is less than 1 FSB, so I would just go
with that. :)

Brian

> Thanks,
> Jan
> 
> -- 
> Jan Tulak
> jtulak@redhat.com / jan@tulak.me

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

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

Re: What is the meaning of XLOG_MIN_RECORD_BSIZE? (Missing a check?)

[Dave Chinner]

On Fri, Feb 19, 2016 at 11:29:13AM -0500, Brian Foster wrote:
> On Fri, Feb 19, 2016 at 05:16:28PM +0100, Jan Tulak wrote:
> > On Fri, Feb 19, 2016 at 4:43 PM, Brian Foster <bfoster@redhat.com> wrote:
> > > On Fri, Feb 19, 2016 at 02:34:52PM +0100, Jan Tulak wrote:
> > > > There is no check for a minimal value, so I can do -l su=1 (or su=0). Are
> > > > there some caveat​s (other than performance) with such a small value? Can
> > > > it be that we are missing a check? Because
> > > > XLOG_BIG_RECORD_BSIZE
> > > > ​ and XLOG_MAX_RECORD_BSIZE are used and the upper bound is limited.​..
> > > >
> > >
> > > On a quick test, it looks like mkfs just ignores certain log stripe unit
> > > values that aren't block aligned. I'd probably expect this to behave
> > > similar to the '-d su' option and complain about invalid input..?
> > 
> > ​Sounds logical and like what I expected and didn't found. I will send a
> > patch adding this check... the only question is, what should be the minimal
> > value? Should I check it against block size and forbid smaller sizes?
> > Aligning a stripe unit with length 1024 on 4096 blocks doesn't looks like a
> > nice thing. :-)
> > (And on a quick check, it seems that -d su is doing just that.)
> 
> The man page says it must be a multiple of the fsb size. Indeed, '-d su'
> complains about anything that is less than 1 FSB, so I would just go
> with that. :)

Keep in mind that a value of 0 in the superblock is completely
acceptible, in which case the kernel treats the log stripe unit as
being a single sector (i.e. same as a v1 log). See, for example,
xlog_sync() where it works out the padding to use for the log buffer
write.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: What is the meaning of XLOG_MIN_RECORD_BSIZE? (Missing a check?)

[Jan Tulak]

[-- Attachment #1.1: Type: text/plain, Size: 468 bytes --]

On Mon, Feb 22, 2016 at 12:18 AM, Dave Chinner <david@fromorbit.com> wrote:

>
> Keep in mind that a value of 0 in the superblock is completely
> acceptible, in which case the kernel treats the log stripe unit as
> being a single sector (i.e. same as a v1 log). See, for example,
> xlog_sync() where it works out the padding to use for the log buffer
> write.
>

​Good to know. :-)

Cheers,
Jan​


-- 
Jan Tulak
jtulak@redhat.com / jan@tulak.me

[-- Attachment #1.2: Type: text/html, Size: 1654 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

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