lkml.org 
[lkml]   [2010]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] drivers/net/tile/: on-chip network drivers for the tile architecture
On 11/3/2010 1:50 PM, Eric Dumazet wrote:
> Le mercredi 03 novembre 2010 à 13:37 -0400, Chris Metcalf a écrit :
>> Stephen, thanks for your feedback!
>>
>> On 11/3/2010 12:59 PM, Stephen Hemminger wrote:
>>> 1. MUST not use volatile, see volatile-considered-harmful.txt
>> The "harmful" use of volatile is in trying to fake out SMP. Believe me,
>> with a 64-core architecture, we know our SMP guidelines. :-) Our use here
>> is simply to force the compiler to issue a load, for the side-effect of
>> populating the TLB, for example.
>>
>> However, your response does suggest that simply the syntactic use of
>> "volatile" will cause a red flag for readers. I'll move this to an inline
>> function in a header with a comment explaining what it's for, and use the
>> function instead.
> Please read Documentation/volatile-considered-harmful.txt

I read it and internalized it long ago, and re-read it when I got Stephen's
original email. I should have said that explicitly instead of a comment
with a smiley -- email is a tricky communication medium sometimes.

Several uses of "*(volatile int *)ptr" in that file are intended as
performance hints. A more obvious way to state this, for our compiler, is
to say "prefetch_L1(ptr)". This generates essentially the same code, but
avoids the red flag for "volatile" and also reads more clearly, so it's a
good change.

The other use is part of a very precise dance that involves detailed
knowledge of the Tile memory subsystem micro-architecture. This doesn't
really belong in the network device driver code, so I've moved it to
<asm/cacheflush.h>, and cleaned it up, with detailed comments. The use
here is that our network hardware's DMA engine can be used in a mode where
it reads directly from memory, in which case you must ensure that any
cached values have been flushed.

/*
* Flush & invalidate a VA range that is homed remotely on a single core,
* waiting until the memory controller holds the flushed values.
*/
static inline void finv_buffer_remote(void *buffer, size_t size)
{
char *p;
int i;

/*
* Flush and invalidate the buffer out of the local L1/L2
* and request the home cache to flush and invalidate as well.
*/
__finv_buffer(buffer, size);

/*
* Wait for the home cache to acknowledge that it has processed
* all the flush-and-invalidate requests. This does not mean
* that the flushed data has reached the memory controller yet,
* but it does mean the home cache is processing the flushes.
*/
__insn_mf();

/*
* Issue a load to the last cache line, which can't complete
* until all the previously-issued flushes to the same memory
* controller have also completed. If we weren't striping
* memory, that one load would be sufficient, but since we may
* be, we also need to back up to the last load issued to
* another memory controller, which would be the point where
* we crossed an 8KB boundary (the granularity of striping
* across memory controllers). Keep backing up and doing this
* until we are before the beginning of the buffer, or have
* hit all the controllers.
*/
for (i = 0, p = (char *)buffer + size - 1;
i < (1 << CHIP_LOG_NUM_MSHIMS()) && p >= (char *)buffer;
++i) {
const unsigned long STRIPE_WIDTH = 8192;

/* Force a load instruction to issue. */
*(volatile char *)p;

/* Jump to end of previous stripe. */
p -= STRIPE_WIDTH;
p = (char *)((unsigned long)p | (STRIPE_WIDTH - 1));
}

/* Wait for the loads (and thus flushes) to have completed. */
__insn_mf();
}


> Then if there is a problem, we can make change to the documentation, but
> volatile use in new code is _strictly_ forbidden.
>
> ACCESS_ONCE() is your friend, we might document it in
> Documentation/volatile-considered-harmful.txt

Good idea, but neither of the use cases at issue here benefit from ACCESS_ONCE.

Thanks for your feedback!

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2010-11-03 20:41    [W:0.061 / U:0.368 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site