On Wed, May 09, 2018 at 09:48:08AM +0200, Christoph Hellwig wrote:
> Simply use iomap_apply to iterate over the file and a submit a bio for
> each non-uptodate but mapped region and zero everything else.  Note that
> as-is this can not be used for file systems with a blocksize smaller than
> the page size, but that support will be added later.
> 
> Signed-off-by: Christoph Hellwig <[email protected]>
.....
> +int
> +iomap_readpages(struct address_space *mapping, struct list_head *pages,
> +             unsigned nr_pages, const struct iomap_ops *ops)
> +{
> +     struct iomap_readpage_ctx ctx = { .pages = pages };
> +     loff_t pos = page_offset(list_entry(pages->prev, struct page, lru));
> +     loff_t last = page_offset(list_entry(pages->next, struct page, lru));
> +     loff_t length = last - pos + PAGE_SIZE, ret = 0;
> +
> +     while (length > 0) {
> +             ret = iomap_apply(mapping->host, pos, length, 0, ops,
> +                             &ctx, iomap_readpages_actor);
> +             if (ret <= 0)
> +                     break;
> +             pos += ret;
> +             length -= ret;
> +     }
> +
> +     ret = 0;

This means the function will always return zero, regardless of
whether iomap_apply returned an error or not.

> +     if (ctx.bio)
> +             submit_bio(ctx.bio);
> +     if (ctx.cur_page) {
> +             if (!ctx.cur_page_in_bio)
> +                     unlock_page(ctx.cur_page);
> +             put_page(ctx.cur_page);
> +     }
> +     WARN_ON_ONCE(ret && !list_empty(ctx.pages));

And this warning will never trigger. Was this intended behaviour?
If it is, it needs a comment, because it looks wrong....

Cheers,

Dave.
-- 
Dave Chinner
[email protected]

Reply via email to