netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Gregory Haskins <gregory.haskins@gmail.com>
Cc: netdev@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@elte.hu>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	hpa@zytor.com, paulmck@linux.vnet.ibm.com
Subject: Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
Date: Wed, 12 Aug 2009 16:47:15 +0300	[thread overview]
Message-ID: <20090812134715.GB29340@redhat.com> (raw)
In-Reply-To: <4A82C68F.8070306@gmail.com>

On Wed, Aug 12, 2009 at 09:41:35AM -0400, Gregory Haskins wrote:
> Michael S. Tsirkin wrote:
> > On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
> >> I think I understand what your comment above meant:  You don't need to
> >> do synchronize_rcu() because you can flush the workqueue instead to
> >> ensure that all readers have completed.
> > 
> > Yes.
> > 
> >>  But if thats true, to me, the
> >> rcu_dereference itself is gratuitous,
> > 
> > Here's a thesis on what rcu_dereference does (besides documentation):
> > 
> > reader does this
> > 
> > 	A: sock = n->sock
> > 	B: use *sock
> > 
> > Say writer does this:
> > 
> > 	C: newsock = allocate socket
> > 	D: initialize(newsock)
> > 	E: n->sock = newsock
> > 	F: flush
> > 
> > 
> > On Alpha, reads could be reordered.  So, on smp, command A could get
> > data from point F, and command B - from point D (uninitialized, from
> > cache).  IOW, you get fresh pointer but stale data.
> > So we need to stick a barrier in there.
> 
> Yes, that is understood.  Perhaps you should just use a normal barrier,
> however.  (Or at least a comment that says "I am just using this for its
> barrier").
> 
> > 
> >> and that pointer is *not* actually
> >> RCU protected (nor does it need to be).
> > 
> > Heh, if readers are lockless and writer does init/update/sync,
> > this to me spells rcu.
> 
> More correctly: it "smells like" RCU, but its not. ;)  It's rcu-like,
> but you are not really using the rcu facilities.  I think anyone that
> knows RCU and reads your code will likely be scratching their heads as well.
> 
> Its probably not a big deal, as I understand your code now.  Just a
> suggestion to help clarify it.
> 
> Regards,
> -Greg
> 

OK, I'll add some comments about that.
Thanks for the review!

-- 
MST

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2009-08-12 13:47 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1249992497.git.mst@redhat.com>
2009-08-11 21:27 ` [PATCHv2 1/2] mm: export use_mm/unuse_mm to modules Michael S. Tsirkin
2009-08-11 22:10   ` Andrew Morton
2009-09-17  5:38     ` Michael S. Tsirkin
2009-09-17  5:43       ` Andrew Morton
2009-08-11 21:28 ` [PATCHv2 2/2] vhost_net: a kernel-level virtio server Michael S. Tsirkin
2009-08-12  0:06   ` Gregory Haskins
2009-08-12  9:02     ` Michael S. Tsirkin
2009-08-12 13:01       ` Gregory Haskins
2009-08-12 13:25         ` Michael S. Tsirkin
2009-08-12 13:41           ` Gregory Haskins
2009-08-12 13:47             ` Michael S. Tsirkin [this message]
2009-08-12 14:11           ` Paul E. McKenney
2009-08-12 14:15             ` Michael S. Tsirkin
2009-08-12 15:26               ` Paul E. McKenney
2009-08-12 15:51                 ` Michael S. Tsirkin
2009-08-12 16:06                   ` Paul E. McKenney
2009-08-12 10:52     ` Michael S. Tsirkin

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=20090812134715.GB29340@redhat.com \
    --to=mst@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=gregory.haskins@gmail.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@elte.hu \
    --cc=netdev@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=virtualization@lists.linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).