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

[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 with adding the O_DIRECT flag to a file pointer
> > after it has been opened -- swapon opens the swapfile before we know if
> > we're using the SWP_FILE infrastructure, and we need to add O_DIRECT so
> > ->{read,write}_iter use direct I/O, but we can't add O_DIRECT to the
> > original open without excluding filesystems that support the old bmap
> > path but not direct I/O.
> 
> In general just adding O_DIRECT is a problem.  However given that the
> swap file is locked against any other access while in use it seems ok
> in this particular case.  Just make sure to clear it on swapoff, and
> write a detailed comment explaining the situation.

I'll admit that I'm a bit confused. I want to do this:

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 8798b2e..5145c09 100644
--- 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 (!mapping->a_ops->direct_IO)
+                       return -EINVAL;
+               swap_file->f_flags |= O_DIRECT;
                ret = mapping->a_ops->swap_activate(sis, swap_file, span);
                if (!ret) {
                        sis->flags |= SWP_FILE;

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 flag.

-- 
Omar
--
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/