Re: [RFC][PATCH 5/14] Introduce union stack

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

 



On 5/14/07, Jan Engelhardt <[email protected]> wrote:

>+static inline void union_lock(struct dentry *dentry)
>+{
>+      if (unlikely(dentry && dentry->d_union)) {
>+              struct union_info *ui = dentry->d_union;
>+
>+              UM_DEBUG_LOCK("\"%s\" locking %p (count=%d)\n",
>+                            dentry->d_name.name, ui,
>+                            atomic_read(&ui->u_count));
>+              __union_lock(dentry->d_union);
>+      }
>+}
>+
>+static inline void union_unlock(struct dentry *dentry)
>+{
>+      if (unlikely(dentry && dentry->d_union)) {
>+              struct union_info *ui = dentry->d_union;
>+
>+              UM_DEBUG_LOCK("\"%s\" unlocking %p (count=%d)\n",
>+                            dentry->d_name.name, ui,
>+                            atomic_read(&ui->u_count));
>+              __union_unlock(dentry->d_union);
>+      }
>+}

Do we really need the unlikely()? d_union may be a new feature,
but it may very well be possible that someone puts the bigger
part of his/her files under a union. And when d_unions get
stable, people will probably begin making their root filesystem
unioned for livecds, and then unlikely() will rather be a
likely penalty. My stance: just
        if (dentry != NULL && dentry->d_union != NULL)
This also goes for union_trylock.

Good question. My intention was that since most of the union code
costs performance (stack traversal, readdir) I optimize for the normal
(not unified) case.

>+static inline int union_trylock(struct dentry *dentry)
>+{
>+      int locked = 1;
>+
>+      if (unlikely(dentry && dentry->d_union)) {
>+              UM_DEBUG_LOCK("\"%s\" try locking %p (count=%d)\n",
>+                            dentry->d_name.name, dentry->d_union,
>+                            atomic_read(&dentry->d_union->u_count));
>+              BUG_ON(!atomic_read(&dentry->d_union->u_count));
>+              locked = mutex_trylock(&dentry->d_union->u_mutex);
>+              UM_DEBUG_LOCK("\"%s\" trylock %p %s\n", dentry->d_name.name,
>+                            dentry->d_union,
>+                            locked ? "succeeded" : "failed");
>+      }
>+      return (locked ? 1 : 0);
>+}

        return locked ? 1 : 0
or even
        return !!locked;
or since we're just passing up from mutex_trylock:
        return locked;
?

Ahh, this seems to be a left-over of the semaphore -> mutex conversion.

>+/*
>+ * This is a *I can't get no sleep* helper

More commonly known as "insomnia". :)


:)


Before I forget this: thank you (and Badari) for reviewing the patches!

Cheers,
Jan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux