All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@osdl.org>, Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <peterz@infradead.org>,
	Nick Piggin <npiggin@kernel.dk>
Subject: Re: [patch 0/5] seqlock consolidation
Date: Thu, 15 Mar 2012 20:42:35 +0000	[thread overview]
Message-ID: <20120315204235.GF8943@ZenIV.linux.org.uk> (raw)
In-Reply-To: <alpine.LFD.2.02.1203152013030.2466@ionos>

On Thu, Mar 15, 2012 at 08:17:12PM +0100, Thomas Gleixner wrote:
> > +			hits += replace_path(&fs->root, old_root, new_root);
> > +			hits += replace_path(&fs->pwd, old_root, new_root);
> 
> Wouldn't it be simpler to just do: 
> 
> +			count += replace_path(&fs->root, old_root, new_root);
> +			count += replace_path(&fs->pwd, old_root, new_root);
> 
> > +			write_seqcount_end(&fs->seq);
> > +			while (hits--) {
> >  				count++;
> 
> Instead of that loop ?

This loop also contains path_get_longterm() and we need to do it before
dropping fs->lock.  We are holding a reference to new_root, all right,
but once it's place into ->fs->{root,pwd} of another task and ->fs->lock
is dropped, there's nothing to stop that task of doing chdir() and dropping
its reference.  Which could outweight the single reference we are holding
pretty soon...

  reply	other threads:[~2012-03-15 20:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-15 11:44 [patch 0/5] seqlock consolidation Thomas Gleixner
2012-03-15 11:44 ` [patch 1/5] seqlock: Remove unused functions Thomas Gleixner
2012-03-15 16:29   ` Linus Torvalds
2012-03-15 17:39     ` Al Viro
2012-03-15 17:53       ` Thomas Gleixner
2012-03-15 11:44 ` [patch 2/5] seqlock: Use seqcount for seqlock Thomas Gleixner
2012-03-15 11:44 ` [patch 3/5] seqlock: Provide seq_spin_* functions Thomas Gleixner
2012-03-15 11:44 ` [patch 4/5] fs: fs_struct use seqlock Thomas Gleixner
2012-03-15 11:44 ` [patch 5/5] fs: Use seqlock in struct dentry Thomas Gleixner
2012-03-15 12:21 ` [patch 0/5] seqlock consolidation Al Viro
2012-03-15 12:28   ` Thomas Gleixner
2012-03-15 17:43     ` Al Viro
2012-03-15 17:55       ` Thomas Gleixner
2012-03-15 18:39         ` Al Viro
2012-03-15 19:17           ` Thomas Gleixner
2012-03-15 20:42             ` Al Viro [this message]
2012-03-15 22:08               ` Thomas Gleixner

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=20120315204235.GF8943@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=npiggin@kernel.dk \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@osdl.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.