All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Tinguely <tinguely@sgi.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 1/8] xfs: check for buffer errors before waiting
Date: Thu, 29 Mar 2012 16:48:11 -0500	[thread overview]
Message-ID: <4F74D89B.1060008@sgi.com> (raw)
In-Reply-To: <20120329211025.GD692@dastard>

On 03/29/12 16:10, Dave Chinner wrote:
> On Thu, Mar 29, 2012 at 02:04:09PM -0500, Mark Tinguely wrote:
>> On 03/29/12 07:23, Dave Chinner wrote:
>>> From: Dave Chinner<dchinner@redhat.com>
>>>
>>> If we call xfs_buf_iowait() on a buffer that failed dispatch due to
>>> an IO error, it will wait forever for an Io that does not exist.
>>> This is hndled in xfs_buf_read, but there is other code that calls
>>> xfs_buf_iowait directly that doesn't.
>>>
>>> Rather than make the call sites have to handle checking for dispatch
>>> errors and then checking for completion errors, make
>>> xfs_buf_iowait() check for dispatch errors on the buffer before
>>> waiting. This means we handle both dispatch and completion errors
>>> with one set of error handling at the caller sites.
>>>
>>> Signed-off-by: Dave Chinner<dchinner@redhat.com>
>>> ---
>>
>>> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
>>> index 396e3bf..64ed6ff 100644
>>> --- a/fs/xfs/xfs_log_recover.c
>>> +++ b/fs/xfs/xfs_log_recover.c
>>> @@ -179,6 +179,7 @@ xlog_bread_noalign(
>>>   	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
>>>   	XFS_BUF_READ(bp);
>>>   	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
>>> +	bp->b_error = 0;
>>>
>>>   	xfsbdstrat(log->l_mp, bp);
>>>   	error = xfs_buf_iowait(bp);
>>> @@ -266,6 +267,7 @@ xlog_bwrite(
>>>   	xfs_buf_hold(bp);
>>>   	xfs_buf_lock(bp);
>>>   	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
>>> +	bp->b_error = 0;
>>>
>>>   	error = xfs_bwrite(bp);
>>>   	if (error)
>>
>> Just curious, were these needed for a particular reason?
>
> If the previous user of the buffer got an error, it is not
> guaranteed to be cleared because the buffer is not re-initialised.
> i.e. it's an uncached buffer that we control completely and reuse
> from IO to IO with just a reset of the bno and length. If b_error is
> non zero here, then the IO can fail because nothing else clears the
> error in the dispatch path....
>
> Cheers,
>
> Dave.

Thank-you for the explanation.

FYI: I am having problems with the patches applying. This patch 
complained at hunk at offset 623. Maybe I am using too new of kernel source.

--Mark.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2012-03-29 21:48 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-29 12:23 [PATCH 0/8] xfs: clean up unit usage in xfs_buf V2 Dave Chinner
2012-03-29 12:23 ` [PATCH 1/8] xfs: check for buffer errors before waiting Dave Chinner
2012-03-29 19:04   ` Mark Tinguely
2012-03-29 21:10     ` Dave Chinner
2012-03-29 21:48       ` Mark Tinguely [this message]
2012-03-29 22:07         ` Dave Chinner
2012-03-29 19:12   ` Christoph Hellwig
2012-03-29 21:17     ` Dave Chinner
2012-03-29 12:23 ` [PATCH 2/8] xfs: fix incorrect b_offset initialisation Dave Chinner
2012-03-29 19:13   ` Christoph Hellwig
2012-03-29 20:43   ` Mark Tinguely
2012-03-29 12:23 ` [PATCH 3/8] xfs: use kmem_zone_zalloc for buffers Dave Chinner
2012-03-29 19:14   ` Christoph Hellwig
2012-03-29 20:45   ` Mark Tinguely
2012-03-29 12:23 ` [PATCH 4/8] xfs: clean up buffer get/read call API Dave Chinner
2012-03-30 19:12   ` Mark Tinguely
2012-03-29 12:23 ` [PATCH 5/8] xfs: kill b_file_offset Dave Chinner
2012-03-30 19:12   ` Mark Tinguely
2012-03-29 12:23 ` [PATCH 6/8] xfs: use blocks for counting length of buffers Dave Chinner
2012-03-29 19:16   ` Christoph Hellwig
2012-03-30 19:13   ` Mark Tinguely
2012-03-29 12:23 ` [PATCH 7/8] xfs: use blocks for storing the desired IO size Dave Chinner
2012-03-30 19:13   ` Mark Tinguely
2012-03-29 12:23 ` [PATCH 8/8] xfs: kill xfs_buf_btoc Dave Chinner
2012-03-29 19:18   ` Christoph Hellwig
2012-03-30 19:13   ` Mark Tinguely

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=4F74D89B.1060008@sgi.com \
    --to=tinguely@sgi.com \
    --cc=david@fromorbit.com \
    --cc=xfs@oss.sgi.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.