On Wed, Oct 03, 2018 at 08:59:52AM -0500, Eric Sandeen wrote: > On 10/3/18 8:39 AM, Carlos Maiolino wrote: > > On Wed, Oct 03, 2018 at 07:47:46AM -0500, Eric Sandeen wrote: > >> On 10/3/18 7:35 AM, Carlos Maiolino wrote: > >>> The addition of FIBT, RMAP and REFCOUNT changed the offsets into > >>> __xfssats structure. > >>> > >>> Although this didn't cause any direct issue, xqmstat_proc_show() relied > >>> on the old offsets to display the xqm statistics. > >> > >> Well, it caused /proc/fs/xfs/xqmstat to display garbage data, right? > >> That seems worth highlighting in the changelog, and not glossing over. > > > > Well, I wouldn't say 'garbage' data. I'd say data from other fields in the > > structure (at this point, specifically fino btree data) :P, but sure, I can add > > it to the changelog. > > That's kind of like saying stale data exposure or kernel memory leaks isn't > garbage data, it's just someone /else's/ data. ;) > > Anyway, however you want to phrase it, aI think the change needs to highlight > that there /is/ a failure and it's not an irrelevant fix. > Sure, I'll resubmit the patch again with updated description together with a new version of patch 2, I am considering to do what Dave suggested and replace the defines by offsetof() macros, but well, will see. I'll add your reviewed-by flag to the patch 1, is it ok? I think that's what you meant with your previous reply? > >> > >> Could maybe use Fixes: tags for: > >> > >> 00f4e4f9 xfs: add rmap btree stats infrastructure > >> aafc3c24 xfs: support the XFS_BTNUM_FINOBT free inode btree type > >> 46eeb521 xfs: introduce refcount btree definitions > >> > > > > No objection. > > > >> and a stable tag as well? Though stable is tricky because different patches > >> would be required for different points along the stats structure evolution... > >> maybe best to ignore it, chances of auto-applying it correctly are slim to > >> none. > > > > Indeed, this may not apply to stable trees, unless the tree itself contains all > > three data structures (rmap finobtree and refcount). > > Yeah. It would go back to 4.10 cleanly, tho. Just a thought. Ok, Cheers > > -Eric -- Carlos
