linux-kernel - Re: [RFC][Patch v11 1/2] mm: page

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 for Android: free password hash cracker in your pocket

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

Message-ID: <fd49381e-cdfa-7ac9-e938-ac790995df24@redhat.com>
Date:   Thu, 11 Jul 2019 12:36:01 -0400
From:   Nitesh Narayan Lal <nitesh@...hat.com>
To:     Dave Hansen <dave.hansen@...el.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        pbonzini@...hat.com, lcapitulino@...hat.com, pagupta@...hat.com,
        wei.w.wang@...el.com, yang.zhang.wz@...il.com, riel@...riel.com,
        david@...hat.com, mst@...hat.com, dodgen@...gle.com,
        konrad.wilk@...cle.com, dhildenb@...hat.com, aarcange@...hat.com,
        alexander.duyck@...il.com, john.starks@...rosoft.com,
        mhocko@...e.com
Subject: Re: [RFC][Patch v11 1/2] mm: page_hinting: core infrastructure


On 7/11/19 12:22 PM, Dave Hansen wrote:
> On 7/11/19 8:25 AM, Nitesh Narayan Lal wrote:
>> On 7/10/19 4:45 PM, Dave Hansen wrote:
>>> On 7/10/19 12:51 PM, Nitesh Narayan Lal wrote:
>>>> +struct zone_free_area {
>>>> +	unsigned long *bitmap;
>>>> +	unsigned long base_pfn;
>>>> +	unsigned long end_pfn;
>>>> +	atomic_t free_pages;
>>>> +	unsigned long nbits;
>>>> +} free_area[MAX_NR_ZONES];
>>> Why do we need an extra data structure.  What's wrong with putting
>>> per-zone data in ... 'struct zone'?
>> Will it be acceptable to add fields in struct zone, when they will only
>> be used by page hinting?
> Wait a sec...  MAX_NR_ZONES the number of zone types not the maximum
> number of *zones* in the system.
>
> Did you test this on a NUMA system?
Yes, I tested it with a guest having 2 and 3 NUMA nodes.
> In any case, yes, you can put these in 'struct zone'.  It will waste
> less space that way, on average, than what you have here (one you scale
> it to MAX_NR_ZONE*MAX_NUM_NODES.
>>>   The cover letter claims that it
>>> doesn't touch core-mm infrastructure, but if it depends on mechanisms
>>> like this, I think that's a very bad thing.
>>>
>>> To be honest, I'm not sure this series is worth reviewing at this point.
>>>  It's horribly lightly commented and full of kernel antipatterns lik
>>>
>>> void func()
>>> {
>>> 	if () {
>>> 		... indent entire logic
>>> 		... of function
>>> 	}
>>> }
>> I usually run checkpatch to detect such indentation issues. For the
>> patches, I shared it didn't show me any issues.
> Just because checkpatch doesn't complain does not mean it is good form.
>  We write the above as:
>
> void func()
> {
> 	if (!something)
> 		goto out;
>
> 	... logic of function here
> out:
> 	// cleanup
> }

Yeap, I got it. I will correct this.


-- 
Thanks
Nitesh