From: Ingo Oeser <[EMAIL PROTECTED]>
Date: Fri, 21 Apr 2006 18:52:47 +0200
> nice to see you getting started with it.
Thanks for reviewing.
> I'm not sure about the queue logic there.
>
> 1867 /* Caller must have exclusive producer access to the netchannel. */
> 1868 int netchannel_enqueue(struct netchannel *np, struct
> netchannel_buftrailer *bp)
> 1869 {
> 1870 unsigned long tail;
> 1871
> 1872 tail = np->netchan_tail;
> 1873 if (tail == np->netchan_head)
> 1874 return -ENOMEM;
>
> This looks wrong, since empty and full are the same condition in your
> case.
Thanks, that's obviously wrong. I'll try to fix this up.
> What about sth. like
>
> struct netchannel {
> /* This is only read/written by the writer (producer) */
> unsigned long write_ptr;
> struct netchannel_buftrailer *netchan_queue[NET_CHANNEL_ENTRIES];
>
> /* This is modified by both */
> atomic_t filled_entries; /* cache_line_align this? */
>
> /* This is only read/written by the reader (consumer) */
> unsigned long read_ptr;
> }
As stated elsewhere, if you add atomic operations you break the entire
idea of net channels. They are meant to be SMP efficient data structures
where the producer has one cache line that only it dirties and the
consumer has one cache line that likewise only it dirties.
> If cacheline bouncing because of the shared filled_entries becomes an issue,
> you are receiving or sending a lot.
Cacheline bouncing is the core issue being addressed by this
data structure, so we really can't consider your idea seriously.
I've just got an off-by-one error, no need to wreck the entire
data structure just to solve that :-)
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
