On Mon, Nov 18, 2024 at 5:27 AM Lorenzo Stoakes lorenzo.stoakes@oracle.com wrote:
On Sat, Nov 16, 2024 at 05:59:17PM +0000, Pasha Tatashin wrote:
Page Detective will be using get_vma_name() that is currently used by fs/proc to show names of VMAs in /proc/<pid>/smaps for example.
Move this function to mm/vma.c, and make it accessible by modules.
This is incorrect.
mm/vma.c is for internal VMA implementation details, whose interface is explicitly mm/vma.h. This is so we can maintain the internal mechanism separate from interfaces and, importantly, are able to userland unit test VMA functionality.
I think this _should_ be in mm/vma.c, but if it were to be exported it would need to be via a wrapper function in mm/mmap.c or somewhere like this.
Ok, I can do that in the next version.
Also you broke the vma tests, go run make in tools/testing/vma/...
Hm interesting, I will take a look, this is surprising, as this patch should not really change the behavior of anything. I guess it would be because of the out of kernel vma.c build?
Your patch also does not apply against Andrew's tree and the mm-unstable branch (i.e. against 6.13 in other words) which is what new mm patches should be based upon.
Maybe I'll comment on the cover letter, but I don't agree you should be doing mm implementation details in a driver.
The core of this should be in mm rather than exporting a bunch of stuff and have a driver do it. You're exposing internal implementation details unnecessarily.
This is not a problem, I will convert Page Detective to be in core mm.
@@ -3474,6 +3474,9 @@ void setattr_copy(struct mnt_idmap *, struct inode *inode,
extern int file_update_time(struct file *file);
+void get_vma_name(struct vm_area_struct *vma, const struct path **path,
const char **name, const char **name_fmt);You're putting something in an mm/ C-file and the header in fs.h? Eh?
This is done so we do not have to include struct path into vma.h. fs.h already has some vma functions like: vma_is_dax() and vma_is_fsdax().