public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Jeffery <djeffery@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Return short read or 0 at end of a raw device, not EIO
Date: Tue, 30 Sep 2014 12:37:07 -0400	[thread overview]
Message-ID: <542ADC33.2040804@redhat.com> (raw)
In-Reply-To: <20140930152840.GA9666@infradead.org>

On 09/30/2014 11:28 AM, Christoph Hellwig wrote:
> On Mon, Sep 29, 2014 at 06:08:11PM -0400, David Jeffery wrote:
>> On 09/29/2014 03:05 PM, Christoph Hellwig wrote:
>>> Seems like this should be changed in the generic code, or is there some
>>> reason why it would return EIO only for devices, but not for regular
>>> files in this case?
>>>
>>
>> Regular files shouldn't be returning EIO and don't in my tests. The file
>> systems manage direct I/O EOF handling in their own block or direct_IO
>> callbacks.  Block devices do not and instead do the size checks up
>> front.  Raw devices were bypassing the block device check, so only the
>> raw driver should be having this issue.
> 
> So I guess the problem is commit
> 
> "blkdev_aio_read(): switch to generic_file_read_iter(), get rid of iov_shorten()"
> 
> which removes the iov_shorten call in blkdev_aio_read?  This should be
> mentioned in the changelog.
> 
> But maybe we should instead make block devices behave more similar to
> regular files in this respect?

The issue's origin should be with bbec0270bdd8 "blkdev_max_block: make
private to fs/buffer.c".  This change intentionally removed the size
checks from blkdev_get_block() to avoid issues with the size variables
changing underneath the calls.  Commit 684c9aaebbb0 is the initial
commit to add the size checks with blkdev_aio_read() since
blkdev_get_block() no longer checks size.  As the raw driver was also
dependent on the size checks in blkdev_get_block(), it was broken by
bbec0270bdd8 but not fixed at all by 684c9aaebbb0.

> 
> Also did you make sure to add your regression test somewhere, e.g. ltp?
> 

No I have not.  I'll see about an ltp test.

  reply	other threads:[~2014-09-30 16:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-29 14:21 [PATCH] Return short read or 0 at end of a raw device, not EIO David Jeffery
2014-09-29 16:46 ` Jeff Moyer
2014-09-29 19:05 ` Christoph Hellwig
2014-09-29 22:08   ` David Jeffery
2014-09-30 15:28     ` Christoph Hellwig
2014-09-30 16:37       ` David Jeffery [this message]
2014-10-10 17:17         ` Jeff Moyer
2014-10-11 10:17           ` Christoph Hellwig
2014-10-31  8:57 ` Christoph Hellwig
2014-10-31 10:35   ` Al Viro

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=542ADC33.2040804@redhat.com \
    --to=djeffery@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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