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
| ||
|
Date: Tue, 18 Oct 2022 19:17:36 +0200 From: Michał Mirosław <emmir@...gle.com> To: Muhammad Usama Anjum <usama.anjum@...labora.com> Cc: 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 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? [...] > >> /* 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? Best Regards Michał Mirosław
Powered by blists - more mailing lists