| Message ID | 20181005130316.4869-1-cmaiolino@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] iomap: Enable alignment check for iomap DIO in holes | expand |
On Fri, Oct 05, 2018 at 03:03:16PM +0200, Carlos Maiolino wrote: > Commit 09230435dffdb13de507e5e40b524b0069fc5c7b refactor iomap_dio_actor > into two different helpers for bio based IO and hole IO, although, the > latter doesn't check for IO alignment anymore. > > Move the DIO alignment check up into the caller, so all iomap DIO has > alignment checked. Why? The only reason to check alignmnet for actual I/O is because we have to. There is no requirement to align holes. > Christoph, I tagged this patch as RFC, because I couldn't really identify if you > intentionally dropped the alignment check for holes and inline actors, if you > didn't drop the alignment check intentionally, please consider it as a normal > patch. The primary intent was to simplify the code and not enforce alignment for inline data support.
On Sat, Oct 06, 2018 at 11:59:51AM +0200, Christoph Hellwig wrote: > On Fri, Oct 05, 2018 at 03:03:16PM +0200, Carlos Maiolino wrote: > > Commit 09230435dffdb13de507e5e40b524b0069fc5c7b refactor iomap_dio_actor > > into two different helpers for bio based IO and hole IO, although, the > > latter doesn't check for IO alignment anymore. > > > > Move the DIO alignment check up into the caller, so all iomap DIO has > > alignment checked. > > Why? The only reason to check alignmnet for actual I/O is because we > have to. There is no requirement to align holes. > > > Christoph, I tagged this patch as RFC, because I couldn't really identify if you > > intentionally dropped the alignment check for holes and inline actors, if you > > didn't drop the alignment check intentionally, please consider it as a normal > > patch. > > The primary intent was to simplify the code and not enforce alignment > for inline data support. Ok, thanks, so please disregard this patch. Cheers.
diff --git a/fs/iomap.c b/fs/iomap.c index 90c2febc93ac..347e31512268 100644 --- a/fs/iomap.c +++ b/fs/iomap.c @@ -1574,9 +1574,7 @@ static loff_t iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, struct iomap_dio *dio, struct iomap *iomap) { - unsigned int blkbits = blksize_bits(bdev_logical_block_size(iomap->bdev)); unsigned int fs_block_size = i_blocksize(inode), pad; - unsigned int align = iov_iter_alignment(dio->submit.iter); struct iov_iter iter; struct bio *bio; bool need_zeroout = false; @@ -1584,9 +1582,6 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, int nr_pages, ret; size_t copied = 0; - if ((pos | length | align) & ((1 << blkbits) - 1)) - return -EINVAL; - if (iomap->type == IOMAP_UNWRITTEN) { dio->flags |= IOMAP_DIO_UNWRITTEN; need_zeroout = true; @@ -1726,6 +1721,11 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length, void *data, struct iomap *iomap) { struct iomap_dio *dio = data; + unsigned int blkbits = blksize_bits(bdev_logical_block_size(iomap->bdev)); + unsigned int align = iov_iter_alignment(dio->submit.iter); + + if ((pos | length | align) & ((1 << blkbits) - 1)) + return -EINVAL; switch (iomap->type) { case IOMAP_HOLE:
Commit 09230435dffdb13de507e5e40b524b0069fc5c7b refactor iomap_dio_actor into two different helpers for bio based IO and hole IO, although, the latter doesn't check for IO alignment anymore. Move the DIO alignment check up into the caller, so all iomap DIO has alignment checked. Fixes: 09230435d iomap: refactor iomap_dio_actor Reported-by: Ondrej Kozina <okozina@redhat.com> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> --- Christoph, I tagged this patch as RFC, because I couldn't really identify if you intentionally dropped the alignment check for holes and inline actors, if you didn't drop the alignment check intentionally, please consider it as a normal patch. fs/iomap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)