Messages in this thread | | | Date | Mon, 07 Jan 2002 16:37:18 -0500 | From | Jeff Garzik <> | Subject | Re: PATCH 2.5.2.9: ext2 unbork fs.h (part 1/7) |
| |
Alexander Viro wrote: > Now, the problems I see with Jeff's variant: > > a) if you make struct inode a part of ext2_inode - WTF bother with pointer?
You mean the typed pointer inside struct inode's union? Because I needed a way to go from struct inode to struct ext2_inode_info, -without- a nasty cast. inode->u.ext2_ip maintains the type information without resorting to a nastier solution like an OFFSET_OF macro. Suggestions for improvement welcome.
> b) ->destroy_inode() / ->clear_inode(). Merge them - that way it's one > method.
Agreed. That would be [not yet written] patch8 in my plan.
> c) get_empty_inode() must die. Make it new_inode() and be done with that. > And have socket.c explicitly set ->i_dev to NODEV afterwards.
In my patch get_empty_inode and new_inode are completely identical. This is easy.
> d) ext2/balloc.c cleanup probably should be merged before.
I don't have an opinion on this one way or the other...
> I can live with "maintain refcounts in common part and leave allocation/freeing > to filesystem". It's definitely better than allocating/freeing opaque objects > in VFS using numeric fields in fs_type.
Yes... the opacity factor in the other patch bothers me.
> We will need to set very strict rules on passing around/storing pointers to > ext2_inode and its ilk, though. There will be bugs when somebody just decides > that keeping such pointers might be a good idea and forgets to be nice with > ->i_count. Or decrement it manually instead of calling iput(), etc.
Not doing so now is a shoot-on-sight offense, I thought...
Jeff
-- Jeff Garzik | Alternate titles for LOTR: Building 1024 | Fast Times at Uruk-Hai MandrakeSoft | The Took, the Elf, His Daughter and Her Lover | Samwise Gamgee: International Hobbit of Mystery - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |