linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	linux-block@vger.kernel.org, clm@fb.com,
	torvalds@linux-foundation.org, david@fromorbit.com
Subject: Re: [PATCH 1/6] fs: add read support for RWF_UNCACHED
Date: Tue, 17 Dec 2019 07:57:49 -0800	[thread overview]
Message-ID: <20191217155749.GC32169@bombadil.infradead.org> (raw)
In-Reply-To: <20191217143948.26380-2-axboe@kernel.dk>

On Tue, Dec 17, 2019 at 07:39:43AM -0700, Jens Axboe wrote:
> +static void buffered_put_page(struct page *page, bool clear_mapping)
> +{
> +	if (clear_mapping)
> +		page->mapping = NULL;
> +	put_page(page);
> +}

I'm not a huge fan of the variable name 'clear_mapping'.  It describes
what it does rather than why it does it.  So maybe 'drop_immediate'?
Or 'uncached'?

>  		if (!page) {
>  			if (iocb->ki_flags & IOCB_NOWAIT)
>  				goto would_block;
> +			/* UNCACHED implies no read-ahead */
> +			if (iocb->ki_flags & IOCB_UNCACHED) {
> +				did_dio_begin = true;
> +				/* block truncate for UNCACHED reads */
> +				inode_dio_begin(inode);

I think this needs to be:

				if (!did_dio_begin)
					inode_dio_begin(inode);
				did_dio_begin = true;

otherwise inode->i_dio_count is going to be increased once per uncached
page.  Do you have a test in your test-suite that does I/O to more than
one page at a time?


  parent reply	other threads:[~2019-12-17 15:57 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-17 14:39 [PATCHSET v5 0/6] Support for RWF_UNCACHED Jens Axboe
2019-12-17 14:39 ` [PATCH 1/6] fs: add read support " Jens Axboe
2019-12-17 15:16   ` Guoqing Jiang
2019-12-17 16:42     ` Jens Axboe
2019-12-17 15:57   ` Matthew Wilcox [this message]
2019-12-17 16:41     ` Jens Axboe
2019-12-18  3:17   ` Dave Chinner
2019-12-17 14:39 ` [PATCH 2/6] mm: make generic_perform_write() take a struct kiocb Jens Axboe
2019-12-17 14:39 ` [PATCH 3/6] mm: make buffered writes work with RWF_UNCACHED Jens Axboe
2019-12-17 14:39 ` [PATCH 4/6] iomap: add struct iomap_ctx Jens Axboe
2019-12-17 19:39   ` Linus Torvalds
2019-12-17 20:26     ` Linus Torvalds
2019-12-18  0:15       ` Jens Axboe
2019-12-18  1:25         ` Darrick J. Wong
2019-12-17 14:39 ` [PATCH 5/6] iomap: support RWF_UNCACHED for buffered writes Jens Axboe
2019-12-18  1:52   ` Darrick J. Wong
2019-12-18  3:18     ` Jens Axboe
2019-12-18  4:15       ` Darrick J. Wong
2019-12-17 14:39 ` [PATCH 6/6] xfs: don't do delayed allocations for uncached " Jens Axboe
2019-12-18  1:57   ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191217155749.GC32169@bombadil.infradead.org \
    --to=willy@infradead.org \
    --cc=axboe@kernel.dk \
    --cc=clm@fb.com \
    --cc=david@fromorbit.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).