Re: [patch 14/23] inodes: Support generic defragmentation

Jörn Engel
2007-11-07T10:36:22Z

On Wed, 7 November 2007 11:17:48 +0100, Jörn Engel wrote:
> > +/*
> > + * Function for filesystems that embedd struct inode into their own
> > + * structures. The offset is the offset of the struct inode in the fs inode.
> > + */
> > +void *fs_get_inodes(struct kmem_cache *s, int nr, void **v,
> > + unsigned long offset)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < nr; i++)
> > + v[i] += offset;
> > +
> > + return get_inodes(s, nr, v);
> > +}
> > +EXPORT_SYMBOL(fs_get_inodes);
>
> The fact that all pointers get changed makes me a bit uneasy:
> struct foo_inode v[20];
> ...
> fs_get_inodes(..., v, ...);
> ...
> v[0].foo_field = bar;
>
> No warning, but spectacular fireworks.
>
> > +void kick_inodes(struct kmem_cache *s, int nr, void **v, void *private)
> > +{
> > + struct inode *inode;
> > + int i;
> > + int abort = 0;
> > + LIST_HEAD(freeable);
> > + struct super_block *sb;
> > +
> > + for (i = 0; i < nr; i++) {
> > + inode = v[i];
> > + if (!inode)
> > + continue;
>
> NULL is legal here? Then fs_get_inodes should check for NULL as well
> and not add the offset to NULL pointers, I guess.

Ignore these two comments. Reading further before making them would
have helped. ;)

Jörn

--
Fancy algorithms are slow when n is small, and n is usually small.
Fancy algorithms have big constants. Until you know that n is
frequently going to be big, don't get fancy.
-- Rob Pike