All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bob Peterson <rpeterso@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [GFS2 PATCH] GFS2: Fix gfs2_log_write and eliminate gfs2_log_bmap
Date: Mon, 4 Dec 2017 09:33:05 -0500 (EST)	[thread overview]
Message-ID: <234147738.38091705.1512397985362.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <2c3401fc-12cc-1af2-fc25-818a22a15c40@redhat.com>

----- Original Message -----
| On 01/12/17 17:34, Bob Peterson wrote:
| > Hi,
| >
| > Before this patch, tiny function gfs2_log_write called small function
| > gfs2_log_bmap to determine which journal block to write. If function
| > gfs2_log_bmap encountered a problem in its block mapping calculations,
| > it would return a block number of -1 to indicate an error. This error
| > was immediately ignored and forgotten: the caller went ahead and
| > tried to write to the log anyway. This patch fixes the problem
| > by checking the results of the block map calculations and doing
| > an assert withdraw if an error occurs. It also eliminates function
| > gfs2_log_bmap in favor of inlining the code to make the code more
| > readable.
| >
| > This should improve overall journal integrity, as a precursor to
| > log writing improvements we are planning.
| Again, I'm not sure that there is anything to be gained in readability
| by inlining gfs2_log_bmap(). Why not just add the new error check in
| that function?
| 
| Steve.

As explained in my previous email: "Death by a thousand cuts."
I much prefer the readability of one 17-line function to two
functions of 9 lines each.

Bob Peterson



  reply	other threads:[~2017-12-04 14:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <634212417.37167771.1512149562990.JavaMail.zimbra@redhat.com>
2017-12-01 17:34 ` [Cluster-devel] [GFS2 PATCH] GFS2: Fix gfs2_log_write and eliminate gfs2_log_bmap Bob Peterson
2017-12-04 12:22   ` Steven Whitehouse
2017-12-04 14:33     ` Bob Peterson [this message]
2017-12-04 14:36       ` Bob Peterson
2017-12-04 14:41         ` Steven Whitehouse

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=234147738.38091705.1512397985362.JavaMail.zimbra@redhat.com \
    --to=rpeterso@redhat.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.