Re: [PATCH 1/7] shared subtree

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

On Thu, 2005-07-28 at 02:57, Miklos Szeredi wrote:
> > > This is an example, where having struct pnode just complicates things.
> > > If there was no struct pnode, this function would be just one line:
> > > setting the shared flag.
> > So your comment is mostly about getting rid of pnode and distributing
> > the pnode functionality in the vfsmount structure.
> 
> Yes, sorry if I didn't make it clear.
> 
> > I know you are thinking of just having the necessary propogation list in
> > the vfsmount structure itself.  Yes true with that implementation the
> > complication is reduced in this part of the code, but really complicates
> > the propogation traversal routines. 
> 
> On the contrary, I think it will simplify the traversal routines.
> 
> Here's an iterator function I coded up.  Not tested at all (may not
> even compile):

Your suggested code has bugs. But I understand what you are aiming at. 

Maybe you are right. I will try out a implementation using your idea.

Hmm.. lots of code change, and testing.

> 
> struct vfsmount {
> 	/* ... */
>         
> 	struct list_head mnt_share;      /* circular list of shared mounts */
> 	struct list_head mnt_slave_list; /* list of slave mounts */
> 	struct list_head mnt_slave;      /* slave list entry */
> 	struct vfsmount *master;         /* slave is on master->mnt_slave_list */
> };
> 
> static inline struct vfsmount *next_shared(struct vfsmount *p)
> {
> 	return list_entry(p->mnt_share.next, struct vfsmount, mnt_share);
> }
> 
> static inline struct vfsmount *first_slave(struct vfsmount *p)
> {
> 	return list_entry(p->mnt_slave_list.next, struct vfsmount, mnt_slave);
> }
> 
> static inline struct vfsmount *next_slave(struct vfsmount *p)
> {
> 	return list_entry(p->mnt_slave.next, struct vfsmount, mnt_slave);
> }
> 
> static struct vfsmount *propagation_next(struct vfsmount *p,
> 					 struct vfsmount *base)
> {
> 	/* first iterate over the slaves */
> 	if (!list_empty(&p->mnt_slave_list))
> 		return first_slave(p);

I think this code should be
		if (!list_empty(&p->mnt_slave))
			return next_slave(p);

Right? I think I get the idea. 



RP

> 
> 	while (1) {
> 		struct vfsmount *q;
> 
> 		/* more vfsmounts belong to the pnode? */
> 		if (!list_empty(&p->mnt_share)) {
> 			p = next_shared(p);
> 			if (list_empty(&p->mnt_slave) && p != base)
> 				return p;
> 		}
> 		if (p == base)
> 			break;
> 		
> 		BUG_ON(list_empty(&p->mnt_slave));
> 
> 		/* more slaves? */
> 		q = next_slave(p);
> 		if (p->master != q)
> 			return q;
> 
> 		/* back at master */
> 		p = q;
> 	}
> 
> 	return NULL;
> }
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html