Re: [RFC v2 4/5] O_NONBLOCK flag for readv2/preadv2

[Jeff Moyer]

Milosz Tanski <milosz@xxxxxxxxx> writes:

> diff --git a/fs/read_write.c b/fs/read_write.c
> index 3db2e87..29b5823 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -864,8 +864,10 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
>  		return -EBADF;
>  	if (!(file->f_mode & FMODE_CAN_READ))
>  		return -EINVAL;
> -	if (flags & ~0)
> +	if (flags & ~RWF_NONBLOCK)
>  		return -EINVAL;
> +	if ((file->f_flags & O_DIRECT) && (flags & RWF_NONBLOCK))
> +		return -EAGAIN;

Just to close out our discussion on EINVAL for O_DIRECT with
RWF_NONBLOCK:

After discussing this with Zach, I agree that EAGAIN would be better.
There may be libraries that lack context and may benefit from the EAGAIN
return value.

> +/* These flags are used for the readv/writev syscalls with flags. */
> +#define RWF_NONBLOCK O_NONBLOCK

I'm not sure this make sense.  As I mentioned earlier, the epoll variant
made sense because the flag was shared (passed unmodified to other vfs
calls that understand it).  Here we can just define an entirely new flag
space.  Unless, of course, someone can come up with a reason why this
/would/ make sense?

> diff --git a/mm/filemap.c b/mm/filemap.c
> index e0919ba..6b7aba8 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1483,7 +1483,10 @@ static ssize_t do_generic_file_read(struct file *filp, loff_t *ppos,
>  		cond_resched();
>  find_page:
>  		page = find_get_page(mapping, index);
> +

Please resist the urge to add whitespace.

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/