On Thu, Aug 14, 2025 at 08:23:58AM +1000, Dave Chinner wrote: > On Tue, Aug 12, 2025 at 02:43:41PM +0200, cem@xxxxxxxxxx wrote: > > From: Carlos Maiolino <cem@xxxxxxxxxx> > > > > An user reported an inconsistency while mounting a V2 log filesystem > > with logbsize equals to the stripe unit being used (192k). > > > > The current validation algorithm for the log buffer size enforces the > > user to pass a power_of_2 value between [16k-256k]. > > The manpage dictates the log buffer size must be a multiple of the log > > stripe unit, but doesn't specify it must be a power_of_2. > > The xfs(5) man page explicitly lists a set of fixed sizes that are > supported, all of which are a power of two. Right, but yet if a log sunit is set, mount will automatically set to a different value (if sunit is not any of those values). This is when it gets confused. > > > Also, if > > logbsize is not specified at mount time, it will be set to > > max(32768, log_sunit), where log_sunit not necessarily is a power_of_2. > > log_sunit is set by mkfs, so the user cannot change that > dynmaically. LSU is generally something that should not be > overridden by users - it is set by mkfs for optimal performance on > devices that require specific write alignment to avoid costly RMW > cycles... > > > It does seem to me then that logbsize being a power_of_2 constraint must > > be relaxed if there is a configured log stripe unit, so this patch > > updates the logbsize validation logic to ensure that: > > > > - It can only be set to a specific range [16k-256k] > > > > - Will be aligned to log stripe unit when the latter is set, > > and will be at least the same size as the log stripe unit. > > Have you tested what happens when LSU is set to, say 72kB and > logbsize is set to 216kB? (i.e. integer multiple of 3). What about a > LSU of 19kB on a 1kB filesystem and a logbsize of 247kB? So far I did test only a 96k logbsize with 32k LSU, I need more time to test different configurations, but I see where you are getting to. > > These are weird and wacky configurations, but this change allows > users to create them. i.e. it greatly expands the test matrix for > these mount and mkfs options by adding an entirely new > dimension to LSU configuration testing matrix. Right, agreed. > > Does the benefit that this change provide outweigh the risk of > uncovering some hidden corner case in the iclog alignment code? And > failure here will result in journal corruption, and that may be very > difficult to detect as it will only be noticed if there are > subsequent recovery failures. That's a pretty nasty failure mode - > it's difficult to triage and find the root cause, and it guarantees > data loss and downtime for the user because it can only be fixed by > running xfs_repair to zero the log. > > So, do the risks associated with greatly expanding the supported > iclogbuf/LSU configurations outweigh the benefits that users will > gain from having this expanded functionality? Agree. It's not worth the increased complexity, I'll update the manpage only to reflect the current behavior. Thanks for the review Dave. Carlos, > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx
