Re: [PATCH 15/16] zram: fix synchronous reads

[Minchan Kim <minchan@kernel.org>]

On Fri, Apr 07, 2023 at 06:37:43AM +0200, Christoph Hellwig wrote:
> On Thu, Apr 06, 2023 at 05:23:51PM -0700, Minchan Kim wrote:
> > rw_page path so that bio comes next to serve the rw_page failure.
> > In the case, zram will always do chained bio so we are fine with
> > asynchronous IO.
> > 
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index b8549c61ff2c..23fa0e03cdc1 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -1264,6 +1264,8 @@ static int __zram_bvec_read(struct zram *zram, struct page *page, u32 index,
> >                 struct bio_vec bvec;
> > 
> >                 zram_slot_unlock(zram, index);
> > +               if (partial_io)
> > +                       return -EAGAIN;
> > 
> >                 bvec.bv_page = page;
> >                 bvec.bv_len = PAGE_SIZE;
> 
> What tree is this supposed to apply to?  6.2 already has the
> zram_bvec_read_from_bdev helper.

It was for prior to 6.1 where doesn't have (94541bc3fbde4, zram: always
expose rw_page). Since 6.1, with returning -EOPNOTSUPP on the rw_page,
it will defer the request to be handled via submit_bio so every IO to
read data from backing device will have parent bio and chained.

> 
> 
> But either way partial_io can be true when called from zram_bvec_write,
> where we can't just -EAGAIN as ->submit_bio is not allowed to return
> that except for REQ_NOWAIT bios, and even then it needs to be handle
> them when submitted without REQ_NOWAIT.

The case returning -EAGAIN is only rw_page path not the submit_bio path
so the request will be deferred to submit_bio path. However, yeah,
I see it's still broken since read_from_bdev_sync never wait the read
complettion again.

Then, as you suggested, just disable the feature for non-4K would be
the simple option.

I see only this way to achieve it and not sure it will cover all the
cases. If you have better idea, let me know.

Thank you!

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 0386b7da02aa..ae7662430789 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -58,6 +58,12 @@ config ZRAM_DEF_COMP
 config ZRAM_WRITEBACK
        bool "Write back incompressible or idle page to backing device"
        depends on ZRAM
+       depends on !PAGE_SIZE_8KB
+       depends on !PAGE_SIZE_16KB
+       depends on !PAGE_SIZE_32KB
+       depends on !PAGE_SIZE_64KB
+       depends on !PAGE_SIZE_256KB
+       depends on !PAGE_SIZE_1MB
        help
         With incompressible page, there is no memory saving to keep it
         in memory. Instead, write it out to backing device.