Re: [PATCH 1/7] shared subtree

Ram Pai
2005-07-29T19:55:21Z

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