From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Layton Subject: Re: [PATCH 2/3] ensure unique i_ino in filesystems without permanent inode numbers (libfs superblock cleanup) Date: Fri, 08 Dec 2006 08:08:03 -0500 Message-ID: <457963B3.3080801@poochiereds.net> References: <457891F4.8030501@redhat.com> <20061208061641.GA24255@filer.fsl.cs.sunysb.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Return-path: Received: from ms-smtp-02.southeast.rr.com ([24.25.9.101]:36931 "EHLO ms-smtp-02.southeast.rr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760695AbWLHNIH (ORCPT ); Fri, 8 Dec 2006 08:08:07 -0500 To: Josef Sipek In-Reply-To: <20061208061641.GA24255@filer.fsl.cs.sunysb.edu> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Josef Sipek wrote: >> - ret = simple_fill_super(sb, IPATHFS_MAGIC, files); >> + ret = simple_fill_super(sb, IPATHFS_MAGIC, files, 1); > > I don't know...the magic looking 1 and 0 (later in the patch) seem a bit > arbitrary. Maybe a #define is in order? Yeah, I'm not fond of that, though the comments on simple_fill_super should explain it. Basically, I need simple_fill_super to operate in two different "modes", and I was using the extra flag to key this. I'm not clear on what sort of #define would make sense here. Can you suggest something? >> -int simple_fill_super(struct super_block *s, int magic, struct tree_descr >> *files) >> +/* >> + * Some filesystems require that particular entries have particular i_ino >> values. Those >> + * callers need to set the "seq" flag to make sure that i_ino is assigned >> sequentially >> + * to the files starting with 0. >> + */ >> +int simple_fill_super(struct super_block *s, int magic, struct tree_descr >> *files, int seq) > > Line wraped. > I thought those were under 80 columns but perhaps they weren't. I'll clean them up and repost, once you clarify what you'd like to see in the #define. >> @@ -399,7 +407,10 @@ int simple_fill_super(struct super_block >> inode->i_blocks = 0; >> inode->i_atime = inode->i_mtime = inode->i_ctime = >> CURRENT_TIME; > > I'd indent CURRENT_TIME a bit. I wasn't planning on touching those parts of the code that don't need to be changed, since formatting deltas can make it harder to see the "actual" changes in the patch. That should probably be addressed in a follow-on patch if you think it needs to be changed. -- Jeff Layton