Hi Michał, Thank you so much for comment! On 2/17/23 8:18 PM, Michał Mirosław wrote: > On Thu, 2 Feb 2023 at 12:30, Muhammad Usama Anjum > <usama.anjum@xxxxxxxxxxxxx> wrote: > [...] >> - The masks are specified in required_mask, anyof_mask, excluded_ mask >> and return_mask. > [...] The interface was suggested by Andrei back on the review of v3 [1]: > I mean we should be able to specify for what pages we need to get info > for. An ioctl argument can have these four fields: > * required bits (rmask & mask == mask) - all bits from this mask have to be set. > * any of these bits (amask & mask != 0) - any of these bits is set. > * exclude masks (emask & mask == 0) = none of these bits are set. > * return mask - bits that have to be reported to user. > > May I suggest a slightly modified interface for the flags? I've added everyone who may be interested in making interface better. > > As I understand, the return_mask is what is applied to page flags to > aggregate the list. > This is a separate thing, and I think it doesn't need changes except > maybe an improvement > in the documentation and visual distinction. > > For the page-selection mechanism, currently required_mask and > excluded_mask have conflicting They are opposite of each other: All the set bits in required_mask must be set for the page to be selected. All the set bits in excluded_mask must _not_ be set for the page to be selected. > responsibilities. I suggest to rework that to: > 1. negated_flags: page flags which are to be negated before applying > the page selection using following masks; Sorry I'm unable to understand the negation (which is XOR?). Lets look at the truth table: Page Flag negated_flags 0 0 0 0 1 1 1 0 1 1 1 0 If a page flag is 0 and negated_flag is 1, the result would be 1 which has changed the page flag. It isn't making sense to me. Why the page flag bit is being fliped? When Anrdei had proposed these masks, they seemed like a fancy way of filtering inside kernel and it was straight forward to understand. These masks would help his use cases for CRIU. So I'd included it. Please can you elaborate what is the purpose of negation? > 2. required_flags: flags which all have to be set in the > (negation-applied) page flags; > 3. anyof_flags: flags of which at least one has to be set in the > (negation-applied) page flags; > > IOW, the resulting algorithm would be: > > tested_flags = page_flags ^ negated_flags; > if (~tested_flags & required_flags) > skip page; > if (!(tested_flags & anyof_flags)) > skip_page; > > aggregate_on(page_flags & return_flags); > > Best Regards > Michał Mirosław [1] https://lore.kernel.org/all/YyiDg79flhWoMDZB@xxxxxxxxx -- BR, Muhammad Usama Anjum
