Re: [RFC] what to do with IOCB_DSYNC?

[Al Viro <viro@zeniv.linux.org.uk>]

On Sun, May 22, 2022 at 12:06:24PM -0600, Jens Axboe wrote:
>  	union {
> +		size_t uaddr_len;

WTF for?

> +#define iterate_uaddr(i, n, base, len, off, STEP) {		\
> +	size_t off = 0;						\
> +	size_t skip = i->iov_offset;				\
> +	len = min(n, i->uaddr_len - skip);			\

How would you ever get offset past the size?  Note that we do
keep track of amount of space left...

> -	if (iter_is_iovec(i))
> +	if (!i->nofault)
>  		might_fault();

Nope.  Sorry, but by default nofault will be false for *all* types.
It's not "I won't fault", it's "pass FOLL_NOFAULT to g_u_p".

> -	if (iov_iter_is_bvec(i) || iov_iter_is_kvec(i) || iov_iter_is_xarray(i)) {
> +	if (iov_iter_is_bvec(i) || iov_iter_is_kvec(i) ||
> +	    iter_is_uaddr(i) || iov_iter_is_xarray(i)) {
>  		void *kaddr = kmap_local_page(page);
>  		size_t wanted = _copy_to_iter(kaddr + offset, bytes, i);
>  		kunmap_local(kaddr);

Nope.  This is bogus - _copy_to_iter() *can* legitimately fault for those, so
the same considerations as for iovec apply.

> @@ -900,7 +934,8 @@ size_t copy_page_from_iter(struct page *page, size_t offset, size_t bytes,
>  		return 0;
>  	if (likely(iter_is_iovec(i)))
>  		return copy_page_from_iter_iovec(page, offset, bytes, i);
> -	if (iov_iter_is_bvec(i) || iov_iter_is_kvec(i) || iov_iter_is_xarray(i)) {
> +	if (iov_iter_is_bvec(i) || iov_iter_is_kvec(i) ||
> +	    iter_is_uaddr(i) || iov_iter_is_xarray(i)) {

Ditto.

> @@ -1319,6 +1368,14 @@ unsigned long iov_iter_alignment(const struct iov_iter *i)
>  	if (iov_iter_is_bvec(i))
>  		return iov_iter_alignment_bvec(i);
>  
> +	if (iter_is_uaddr(i)) {
> +		size_t len = i->count - i->iov_offset;
> +
> +		if (len)
> +			return (unsigned long) i->uaddr + i->iov_offset;

Huh?  iov_iter_alignment() wants the worse of beginning and end, if there's
any data at all.

> @@ -1527,6 +1584,9 @@ ssize_t iov_iter_get_pages(struct iov_iter *i,
>  	if (!maxsize)
>  		return 0;
>  
> +	if (WARN_ON_ONCE(iter_is_uaddr(i)))
> +		return 0;

So no DIO read(2) or write(2) anymore?  Harsh...

> @@ -1652,6 +1712,8 @@ ssize_t iov_iter_get_pages_alloc(struct iov_iter *i,
>  		maxsize = i->count;
>  	if (!maxsize)
>  		return 0;
> +	if (WARN_ON_ONCE(iter_is_uaddr(i)))
> +		return 0;

Ditto.

> +	if (iter_is_uaddr(i)) {
> +		unsigned long uaddr = (unsigned long) i->uaddr;
> +		unsigned long start, end;
> +
> +		end = (uaddr + i->count - i->iov_offset + PAGE_SIZE - 1)
> +				>> PAGE_SHIFT;
> +		start = uaddr >> PAGE_SHIFT;
> +		return min_t(int, end - start, maxpages);

Nope.  The value is wrong (sanity check - decrement the base and increment
iov_offset by the same amount; the range of addresses is the same,
and the same should go for the result; yours fails that test).

FWIW, here it's
        if (likely(iter_is_ubuf(i))) {
		unsigned offs = offset_in_page(i->ubuf + i->iov_offset);
		int npages = DIV_ROUND_UP(offs + i->count, PAGE_SIZE);
		return min(npages, maxpages);
	}

BTW, you've missed iov_iter_gap_alignment()...