schedule suggestion

Just wanted to suggest to push HMM status down one slot in the agenda to avoid having FS and MM first going into their own room and then merging back for GUP and DAX, and re-splitting after. More over HMM and NUMA talks will be good to have back to back as they deal with same kind of thing mostly.

Just wanted to suggest to push HMM status down one slot in the agenda to avoid having FS and MM first going into their own room and then merging back for GUP and DAX, and re-splitting after. More over HMM and NUMA talks will be good to have back to back as they deal with same kind of thing mostly.

Quoted 5 lines · Jerome Glisse

Just wanted to suggest to push HMM status down one slot in the agenda to avoid having FS and MM first going into their own room and then merging back for GUP and DAX, and re-splitting after. More over HMM and NUMA talks will be good to have back to back as they deal with same kind of thing mostly.

=

So while we are talking about schedule suggestions, we see that there's lots of empty slots in the FS track. We (xfs guys) were just chatting on #xfs about whether we'd have time to have a "XFS devel meeting" at some point during LSF/MM as we are rarely in the same place at the same time.
=

I'd like to propose that we compact the fs sessions so that we get a 3-slot session reserved for "Individual filesystem discussions" one afternoon. That way we've got time in the schedule for the all the ext4/btrfs/XFS/NFS/CIFS devs to get together with each other and talk about things of interest only to their own fileystems. =

That means we all don't have to find time outside the schedule to do this, and think this wold be time very well spent for most fs people at the conf....

Oh can i get one more small slot for fs ? I want to ask if they are any people against having a callback everytime a struct file is added to a task_struct and also having a secondary array so that special file like device file can store something opaque per task_struct per struct file.

Quoted 5 lines · Jerome Glisse

Just wanted to suggest to push HMM status down one slot in the agenda to avoid having FS and MM first going into their own room and then merging back for GUP and DAX, and re-splitting after. More over HMM and NUMA talks will be good to have back to back as they deal with same kind of thing mostly.

So while we are talking about schedule suggestions, we see that there's lots of empty slots in the FS track. We (xfs guys) were just chatting on #xfs about whether we'd have time to have a "XFS devel meeting" at some point during LSF/MM as we are rarely in the same place at the same time.

I'd like to propose that we compact the fs sessions so that we get a 3-slot session reserved for "Individual filesystem discussions" one afternoon. That way we've got time in the schedule for the all the ext4/btrfs/XFS/NFS/CIFS devs to get together with each other and talk about things of interest only to their own fileystems.

That means we all don't have to find time outside the schedule to do this, and think this wold be time very well spent for most fs people at the conf....

Quoted 9 lines

I'd like to propose that we compact the fs sessions so that we get a 3-slot session reserved for "Individual filesystem discussions" one afternoon. That way we've got time in the schedule for the all the ext4/btrfs/XFS/NFS/CIFS devs to get together with each other and talk about things of interest only to their own fileystems.

That means we all don't have to find time outside the schedule to do this, and think this wold be time very well spent for most fs people at the conf....

I'd love this as well.

Quoted 5 lines · Jerome Glisse

Oh can i get one more small slot for fs ? I want to ask if they are any people against having a callback everytime a struct file is added to a task_struct and also having a secondary array so that special file like device file can store something opaque per task_struct per struct file.

Do you really want something per _thread_, and not per _mm_?

Quoted 8 lines · Matthew Wilcox

Quoted 5 lines · Jerome Glisse

Oh can i get one more small slot for fs ? I want to ask if they are any people against having a callback everytime a struct file is added to a task_struct and also having a secondary array so that special file like device file can store something opaque per task_struct per struct file.

Do you really want something per _thread_, and not per _mm_?

Well per mm would be fine but i do not see how to make that happen with reasonable structure. So issue is that you can have multiple task with same mm but different file descriptors (or am i wrong here ?) thus there would be no easy way given a struct file to lookup the per mm struct.

So as a not perfect solution i see a new array in filedes which would allow device driver to store a pointer to their per mm data structure. To be fair usualy you will only have a single fd in a single task for a given device.

If you see an easy way to get a per mm per inode pointer store somewhere with easy lookup i am all ears :)

Quoted 15 lines · Jerome Glisse

Quoted 8 lines · Matthew Wilcox

Quoted 5 lines · Jerome Glisse

Oh can i get one more small slot for fs ? I want to ask if they are any people against having a callback everytime a struct file is added to a task_struct and also having a secondary array so that special file like device file can store something opaque per task_struct per struct file.

Do you really want something per _thread_, and not per _mm_?

Well per mm would be fine but i do not see how to make that happen with reasonable structure. So issue is that you can have multiple task with same mm but different file descriptors (or am i wrong here ?) thus there would be no easy way given a struct file to lookup the per mm struct.

Quoted 8 lines

So as a not perfect solution i see a new array in filedes which would allow device driver to store a pointer to their per mm data structure. To be fair usualy you will only have a single fd in a single task for a given device.

If you see an easy way to get a per mm per inode pointer store somewhere with easy lookup i am all ears :)

I may be misunderstanding, but to be clear: struct files don't get added to a thread, per-se.

When userland calls open() or similar, the struct file gets added to the files_struct. Those are generally shared with other threads within the same process. The files_struct can also be shared with other processes if you clone() with the right flags.

Doing something per-thread on every open may be rather difficult to do.

Quoted 28 lines · Jeff Layton

Quoted 15 lines · Jerome Glisse

Quoted 8 lines · Matthew Wilcox

Quoted 5 lines · Jerome Glisse

Oh can i get one more small slot for fs ? I want to ask if they are any people against having a callback everytime a struct file is added to a task_struct and also having a secondary array so that special file like device file can store something opaque per task_struct per struct file.

Do you really want something per _thread_, and not per _mm_?

Well per mm would be fine but i do not see how to make that happen with reasonable structure. So issue is that you can have multiple task with same mm but different file descriptors (or am i wrong here ?) thus there would be no easy way given a struct file to lookup the per mm struct.

Quoted 8 lines

So as a not perfect solution i see a new array in filedes which would allow device driver to store a pointer to their per mm data structure. To be fair usualy you will only have a single fd in a single task for a given device.

If you see an easy way to get a per mm per inode pointer store somewhere with easy lookup i am all ears :)

I may be misunderstanding, but to be clear: struct files don't get added to a thread, per-se.

Quoted 6 lines

When userland calls open() or similar, the struct file gets added to the files_struct. Those are generally shared with other threads within the same process. The files_struct can also be shared with other processes if you clone() with the right flags.

Doing something per-thread on every open may be rather difficult to do.

Basicly i want a callback in __fd_install(), do_dup2(), dup_fd() and add void * *private_data; to struct fdtable (also a default array to struct files_struct). The callback would be part of struct file_operations. and only call if it exist (os overhead is only for device driver that care).

Did i miss something fundamental ? copy_files() call dup_fd() so i should be all set here.

I will work on patches i was hoping this would not be too much work.

Quoted 42 lines · Jerome Glisse

Quoted 28 lines · Jeff Layton

Quoted 15 lines · Jerome Glisse

Quoted 8 lines · Matthew Wilcox

Quoted 5 lines · Jerome Glisse

Oh can i get one more small slot for fs ? I want to ask if they are any people against having a callback everytime a struct file is added to a task_struct and also having a secondary array so that special file like device file can store something opaque per task_struct per struct file.

Do you really want something per _thread_, and not per _mm_?

Well per mm would be fine but i do not see how to make that happen with reasonable structure. So issue is that you can have multiple task with same mm but different file descriptors (or am i wrong here ?) thus there would be no easy way given a struct file to lookup the per mm struct.

Quoted 8 lines

So as a not perfect solution i see a new array in filedes which would allow device driver to store a pointer to their per mm data structure. To be fair usualy you will only have a single fd in a single task for a given device.

If you see an easy way to get a per mm per inode pointer store somewhere with easy lookup i am all ears :)

I may be misunderstanding, but to be clear: struct files don't get added to a thread, per-se.

Quoted 6 lines

When userland calls open() or similar, the struct file gets added to the files_struct. Those are generally shared with other threads within the same process. The files_struct can also be shared with other processes if you clone() with the right flags.

Doing something per-thread on every open may be rather difficult to do.

Basicly i want a callback in __fd_install(), do_dup2(), dup_fd() and add void * *private_data; to struct fdtable (also a default array to struct files_struct). The callback would be part of struct file_operations. and only call if it exist (os overhead is only for device driver that care).

Quoted 5 lines

Did i miss something fundamental ? copy_files() call dup_fd() so i should be all set here.

I will work on patches i was hoping this would not be too much work.

No, I think I misunderstood. I was thinking you wanted to iterate over all of the threads that might be associated with a struct file, and that's rather non-trivial.

A callback when you add a file to the files_struct seems like it would probably be OK (in principle).

Quoted 6 lines

Quoted 6 lines

Basicly i want a callback in __fd_install(), do_dup2(), dup_fd() and add void * *private_data; to struct fdtable (also a default array to struct files_struct). The callback would be part of struct file_operations. and only call if it exist (os overhead is only for device driver that care).

Quoted 4 lines

Quoted 4 lines

Did i miss something fundamental ? copy_files() call dup_fd() so i should be all set here.

I will work on patches i was hoping this would not be too much work.

Well scratch that whole idea, i would need to add a new array to task struct which make it a lot less appealing. Hence a better solution is to instead have this as part of mm (well indirectly).

Quoted 11 lines · Jerome Glisse

Quoted 10 lines

Quoted 10 lines

Basicly i want a callback in __fd_install(), do_dup2(), dup_fd() and add void * *private_data; to struct fdtable (also a default array to struct files_struct). The callback would be part of struct file_operations. and only call if it exist (os overhead is only for device driver that care).

Did i miss something fundamental ? copy_files() call dup_fd() so i should be all set here.

I will work on patches i was hoping this would not be too much work.

Quoted 3 lines

Well scratch that whole idea, i would need to add a new array to task struct which make it a lot less appealing. Hence a better solution is to instead have this as part of mm (well indirectly).

It shouldn't be too bad to add a struct radix_tree to the fdtable.

I'm sure we could just not support weird cases like sharing the fdtable without sharing the mm. Does anyone actually do that?

Quoted 8 lines · Matthew Wilcox

Quoted 3 lines

Well scratch that whole idea, i would need to add a new array to task struct which make it a lot less appealing. Hence a better solution is to instead have this as part of mm (well indirectly).

It shouldn't be too bad to add a struct radix_tree to the fdtable.

I'm sure we could just not support weird cases like sharing the fdtable without sharing the mm. Does anyone actually do that?

Well like you pointed out what i really want is a 1:1 structure linking a device struct an a mm_struct. Given that this need to be cleanup when mm goes away hence tying this to mmu_notifier sounds like a better idea.

I am thinking of adding a hashtable to mmu_notifier_mm using file id for hash as this should be a good hash value for common cases. I only expect few drivers to need that (GPU drivers, RDMA). Today GPU drivers do have a hashtable inside their driver and they has on the mm struct pointer, i believe hash mmu_notifier_mm using file id will be better.

Basicly i want a callback in __fd_install(), do_dup2(), dup_fd() and add void * *private_data; to struct fdtable (also a default array to struct files_struct). The callback would be part of struct file_operations. and only call if it exist (os overhead is only for device driver that care).

Did i miss something fundamental ? copy_files() call dup_fd() so i should be all set here.

Quoted 14 lines · Jerome Glisse

Quoted 8 lines · Matthew Wilcox

Quoted 3 lines

Well scratch that whole idea, i would need to add a new array to task struct which make it a lot less appealing. Hence a better solution is to instead have this as part of mm (well indirectly).

It shouldn't be too bad to add a struct radix_tree to the fdtable.

I'm sure we could just not support weird cases like sharing the fdtable without sharing the mm. Does anyone actually do that?

Well like you pointed out what i really want is a 1:1 structure linking a device struct an a mm_struct. Given that this need to be cleanup when mm goes away hence tying this to mmu_notifier sounds like a better idea.

Quoted 5 lines

I am thinking of adding a hashtable to mmu_notifier_mm using file id for hash as this should be a good hash value for common cases. I only expect few drivers to need that (GPU drivers, RDMA). Today GPU drivers do have a hashtable inside their driver and they has on the mm struct pointer, i believe hash mmu_notifier_mm using file id will be better.

file descriptors are small positive integers ...

ideal for the radix tree.
If you need to find your data based on the struct file address, then by all means a hashtable is the better data structure.

Well like you pointed out what i really want is a 1:1 structure linking a device struct an a mm_struct. Given that this need to be cleanup when mm goes away hence tying this to mmu_notifier sounds like a better idea.

I am thinking of adding a hashtable to mmu_notifier_mm using file id for hash as this should be a good hash value for common cases. I only expect few drivers to need that (GPU drivers, RDMA). Today GPU drivers do have a hashtable inside their driver and they has on the mm struct pointer, i believe hash mmu_notifier_mm using file id will be better.

Quoted 5 lines · Jerome Glisse

Basicly i want a callback in __fd_install(), do_dup2(), dup_fd() and add void * *private_data; to struct fdtable (also a default array to struct files_struct). The callback would be part of struct file_operations. and only call if it exist (os overhead is only for device driver that care).

Hell, *NO*. This is insane - you would need to maintain extra counts ("how many descriptors refer to this struct file... for this descriptor table").

Besides, _what_ private_data? What would own and maintain it? A specific driver? What if more than one of them wants that thing?

Quoted 3 lines

Did i miss something fundamental ? copy_files() call dup_fd() so i should be all set here.

That looks like an extremely misguided kludge for hell knows what purpose, almost certainly architecturally insane. What are you actually trying to achieve?

Quoted 15 lines · Matthew Wilcox

Quoted 14 lines · Jerome Glisse

Quoted 8 lines · Matthew Wilcox

Quoted 3 lines

Well scratch that whole idea, i would need to add a new array to task struct which make it a lot less appealing. Hence a better solution is to instead have this as part of mm (well indirectly).

It shouldn't be too bad to add a struct radix_tree to the fdtable.

I'm sure we could just not support weird cases like sharing the fdtable without sharing the mm. Does anyone actually do that?

Well like you pointed out what i really want is a 1:1 structure linking a device struct an a mm_struct. Given that this need to be cleanup when mm goes away hence tying this to mmu_notifier sounds like a better idea.

Quoted 7 lines

Quoted 5 lines

I am thinking of adding a hashtable to mmu_notifier_mm using file id for hash as this should be a good hash value for common cases. I only expect few drivers to need that (GPU drivers, RDMA). Today GPU drivers do have a hashtable inside their driver and they has on the mm struct pointer, i believe hash mmu_notifier_mm using file id will be better.

file descriptors are small positive integers ...

... except when there's a lot of them. Or when something uses dup2() in interesting ways, but hey - we could "just not support" that, right?

Quoted 3 lines

ideal for the radix tree.
If you need to find your data based on the struct file address, then by all means a hashtable is the better data structure.

Perhaps it would be a good idea to describe whatever is being attempted?

FWIW, passing around descriptors is almost always a bloody bad idea. There are very few things really associated with those and just about every time I'd seen internal APIs that work in terms of those "small positive numbers" they had been badly racy and required massive redesign to get something even remotely sane.

I need a struct to link part of device context with mm struct for a process. Most of device context is link to the struct file of the device file (ie process open has a file descriptor for the device file).

Device driver for GPU have some part of their process context tied to the process mm (accessing process address space directly from the GPU). However we can not store this context information in the struct file private data because of clone (same struct file accross different mm).

So today driver have an hashtable in their global device structure to lookup context information for a given mm. This is sub-optimal and duplicate a lot of code among different drivers.

Hence why i want something generic that allow a device driver to store context structure that is specific to a mm. I thought that adding a new array on the side of struct file array would be a good idea but it has too many kludges.

So i will do something inside mmu_notifier and there will be no tie to any fs aspect. I expect only a handful of driver to care about this and for a given platform you won't see that many devices hence you won't have that many pointer to deal with.

Quoted 4 lines · Jerome Glisse

I need a struct to link part of device context with mm struct for a process. Most of device context is link to the struct file of the device file (ie process open has a file descriptor for the device file).

Er... You do realize that

	fd = open(...)
	mmap(fd, ...)
	close(fd)

is absolutely legitimate, right? IOW, existence/stability/etc. of a file descriptor is absolutely *not* guaranteed - in fact, there might be not a single file descriptor referring to a given openen and mmaped file.

Quoted 9 lines

Device driver for GPU have some part of their process context tied to the process mm (accessing process address space directly from the GPU). However we can not store this context information in the struct file private data because of clone (same struct file accross different mm).

So today driver have an hashtable in their global device structure to lookup context information for a given mm. This is sub-optimal and duplicate a lot of code among different drivers.

Umm... Examples?

Quoted 10 lines

Hence why i want something generic that allow a device driver to store context structure that is specific to a mm. I thought that adding a new array on the side of struct file array would be a good idea but it has too many kludges.

So i will do something inside mmu_notifier and there will be no tie to any fs aspect. I expect only a handful of driver to care about this and for a given platform you won't see that many devices hence you won't have that many pointer to deal with.

Let's step back for a second - lookups by _what_? If you are associating somethin with a mapping, vm_area_struct would be a natural candidate for storing such data, wouldn't it?

What do you have and what do you want to find?

Quoted 7 lines · Al Viro

Quoted 4 lines · Jerome Glisse

I need a struct to link part of device context with mm struct for a process. Most of device context is link to the struct file of the device file (ie process open has a file descriptor for the device file).

Quoted 8 lines

Er... You do realize that

	fd = open(...)
	mmap(fd, ...)
	close(fd)

is absolutely legitimate, right? IOW, existence/stability/etc. of a file descriptor is absolutely *not* guaranteed - in fact, there might be not a single file descriptor referring to a given openen and mmaped file.

Yes and that's fine, on close(fd) the device driver is tear down

and
struct i want to store is tear down too and free.