Date: Thu, 15 Sep 2011 17:28:13 -0400 From: Arnaud Lacombe <lacombar@gmail.com> To: Robert Watson <rwatson@freebsd.org> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org>, "K. Macy" <kmacy@freebsd.org> Subject: Re: buf_ring(9) API precisions Message-ID: <CACqU3MV_wZN3PQ1%2B-gNHPeEy4MZmp5YGks=HTAbK_7v7vKHyJg@mail.gmail.com> In-Reply-To: <alpine.BSF.2.00.1109152219380.14815@fledge.watson.org> References: <CACqU3MXQ6tD804fKymeFeKDnHndSXVvHJwepYztB4DsnNmtMiw@mail.gmail.com> <CAHM0Q_P7NRQXay-ho1E--P4QnV5kr0eTo48NT21dbJjpbmAF=Q@mail.gmail.com> <alpine.BSF.2.00.1109152219380.14815@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
Hi,
On Thu, Sep 15, 2011 at 5:21 PM, Robert Watson <rwatson@freebsd.org> wrote:
> On Thu, 15 Sep 2011, K. Macy wrote:
>
>>> Why are you making an MD guess, the amount of padding to fit the size of
>>> a cache line, in MI API ? Strangely enough, you did not make this assumption
>>> in, say r205488 (picked randomly).
>>
>> It has been several years, and I haven't done any work in svn in over a
>> year, I don't remember. I probably meant to refine it in a later iteration.
>>
>> If you would like to send me a patch addressing this I'd be more than
>> happy to apply it if appropriate. Otherwise, I will deal with it some time
>> after 9 settles.
>>
>> Thanks for pointing this out.
>
> I'm not sure if gcc (and friends) allow __aligned(CACHE_LINE_SIZE) to be
> used on individual elements of a struct (causing appropriate padding to be
> added), but that may be one option here.
>
It definitively does, it is used in several structure in the tree.
Attached a patch to convert buf_ring(9), however, I'm not a huge fan
of dedicating a full cache line to the lock pointer.
- Arnaud
> Of course, that introduces a
> further alignment requirement on the struct itself, so a moderate amount of
> care would need to be used.
>
> Robert
>
[-- Attachment #2 --]
diff --git a/sys/sys/buf_ring.h b/sys/sys/buf_ring.h
index 57e42e5..01f399e 100644
--- a/sys/sys/buf_ring.h
+++ b/sys/sys/buf_ring.h
@@ -49,25 +49,17 @@ struct buf_ring {
uint64_t br_drops;
uint64_t br_prod_bufs;
uint64_t br_prod_bytes;
- /*
- * Pad out to next L2 cache line
- */
- uint64_t _pad0[11];
- volatile uint32_t br_cons_head;
+ volatile uint32_t br_cons_head __aligned(CACHE_LINE_SIZE);
volatile uint32_t br_cons_tail;
int br_cons_size;
int br_cons_mask;
- /*
- * Pad out to next L2 cache line
- */
- uint64_t _pad1[14];
#ifdef DEBUG_BUFRING
- struct mtx *br_lock;
+ struct mtx *br_lock __aligned(CACHE_LINE_SIZE);
#endif
- void *br_ring[0];
-};
+ void *br_ring[0] __aligned(CACHE_LINE_SIZE);
+} __aligned(CACHE_LINE_SIZE);
/*
* multi-producer safe lock-free ring buffer enqueue
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACqU3MV_wZN3PQ1%2B-gNHPeEy4MZmp5YGks=HTAbK_7v7vKHyJg>