Re: [PATCH 07/16] GFS2: Directory handling

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Fri, 2006-04-21 at 17:16 +0100, Christoph Hellwig wrote:
> > +/*
> > +* Implements Extendible Hashing as described in:
> > +*   "Extendible Hashing" by Fagin, et al in
> > +*     __ACM Trans. on Database Systems__, Sept 1979.
> > +*
> > +*
> 
> please follow the normal comment style, that is leave a space before the *
> for block comments so it lines up nicely with the * in the start tag.
> 
> > +#include <linux/sched.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/completion.h>
> 
> you don't seem to be using any completion in this file
> 
> > +#include <linux/buffer_head.h>
> > +#include <linux/sort.h>
> > +#include <linux/gfs2_ondisk.h>
> > +#include <linux/crc32.h>
> > +#include <linux/vmalloc.h>
> > +#include <asm/semaphore.h>
> 
> you're not using any semaphore in this file
> 
> > +int gfs2_dir_get_buffer(struct gfs2_inode *ip, uint64_t block, int new,
> > +		         struct buffer_head **bhp)
[function body cut for clarity]
> 
> the code is completely different for the new vs !new case, so there's no
> point in merging it to a single function.
> 
These points are now fixed in the git tree:
http://www.kernel.org/git/?p=linux/kernel/git/steve/gfs2-2.6.git;a=commitdiff;h=61e085a88cb59232eb8ff5b446d70491c7bf2c68

Thanks for the comments, please let me know if I missed anything,

Steve.