Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: move readv/writev to the kernel #13498

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft

WIP: move readv/writev to the kernel #13498

wants to merge 20 commits into from

Conversation

yamt
Copy link
Contributor

@yamt yamt commented Sep 17, 2024

Summary

Impact

Testing

@yamt yamt marked this pull request as draft September 17, 2024 08:38
@yamt
Copy link
Contributor Author

yamt commented Sep 17, 2024

@xiaoxiang781216 can you please review this before i apply the mechanical change to hundreds of files? thank you.

CODE off_t (*seek)(FAR struct file *filep, off_t offset, int whence);
CODE int (*ioctl)(FAR struct file *filep, int cmd, unsigned long arg);
CODE int (*mmap)(FAR struct file *filep,
FAR struct mm_map_entry_s *map);
CODE int (*truncate)(FAR struct file *filep, off_t length);
CODE int (*poll)(FAR struct file *filep, FAR struct pollfd *fds,
bool setup);
CODE ssize_t (*compat_read)(FAR struct file *filep, FAR char *buffer,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

ret = (int)inode->u.i_ops->read(filep,
(FAR char *)buf,
(size_t)nbytes);
ret = inode->u.i_ops->readv(filep, iov, iovcnt);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's fallback to compact_readv here if readv is NULL, instead changing each individual driver or fs.

*
****************************************************************************/

ssize_t iovec_compat_readv(FAR struct file *filep,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could move to fs_read.c as a static function, and call it directly if driver/fs doesn't implement readv

*
****************************************************************************/

ssize_t iovec_compat_writev(FAR struct file *filep,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@yamt
Copy link
Contributor Author

yamt commented Sep 18, 2024

@xiaoxiang781216 do you prefer the latest push?

@@ -227,6 +229,10 @@ struct file_operations

CODE int (*poll)(FAR struct file *filep, FAR struct pollfd *fds,
bool setup);
CODE ssize_t (*readv)(FAR struct file *filep, FAR const struct iovec *iov,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we move the new callback to the end? otherwise all drivers/fs has to been modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where exactly do you mean by "the end"?

@@ -323,6 +329,11 @@ struct mountpt_operations
CODE int (*truncate)(FAR struct file *filep, off_t length);
CODE int (*poll)(FAR struct file *filep, FAR struct pollfd *fds,
bool setup);
CODE ssize_t (*readv)(FAR struct file *filep, FAR const struct iovec *iov,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

{
struct iovec iov;

iov.iov_base = (void *)buf;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
iov.iov_base = (void *)buf;
iov.iov_base = (FAR void *)buf;

****************************************************************************/

struct file;
ssize_t iovec_compat_readv(FAR struct file *filep,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we move to include/nuttx/fs/fs.h

Suggested change
ssize_t iovec_compat_readv(FAR struct file *filep,
ssize_t file_compat_readv(FAR struct file *filep,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

*
****************************************************************************/

ssize_t iovec_compat_writev(FAR struct file *filep,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants