Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-02-03 Thread Dave Chinner
On Mon, Feb 03, 2020 at 06:46:41PM +0100, Christoph Hellwig wrote: > On Sat, Jan 18, 2020 at 08:28:38PM +1100, Dave Chinner wrote: > > I think it's pretty gross, actually. It makes the same mistake made > > with locking in the old direct IO code - it encodes specific lock > > operations via flags

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-02-03 Thread Christoph Hellwig
On Sat, Jan 18, 2020 at 08:28:38PM +1100, Dave Chinner wrote: > I think it's pretty gross, actually. It makes the same mistake made > with locking in the old direct IO code - it encodes specific lock > operations via flags into random locations in the DIO path. This is > a very slippery slope,

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-02-03 Thread Christoph Hellwig
On Thu, Jan 16, 2020 at 03:00:04PM +0100, Jan Kara wrote: > I'd like to note that using i_dio_count has also one advantage you didn't > mention. For AIO case, if you need to hold i_rwsem in exclusive mode, > holding the i_rwsem just for submission part is a significant performance > advantage

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-18 Thread Matthew Wilcox
On Wed, Jan 15, 2020 at 03:33:47PM +0100, Peter Zijlstra wrote: > On Wed, Jan 15, 2020 at 09:24:28AM -0400, Jason Gunthorpe wrote: > > > I was interested because you are talking about allowing the read/write side > > of a rw sem to be held across a return to user space/etc, which is the > > same

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-18 Thread Dave Chinner
On Tue, Jan 14, 2020 at 05:12:13PM +0100, Christoph Hellwig wrote: > Hi all, > > Asynchronous read/write operations currently use a rather magic locking > scheme, were access to file data is normally protected using a rw_semaphore, > but if we are doing aio where the syscall returns to userspace

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-16 Thread Jan Kara
Hello! On Tue 14-01-20 17:12:13, Christoph Hellwig wrote: > Asynchronous read/write operations currently use a rather magic locking > scheme, were access to file data is normally protected using a rw_semaphore, > but if we are doing aio where the syscall returns to userspace before the > I/O has

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-15 Thread Waiman Long
On 1/15/20 9:49 AM, Jason Gunthorpe wrote: > On Wed, Jan 15, 2020 at 03:33:47PM +0100, Peter Zijlstra wrote: >> On Wed, Jan 15, 2020 at 09:24:28AM -0400, Jason Gunthorpe wrote: >> >>> I was interested because you are talking about allowing the read/write side >>> of a rw sem to be held across a

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-15 Thread Jason Gunthorpe
On Wed, Jan 15, 2020 at 04:36:14PM +0100, Christoph Hellwig wrote: > synchronous and currently hack that up, so a version of the percpu_ref > that actually waits for the other references to away like we hacked > up various places seems to exactly suit your requirements. Ah, yes, sounds like a

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-15 Thread Christoph Hellwig
On Wed, Jan 15, 2020 at 09:24:28AM -0400, Jason Gunthorpe wrote: > > Your requirement seems a little different, and in fact in many ways > > similar to the percpu_ref primitive. > > I was interested because you are talking about allowing the read/write side > of a rw sem to be held across a

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-15 Thread Jason Gunthorpe
On Wed, Jan 15, 2020 at 03:33:47PM +0100, Peter Zijlstra wrote: > On Wed, Jan 15, 2020 at 09:24:28AM -0400, Jason Gunthorpe wrote: > > > I was interested because you are talking about allowing the read/write side > > of a rw sem to be held across a return to user space/etc, which is the > > same

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-15 Thread Peter Zijlstra
On Wed, Jan 15, 2020 at 09:24:28AM -0400, Jason Gunthorpe wrote: > I was interested because you are talking about allowing the read/write side > of a rw sem to be held across a return to user space/etc, which is the > same basic problem. No it is not; allowing the lock to be held across

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-15 Thread Jason Gunthorpe
On Wed, Jan 15, 2020 at 07:56:14AM +0100, Christoph Hellwig wrote: > On Tue, Jan 14, 2020 at 03:27:00PM -0400, Jason Gunthorpe wrote: > > I've seen similar locking patterns quite a lot, enough I've thought > > about having a dedicated locking primitive to do it. It really wants > > to be a rwsem,

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-15 Thread Jason Gunthorpe
On Tue, Jan 14, 2020 at 05:12:13PM +0100, Christoph Hellwig wrote: > Hi all, > > Asynchronous read/write operations currently use a rather magic locking > scheme, were access to file data is normally protected using a rw_semaphore, > but if we are doing aio where the syscall returns to userspace

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-14 Thread Christoph Hellwig
On Tue, Jan 14, 2020 at 03:27:00PM -0400, Jason Gunthorpe wrote: > I've seen similar locking patterns quite a lot, enough I've thought > about having a dedicated locking primitive to do it. It really wants > to be a rwsem, but as here the rwsem rules don't allow it. > > The common pattern I'm

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-14 Thread Christoph Hellwig
On Tue, Jan 14, 2020 at 10:47:07AM -0800, Matthew Wilcox wrote: > It would be helpful if we could also use the same lockdep logic > for PageLocked. Again, it's a case where returning to userspace with > PageLock held is fine, because we're expecting an interrupt to come in > and drop the lock for

Re: [Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-14 Thread Matthew Wilcox
On Tue, Jan 14, 2020 at 05:12:13PM +0100, Christoph Hellwig wrote: > Second I/O > completions often come from interrupt context, which means the re-acquire > is recorded as from irq context, leading to warnings about incorrect > contexts. I wonder if we could just have a bit in lockdep that says

[Cluster-devel] RFC: hold i_rwsem until aio completes

2020-01-14 Thread Christoph Hellwig
Hi all, Asynchronous read/write operations currently use a rather magic locking scheme, were access to file data is normally protected using a rw_semaphore, but if we are doing aio where the syscall returns to userspace before the I/O has completed we also use an atomic_t to track the outstanding