linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Kinglong Mee <kinglongmee@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Trond Myklebust <trond.myklebust@primarydata.com>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfs/blocklayout: make sure making a aligned read request
Date: Mon, 29 Feb 2016 05:34:39 -0800	[thread overview]
Message-ID: <20160229133439.GA4416@infradead.org> (raw)
In-Reply-To: <56D432D5.7000905@gmail.com>

On Mon, Feb 29, 2016 at 08:00:21PM +0800, Kinglong Mee wrote:
> On 2/29/2016 17:57, Christoph Hellwig wrote:
> > On Sat, Feb 13, 2016 at 09:51:31PM +0800, Kinglong Mee wrote:
> >> Only treat write goes up to the inode size as aligned request,
> >> because it always write PAGE_CACHE_SIZE, but read a dynamic size.
> > 
> > Can you explain what the point is? 
> 
> I run ltp tests with read02 hang.
> There seams a loop in block codes.
> It is caused by passing an unaligned read to bio.
> So this patch is out as making a aligned read request.

Do you have any additional details?

> > We'll never use data pas the block size
> > in the page cache, but per the block size requirement in the spec we must
> > be able to read it.  This patch means we can't direct storage reads where
> > we previously could, without any obvious upside.
> 
> bl_pg_init_read/bl_pg_test_read checks aligned base on SECTOR_SIZE.
> bl_pg_init_write/bl_pg_test_write checks aligned base on PAGE_SIZE.
> 
> If according the codes, reads data per block size is okay.
> 
> But, there is a comment in bl_read_pagelist() as,
> 
> 250         isect = (sector_t) (f_offset >> SECTOR_SHIFT);
> 251         /* Code assumes extents are page-aligned */
> 252         for (i = pg_index; i < header->page_array.npages; i++) {
> 253                 if (extent_length <= 0) {
> 
> I don't known the meaning of "extents are page-aligned", 
> extent's start offset is aligned to page size?
> or extent's start offset is aligned to page size and length
> is equal to PAGE_SIZE too ?

All of them should start aligned to PAGE_SIZE, and also have a length
that is a multiple of PAGE_SIZE.

  reply	other threads:[~2016-02-29 13:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-13 13:51 [PATCH] nfs/blocklayout: make sure making a aligned read request Kinglong Mee
2016-02-29  9:57 ` Christoph Hellwig
2016-02-29 12:00   ` Kinglong Mee
2016-02-29 13:34     ` Christoph Hellwig [this message]
2016-03-01 12:35       ` Kinglong Mee
2016-03-16 19:41         ` Trond Myklebust
2016-03-21 15:30           ` Christoph Hellwig
2016-03-21 18:27             ` William Dauchy

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=20160229133439.GA4416@infradead.org \
    --to=hch@infradead.org \
    --cc=kinglongmee@gmail.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.com \
    /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).