linux-kernel - Re: [PATCH v3 1/3] resource: Add @flags to region

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPcyv4g2n9yTWye2aVvKMP0X7mrm_NLKmGd5WBO2SesTj77gbg@mail.gmail.com>
Date:	Tue, 1 Dec 2015 08:54:23 -0800
From:	Dan Williams <dan.j.williams@...el.com>
To:	Borislav Petkov <bp@...en8.de>
Cc:	Toshi Kani <toshi.kani@....com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Tony Luck <tony.luck@...el.com>,
	Vishal L Verma <vishal.l.verma@...el.com>,
	Linux MM <linux-mm@...ck.org>,
	"linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
	Linux ACPI <linux-acpi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] resource: Add @flags to region_intersects()

On Tue, Dec 1, 2015 at 5:50 AM, Borislav Petkov <bp@...en8.de> wrote:
> On Tue, Nov 24, 2015 at 03:33:36PM -0700, Toshi Kani wrote:
>> region_intersects() checks if a specified region partially overlaps
>> or fully eclipses a resource identified by @name.  It currently sets
>> resource flags statically, which prevents the caller from specifying
>> a non-RAM region, such as persistent memory.  Add @flags so that
>> any region can be specified to the function.
>>
>> A helper function, region_intersects_ram(), is added so that the
>> callers that check a RAM region do not have to specify its iomem
>> resource name and flags.  This interface is exported for modules,
>> such as the EINJ driver.
>>
>> Signed-off-by: Toshi Kani <toshi.kani@....com>
>> Reviewed-by: Dan Williams <dan.j.williams@...el.com>
>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>> Cc: Vishal Verma <vishal.l.verma@...el.com>
>> ---
>>  include/linux/mm.h |    4 +++-
>>  kernel/memremap.c  |    5 ++---
>>  kernel/resource.c  |   23 ++++++++++++++++-------
>>  3 files changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 00bad77..c776af3 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -362,7 +362,9 @@ enum {
>>       REGION_MIXED,
>>  };
>>
>> -int region_intersects(resource_size_t offset, size_t size, const char *type);
>> +int region_intersects(resource_size_t offset, size_t size, const char *type,
>> +                     unsigned long flags);
>> +int region_intersects_ram(resource_size_t offset, size_t size);
>>
>>  /* Support for virtually mapped pages */
>>  struct page *vmalloc_to_page(const void *addr);
>> diff --git a/kernel/memremap.c b/kernel/memremap.c
>> index 7658d32..98f52f1 100644
>> --- a/kernel/memremap.c
>> +++ b/kernel/memremap.c
>> @@ -57,7 +57,7 @@ static void *try_ram_remap(resource_size_t offset, size_t size)
>>   */
>>  void *memremap(resource_size_t offset, size_t size, unsigned long flags)
>>  {
>> -     int is_ram = region_intersects(offset, size, "System RAM");
>
> Ok, question: why do those resource things types gets identified with
> a string?! We have here "System RAM" and next patch adds "Persistent
> Memory".
>
> And "persistent memory" or "System RaM" won't work and this is just
> silly.
>
> Couldn't struct resource have gained some typedef flags instead which we
> can much easily test? Using the strings looks really yucky.
>

At least in the case of region_intersects() I was just following
existing strcmp() convention from walk_system_ram_range.

We could define 'const char *system_ram = "System RAM"' somewhere and
then do pointer comparisons to cut down on the thrash of adding new
flags to 'struct resource'?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/