Packet socket: mmapped IO: PACKET_TX_RING
Subsystems (2)
Hi Patrick,
Quoted 2 lines
The reason why its not in there is to avoid the unnecessary barrier when setting up the ring in
packet_set_ring().
Is it a serious problem? packet_set_status() and packet_get_status() will be called from multiple places and we will obtain a write/read barrier at each call. I imagine that's more clean to insert those barriers inside status functions even if we add only one useless flush at the beginning. No?
Quoted 5 lines
The code is fishy though, I agree. Have a look at this discussion:
My approach was to use the mechanism of RX_RING where the kernel is allocating the buffer. In order to get something similar from kernel and user point of view. Moving kernel buffer to user space could be studied as global enhancement for RX_RING and TX_RING but that will need user application changes.
I'm wondering if I also need to use flush_dcache_page() before getting status buffer and attaching circular buffer pages to skb (both written by user).
According to this quote in [cachetlb.txt] of flush_dcache_page part:
Any time the kernel writes to a page cache page, _OR_ the kernel is about to read from a page cache page and user space shared/writable mappings of this page potentially exist, this routine is called.
...
The phrase "kernel writes to a page cache page" means,
...
The corollary case is just as important, if there are users which have shared+writable mappings of this file, we must make sure that kernel reads of these pages will see the most recent stores done by the user.
Can someone confirm?
Thanks in advance,
Johann
Quoted 12 lines
-----Original Message-----
From: Johann Baudy [mailto:[email protected]]
Sent: November-05-08 7:17 AM
To: Patrick McHardy
Cc: Lovich, Vitali; [email protected]; Evgeniy Polyakov
Subject: Re: [PATCH] Packet socket: mmapped IO:PACKET_TX_RINGHi Patrick,
Quoted 8 lines
Quoted 2 lines
The reason why its not in there is to avoid the unnecessary barrier when setting up the ring in
packet_set_ring().Is it a serious problem?
packet_set_status()andpacket_get_status()will be called from multiple places and we will obtain a write/read barrier at each call. I imagine that's more clean to insert those barriers inside status functions even if we add only one useless flush at the beginning. No?
You can put a smp_rmb after reading the status in packet_get_status, and a smp_wmb before setting the status in packet_set_status. Avoiding it because packet_set_ring doesn't need it is an unnecessary optimization in my opinion; it's not the performance critical path so it's not going to cost anything (allocating the memory is far more expensive anyways than a memory barrier).
Quoted 4 lines
Quoted 4 lines
The code is fishy though, I agree. Have a look at this discussion:
Quoted 8 lines
Quoted 2 lines
My approach was to use the mechanism of
RX_RINGwhere the kernel is allocating the buffer. In order to get something similar from kernel and
user point of view. Moving kernel buffer to user space could be studied as global enhancement forRX_RINGandTX_RINGbut that will need user application changes.
I'm not seeing a resolution in the above thread - is af_packet wrong in the order it flushes the pages? It certainly seems like a valid question (I'm on x86, so this is an academic issue for me).
Quoted 17 lines
I'm wondering if I also need to use
flush_dcache_page()before getting status buffer and attaching circular buffer pages to skb (both written by user).
According to this quote in [cachetlb.txt] offlush_dcache_pagepart:
Any time the kernel writes to a page cache page, _OR_ the kernel is about to read from a page cache page and user space shared/writable mappings of this page potentially exist, this routine is called.
...
The phrase "kernel writes to a page cache page" means,
...
The corollary case is just as important, if there are users which have shared+writable mappings of this file, we must make sure that kernel reads of these pages will see the most recent stores done by the user.
Quoted 1 line
Can someone confirm?
It sounds like you are correct - you need to call flush_dcache_page. Presumably this can happen after you read the status flag, unless there's a possibility that you never actually get that update, in which case you should be safe and do it before, although I'm not sure then of how significant the performance impact is. More importantly, I think is the comment in the documentation that the deferred flush must happen on the same CPU as where the stores occurred, so what I come away from this is that I don't think I understand the situation well enough with flush_dcache_page.
Quoted 4 lines
Thanks in advance,
Johann
Thanks,
Vitali