On (25/07/16 17:38), Sergey Senozhatsky wrote:
> Hi Christoph,
>
> > static int read_from_bdev(struct zram *zram, struct page *page,
> > - unsigned long entry, struct bio *parent, bool sync)
> > + unsigned long entry, struct bio *parent)
> > {
> > atomic64_inc(&zram->stats.bd_reads);
> > - if (sync) {
> > + if (!parent) {
> > if (WARN_ON_ONCE(!IS_ENABLED(ZRAM_PARTIAL_IO)))
> > return -EIO;
> > - return read_from_bdev_sync(zram, page, entry, parent);
> > + return read_from_bdev_sync(zram, page, entry);
> > }
> > read_from_bdev_async(zram, page, entry, parent);
> > return 1;
>
> I was looking at zram's bdev (read from a backing device) today
> and got a bit puzzled by that !parent check in read_from_bdev():
>
> zram_bio_read(zram, bio)
> zram_bvec_read(bio)
> zram_read_page(bio)
> read_from_bdev(bio) {
> if (!parent)
> return read_from_bdev_sync()
> }
>
> The thing is, that "parent" is basically "bio" which is passed to
> zram_bio_read(), and it cannot be NULL (zram_bio_read() dereferences
> it multiple times before passing it down the call chain.) Is sync
> read basically a dead code now? Am I missing something?
Ah, wait, I think I see it now.
Synchronous reads seem to be only for partial IO:
zram_bvec_read()
zram_bvec_read_partial()
zram_read_page(NULL)
read_from_bdev(NULL)
read_from_bdev(NULL)
if (!parent)
read_from_bdev_sync()
