Messages in this thread | | | From | Krzysztof Sachanowicz <> | Subject | Re: [PATCH] proc: proc_get_inode should de_put when inode already initialized | Date | Tue, 24 Feb 2009 00:56:25 +0100 |
| |
Tuesday 24 February 2009 00:25:55 Andrew Morton napisał(a): > On Mon, 23 Feb 2009 22:21:55 +0100 > > Krzysztof Sachanowicz <analyzer1@gmail.com> wrote: > > de_get is called before every proc_get_inode, but corresponding de_put is > > called only when dropping last reference to an inode. This might cause > > something like > > remove_proc_entry: /proc/stats busy, count=14496 > > to be printed to the syslog. > > > > The fix is to call de_put in case of an already initialized inode in > > proc_get_inode. > > > > Signed-off-by: Krzysztof Sachanowicz <analyzer1@gmail.com> > > Tested-by: Marcin Pilipczuk <marcin.pilipczuk@gmail.com> > > --- > > --- linux-2.6.29-rc6.orig/fs/proc/inode.c 2009-02-23 20:43:32.000000000 > > +0100 +++ linux-2.6.29-rc6/fs/proc/inode.c 2009-02-23 20:46:37.000000000 > > +0100 @@ -485,8 +485,10 @@ struct inode *proc_get_inode(struct supe > > } > > } > > unlock_new_inode(inode); > > - } else > > + } else { > > module_put(de->owner); > > + de_put(de); > > + } > > return inode; > > > > out_ino: > > This code area looks quite different in linux-next, although the > changes there are removing proc_dir_entry.owner altogether and aren't > obviously targetted at fixing this bug. > > Also... > > It's unpleasing to have the de_get() inside the caller and the de_put() > inside the callee - it is better to have them both happening at the > same level. If it is the case that "de_get is called before every > proc_get_inode", then perhaps that operation should simply be moved > into proc_get_inode().
Yes, but unfortunately in proc_lookup_de() (fs/proc/generic.c) we have: 391 de_get(de); 392 spin_unlock(&proc_subdir_lock); 393 error = -EINVAL; 394 inode = proc_get_inode(dir->i_sb, ino, de);
So if we move de_get() into proc_get_inode(), we will also have to move spin_unlock there. Then we will have spin_lock in proc_lookup_de but spin_unlock in proc_get_inode...
Maybe my solution is not that bad, because usually de_put is called from proc_delete_inode(). Only if iget_locked() returns an already initialized inode we want de_put to be called in proc_get_inode. So the callee need not care about who will eventually call de_put. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |