Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-23 Thread Christoph Hellwig
On Sat, Dec 20, 2014 at 06:51:33AM +, Al Viro wrote: > > The problem is that the use of ->direct_IO by the swap code is a gross > > layering violation. ->direct_IO is a callback for the filesystem, and > > the swap code need to call ->read_iter instead of ->readpage and > > ->write_tier

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-23 Thread Christoph Hellwig
On Sat, Dec 20, 2014 at 06:51:33AM +, Al Viro wrote: The problem is that the use of -direct_IO by the swap code is a gross layering violation. -direct_IO is a callback for the filesystem, and the swap code need to call -read_iter instead of -readpage and -write_tier instead of

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-21 Thread Omar Sandoval
On Sat, Dec 20, 2014 at 06:51:33AM +, Al Viro wrote: > On Mon, Dec 15, 2014 at 08:56:15AM -0800, Christoph Hellwig wrote: > > On Mon, Dec 15, 2014 at 05:27:05PM +0100, Jan Kara wrote: > > > On Sun 14-12-14 21:26:56, Omar Sandoval wrote: > > > > The generic write code locks i_mutex for a

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-21 Thread Omar Sandoval
On Sat, Dec 20, 2014 at 06:51:33AM +, Al Viro wrote: On Mon, Dec 15, 2014 at 08:56:15AM -0800, Christoph Hellwig wrote: On Mon, Dec 15, 2014 at 05:27:05PM +0100, Jan Kara wrote: On Sun 14-12-14 21:26:56, Omar Sandoval wrote: The generic write code locks i_mutex for a direct_IO.

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-19 Thread Al Viro
On Mon, Dec 15, 2014 at 08:56:15AM -0800, Christoph Hellwig wrote: > On Mon, Dec 15, 2014 at 05:27:05PM +0100, Jan Kara wrote: > > On Sun 14-12-14 21:26:56, Omar Sandoval wrote: > > > The generic write code locks i_mutex for a direct_IO. Swap-over-NFS > > > doesn't grab the mutex because

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-19 Thread Al Viro
On Mon, Dec 15, 2014 at 08:56:15AM -0800, Christoph Hellwig wrote: On Mon, Dec 15, 2014 at 05:27:05PM +0100, Jan Kara wrote: On Sun 14-12-14 21:26:56, Omar Sandoval wrote: The generic write code locks i_mutex for a direct_IO. Swap-over-NFS doesn't grab the mutex because nfs_direct_IO

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-18 Thread Al Viro
On Thu, Dec 18, 2014 at 10:24:05PM -0800, Omar Sandoval wrote: > + swap_file = file_open_name(name, O_RDWR | O_LARGEFILE | O_DIRECT, 0); > + if (IS_ERR(swap_file) && PTR_ERR(swap_file) == -EINVAL) ITYM if (swap_file == ERR_PTR(-EINVAL)) here... -- To unsubscribe from this list: send

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-18 Thread Omar Sandoval
On Wed, Dec 17, 2014 at 10:03:13PM +, Al Viro wrote: > On Wed, Dec 17, 2014 at 10:52:56AM -0800, Christoph Hellwig wrote: > > On Wed, Dec 17, 2014 at 06:58:32AM -0800, Omar Sandoval wrote: > > > See my previous message. If we use O_DIRECT on the original open, then > > > filesystems that

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-18 Thread Omar Sandoval
On Wed, Dec 17, 2014 at 10:03:13PM +, Al Viro wrote: On Wed, Dec 17, 2014 at 10:52:56AM -0800, Christoph Hellwig wrote: On Wed, Dec 17, 2014 at 06:58:32AM -0800, Omar Sandoval wrote: See my previous message. If we use O_DIRECT on the original open, then filesystems that implement bmap

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-18 Thread Al Viro
On Thu, Dec 18, 2014 at 10:24:05PM -0800, Omar Sandoval wrote: + swap_file = file_open_name(name, O_RDWR | O_LARGEFILE | O_DIRECT, 0); + if (IS_ERR(swap_file) PTR_ERR(swap_file) == -EINVAL) ITYM if (swap_file == ERR_PTR(-EINVAL)) here... -- To unsubscribe from this list: send the

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Al Viro
On Wed, Dec 17, 2014 at 10:52:56AM -0800, Christoph Hellwig wrote: > On Wed, Dec 17, 2014 at 06:58:32AM -0800, Omar Sandoval wrote: > > See my previous message. If we use O_DIRECT on the original open, then > > filesystems that implement bmap but not direct_IO will no longer work. > > These are

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Christoph Hellwig
On Wed, Dec 17, 2014 at 06:58:32AM -0800, Omar Sandoval wrote: > See my previous message. If we use O_DIRECT on the original open, then > filesystems that implement bmap but not direct_IO will no longer work. > These are the ones that I found in my tree: In the long run I don't think they are

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Omar Sandoval
On Wed, Dec 17, 2014 at 12:24:37AM -0800, Christoph Hellwig wrote: > On Wed, Dec 17, 2014 at 08:20:21AM +, Al Viro wrote: > > Where the hell would those other references come from? We open the damn > > thing in sys_swapon(), never put it into descriptor tables, etc. and > > the only reason

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Christoph Hellwig
On Wed, Dec 17, 2014 at 08:20:21AM +, Al Viro wrote: > Where the hell would those other references come from? We open the damn > thing in sys_swapon(), never put it into descriptor tables, etc. and > the only reason why we use filp_close() instead of fput() is that we > would miss ->flush()

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Al Viro
On Wed, Dec 17, 2014 at 12:06:10AM -0800, Christoph Hellwig wrote: > > This seems to be more or less equivalent to doing a fcntl(F_SETFL) to > > add the O_DIRECT flag to swap_file (which is a struct file *). Swapoff > > calls filp_close on swap_file, so I don't see why it's necessary to > > clear

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Christoph Hellwig
On Tue, Dec 16, 2014 at 12:56:24AM -0800, Omar Sandoval wrote: > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -1728,6 +1728,9 @@ static int setup_swap_extents(struct swap_info_struct > *sis, sector_t *span) > } > > if (mapping->a_ops->swap_activate) { > + if

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Christoph Hellwig
On Tue, Dec 16, 2014 at 12:56:24AM -0800, Omar Sandoval wrote: --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -1728,6 +1728,9 @@ static int setup_swap_extents(struct swap_info_struct *sis, sector_t *span) } if (mapping-a_ops-swap_activate) { + if

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Al Viro
On Wed, Dec 17, 2014 at 12:06:10AM -0800, Christoph Hellwig wrote: This seems to be more or less equivalent to doing a fcntl(F_SETFL) to add the O_DIRECT flag to swap_file (which is a struct file *). Swapoff calls filp_close on swap_file, so I don't see why it's necessary to clear the

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Christoph Hellwig
On Wed, Dec 17, 2014 at 08:20:21AM +, Al Viro wrote: Where the hell would those other references come from? We open the damn thing in sys_swapon(), never put it into descriptor tables, etc. and the only reason why we use filp_close() instead of fput() is that we would miss -flush()

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Omar Sandoval
On Wed, Dec 17, 2014 at 12:24:37AM -0800, Christoph Hellwig wrote: On Wed, Dec 17, 2014 at 08:20:21AM +, Al Viro wrote: Where the hell would those other references come from? We open the damn thing in sys_swapon(), never put it into descriptor tables, etc. and the only reason why we

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Christoph Hellwig
On Wed, Dec 17, 2014 at 06:58:32AM -0800, Omar Sandoval wrote: See my previous message. If we use O_DIRECT on the original open, then filesystems that implement bmap but not direct_IO will no longer work. These are the ones that I found in my tree: In the long run I don't think they are worth

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-17 Thread Al Viro
On Wed, Dec 17, 2014 at 10:52:56AM -0800, Christoph Hellwig wrote: On Wed, Dec 17, 2014 at 06:58:32AM -0800, Omar Sandoval wrote: See my previous message. If we use O_DIRECT on the original open, then filesystems that implement bmap but not direct_IO will no longer work. These are the ones

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-16 Thread Omar Sandoval
On Tue, Dec 16, 2014 at 12:35:43AM -0800, Christoph Hellwig wrote: > On Mon, Dec 15, 2014 at 02:11:00PM -0800, Omar Sandoval wrote: > > Ok, I got the swap code working with ->read_iter/->write_iter without > > too much trouble. I wanted to double check before I submit if there's > > any gotchas

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-16 Thread Christoph Hellwig
On Mon, Dec 15, 2014 at 02:11:00PM -0800, Omar Sandoval wrote: > Ok, I got the swap code working with ->read_iter/->write_iter without > too much trouble. I wanted to double check before I submit if there's > any gotchas involved with adding the O_DIRECT flag to a file pointer > after it has been

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-16 Thread Christoph Hellwig
On Mon, Dec 15, 2014 at 02:11:00PM -0800, Omar Sandoval wrote: Ok, I got the swap code working with -read_iter/-write_iter without too much trouble. I wanted to double check before I submit if there's any gotchas involved with adding the O_DIRECT flag to a file pointer after it has been opened

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-16 Thread Omar Sandoval
On Tue, Dec 16, 2014 at 12:35:43AM -0800, Christoph Hellwig wrote: On Mon, Dec 15, 2014 at 02:11:00PM -0800, Omar Sandoval wrote: Ok, I got the swap code working with -read_iter/-write_iter without too much trouble. I wanted to double check before I submit if there's any gotchas involved

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-15 Thread Omar Sandoval
On Mon, Dec 15, 2014 at 08:56:15AM -0800, Christoph Hellwig wrote: > On Mon, Dec 15, 2014 at 05:27:05PM +0100, Jan Kara wrote: > > On Sun 14-12-14 21:26:56, Omar Sandoval wrote: > > > The generic write code locks i_mutex for a direct_IO. Swap-over-NFS > > > doesn't grab the mutex because

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-15 Thread Christoph Hellwig
On Mon, Dec 15, 2014 at 05:27:05PM +0100, Jan Kara wrote: > On Sun 14-12-14 21:26:56, Omar Sandoval wrote: > > The generic write code locks i_mutex for a direct_IO. Swap-over-NFS > > doesn't grab the mutex because nfs_direct_IO doesn't expect i_mutex to > > be held, but most direct_IO

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-15 Thread Jan Kara
On Sun 14-12-14 21:26:56, Omar Sandoval wrote: > The generic write code locks i_mutex for a direct_IO. Swap-over-NFS > doesn't grab the mutex because nfs_direct_IO doesn't expect i_mutex to > be held, but most direct_IO implementations do. I think you are speaking about direct IO writes only,

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-15 Thread Jan Kara
On Sun 14-12-14 21:26:56, Omar Sandoval wrote: The generic write code locks i_mutex for a direct_IO. Swap-over-NFS doesn't grab the mutex because nfs_direct_IO doesn't expect i_mutex to be held, but most direct_IO implementations do. I think you are speaking about direct IO writes only,

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-15 Thread Christoph Hellwig
On Mon, Dec 15, 2014 at 05:27:05PM +0100, Jan Kara wrote: On Sun 14-12-14 21:26:56, Omar Sandoval wrote: The generic write code locks i_mutex for a direct_IO. Swap-over-NFS doesn't grab the mutex because nfs_direct_IO doesn't expect i_mutex to be held, but most direct_IO implementations do.

Re: [PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-15 Thread Omar Sandoval
On Mon, Dec 15, 2014 at 08:56:15AM -0800, Christoph Hellwig wrote: On Mon, Dec 15, 2014 at 05:27:05PM +0100, Jan Kara wrote: On Sun 14-12-14 21:26:56, Omar Sandoval wrote: The generic write code locks i_mutex for a direct_IO. Swap-over-NFS doesn't grab the mutex because nfs_direct_IO

[PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-14 Thread Omar Sandoval
The generic write code locks i_mutex for a direct_IO. Swap-over-NFS doesn't grab the mutex because nfs_direct_IO doesn't expect i_mutex to be held, but most direct_IO implementations do. Signed-off-by: Omar Sandoval --- mm/page_io.c | 3 +++ 1 file changed, 3 insertions(+) diff --git

[PATCH 2/8] swap: lock i_mutex for swap_writepage direct_IO

2014-12-14 Thread Omar Sandoval
The generic write code locks i_mutex for a direct_IO. Swap-over-NFS doesn't grab the mutex because nfs_direct_IO doesn't expect i_mutex to be held, but most direct_IO implementations do. Signed-off-by: Omar Sandoval osan...@osandov.com --- mm/page_io.c | 3 +++ 1 file changed, 3 insertions(+)