mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Trond Myklebust <trond.myklebust@fys.uio.no>
To: dipankar@in.ibm.com
Cc: John M Flinchbaugh <glynis@butterfly.hjsoft.com>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@transmeta.com>,
	Maneesh Soni <maneesh@in.ibm.com>
Subject: Re: 2.5.70-bk16: nfs crash
Date: Thu, 12 Jun 2003 22:24:16 -0700	[thread overview]
Message-ID: <16105.24576.901270.856844@charged.uio.no> (raw)
In-Reply-To: <20030612195302.GH1438@in.ibm.com>

>>>>> " " == Dipankar Sarma <dipankar@in.ibm.com> writes:

    >> Look at all those functions that take dcache_lock, and then
    >> test
    >> dentry-> d_count. Unless I'm missing something here, your
    >> d_lookup() clearly has them all screwed, no?

     > Not necessarily. One example is the fact that d_lookup() can
     > only increase d_count. Besides, dput() decrements d_count
     > without dcache_lock, so I am not sure holding dcache_lock
     > during d_count test buys you much.

Wrong. Look at the VFS code. In all cases the test is of the form.

    spin_lock(&dcache_lock);
    /* Are we the sole users of this dentry */
    if (atomic_read(&dentry->d_count) == 1) {
       /* Yes - do some operation */
    }


Knowing that d_lookup() can *increase* d_count is not a plus here. The
whole idea is to have a test for sole use.

In most cases, the "do some operation" above is

	d_drop(dentry);

in order (for instance) to ensure that nobody else can look up this
dentry while we're working on it (e.g. rename or unlink,...).

Your d_lookup() screws the above example of code which you can find in
any number of VFS functions. dput(), d_delete(), d_invalidate(),
d_prune_aliases(), prune_dcache(), shrink_dcache_sb() are but a few
functions that rely on the above code snippet working to keep
d_lookup() from intruding.

Cheers,
   Trond

  reply	other threads:[~2003-06-13  5:11 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-06-12 12:56 John M Flinchbaugh
2003-06-12 13:52 ` Dipankar Sarma
2003-06-12 15:33   ` Dipankar Sarma
2003-06-12 15:35   ` Trond Myklebust
2003-06-12 15:53     ` Dipankar Sarma
2003-06-12 16:26       ` Trond Myklebust
2003-06-12 16:49         ` Linus Torvalds
2003-06-12 16:55           ` Linus Torvalds
2003-06-12 19:53         ` Dipankar Sarma
2003-06-13  5:24           ` Trond Myklebust [this message]
2003-06-13  5:50             ` Dipankar Sarma
2003-06-13  6:13               ` Trond Myklebust
2003-06-13  6:54                 ` Dipankar Sarma
2003-06-13  6:06             ` Dipankar Sarma
2003-06-12 16:30       ` viro
2003-06-12 16:55         ` Dipankar Sarma
2003-06-12 15:49   ` Linus Torvalds
2003-06-12 16:05     ` Dipankar Sarma
2003-06-12 16:18     ` Linus Torvalds
2003-06-12 16:35       ` Dipankar Sarma
2003-06-12 16:47         ` Linus Torvalds
2003-06-13 12:48       ` Maneesh Soni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=16105.24576.901270.856844@charged.uio.no \
    --to=trond.myklebust@fys.uio.no \
    --cc=dipankar@in.ibm.com \
    --cc=glynis@butterfly.hjsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maneesh@in.ibm.com \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®