Re: [PATCH v8 06/10] iomap: fix iomap_dio_zero() for fs bs > system page size

[Pankaj Raghav (Samsung)]

> > > A WARN_ON without actually erroring out here is highly dangerous. 
> > 
> > I agree but I think we decided that we are safe with 64k for now as fs 
> > that uses iomap will not have a block size > 64k. 
> > 
> > But this function needs some changes when we decide to go beyond 64k
> > by returning error instead of not returning anything. 
> > Until then WARN_ON_ONCE would be a good stop gap for people developing
> > the feature to go beyond 64k block size[1]. 
> 
> Sure, but please make it return an error and return that instead of
> just warning and going beyond the allocated page.

Does this make sense?

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 61d09d2364f7..14be34703588 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -240,16 +240,19 @@ void iomap_dio_bio_end_io(struct bio *bio)
 }
 EXPORT_SYMBOL_GPL(iomap_dio_bio_end_io);
 
-static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
+static int iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
                loff_t pos, unsigned len)
 {
        struct inode *inode = file_inode(dio->iocb->ki_filp);
        struct bio *bio;
 
+       if (!len)
+               return 0;
        /*
         * Max block size supported is 64k
         */
-       WARN_ON_ONCE(len > ZERO_PAGE_64K_SIZE);
+       if (len > ZERO_PAGE_64K_SIZE)
+               return -EINVAL;
 
        bio = iomap_dio_alloc_bio(iter, dio, 1, REQ_OP_WRITE | REQ_SYNC | REQ_IDLE);
        fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
@@ -260,6 +263,7 @@ static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
 
        __bio_add_page(bio, zero_page_64k, len, 0);
        iomap_dio_submit_bio(iter, dio, bio, pos);
+       return 0;
 }
 
 /*
@@ -368,8 +372,10 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
        if (need_zeroout) {
                /* zero out from the start of the block to the write offset */
                pad = pos & (fs_block_size - 1);
-               if (pad)
-                       iomap_dio_zero(iter, dio, pos - pad, pad);
+
+               ret = iomap_dio_zero(iter, dio, pos - pad, pad);
+               if (ret)
+                       goto out;
        }
 
        /*
@@ -443,7 +449,7 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
                /* zero out from the end of the write to the end of the block */
                pad = pos & (fs_block_size - 1);
                if (pad)
-                       iomap_dio_zero(iter, dio, pos, fs_block_size - pad);
+                       ret = iomap_dio_zero(iter, dio, pos, fs_block_size - pad);
        }
 out:
        /* Undo iter limitation to current extent */

--
Pankaj