Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread Al Viro
On Thu, Sep 13, 2018 at 04:51:35PM +0100, David Howells wrote:
> 
> Hi Al,
> 
> Here's a set of patches that adds two new iov_iter types and then makes AFS
> use them to do I/O.  The iov_iter changes are:
> 
>  (1) Separate the type from the direction in the iov_iter struct and
>  provide accessor functions to wrap type checking.
> 
>  (2) Renumber the type constants to be contiguous small unsigned integers,
>  starting from 0 and then use switch-statements rather than if-else
>  chains using bit-testing.
> 
>  Note that the compiler can optimise this better by using CMP rather
>  than AND/TEST, say, as comparing integers requires fewer CMP
>  instructions or can use jump tables.
> 
>  (3) Change iov_offset from size_t to loff_t.  This allows iov_offset to be
>  then used as a byte offset with ITER_MAPPING and allows 4GiB and larger
>  reads and writes to be proposed.
> 
>  This makes no difference on a 64-bit system, but does make a 32-bit
>  compilation a bit larger.
> 
>  (4) Add an ITER_MAPPING iterator type.  This provides an iterator that
>  directly accesses an address_space, and assumes that the target pages
>  are in some way locked (eg. PG_lock or PG_writeback).
> 
>  (5) Add an ITER_DISCARD iterator type.  This provides an iterator that
>  simply discards anything written to it.  It cannot be used as a data
>  source.

Hmm...   I'll take a look in about an hour (need to finish the sodding tty-ioctl
stuff first), will post review then.  Sorry, I remember seeing it posted several
weeks ago, slipped through the cracks then ;-/


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread Al Viro
On Thu, Sep 13, 2018 at 04:51:35PM +0100, David Howells wrote:
> 
> Hi Al,
> 
> Here's a set of patches that adds two new iov_iter types and then makes AFS
> use them to do I/O.  The iov_iter changes are:
> 
>  (1) Separate the type from the direction in the iov_iter struct and
>  provide accessor functions to wrap type checking.
> 
>  (2) Renumber the type constants to be contiguous small unsigned integers,
>  starting from 0 and then use switch-statements rather than if-else
>  chains using bit-testing.
> 
>  Note that the compiler can optimise this better by using CMP rather
>  than AND/TEST, say, as comparing integers requires fewer CMP
>  instructions or can use jump tables.
> 
>  (3) Change iov_offset from size_t to loff_t.  This allows iov_offset to be
>  then used as a byte offset with ITER_MAPPING and allows 4GiB and larger
>  reads and writes to be proposed.
> 
>  This makes no difference on a 64-bit system, but does make a 32-bit
>  compilation a bit larger.
> 
>  (4) Add an ITER_MAPPING iterator type.  This provides an iterator that
>  directly accesses an address_space, and assumes that the target pages
>  are in some way locked (eg. PG_lock or PG_writeback).
> 
>  (5) Add an ITER_DISCARD iterator type.  This provides an iterator that
>  simply discards anything written to it.  It cannot be used as a data
>  source.

Hmm...   I'll take a look in about an hour (need to finish the sodding tty-ioctl
stuff first), will post review then.  Sorry, I remember seeing it posted several
weeks ago, slipped through the cracks then ;-/


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread David Howells
Matthew Wilcox  wrote:

> It might serve to replace iov_iter_zero().

I don't think it can work like that.  iov_iter_zero() writes zeros into an
iterator someone else set up.

To use ITER_ZERO, you'd be setting up the iterator and passing it to someone
else to comsume.  I think these are at opposite ends of the concept.

David


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread David Howells
Matthew Wilcox  wrote:

> It might serve to replace iov_iter_zero().

I don't think it can work like that.  iov_iter_zero() writes zeros into an
iterator someone else set up.

To use ITER_ZERO, you'd be setting up the iterator and passing it to someone
else to comsume.  I think these are at opposite ends of the concept.

David


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread Matthew Wilcox
On Thu, Sep 13, 2018 at 05:18:23PM +0100, David Howells wrote:
> Matthew Wilcox  wrote:
> 
> > >  (5) Add an ITER_DISCARD iterator type.  This provides an iterator that
> > >  simply discards anything written to it.  It cannot be used as a data
> > >  source.
> > 
> > May I suggest an ITER_ZERO iterator type instead?  It acts like /dev/zero;
> > writes are discarded (as you have here) and reads return zeroes.
> > I've wanted such a thing in the past, but got distracted away from
> > that project.
> 
> I thought about that, but the zero-filling is not as easy to implement as the
> discard side - plus I don't have any use case to test it with.
> 
> Do you have a use case in mind?

At the time I did it, it was useful in dax, but I don't know if it is
any more.  It might serve to replace iov_iter_zero().


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread Matthew Wilcox
On Thu, Sep 13, 2018 at 05:18:23PM +0100, David Howells wrote:
> Matthew Wilcox  wrote:
> 
> > >  (5) Add an ITER_DISCARD iterator type.  This provides an iterator that
> > >  simply discards anything written to it.  It cannot be used as a data
> > >  source.
> > 
> > May I suggest an ITER_ZERO iterator type instead?  It acts like /dev/zero;
> > writes are discarded (as you have here) and reads return zeroes.
> > I've wanted such a thing in the past, but got distracted away from
> > that project.
> 
> I thought about that, but the zero-filling is not as easy to implement as the
> discard side - plus I don't have any use case to test it with.
> 
> Do you have a use case in mind?

At the time I did it, it was useful in dax, but I don't know if it is
any more.  It might serve to replace iov_iter_zero().


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread David Howells
Matthew Wilcox  wrote:

> >  (5) Add an ITER_DISCARD iterator type.  This provides an iterator that
> >  simply discards anything written to it.  It cannot be used as a data
> >  source.
> 
> May I suggest an ITER_ZERO iterator type instead?  It acts like /dev/zero;
> writes are discarded (as you have here) and reads return zeroes.
> I've wanted such a thing in the past, but got distracted away from
> that project.

I thought about that, but the zero-filling is not as easy to implement as the
discard side - plus I don't have any use case to test it with.

Do you have a use case in mind?

David


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread David Howells
Matthew Wilcox  wrote:

> >  (5) Add an ITER_DISCARD iterator type.  This provides an iterator that
> >  simply discards anything written to it.  It cannot be used as a data
> >  source.
> 
> May I suggest an ITER_ZERO iterator type instead?  It acts like /dev/zero;
> writes are discarded (as you have here) and reads return zeroes.
> I've wanted such a thing in the past, but got distracted away from
> that project.

I thought about that, but the zero-filling is not as easy to implement as the
discard side - plus I don't have any use case to test it with.

Do you have a use case in mind?

David


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread Matthew Wilcox
On Thu, Sep 13, 2018 at 04:51:35PM +0100, David Howells wrote:
>  (5) Add an ITER_DISCARD iterator type.  This provides an iterator that
>  simply discards anything written to it.  It cannot be used as a data
>  source.

May I suggest an ITER_ZERO iterator type instead?  It acts like /dev/zero;
writes are discarded (as you have here) and reads return zeroes.
I've wanted such a thing in the past, but got distracted away from
that project.


Re: [PATCH 00/10] iov_iter: Add new iters and use with AFS

2018-09-13 Thread Matthew Wilcox
On Thu, Sep 13, 2018 at 04:51:35PM +0100, David Howells wrote:
>  (5) Add an ITER_DISCARD iterator type.  This provides an iterator that
>  simply discards anything written to it.  It cannot be used as a data
>  source.

May I suggest an ITER_ZERO iterator type instead?  It acts like /dev/zero;
writes are discarded (as you have here) and reads return zeroes.
I've wanted such a thing in the past, but got distracted away from
that project.