6/6 cacheline align files_lock

I'm getting a lot of cacheline bounce from .text.lock.file_table due to false sharing of the cahceline. The following patch just aligns the lock in it's own cacheline.

only changes in profile under 50 ticks are:

-4832 -22.2% .text.lock.file_table
-6357 -12.8% default_idle
-10374 -6.2% total

Difference in results below (note system times as well as elapsed).

Kernbench: (make -j N vmlinux, where N = 2 x num_cpus)

                              Elapsed      System        User         CPU
                 no-align       44.09       94.38      557.26     1477.00
                    align       44.38       94.18      558.00     1468.25

Kernbench: (make -j N vmlinux, where N = 16 x num_cpus)

                              Elapsed      System        User         CPU
                 no-align       45.53      118.06      560.48     1489.50
                    align       44.84      111.77      560.63     1502.50

Kernbench: (make -j vmlinux, maximal tasks)

                              Elapsed      System        User         CPU
                 no-align       45.17      117.80      560.62     1500.50
                    align       44.94      113.36      560.59     1500.00

diff -urpN -X /home/fletch/.diff.exclude 020-prof_docs/fs/file_table.c 030-align_files_lock/fs/file_table.c
@@ -27,7 +27,7 @@ static LIST_HEAD(anon_list);
 /* And here the free ones sit */
 static LIST_HEAD(free_list);
 /* public *and* exported. Not pretty! */
-spinlock_t files_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t files_lock __cacheline_aligned_in_smp = SPIN_LOCK_UNLOCKED;
 
 /* Find an unused file structure and return a pointer to it.
  * Returns NULL, if there are no more free file structures or

Seriously, I'd love to do that ... files_lock is the highest item of the whole kernel compile profile overall, and accounts for about 10% of all systime.

By I inlining the spinlocks, I can see the heavy users seem to be:

      6278   166.0% get_empty_filp
      5482   239.3% __fput
      4770   254.7% file_move
    -14464  -100.0% .text.lock.file_table

It seems to lock several different things though - free lists, tty lists, etc, etc. I could break it up to one per list, but that means taking two locks to shift between lists, which I'd prefer not to do if possible.

Andrew was talking about something more dramatic for it, but I'm not sure exactly what ... any suggestions on how to drive a stake through it's heart gladly accepted.

Ironically, the latest scheduler changes *seem* to get rid of about 57% of the cost of it .... I have *no* idea why. Diffprofile between 64 and 64-bk is just wierd:

      7222    45.1% page_remove_rmap
       521    18.4% __copy_from_user_ll
       422     8.9% __copy_to_user_ll
       334   105.4% __wake_up
       248    48.5% pte_alloc_one
       243   205.9% pgd_ctor
       211     4.6% vm_enough_memory
       198     6.3% zap_pte_range
       179    29.3% kmem_cache_free
       118    12.2% clear_page_tables
        99    13.2% __pte_chain_free
        84     6.1% do_no_page
        76     6.6% release_pages
        74     9.0% strnlen_user
        67     5.6% free_hot_cold_page
        65    14.4% copy_page_range
...
       -54   -26.1% generic_file_open
       -54    -7.8% dentry_open
       -55   -16.3% do_lookup
       -61    -1.7% find_get_page
       -74   -58.7% __wake_up_sync
       -74    -8.2% current_kernel_time
       -82   -16.1% file_ra_state_init
      -127    -8.0% schedule
      -132   -19.1% fd_install
      -148    -2.2% page_add_rmap
      -246    -3.0% d_lookup
      -263    -9.3% atomic_dec_and_lock
      -407   -25.7% file_move
      -455   -20.5% __fput
      -853   -21.1% get_empty_filp
      -870    -1.9% default_idle
     -1724    -1.0% total
     -8473   -59.8% .text.lock.file_table

page_remove_rmap I'm not too worried about, as I know objrmap will kill that already (tried to combine them last night, but got an oops, which I'm just about to try to recreate with just virgin -bk). But *why* things are shifting about so much from (presumably) just the scheduler changes is a mystery to me at least.

Any ideas?

M.

The free list should go away - we have slab for that. The tty stuff should get a per-tty lock.

I doubt either of these would actually fix the lock contention, though.

The tty stuff is not likely to be a real issue for any real load (just how often do you kill off sessions etc?) And the free list isn't the reason for the file lock - ues, the file lock protects it, but every time we touch the free list we touch _real_ lists too (ie either we move a file from the free list to another list, _or_ we move a unused entry from a real list to the free list), so we'd need the lock anyway.

So to actually fix file_lock, you need to do something else. I _think_ that "something else" may be to make it be a per-super-block lock, since I think that's the only thing the f_list thing is actually used for. Then you should probably pass in the superblock pointer to "get_empty_filp()", and _then_ you can get rid of the free list and the current global lock.

Oh, and you need to make the "tty" stuff be a superblock too. Of course, it might actually be a perfectly fine thing to make that tty stuff use a totally separate pointer chain anyway, the current thing makes me worry that "umount()" actually might do the wrong thing if the only file open on a filesystem are tty files.

Linus

The high count for pgd_ctor() is bizarre. It fiddles with less data and should be called less often than pmd_ctor() or pte_alloc_one(), both of which take the cache hit for constructing full pages, and pgd_ctor() can't really hope to do better. This otherwise appears to be in order of cacheline bounciness. DCU_MISS_OUTSTANDING and IFU_MEM_STALL?

page_remove_rmap() as usual takes top position b/c of its long list walks; I think you've got something brewing for that already.

-- wli

Agreed, that's what I actually though when looking into that stuff in more detail a while ago - I just couldn't remember everything now that Martin brought it up again. Killing the freelist seems like a good idea anyway (or rather keep a small list for the reserved filp that is used only _after_ kmem_cache_alloc() failed)

Well, the sad part, though, is that the file lists are almost never actually _used_, so even if this is made per-superblock that won't get over the fact that

(a) much of the time you'll still get the lock overhead (not very many superblocks would be impacted: it would usually spread out the current load over two or three super-blocks: pipes, networking and "real" filesystem) and

(b) it would _still_ be for something that almost never happens - relatively speaking (ie it's currently used for hanging up terminals and for unmounts, and nothing else, I think)

I think we might actually be able to make it really go away, if the file list was made per-dentry or something like that (and then use the existing dentry->lock instead of dcache_lock). We already keep track of dentries for "umount", and if we made the tty layer use another list, we'd get rid of this whole lock entirely.

At this point, though, the 2.6.x thing is clearly to just avoid the false sharing. But somebody can work on this if they feel like a 2.7.x challenge.

Linus