xiaoxiang781216 commented on code in PR #8000:
URL: https://github.com/apache/nuttx/pull/8000#discussion_r1059368140
##########
include/nuttx/fs/fs.h:
##########
@@ -206,16 +207,21 @@ struct file_operations
* treated like unions.
*/
- int (*close)(FAR struct file *filep);
- ssize_t (*read)(FAR struct file *filep, FAR char *buffer, size_t buflen);
- ssize_t (*write)(FAR struct file *filep, FAR const char *buffer,
- size_t buflen);
- off_t (*seek)(FAR struct file *filep, off_t offset, int whence);
- int (*ioctl)(FAR struct file *filep, int cmd, unsigned long arg);
+ int (*close)(FAR struct file *filep);
+ ssize_t (*read)(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ ssize_t (*write)(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+ off_t (*seek)(FAR struct file *filep, off_t offset, int whence);
+ int (*ioctl)(FAR struct file *filep, int cmd, unsigned long arg);
+ int (*truncate)(FAR struct file *filep, off_t length);
+ FAR void *(*mmap)(FAR struct file *filep, off_t start, size_t length);
+ int (*munmap)(FAR struct task_group_s *group, FAR struct inode *inode,
Review Comment:
> We've been using the mmap returned address and length as the identifier
and stored those, together with a reference to the inode in the group-specific
list of mappings. If there is really an inode-backed mapping, the inode is the
reference. It doesn't really then matter whether the fd is open or close when
it is being unmapped, the inode can be preserved with the reference counter it
already has.
>
It's fine for some pseudo file(e.g., char driver) sine there is one to one
mapping between inode and hardware instance, but there are other special cases:
1. For real file system, only the mount point bind to an inode, all files
inside mount point doesn't have inode at all
2. Some special char drivers(e.g., sock, eventfd, timerfd, signalfd) share
one inode with all instances
3. mtd_dev_s doesn't bind to an inode at all
So, the combination is complex.
> In all normal cases there is no need to pass the "group" as an argument
for mmap or munmap; it is always the current group. The only reason why group
is passed in munmap is that during process exit, the current group is something
else. Doing actual MMU unmapping cannot be done in that case (an doesn't need
to be done, since the whole address environment is destroyed anyways), but
there may still be kernel-allocated resources (such as the inode), which need
to be freed. So what we have done is that we pass "NULL" as group at task
deletion and the current group when we enter munmap normally from "munmap" user
call.
>
> Then the driver can do MMU mapping deletion for example by checking "if
(group) { }, or such.
>
> We could also pass just some flag, e.g. "bool on_exit", or such, but were
thinking that passing the group is more generic.
>
> If you have some clear vision on different interface, please give a
suggestion, these are easy to change.
I think we can let's mmap return a void * as cookie (the implementation
decides what should contain in it) and save this cookie together with address
and len in task_group, and pass back in munmap.
Or like Linux:
https://github.com/torvalds/linux/blob/master/include/linux/fs.h#L2102
pass a new struct(e.g. file_map_s?) which contains all field required to
manage the memory map:
1. link list (or rb tree) node
2. start and length
2. task_group
4. FAR void *priv
With the last field, the implementation could save the private info into
priv and retrieve back in munmap, just like how file_s used in open/close
callback.
> It is possible to add the group parameter of course also to the mmap, if
you can think of some potential use for it. I would think that trying to mmap
in anything else than currently running context would be illegal.
>
> Also, you really need to store the mappings in the group anyways, since
the user side only knows about the virtual address and length of the mapping.
Also, when you store the mappings in the group, you don't need any "context"
specific token; the mmap and unmap are always done in the same currently
running context, except in the case of process deletion.
The driver may need store the additional info beside address and length. For
example, shm driver may need record the physical page.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
