Re: [PATCH v5 2/3] fs/proc/task_mmu: Implement IOCTL to get and/or the clear info about PTEs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michał,

Thank you so much for reviewing.

On 11/7/22 5:26 PM, Michał Mirosław wrote:
+
+/*
+ * struct page_region - Page region with bitmap flags
+ * @start:     Start of the region
+ * @len:       Length of the region
+ * bitmap:     Bits sets for the region
+ */
+struct page_region {
+       __u64 start;
+       __u64 len;
+       __u32 bitmap;
+       __u32 __reserved;

"u64 flags"? If an extension is needed it would already require a new
ioctl or something in the `arg` struct.
I feel like the masks must have the same type as this bitmap variable as the return_mask specifies the flags to be returned in bitmap. All the masks are of type __u32. This is why I'd kept the bitmap of type _u32 as well. I've kept them of 32 bit size as currently we are adding support for 4 flags and there is still room to add 28 more bits in the future. Do you still think that I should update the masks and bitmap to _u64?

+ * @start:             Starting address of the region
+ * @len:               Length of the region (All the pages in this length are included)
+ * @vec:               Address of page_region struct array for output
+ * @vec_len:           Length of the page_region struct array
+ * @max_pages:         Optional max return pages (It must be less than vec_len if specified)

I think we discussed that this is not counting the same things as
vec_len, so there should not be a reference between the two. The limit
is whatever fits under both conditions (IOW: n_vecs <= vec_len &&
(!max_pages || n_pages <= max_pages).
In worse case when pages cannot be folded into the page_region, the one page_region may have information of only one page. This is why I've compared them. I want to communicate to the user that if max_pages is used, the vec_len should be of equal or greater size (to cater worse case which can happen at any time). Otherwise in worse case, the api can return without finding the max_pages number of pages. I don't know how should I put this in the comment.

(I only reviewed the API now. The implementation I think could be
simpler, but let's leave that to after the API is agreed on.)

Best Regards
Michał Mirosław



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux