On Tue, Dec 09, 2014 at 04:35:33PM +0100, Laszlo Ersek wrote:
> I disagree with the patch in general, and I strongly disagree with this
> specific change in particular. This part:
>
> - breaks the alignment for the right side of the table
Well, we can start by removing the sizes - they're useless anyway and
can be computed with a simple awk script or whatever. I.e.,
...
[ 0.000000] efi: mem23: [0x000000007ee3b000-0x000000007ee4e000) [RtData |RT|WB|WT|WC|UC])
[ 0.000000] efi: mem24: [0x000000007ee4e000-0x000000007fd4e000) [BootData |WB|WT|WC|UC]
[ 0.000000] efi: mem25: [0x000000007fd4e000-0x000000007fece000) [BootCode |WB|WT|WC|UC]
[ 0.000000] efi: mem26: [0x000000007fece000-0x000000007fefe000) [RtCode |RT|WB|WT|WC|UC]
[ 0.000000] efi: mem27: [0x000000007fefe000-0x000000007ff22000) [RtData |RT|WB|WT|WC|UC]
...
Then we could do some more work like fishing out columns and removing
those which are not set in any region from the output completely and
then have alignment but with only the relevant fields, ...
> - completely defeats another of my goals with the original patch, which
> was to be able to get an overview of memory attributes *at a glance*.
> That only works if each column is strictly associated with one bit,
> and your gaze can slide over empty stretches.
Well, and empty column doesn't tell me a whole lot, does it?
[ 0.000000] efi: mem00: [Conventional Memory| | | | | |WB|WT|WC|UC] range=[0x0000000000000000-0x0000000000001000) (0MB)
I still have to go and look up what those 5 empty bits mean. Thus my
angle was: if it is not set, don't print it.
> Please compare how you visually execute a search for the next
> runtime region (or the next non-WB line) before and after.
I'd look for region names RtData/RtCode.
Unless this dump is supposed to be used to sanity-check region attribute
flags too, then I see your point about the alignment.
> Your patch shaves off 30 characters of the width if I measured it right
> (goes from 131 to 101.) I don't believe that would justify breaking the
> alignment of the columns. For example, you could remove 15 characters
> from the right just with "dmesg -t" (timestamps are probably not overly
> important for printing the memmap).
Yeah, but you can look at that output with other means, i.e. serial
console/netconsole/guest boot log, etc.
> Anyway it's a matter of taste. I know you work with this every day so if
> it doesn't meet your needs then I won't insist preserving it. I won't
> NACK the patch it but I certainly won't ACK it either.
Ok, fair enough. Let's see what the others think.
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
