Re: [PATCH] bcache: untagle cache_aolloc

From	Kent Overstreet <kent.overstreet@gmail.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH] bcache: untagle cache_aolloc
Date	2016-07-18 12:30 +0200
Message-ID	<rWdYd-4WI-27@gated-at.bofh.it> (permalink)
References	<rWdOy-4To-9@gated-at.bofh.it> <rWdOy-4To-7@gated-at.bofh.it> <rWdYd-4WI-29@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Mon, Jul 18, 2016 at 12:24:11PM +0200, Johannes Thumshirn wrote:
> On Mon, Jul 18, 2016 at 02:13:33AM -0800, Kent Overstreet wrote:
> > On Mon, Jul 18, 2016 at 12:11:09PM +0200, Johannes Thumshirn wrote:
> > > bcache's cache_alloc() function currenty has no way freeing memory if one of
> > > the allocations fails. Untangle the if + allocation statement so we have
> > > defined checkpoints to free previous allocations if one fails.
> > 
> > nack. The existing error path handles failure midway through just fine.
> 
> Come on, the patch improves the readability of the if statement by some orders
> of magnitude as well.
> 
> Are you OK with it if I change the subject/commit log?

No, it's just churn and I don't agree that it improves readability. On the
contrary, now the cleanup code has to be duplicated in two places - which
invites them getting out of sync and introducing bugs.

Thread

Re: [PATCH] bcache: untagle cache_aolloc Kent Overstreet <kent.overstreet@gmail.com> - 2016-07-18 12:20 +0200
  Re: [PATCH] bcache: untagle cache_aolloc Kent Overstreet <kent.overstreet@gmail.com> - 2016-07-18 12:30 +0200
  Re: [PATCH] bcache: untagle cache_aolloc Johannes Thumshirn <jthumshirn@suse.de> - 2016-07-18 12:30 +0200

csiph-web