On Fri 2014-11-21 12:06 -0500, Dave Anderson wrote: > > Aaron, > > I like the addition. But I've got a few nits regarding the patch. > > Given that "kmem -i" is so commonly-used, I prefer not to introduce something > that could conceivably generate a command-killing "invalid structure member > offset" error. And that could happen in your hugetlb_total_pages() function > because of the hstate structure dependencies. (See dump_hstates() for example) > > Also, in dump_kmeminfo() you use MEMBER_OFFSET(), which should be avoided > because it will quietly return -1 if the structure member doesn't exist, > leading to bogus output. That's the whole purpose behind using OFFSET(), > or at least storing/validating the MEMBER_OFFSET() return value before > blindly using it. > > It may seem pedantic, but if history has shown us anything, it's that > kernel developers cannot stop themselves from changing structure/member > names. OK - understood. > So if you could just adjust those kinds of issues, this patch would be > a nice addition. Thanks. I'll work on a v2. -- Aaron Tomlin -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
