linux-kernel - Re: [PATCH v3 0/4] Implement IOCTL to get and clear soft dirty PTE

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] [day] [month] [year] [list]

Message-ID: <5725c4e2-e0b3-7573-5198-da6bb9637f3b@collabora.com>
Date:   Wed, 19 Oct 2022 12:18:35 +0500
From:   Muhammad Usama Anjum <usama.anjum@...labora.com>
To:     Michał Mirosław <emmir@...gle.com>
Cc:     Muhammad Usama Anjum <usama.anjum@...labora.com>,
        Danylo Mocherniuk <mdanylo@...gle.com>, avagin@...il.com,
        linux-mm@...ck.org, akpm@...ux-foundation.org,
        gregkh@...uxfoundation.org, corbet@....net, david@...hat.com,
        kernel@...labora.com, krisman@...labora.com,
        linux-doc@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        peter.enderborg@...y.com, shuah@...nel.org,
        viro@...iv.linux.org.uk, willy@...radead.org, figiel@...gle.com,
        kyurtsever@...gle.com, Paul Gofman <pgofman@...eweavers.com>,
        surenb@...gle.com
Subject: Re: [PATCH v3 0/4] Implement IOCTL to get and clear soft dirty PTE

On 10/18/22 10:17 PM, Michał Mirosław wrote:
> On Tue, 18 Oct 2022 at 15:23, Muhammad Usama Anjum
> <usama.anjum@...labora.com> wrote:
>>
>> On 10/18/22 4:11 PM, Michał Mirosław wrote:
>>> On Tue, 18 Oct 2022 at 12:36, Muhammad Usama Anjum
>>> <usama.anjum@...labora.com> wrote:
> [...]
>>>>     * @start:             Starting address
>>>>     * @len:               Length of the region
>>>>     * @vec:               Output page_region struct array
>>>>     * @vec_len:           Length of the page_region struct array
>>>>     * @max_out_page:      Optional max output pages (It must be less than
>>>> vec_len if specified)
>>>
>>> Why is it required to be less than vec_len? vec_len effectively
>>> specifies max number of ranges to find, and this new additional field
>>> counts pages, I suppose?
>>> BTW, if we count pages, then what size of them? Maybe using bytes
>>> (matching start/len fields) would be more consistent?
>> Yes, it if for counting pages. As the regions can have multiple pages,
>> user cannot specify through the number of regions that how many pages
>> does he need. Page size is used here as well like the start and len.
>> This is optional argument as this is only needed to emulate the Windows
>> syscall getWriteWatch.
> 
> I'm wondering about the condition that max_out_page < vec_len. Since
> both count different things (pages vs ranges) I would expect there is
> no strict relation between them and information returned is as much as
> fits both (IOW: at most vec_len ranges spanning not more than
> max_out_page pages). The field's name and description I'd suggest
> improving: maybe 'max_pages' with a comment that 0 = unlimited?
Correct, max_pages with this comment is what I want. I'll update. 
vec_len represents the total number of the page_range array elements. If 
the pages which we want to return are sparse or the consective pages 
have different flags, we'll only return one page in one page_range 
struct. In this case if someone has specified max_pages to be 10, 
vec_len must be at least 10 to keep store the 10 pages. So max_pages <= 
vec_len.

> 
> [...]
>>>> /* Special flags */
>>>> #define PAGEMAP_NO_REUSED_REGIONS       0x1
>>>
>>> What does this flag do?
>> Some non-dirty pages get marked as dirty because of the kernel's
>> internal activity. The dirty bit of the pages is stored in the VMA flags
>> and in the per page flags. If any of these two bits are set, the page is
>> considered to be dirty. Suppose you have cleared the dirty bit of half
>> of VMA which will be done by splitting the VMA and clearing dirty flag
>> in the half VMA and the pages in it. Now kernel may decide to merge the
>> VMAs again as dirty bit of VMAs isn't considered if the VMAs should be
>> merged. So the half VMA becomes dirty again. This splitting/merging
>> costs performance. The application receives a lot of pages which aren't
>> dirty in reality but marked as dirty. Performance is lost again here.
>>
>> This PAGEMAP_NO_REUSED_REGIONS flag is used to don't depend on the dirty
>> flag in the VMA flags. It only depends on the individual page dirty bit.
>> With doing this, the new memory regions which are just created, doesn't
>> look like dirty when seen with the IOCTL, but look dirty when seen from
>> pagemap. This seems okay as the user of this flag know the implication
>> of using it.
> 
> Thanks for explaining! Could you include this as a comment in the patch?
Will do.

> 
> Best Regards
> Michał Mirosław