From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fernando Luis Vazquez Cao Subject: Re: [PATCH 1/9] vfs: add __iterate_supers() helper Date: Fri, 14 Sep 2012 11:40:41 +0900 Message-ID: <50529929.6010403@lab.ntt.co.jp> References: <1347533862.5646.2.camel@nexus.lab.ntt.co.jp> <1347534020.5646.4.camel@nexus.lab.ntt.co.jp> <50529837.5060307@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Al Viro , Josef Bacik , Dave Chinner , Christoph Hellwig , Jan Kara , linux-fsdevel@vger.kernel.org To: Eric Sandeen Return-path: Received: from tama500.ecl.ntt.co.jp ([129.60.39.148]:39593 "EHLO tama500.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754588Ab2INCk4 (ORCPT ); Thu, 13 Sep 2012 22:40:56 -0400 In-Reply-To: <50529837.5060307@redhat.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 2012/09/14 11:36, Eric Sandeen wrote: > On 9/13/12 6:00 AM, Fernando Luis V=C3=A1zquez Cao wrote: >> iterate_supers() calls a function provided by the caller with the s_= umount >> semaphore taken in read mode. However, there may be cases where writ= e mode >> is preferable, so we add a new helper, __iterate_supers(), which let= s one >> specify the mode of the lock. >> >> This will be used to fix the emergency thaw (filesystem unfreeze) co= de, which >> iterates over the list of superblocks but needs to hold the s_umount= semaphore >> in _write_ mode bebore carrying out the actual thaw operation. > I'll go ahead & comment on this even though all of this is to support= the > emergency thaw misfeature which I'm liking less and less this evening= =2E > > I think this split is a little odd, usually __foo() provides some fun= damental > action, and foo_*() functions call it after possibly defining or alte= ring aspects of > that action, or preparing for that action. > > So having iterate_supers() do read locks, and __iterate_supers do eit= her read > or write, seems asymmetric, and isn't very well self-documenting. > > Why not have i.e. iterate_supers_read() and iterate_supers_write(), e= ach > of which can call __iterate_supers(blah, blah, locktype). > > Anyway, it'd mean changing 5 callers but it's a little clearer and mo= re symmetric > to me. What do you think? > > The logic seems fine, just a question about the style. I agree with you. I will fix the style as you suggest. Thanks, =46ernando -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html