linux-kernel - Re: [PATCH 39/51] writeback: make writeback_in_progress() take bdi_writeback instead of backing_dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150702022843.GI26440@mtj.duckdns.org>
Date:	Wed, 1 Jul 2015 22:28:43 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Jan Kara <jack@...e.cz>
Cc:	axboe@...nel.dk, linux-kernel@...r.kernel.org, hch@...radead.org,
	hannes@...xchg.org, linux-fsdevel@...r.kernel.org,
	vgoyal@...hat.com, lizefan@...wei.com, cgroups@...r.kernel.org,
	linux-mm@...ck.org, mhocko@...e.cz, clm@...com,
	fengguang.wu@...el.com, david@...morbit.com, gthelen@...gle.com,
	khlebnikov@...dex-team.ru
Subject: Re: [PATCH 39/51] writeback: make writeback_in_progress() take
 bdi_writeback instead of backing_dev_info

Hello, Jan.

On Wed, Jul 01, 2015 at 09:47:08AM +0200, Jan Kara wrote:
> BTW: It would have been easier for me to review this if e.g. a move from
> bdi to wb parameter was split among less patches. The intermediate state
> where some functions call partly bdi and party wb functions is strange and
> it always makes me go search in the series whether the other part of the
> function gets converted and whether they play well together...

Similar argument.  When reviewing big picture transitions, it *could*
be easier to have larger lumps but I believe that's not necessarily
because reviewing itself becomes easier but more because it becomes
easier to skip what's uninteresting like actually verifying each
change.  Another aspect is that some of the changes are spread out.
When each patch modifies one part, it's clear that all changes in the
patch belong to that specific part; however, in larger lumps, there
usually are a number of stragglers across the changes and associating
them with other parts aren't necessarily trivial.  This happens with
patch descrption too.  It becomes easier to slip in, intentionally or
by mistake, unrelated changes without explaining what's going on.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/