From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f193.google.com ([209.85.223.193]:42384 "EHLO mail-io0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753130AbeF0LO0 (ORCPT ); Wed, 27 Jun 2018 07:14:26 -0400 Received: by mail-io0-f193.google.com with SMTP id r24-v6so1516734ioh.9 for ; Wed, 27 Jun 2018 04:14:25 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20180627003906.15571-1-agruenba@redhat.com> References: <20180627003906.15571-1-agruenba@redhat.com> From: Andreas Gruenbacher Date: Wed, 27 Jun 2018 13:14:24 +0200 Message-ID: Subject: Re: [PATCH 0/1] iomap: Direct I/O for inline data To: Christoph Hellwig Cc: cluster-devel , linux-ext4 , linux-fsdevel , Andreas Gruenbacher Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 27 June 2018 at 02:39, Andreas Gruenbacher wrote: > Here's a patch that implements direct I/O for inline data. Direct I/O > to inline data is a bit weird because it's not direct in the usual > sense, but since Christoph's been asking for it ... > > The usual alignment restrictions to the logical block size of the > underlying block device still apply. I don't see a reason for changing > that; the resulting behavior would only become very weird for no > benefit. > > I've tested this against a hacked-up version of gfs2. However, the > "real" gfs2 will keep falling back to buffered I/O for writes to inline > data: gfs2 takes a shared lock during direct I/O, and writing to the > inode under that shared lock is not allowed. Ext4 may become the first > actual user of this part of the patch. One further issue is the alignment check in iomap_dio_actor: > if ((pos | length | align) & ((1 << blkbits) - 1)) > return -EINVAL; For inline data, iomap->length is set to the file size. iomap_apply truncates the requested length down to that, so iomap_dio_actor sees the truncated length instead of the requested length and fails with -EINVAL. This causes tests like the following to fail (also see xfstest generic/120): xfs_io -fd -c 'truncate 300' -c 'pread -v 0 4096' /mnt/test/foo A possible fix is to change the alignment check in iomap_dio_actor as follows: - if ((pos | length | align) & ((1 << blkbits) - 1)) + if ((pos | align) & ((1 << blkbits) - 1)) + return -EINVAL; + if (length & ((1 << blkbits) - 1) && + pos + length != iomap->offset + iomap->length) return -EINVAL; Moving the alignment check from iomap_dio_actor to iomap_dio_rw isn't that easy because iomap->bdev isn't known there. Any thoughts? Thanks, Andreas