Bug about devfs?

From: Kohji Okuno <okuno.kohji_at_jp.panasonic.com>
Date: Tue, 12 Jul 2011 19:10:28 +0900 (JST)
Hello,

I think that devfs has a problem.
I encountered the problem that open("/dev/AAA") returned ENOENT.
Of course, /dev/AAA exists.

ENOENT was created by the point(***) in devfs_allocv().
I think that the race condition had occurred between process A and
vnlru kernel thread.

Please check the following.

If vnlru set VI_DOOMED to vp->v_iflag but vnlru didn't still execute
VOP_RECLAIM(), process A cat get valid vp from de->de_vnode.
But, vget() will return ENOENT, because vp->v_iflag has VI_DOOMED.

When I set the break point to (***), I checked that de->de_vnode and
vp->v_data were NULL.


process A:				vnlru:

devfs_allocv() {
					vgonel(vp) {
   ...					   ...
					  vp->v_iflag |= VI_DOOMED;
  mtx_lock(&devfs_de_interlock);	   ...
  vp = de->de_vnode;			  
  if (vp != NULL) {			  VI_UNLOCK(vp);
		            _____________/ ...
  VI_LOCK(vp); ____________/              if (VOP_RECLAIM(vp, td))
  mtx_unlock(&devfs_de_interlock);	   ...
   ...				\	  devfs_reclaim(ap) {
  error = vget(vp,...);		 \	    
   ...				  \______   mtx_lock(&devfs_de_interlock); 
  if (devfs_allocv_drop_refs(...)) {        de = vp->v_data;
    ...				   	    if (de != NULL) {
  } 					      de->de_vnode = NULL;	    
  else if (error) {			      vp->v_data = NULL;
    ...	  	  			    }
    rturn (error); (***)		    mtx_unlock(&devfs_de_interlock);
  }					      ...
					  }



I think that devfs_allocv() should be fixed as below.
How do you think?

devfs_allocv(struct devfs_dirent *de, struct mount *mp, struct vnode **vpp)
{
        int error;
 	struct vnode *vp;
 	struct cdev *dev;
 	struct devfs_mount *dmp;
 
 	dmp = VFSTODEVFS(mp);
+#if 1
+ retry:
+#endif
        if (de->de_flags & DE_DOOMED) {

           ...

        mtx_lock(&devfs_de_interlock);
        vp = de->de_vnode;
        if (vp != NULL) {
                VI_LOCK(vp);
                mtx_unlock(&devfs_de_interlock);
                sx_xunlock(&dmp->dm_lock);
                error = vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, curthread);
                sx_xlock(&dmp->dm_lock);
                if (devfs_allocv_drop_refs(0, dmp, de)) {
                        if (error == 0)
                                vput(vp);
                        return (ENOENT);
                }
                else if (error) {
+#if 1
+			if (error == ENOENT)
+				goto retry;
+#endif
 			sx_xunlock(&dmp->dm_lock);
 			return (error);
 		}


Thanks,
 Kohji Okuno.
Received on Tue Jul 12 2011 - 08:31:26 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:15 UTC