DragonFly kernel List (threaded) for 2008-01
DragonFly BSD
DragonFly kernel List (threaded) for 2008-01
[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]

Re: Patch for inode SLIST conversion


To: Simon 'corecode' Schubert <corecode@xxxxxxxxxxxx>
From: Michael Neumann <mneumann@xxxxxxxx>
Date: Sat, 19 Jan 2008 17:55:37 +0100

Simon 'corecode' Schubert wrote:
Michael Neumann wrote:
Matthias Schmidt wrote:
Hi,

* Michael Neumann wrote:
Hi,

Appended my first patch. And it wasn't that hard at all. The Projects page of the DF wiki mentioned to convert the inode i_next field to use a SLIST. This is what the patch does. I'm not sure if it is that usable :

Just for the record: This issue is also covered by http://bugs.dragonflybsd.org/issue694

Thanks for pointing this out. Next time I'll take a look at the issue tracker first.

I think we should commit one of them. The submission by Michael seems more thorough, so I'd go for that one.

Please wait because I'm doing some further cleanup on it. But before I have a question according to serialization tokens...

In ufs_ihash.c there are only two functions that modify
the inode hash table, ufs_ihashins and ufs_ihashrem given below:

int
ufs_ihashins(struct inode *ip)
{
        struct inode *iq;
        struct inode *prev;
        struct ihashtbl_bucket *head;
        lwkt_tokref ilock;

        KKASSERT((ip->i_flag & IN_HASHED) == 0);
        lwkt_gettoken(&ilock, &ufs_ihash_token);

        head = INOHASH(ip->i_dev, ip->i_number);
        SLIST_FOREACH_WITH_PREV(iq, prev, head, i_next) {
                if (INOEQL(ip, iq->i_dev, iq->i_number)) {
                        lwkt_reltoken(&ilock);
                        return(EBUSY);
                }
        }
        if (prev) SLIST_NEXT(prev, i_next) = ip;
        else      SLIST_FIRST(head) = ip;
        SLIST_NEXT(ip, i_next) = NULL;
        ip->i_flag |= IN_HASHED;
        lwkt_reltoken(&ilock);
        return(0);
}

void
ufs_ihashrem(struct inode *ip)
{
        lwkt_tokref ilock;

        lwkt_gettoken(&ilock, &ufs_ihash_token);
        if (ip->i_flag & IN_HASHED) {
                SLIST_REMOVE(INOHASH(ip->i_dev, ip->i_number), ip,
                    inode, i_next);
                SLIST_NEXT(ip, i_next) = NULL;
                ip->i_flag &= ~IN_HASHED;
        }
        lwkt_reltoken(&ilock);
}

If I understand serialization tokens correctly, they lead in case of the two functions above to mutual exclusive execution, because none of them voluntarily blocks or calls a blocking method. Is this assumption correct?

Then there is ufs_ihashget:

struct vnode *
ufs_ihashget(cdev_t dev, ino_t inum)
{
        lwkt_tokref ilock;
        struct inode *ip;
        struct vnode *vp;

lwkt_gettoken(&ilock, &ufs_ihash_token);

loop:
        INOFIND(ip, dev, inum);
        vp = NULL;
        if (ip)
        {
                vp = ITOV(ip);
                if (vget(vp, LK_EXCLUSIVE))
                        goto loop;
                /*
                 * We must check to see if the inode has been ripped
                 * out from under us after blocking.
                 */
                INOFIND(ip, dev, inum);
                if (ip == NULL || ITOV(ip) != vp) {
       			vput(vp);
                        goto loop;
                }
        }

        lwkt_reltoken(&ilock);
        return (vp);
}


See the comment on "ripped out from under us". This can only happen because vget might block, right?


Now, ufs_ihashget doesn't modify the hashtable at all. So why not store a "transaction id" in each hash table bucket, which gets increased by ufs_ihashins and ufs_ihashrem. Then in ufs_ihashget I read in this id *before* calling vget and check afterwards if it has changed. If it is unchanged, I can omit a second pass over the linked list.

It's actually quite easy to implement and if all my assumptions apply, the correct way of doing it. Please correct me if I'm wrong.


Regards,


Michael



[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]