From: Dexuan Cui <decui@microsoft.com>
To: Michael Kelley <mikelley@microsoft.com>,
kimbrownkd <kimbrownkd@gmail.com>, Long Li <longli@microsoft.com>,
Sasha Levin <Alexander.Levin@microsoft.com>
Cc: KY Srinivasan <kys@microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Stephen Hemminger <sthemmin@microsoft.com>,
"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] Drivers: hv: vmbus: Expose counters for interrupts and full conditions
Date: Thu, 10 Jan 2019 03:58:34 +0000 [thread overview]
Message-ID: <PU1P153MB01695102421655A3DEB4F98EBF840@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <CY4PR21MB07733E3D30FD5FCB7A945CA6D78F0@CY4PR21MB0773.namprd21.prod.outlook.com>
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Saturday, January 5, 2019 1:01 PM
> > From: Kimberly Brown <kimbrownkd@gmail.com> Sent: Friday, January 4,
> > 2019 8:35 PM
> >
> > static inline void set_channel_pending_send_size(struct vmbus_channel *c,
> > u32 size)
> > {
> > + if (size) {
> > + ++c->out_full_total;
> > +
> > + if (!c->out_full_flag) {
> > + ++c->out_full_first;
> > + c->out_full_flag = true;
> > + }
> > + } else {
> > + c->out_full_flag = false;
> > + }
> > +
> > c->outbound.ring_buffer->pending_send_sz = size;
> > }
> >
>
> I think there may be an atomicity problem with the above code. I looked
> in the hv_sock code, and didn't see any locks being held when
> set_channel_pending_send_size() is called. The original code doesn't need
> a lock because it is just storing a single value into pending_send_sz.
>
> In the similar code in hv_ringbuffer_write(), the ring buffer spin lock is held
> while the counts are incremented and the out_full_flag is maintained, so all is
> good there. But some locking may be needed here. Dexuan knows the
> hv_sock
> code best and can comment on whether there is any higher level
> synchronization
> that prevents multiple threads from running the above code on the same
> channel.
> Even if there is such higher level synchronization, this code probably shouldn't
> depend on it for correctness.
>
> Michael
Yes, there is indeed a higher level per-"sock" lock, e.g. in the code path
vsock_stream_sendmsg() -> vsock_stream_has_space() ->
transport->stream_has_space() -> hvs_stream_has_space() ->
hvs_set_channel_pending_send_size(), we acquire the lock by
lock_sock(sk) at the beginning of vsock_stream_sendmsg(), and call
release_sock(sk) at the end of the function.
So we don't have an atomicity issue here for hv_sock, which is the only user
of set_channel_pending_send_size(), so far.
Thanks,
-- Dexuan
next prev parent reply other threads:[~2019-01-10 3:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <0181213190901.GA3843@ubu-Virtual-Machine>
2019-01-05 4:35 ` [PATCH v2] Drivers: hv: vmbus: Expose counters for interrupts and full conditions Kimberly Brown
2019-01-05 21:01 ` Michael Kelley
2019-01-10 3:58 ` Dexuan Cui [this message]
2019-01-10 3:46 ` Dexuan Cui
2019-01-17 4:37 ` [PATCH v3] " Kimberly Brown
2019-01-17 6:45 ` Dexuan Cui
2019-01-17 17:11 ` Stephen Hemminger
2019-01-21 16:29 ` Kimberly Brown
2019-01-17 16:04 ` Michael Kelley
2019-02-04 7:13 ` [PATCH v4] " Kimberly Brown
2019-02-04 16:25 ` Michael Kelley
2019-02-15 1:57 ` Sasha Levin
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=PU1P153MB01695102421655A3DEB4F98EBF840@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM \
--to=decui@microsoft.com \
--cc=Alexander.Levin@microsoft.com \
--cc=devel@linuxdriverproject.org \
--cc=haiyangz@microsoft.com \
--cc=kimbrownkd@gmail.com \
--cc=kys@microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=longli@microsoft.com \
--cc=mikelley@microsoft.com \
--cc=sthemmin@microsoft.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;
as well as URLs for NNTP newsgroup(s).