Please revert to static zero initialization of a const: when thus
initialized it's linked into a readonly cacheline shared between cpus;
otherwise it's linked into bss, likely to be in a dirty cacheline
bouncing between cpus.
--- 2.6.0-test2-bk/mm/shmem.c Tue Aug 5 15:57:31 2003
+++ linux/mm/shmem.c Tue Aug 5 16:16:55 2003
@@ -296,7 +296,7 @@
struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
struct page *page = NULL;
swp_entry_t *entry;
- static const swp_entry_t unswapped;
+ static const swp_entry_t unswapped = {0};
if (sgp != SGP_WRITE &&
((loff_t) index << PAGE_CACHE_SHIFT) >= i_size_read(inode))
On Tue, 2003-08-05 at 17:48, Hugh Dickins wrote:
> Please revert to static zero initialization of a const: when thus
> initialized it's linked into a readonly cacheline shared between cpus;
> otherwise it's linked into bss, likely to be in a dirty cacheline
> bouncing between cpus.
how about adding a big fat comment here that says this?
Otherwise this keeps happening all the time...
On Tue, 5 Aug 2003 16:48:02 +0100 (BST) Hugh Dickins <[email protected]> wrote:
| Please revert to static zero initialization of a const: when thus
| initialized it's linked into a readonly cacheline shared between cpus;
| otherwise it's linked into bss, likely to be in a dirty cacheline
| bouncing between cpus.
|
| --- 2.6.0-test2-bk/mm/shmem.c Tue Aug 5 15:57:31 2003
| +++ linux/mm/shmem.c Tue Aug 5 16:16:55 2003
| @@ -296,7 +296,7 @@
| struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
| struct page *page = NULL;
| swp_entry_t *entry;
| - static const swp_entry_t unswapped;
| + static const swp_entry_t unswapped = {0};
In all of the "don't init statics to 0" patches, should we
check for "const" also and leave those with 0 initializers
(with explanation as Arjan requested)?
--
~Randy
Hugh Dickins wrote:
>Please revert to static zero initialization of a const: when thus
>initialized it's linked into a readonly cacheline shared between cpus;
>otherwise it's linked into bss, likely to be in a dirty cacheline
>bouncing between cpus.
>
>--- 2.6.0-test2-bk/mm/shmem.c Tue Aug 5 15:57:31 2003
>+++ linux/mm/shmem.c Tue Aug 5 16:16:55 2003
>@@ -296,7 +296,7 @@
> struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
> struct page *page = NULL;
> swp_entry_t *entry;
>- static const swp_entry_t unswapped;
>+ static const swp_entry_t unswapped = {0};
>
> if (sgp != SGP_WRITE &&
> ((loff_t) index << PAGE_CACHE_SHIFT) >= i_size_read(inode))
>
>
If this static zero initialization makes it into the kernel distro for
the given reason, please also add a comment sharing your above mentioned
reasoning.
On Tue, Aug 05, 2003 at 04:48:02PM +0100, Hugh Dickins wrote:
> Please revert to static zero initialization of a const: when thus
> initialized it's linked into a readonly cacheline shared between cpus;
> otherwise it's linked into bss, likely to be in a dirty cacheline
> bouncing between cpus.
>
> --- 2.6.0-test2-bk/mm/shmem.c Tue Aug 5 15:57:31 2003
> +++ linux/mm/shmem.c Tue Aug 5 16:16:55 2003
> @@ -296,7 +296,7 @@
> struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
> struct page *page = NULL;
> swp_entry_t *entry;
> - static const swp_entry_t unswapped;
> + static const swp_entry_t unswapped = {0};
>
> if (sgp != SGP_WRITE &&
> ((loff_t) index << PAGE_CACHE_SHIFT) >= i_size_read(inode))
>
If it's const, it shouldn't be linked into anything at all... right?
Jeff
Please revert to static zero initialization of a const: when thus
initialized it's linked into a readonly cacheline shared between cpus;
otherwise it's linked into bss, likely to be in a dirty cacheline
bouncing between cpus.
On 5 Aug 2003, Arjan van de Ven wrote:
> how about adding a big fat comment here that says this?
> Otherwise this keeps happening all the time...
and others concurred.
Okay, by popular request, here it is with a small thin comment.
--- 2.6.0-test2-bk/mm/shmem.c Tue Aug 5 15:57:31 2003
+++ linux/mm/shmem.c Tue Aug 5 19:29:16 2003
@@ -296,7 +296,8 @@
struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
struct page *page = NULL;
swp_entry_t *entry;
- static const swp_entry_t unswapped;
+ static const swp_entry_t unswapped = {0};
+ /* = {0} to go in readonly data, to avoid bss cacheline bounce */
if (sgp != SGP_WRITE &&
((loff_t) index << PAGE_CACHE_SHIFT) >= i_size_read(inode))
On Tue, 5 Aug 2003, Randy.Dunlap wrote:
>
> In all of the "don't init statics to 0" patches, should we
> check for "const" also and leave those with 0 initializers
> (with explanation as Arjan requested)?
It's certainly something to consider. This was my "= {0}" so
I know the reasoning, but I've not looked at and won't speak for others.
Hugh
On Tue, 5 Aug 2003, Jeff Garzik wrote:
>
> If it's const, it shouldn't be linked into anything at all... right?
Sorry, Jeff, I don't get your point.
Hugh
On Tue, 5 Aug 2003, Jeff Garzik wrote:
>> If it's const, it shouldn't be linked into anything at all... right?
On Tue, Aug 05, 2003 at 07:51:41PM +0100, Hugh Dickins wrote:
> Sorry, Jeff, I don't get your point.
I suspect this assumes const values will get constant folded, which I'm
not sure is the case, though it certainly sounds legal and worthwhile
for a compiler to do when reasonable (i.e. for small structures and/or
extractions of small fields of const structures).
-- wli
William Lee Irwin III wrote:
> On Tue, 5 Aug 2003, Jeff Garzik wrote:
>
>>>If it's const, it shouldn't be linked into anything at all... right?
>
>
> On Tue, Aug 05, 2003 at 07:51:41PM +0100, Hugh Dickins wrote:
>
>>Sorry, Jeff, I don't get your point.
>
>
> I suspect this assumes const values will get constant folded, which I'm
> not sure is the case, though it certainly sounds legal and worthwhile
> for a compiler to do when reasonable (i.e. for small structures and/or
> extractions of small fields of const structures).
Correct. In fact, some Linux kernel code _assumes_ the compiler will
fold constants...
Jeff