Re: [PATCH 2/3] ensure unique i_ino in filesystems without permanentinode numbers (libfs superblock cleanup)

From: Jeff Layton
Date: Fri Dec 08 2006 - 08:08:27 EST


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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/