On Tue, Feb 21, 2023 at 4:42 AM Michał Mirosław emmir@google.com wrote:
On Tue, 21 Feb 2023 at 11:28, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
Hi Michał,
Thank you so much for comment!
On 2/17/23 8:18 PM, Michał Mirosław wrote:
[...]
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:
- 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?
The XOR is a way to invert the tested value of a flag (from positive to negative and the other way) without having the API with invalid values (with required_flags and excluded_flags you need to define a rule about what happens if a flag is present in both of the masks - either prioritise one mask over the other or reject the call). (Note: the XOR is applied only to the value of the flags for the purpose of testing page-selection criteria.)
Michał,
Your API isn't much different from the current one, but it requires a bit more brain activity for understanding.
The current set of masks can be easy translated to the new one: negated_flags = excluded_flags required_flags_new = excluded_flags | required_flags
As for invalid values, I think it is an advantage of the current API. I mean we can easily detect invalid values and return EINVAL. With your API, such mistakes will be undetectable.
As for priorities, I don't see this problem here If I don't miss something.
We can rewrite the code this way: ``` if (required_mask && ((page_flags & required_mask) != required_mask) skip page; if (anyof_mask && !(page_flags & anyof_mask)) skip page; if (page_flags & excluded_mask) skip page; ```
I think the result is always the same no matter in what order each mask is applied.
Thanks, Andrei