On Sun, Apr 19, 2015 at 12:46 AM, Boaz Harrosh <b...@plexistor.com> wrote:
> On 04/18/2015 04:35 AM, Dan Williams wrote:
[..]
>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>> index 11cc7d54ec3f..410af501a941 100644
>> --- a/arch/x86/kernel/e820.c
>> +++ b/arch/x86/kernel/e820.c
>> @@ -137,6 +137,8 @@ static void __init e820_print_type(u32 type)
>> case E820_RESERVED_KERN:
>> printk(KERN_CONT "usable");
>> break;
>> + case E820_PMEM:
>> + case E820_PRAM:
>
> NACK!
>
> This is the most important print in the system and it is a pure
> user Interface. It has no effect what so ever on functionality
> It is to Inform the user through dmesg what is the content of the
> table.
It still describes how the memory is used which is "reserved" for a
driver, I don't see how increasing the verbosity here improves debug
given the alternatives, see below...
>
>> case E820_RESERVED:
>> printk(KERN_CONT "reserved");
>> break;
>> @@ -149,9 +151,6 @@ static void __init e820_print_type(u32 type)
>> case E820_UNUSABLE:
>> printk(KERN_CONT "unusable");
>> break;
>
> + case E820_PMEM:
>> - case E820_PRAM:
>> - printk(KERN_CONT "persistent (type %u)", type);
>> - break;
>
> Just add the new (7) entry here please. Here Christoph has bike shed
> it for you.
/proc/iomem has these details to differentiate PRAM and PMEM as well
as show which driver(s)/device(s) have claimed the range(s).
>> default:
>> printk(KERN_CONT "type %u", type);
>> break;
Here is where you can see undefined/unknown types.
>> @@ -919,10 +918,26 @@ static inline const char *e820_type_to_string(int
>> e820_type)
>> case E820_NVS: return "ACPI Non-volatile Storage";
>> case E820_UNUSABLE: return "Unusable memory";
>> case E820_PRAM: return "Persistent RAM";
>> + case E820_PMEM: return "Persistent I/O Memory";
>> default: return "reserved";
>> }
>> }
>>
>> +static bool do_mark_busy(u32 type, struct resource *res)
>> +{
>> + if (res->start < (1ULL<<20))
>> + return true;
>> +
>> + switch (type) {
>> + case E820_RESERVED:
>> + case E820_PRAM:
>> + case E820_PMEM:
>> + return false;
>> + default:
>> + return true;
>> + }
>
> Sigh. Again an unknown type comes out busy. Busy means
> resource used. It does *not* mean "unknown type".
>
> It just forces researchers to ignore the return value of
> request_region. And not be protected by double lock. It
> does not really prevent anything
You're free to submit a standalone patch to change this policy... see
the new "OEM-reserved" memory types in ACPI 6.
That said, I think we're better off with the current policy. If
unknown memory types were treated as permanently-busy back when we
initially started experimenting with NVDIMM support (2010) then I
doubt the e820-type-12 prototype would ever have escaped the lab. We
could have avoided a good amount of confusion.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
