"J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote on 04/22/2009 08:24:27 AM:
> > +static int k_nfsd_thread(void *unused)
> > +{
> > + while (!kthread_should_stop()) {
> > + schedule_timeout_interruptible(NFSD_CACHE_JIFFIES);
> > +
> > + if (kthread_should_stop())
> > + break;
> > +
> > + daemon_free_entries();
> > + }
> > + __set_current_state(TASK_RUNNING);
>
> Is this necessary? The comment before schedule_timeout() claims "The
> current task state is guaranteed to be TASK_RUNNING when this routine
> returns."
Right, it is not required.
> > +static inline struct fhcache *
> > +nfsd_get_fhcache(__u32 auth)
> > +{
> > + struct fhcache *fh, **fhp, **ffhp = NULL;
> > + int depth = 0;
> > + unsigned int hash;
> > + struct fhcache_hbucket *fhb;
> > +
> > + if (!auth)
> > + return NULL;
> > +
> > + hash = jhash_1word(auth, 0xfeedbeef) & FHPARM_HASH_MASK;
> > + fhb = &fhcache_hash[hash];
> > +
> > + spin_lock(&fhb->pb_lock);
> > + for (fhp = &fhb->pb_head; (fh = *fhp); fhp = &fh->p_next) {
> > + if (fh->p_auth == auth) {
> > + /* Same inode */
> > + if (unlikely(!fh->p_filp)) {
> > + /*
> > + * Someone is racing in the same code, and
> > + * this is the first reference to this file.
> > + */
> > + spin_unlock(&fhb->pb_lock);
>
> The locking seems over-complicated. But I have to admit, I don't
> understand it yet.
I have one new lock - k_nfsd_lock, that is used to add/delete the entry to
nfsd_daemon_free_list. Lock is used by either the daemon, or when an
operation results in creating or deleting a cache entry (read and
unlink).
>
> > + return NULL;
> > + }
> > +
> > + /*
> > + * Hold an extra reference to dentry/exp since these
> > + * are released in fh_put(). 'file' already has an
> > + * extra hold from the first lookup which was never
> > + * dropped.
>
> Why not take a reference on the file instead of the dentry?
I am holding an extra reference to file in the first lookup, but unless
I hold dentry also, it gets freed up by fh_put.
> > + depth = nfsdstats.ra_size*11/10;
>
> Help, I'm confused--what's the meaning of this calculation?
That is original code, and I also find it confusing :) What I understand is
that when an entry is not found in the cache, ra_depth[10] is incremented,
and
when it is found, ra_depth[0] is incremented (unless the hash table entry
contains elements in the range of 20-30 or more, then ra_depth[2], [3],
etc,
get incremented. It seems to give an indication of the efficiency of the
hash
table.
thanks,
- KK
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
