On (23/04/11 19:14), Christoph Hellwig wrote:
> Currently nothing waits for the synchronous reads before accessing
> the data. Switch them to an on-stack bio and submit_bio_wait to
> make sure the I/O has actually completed when the work item has been
> flushed. This also removes the call to page_endio that would unlock
> a page that has never been locked.
>
> Drop the partial_io/sync flag, as chaining only makes sense for the
> asynchronous reads of the entire page.
[..]
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?
