All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@redhat.com>
To: Boaz Harrosh <openosd@gmail.com>, NeilBrown <neilb@suse.com>,
	Andreas Dilger <adilger@dilger.ca>
Cc: linux-block@vger.kernel.org,
	linux-scsi <linux-scsi@vger.kernel.org>,
	lsf-pc <lsf-pc@lists.linuxfoundation.org>,
	Neil Brown <neilb@suse.de>, LKML <linux-kernel@vger.kernel.org>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	linux-mm <linux-mm@kvack.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [Lsf-pc] [LSF/MM TOPIC] do we really need PG_error at all?
Date: Tue, 28 Feb 2017 06:32:39 -0500	[thread overview]
Message-ID: <1488281559.2874.1.camel@redhat.com> (raw)
In-Reply-To: <0bea2b1c-ddb1-f2bf-8ef7-b83d6a6404fc@gmail.com>

On Tue, 2017-02-28 at 12:12 +0200, Boaz Harrosh wrote:
> On 02/28/2017 03:11 AM, Jeff Layton wrote:
> <>
> > 
> > I'll probably have questions about the read side as well, but for now it
> > looks like it's mostly used in an ad-hoc way to communicate errors
> > across subsystems (block to fs layer, for instance).
> 
> If memory does not fail me it used to be checked long time ago in the
> read-ahead case. On the buffered read case, the first page is read synchronous
> and any error is returned to the caller, but then a read-ahead chunk is
> read async all the while the original thread returned to the application.
> So any errors are only recorded on the page-bit, since otherwise the uptodate
> is off and the IO will be retransmitted. Then the move to read_iter changed
> all that I think.
> But again this is like 5-6 years ago, and maybe I didn't even understand
> very well.
> 

Yep, that's what I meant about using it to communicate errors between
layers. e.g. end_buffer_async_read will check PageError and only
SetPageUptodate if it's not set. That has morphed a lot in the last few
years though and it looks like it may rely on PG_error less than it used
to.

> 
> I would like a Documentation of all this as well please. Where are the
> tests for this?
> 

Documentation is certainly doable (and I'd like to write some once we
have this all straightened out). In particular, I think we need clear
guidelines for fs authors on how to handle pagecache read and write
errors. Tests are a little tougher -- this is all kernel-internal stuff
and not easily visible to userland.

The one thing I have noticed is that even if you set AS_ENOSPC in the
mapping, you'll still get back -EIO on the first fsync if any PG_error
bits are set. I think we ought to fix that by not doing the
TestClearPageError call in __filemap_fdatawait_range, and just rely on
the mapping error there.

We could maybe roll a test for that, but it's rather hard to test ENOSPC
conditions in a fs-agnostic way. I'm open to suggestions here though.

-- 
Jeff Layton <jlayton@redhat.com>

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Layton <jlayton@redhat.com>
To: Boaz Harrosh <openosd@gmail.com>, NeilBrown <neilb@suse.com>,
	Andreas Dilger <adilger@dilger.ca>
Cc: linux-block@vger.kernel.org,
	linux-scsi <linux-scsi@vger.kernel.org>,
	lsf-pc <lsf-pc@lists.linuxfoundation.org>,
	Neil Brown <neilb@suse.de>, LKML <linux-kernel@vger.kernel.org>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	linux-mm <linux-mm@kvack.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [Lsf-pc] [LSF/MM TOPIC] do we really need PG_error at all?
Date: Tue, 28 Feb 2017 06:32:39 -0500	[thread overview]
Message-ID: <1488281559.2874.1.camel@redhat.com> (raw)
In-Reply-To: <0bea2b1c-ddb1-f2bf-8ef7-b83d6a6404fc@gmail.com>

On Tue, 2017-02-28 at 12:12 +0200, Boaz Harrosh wrote:
> On 02/28/2017 03:11 AM, Jeff Layton wrote:
> <>
> > 
> > I'll probably have questions about the read side as well, but for now it
> > looks like it's mostly used in an ad-hoc way to communicate errors
> > across subsystems (block to fs layer, for instance).
> 
> If memory does not fail me it used to be checked long time ago in the
> read-ahead case. On the buffered read case, the first page is read synchronous
> and any error is returned to the caller, but then a read-ahead chunk is
> read async all the while the original thread returned to the application.
> So any errors are only recorded on the page-bit, since otherwise the uptodate
> is off and the IO will be retransmitted. Then the move to read_iter changed
> all that I think.
> But again this is like 5-6 years ago, and maybe I didn't even understand
> very well.
> 

Yep, that's what I meant about using it to communicate errors between
layers. e.g. end_buffer_async_read will check PageError and only
SetPageUptodate if it's not set. That has morphed a lot in the last few
years though and it looks like it may rely on PG_error less than it used
to.

> 
> I would like a Documentation of all this as well please. Where are the
> tests for this?
> 

Documentation is certainly doable (and I'd like to write some once we
have this all straightened out). In particular, I think we need clear
guidelines for fs authors on how to handle pagecache read and write
errors. Tests are a little tougher -- this is all kernel-internal stuff
and not easily visible to userland.

The one thing I have noticed is that even if you set AS_ENOSPC in the
mapping, you'll still get back -EIO on the first fsync if any PG_error
bits are set. I think we ought to fix that by not doing the
TestClearPageError call in __filemap_fdatawait_range, and just rely on
the mapping error there.

We could maybe roll a test for that, but it's rather hard to test ENOSPC
conditions in a fs-agnostic way. I'm open to suggestions here though.

-- 
Jeff Layton <jlayton@redhat.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-02-28 11:32 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-26 14:42 [LSF/MM TOPIC] do we really need PG_error at all? Jeff Layton
2017-02-26 14:42 ` Jeff Layton
2017-02-26 17:10 ` James Bottomley
2017-02-26 17:10   ` James Bottomley
2017-02-26 21:03   ` NeilBrown
2017-02-26 22:43     ` Jeff Layton
2017-02-26 22:43       ` Jeff Layton
2017-02-26 23:30     ` James Bottomley
2017-02-26 23:57       ` Jeff Layton
2017-02-26 23:57         ` Jeff Layton
2017-02-27  0:27       ` NeilBrown
2017-02-27 15:07         ` Jeff Layton
2017-02-27 15:07           ` Jeff Layton
2017-02-27 22:51           ` Andreas Dilger
2017-02-27 23:02             ` Jeff Layton
2017-02-27 23:02               ` Jeff Layton
2017-02-27 23:32             ` NeilBrown
2017-02-28  1:11               ` [Lsf-pc] " Jeff Layton
2017-02-28  1:11                 ` Jeff Layton
2017-02-28  1:11                 ` Jeff Layton
2017-02-28 10:12                 ` Boaz Harrosh
2017-02-28 10:12                   ` Boaz Harrosh
2017-02-28 11:32                   ` Jeff Layton [this message]
2017-02-28 11:32                     ` Jeff Layton
2017-02-28 20:45                 ` NeilBrown

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=1488281559.2874.1.camel@redhat.com \
    --to=jlayton@redhat.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=adilger@dilger.ca \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=lsf-pc@lists.linuxfoundation.org \
    --cc=neilb@suse.com \
    --cc=neilb@suse.de \
    --cc=openosd@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.