Re: [PATCH v2 2/4] fd/direct-io: introduce should_dirty for kernel aio

From: Christoph Hellwig
Date: Sun Jan 25 2015 - 08:34:40 EST


On Tue, Jan 13, 2015 at 11:44:46PM +0800, Ming Lei wrote:
>
> - if (dio->is_async && dio->rw == READ)
> + if (dio->is_async && dio->rw == READ && dio->should_dirty)
> bio_set_pages_dirty(bio);
>
> if (sdio->submit_io)
> @@ -463,13 +464,14 @@ static int dio_bio_complete(struct dio *dio, struct bio *bio)
> if (!uptodate)
> dio->io_error = -EIO;
>
> - if (dio->is_async && dio->rw == READ) {
> + if (dio->is_async && dio->rw == READ && dio->should_dirty) {

I'd rather have a ->should_dirrty flag that means we need to call
bio_check_pages_dirty, And set that either if we have a kernel
iovec/bvec or dio->is_async && dio->rw == READ.

But why would we even set this if writing from an iovec?

> bio_check_pages_dirty(bio); /* transfers ownership */
> } else {
> bio_for_each_segment_all(bvec, bio, i) {
> struct page *page = bvec->bv_page;
>
> - if (dio->rw == READ && !PageCompound(page))
> + if (dio->rw == READ && !PageCompound(page) &&
> + dio->should_dirty)
> set_page_dirty_lock(page);

And this unk could also use some explanation.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/