On Thu, Feb 13, 2025 at 06:14:48AM +0100, Christoph Hellwig wrote: > On Fri, Feb 07, 2025 at 09:39:42AM -0800, Darrick J. Wong wrote: > > > + /* XXX: this is a little verbose, but let's keep it for now */ > > > + xfs_info(mp, "using zone %u (%u)", > > > + rtg_rgno(oz->oz_rtg), zi->zi_nr_open_zones); > > > > Should this XXX become a tracepoint? > > > > > + trace_xfs_zone_activate(oz->oz_rtg); > > The tracepoint is just below, but yes - this obviously was left as a > canary in the coalmine to check if anyone actually reviews the code :) > > > > + if (xfs_is_shutdown(mp)) > > > + goto out_error; > > > + > > > + /* > > > + * If we don't have a cached zone in this write context, see if the > > > + * last extent before the one we are writing points of an active zone. > > > > "...writing points *to the end* of an active zone" ? > > We only really care about the same zone. Even if that doesn't create a > contiguous extent, it means the next GC cycle will make it contiguous. > But even before that the locality is kinda useful at least on HDD. > > There's still grammar issues, though which I've fixed up. > > > > > > + * If so, just continue writing to it. > > > + */ > > > + if (!*oz && ioend->io_offset) > > > + *oz = xfs_last_used_zone(ioend); > > > > Also, why not return oz instead of passing it out via double pointer? > > I remember going back and forth a few times. Let me give it a try to > see how it works out this time. > > > > + mp->m_zone_info = xfs_alloc_zone_info(mp); > > > + if (!mp->m_zone_info) > > > + return -ENOMEM; > > > + > > > + xfs_info(mp, "%u zones of %u blocks size (%u max open)", > > > + mp->m_sb.sb_rgcount, mp->m_groups[XG_TYPE_RTG].blocks, > > > + mp->m_max_open_zones); > > > > Tracepoint? > > I think this actually pretty usueful mount time information in the > kernel log. But if you mean a trace point on top of the message and > not instead I can look into it. Yeah, I like to just set up ftrace for 'xfs_zone*' and see what falls out of a test run, instead of pulling in printk and then having to filter out a bunch of other stuff. :) --D
