Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752071AbbG3PVt (ORCPT ); Thu, 30 Jul 2015 11:21:49 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:48242 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750734AbbG3PVr (ORCPT ); Thu, 30 Jul 2015 11:21:47 -0400 MIME-Version: 1.0 In-Reply-To: <20150730150900.GA13082@infradead.org> References: <1438256184-23645-1-git-send-email-ming.lei@canonical.com> <1438256184-23645-5-git-send-email-ming.lei@canonical.com> <20150730150900.GA13082@infradead.org> Date: Thu, 30 Jul 2015 11:21:44 -0400 Message-ID: Subject: Re: [PATCH v8 4/6] block: loop: prepare for supporing direct IO From: Ming Lei To: Christoph Hellwig Cc: Jens Axboe , Linux Kernel Mailing List , Dave Kleikamp , Zach Brown , Maxim Patlasov , Andrew Morton , Alexander Viro , Tejun Heo , Dave Chinner , linux-api@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1952 Lines: 52 On Thu, Jul 30, 2015 at 11:09 AM, Christoph Hellwig wrote: > On Thu, Jul 30, 2015 at 07:36:22AM -0400, Ming Lei wrote: >> This patches provides one interface for enabling direct IO >> from user space: >> >> - userspace(such as losetup) can pass 'file' which is >> opened/fcntl as O_DIRECT >> >> Also __loop_update_dio() is introduced to check if direct I/O >> can be used on current loop setting. >> >> The last big change is to introduce LO_FLAGS_DIRECT_IO flag >> for userspace to know if direct IO is used to access backing >> file. > > Maybe i'm missing something because I was too busy to follow the > current discussion, but this still doesn't check that the loop > device sector size is aligned to the sector size of the underlying > filesystem. > > E.g. with this you could create a loop device with a 512 byte > sector size on a filesystem with 4k sector size, and it would attempt > to use direct I/O and fail. The I/O size & offset has to be checked in I/O path, see patch 6 please, could you comment on that patch? > >> + unsigned dio_align = inode->i_sb->s_bdev ? >> + (bdev_io_min(inode->i_sb->s_bdev) - 1) : 0; >> + >> + /* >> + * We support direct I/O only if lo_offset is aligned >> + * with the min I/O size of backing device. >> + * >> + * Request's offset and size will be checked in I/O path. >> + */ >> + if (dio) { >> + if (!dio_align || (lo->lo_offset & dio_align)) >> + use_dio = false; > > Also this means you'll never use direct I/O on network filesystems, > which really would benefit from it. OK, that can be done by removing !dio_align in above check. Thanks, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/