On Thu, Dec 22, 2022 at 2:28 PM Jukka Laitinen <jukka.laiti...@iki.fi> wrote:
> Hi, > > We've implemented and been using "posix shm" support for nuttx ( not > up-streamed to mainline nuttx so far - I am open to discussion whether > there is interest for it ). For this, we've naturally utilized the > interfaces mentioned in the subject. > > Yes, it's a very useful feature. > Currently those operations are implemented as FIOC_ ioctls:s to the > drivers - such as our shmfs and upstream video, leds and and shmfs. > > While this works fine, I can't help thinking that implementing this > functionality as IOCTLs is wrong, and those should be specified in > "struct file_operations" instead. > > You mean FIOC_MMAP? We also find that the rigid design of FIOC_MMAP makes it hard to implement some advanced features inside the driver since it doesn't map to mmap/mumap very well. > This is my thinking around the subject: > > - IOCTLs are ment for communication from userspace application to the > drivers. It doesn't make sense for an application to perform a direct > IOCTL (such as FIOC_MMAP, FIOC_MUNMAP or FIOC_TRUNCATE). Application > should always use posix mmap, munmap and truncate calls instead. > > - For mmap, munmap and truncate there is a syscall lookup already in > place in nuttx - there is no ioctl needed for the userspace-kernel > communication > > - Especially for "unmap", the ioctl doesn't make sense, since file_ioctl > is supposed to work on a file pointer. But unmap cannot be done on a > file. Even when unmapping a previously mapped "file", unmap can only > work on inode, since there may not be an open file, or even a linked > inode present. It must be possible to do "munmap" for a mapped file, > even if the actual file is closed and/or deleted after mapping. (and > yes, I realize that I earlier added the FIOC_MUNMAP myself - since the > others were implemented as ioctls, and we really need the unmap). > > So what do you think, is my reasoning above right, and should the above > operations moved to file_struct instead of the being IOCTLs? > > It's fine to define map/unmap related operation directly in file_operations since other OS make the similar decision: https://github.com/torvalds/linux/blob/master/include/linux/fs.h#L2102 https://github.com/torvalds/linux/blob/master/include/linux/mm.h#L540-L612 But it's important to convert the current fs/driver to utilize the new interface to demonstrate that the design is flexible enough to support all possible usage. > Br, > > Jukka > > > > > >
