From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751480AbdILXE3 (ORCPT ); Tue, 12 Sep 2017 19:04:29 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:49713 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751003AbdILXE2 (ORCPT ); Tue, 12 Sep 2017 19:04:28 -0400 Subject: Re: [PATCH v3 11/13] xen/pvcalls: implement poll command To: Stefano Stabellini References: <1501541855-7354-1-git-send-email-sstabellini@kernel.org> <1501541855-7354-11-git-send-email-sstabellini@kernel.org> <702cfa9c-5f14-07a3-63ba-93648ff66d9b@oracle.com> <6c9613d8-8219-d06f-9095-fa57474ed518@oracle.com> Cc: xen-devel@lists.xen.org, linux-kernel@vger.kernel.org, jgross@suse.com, Stefano Stabellini From: Boris Ostrovsky Message-ID: <73a6ae0f-077c-9d42-fd56-0738ccb30cec@oracle.com> Date: Tue, 12 Sep 2017 19:04:07 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: aserv0022.oracle.com [141.146.126.234] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/12/2017 06:17 PM, Stefano Stabellini wrote: > On Tue, 12 Sep 2017, Boris Ostrovsky wrote: >>>>> + >>>>> +unsigned int pvcalls_front_poll(struct file *file, struct socket *sock, >>>>> + poll_table *wait) >>>>> +{ >>>>> + struct pvcalls_bedata *bedata; >>>>> + struct sock_mapping *map; >>>>> + >>>>> + if (!pvcalls_front_dev) >>>>> + return POLLNVAL; >>>>> + bedata = dev_get_drvdata(&pvcalls_front_dev->dev); >>>>> + >>>>> + map = (struct sock_mapping *) READ_ONCE(sock->sk->sk_send_head); >>>> I just noticed this --- why is it READ_ONCE? Are you concerned that >>>> sk_send_head may change? >>> No, but I wanted to avoid partial reads. A caller could call >>> pvcalls_front_accept and pvcalls_front_poll on newsock almost at the >>> same time (it is probably not the correct way to use the API), I wanted >>> to make sure that "map" is either read correctly, or not read at all. >> How can you have a partial read on a pointer? > I don't think that the compiler makes any promises on translating a > pointer read into a single read instruction. Of couse, I expect gcc to > actually do it without any need for READ/WRITE_ONCE. READ_ONCE() only guarantees ordering but not atomicity. It resolves (for 64-bit pointers) to case 8: *(__u64 *)res = *(volatile __u64 *)p; break; so if compiler was breaking accesses into two then nothing would have prevented it from breaking them here (I don't think volatile declaration would affect this). Moreover, for sizes >8 bytes READ_ONCE() is __builtin_memcpy() which is definitely not atomic. So you can't rely on READ_ONCE being atomic from that perspective. OTOH, I am pretty sure pointer accesses are guaranteed to be atomic. For example, atomic64_read() is READ_ONCE(u64), which (per above) is dereferencing of a 64-bit pointer in C. -boris