Re: [RFC PATCH 1/8] share/private/slave a subtree

[Ram <linuxram@us.ibm.com>]

On Fri, 2005-07-08 at 04:17, Pekka Enberg wrote:
> On 7/8/05, Ram <linuxram@us.ibm.com> wrote:
> > This patch adds the shared/private/slave support for VFS trees.
> 
> Inlining the patches to email would be greatly appreciated. Here are
> some comments.
> 
> > +int
> > +_do_make_mounted(struct nameidata *nd, struct vfsmount **mnt)
> 
> Use two underscores to follow naming conventions.

I have renamed this function as make_mounted() in the 2nd patch.
Sure, will follow the convention.

> 
> > Index: 2.6.12/fs/pnode.c
> > ===================================================================
> > --- /dev/null
> > +++ 2.6.12/fs/pnode.c
> > @@ -0,0 +1,362 @@
> > +
> > +#define PNODE_MEMBER_VFS  0x01
> > +#define PNODE_SLAVE_VFS   0x02
> 
> Enums, please.

ok.

> 
> > +
> > +static kmem_cache_t * pnode_cachep;
> > +
> > +/* spinlock for pnode related operations */
> > + __cacheline_aligned_in_smp DEFINE_SPINLOCK(vfspnode_lock);
> > +
> > +
> > +static void
> > +pnode_init_fn(void *data, kmem_cache_t *cachep, unsigned long flags)
> > +{
> > +	struct vfspnode *pnode = (struct vfspnode *)data;
> 
> Redundant cast.

ok.

> 
> > +	INIT_LIST_HEAD(&pnode->pnode_vfs);
> > +	INIT_LIST_HEAD(&pnode->pnode_slavevfs);
> > +	INIT_LIST_HEAD(&pnode->pnode_slavepnode);
> > +	INIT_LIST_HEAD(&pnode->pnode_peer_slave);
> > +	pnode->pnode_master = NULL;
> > +	pnode->pnode_flags = 0;
> > +	atomic_set(&pnode->pnode_count,0);
> > +}
> > +
> > +void __init
> > +pnode_init(unsigned long mempages)
> > +{
> > +	pnode_cachep = kmem_cache_create("pnode_cache",
> > +                       sizeof(struct vfspnode), 0,
> > +                       SLAB_HWCACHE_ALIGN|SLAB_PANIC, pnode_init_fn, NULL);
> > +}
> > +
> > +
> > +struct vfspnode *
> > +pnode_alloc(void)
> > +{
> > +	struct vfspnode *pnode =  (struct vfspnode *)kmem_cache_alloc(
> > +			pnode_cachep, GFP_KERNEL);
> 
> Redundant cast.

ok.

> 
> > +struct inoutdata {
> 
> Wants a better name.

ok. something like propogation_data? or pdata?

> 
> > +	void *my_data; /* produced and consumed by me */
> > +	void *in_data; /* produced by master, consumed by slave */
> > +	void *out_data; /* produced by slave, comsume by master */
> > +};
> > +
> > +struct pcontext {
> > +	struct vfspnode *start;
> > +	int 	flag;
> > +	int 	traversal;
> > +	int	level;
> > +	struct vfspnode *master_pnode;
> > +	struct vfspnode *pnode;
> > +	struct vfspnode *slave_pnode;
> > +};
> > +
> > +
> > +#define PNODE_UP 1
> > +#define PNODE_DOWN 2
> > +#define PNODE_MID 3
> 
> Enums, please.

ok. I will incorporate all the rest of the comments.  There are lots of
places as noted by you I need to following the coding style. I will.

Thanks,
RP