Re: [PATCH] Shrinks sizeof(files_struct) and better layout

[Andi Kleen <ak@suse.de>]

Eric Dumazet <dada1@cosmosbay.com> writes:
> 
> 1) Reduces the size of (struct fdtable) to exactly 64 bytes on 32bits
> platforms, lowering kmalloc() allocated space by 50%.

It should be probably a kmem_cache_alloc() instead of a kmalloc
in the first place anyways. This would reduce fragmentation.

> 2) Reduces the size of (files_struct), using a special 32 bits (or
> 64bits) embedded_fd_set, instead of a 1024 bits fd_set for the
> close_on_exec_init and open_fds_init fields. This save some ram (248
> bytes per task) as most tasks dont open more than 32 files. D-Cache
> footprint for such tasks is also reduced to the minimum.
> 
> 3) Reduces size of allocated fdset. Currently two full pages are
> allocated, that is 32768 bits on x86 for example, and way too
> much. The minimum is now L1_CACHE_BYTES.
> 
> UP and SMP should benefit from this patch, because most tasks will
> touch only one cache line when open()/close() stdin/stdout/stderr
> (0/1/2), (next_fd, close_on_exec_init, open_fds_init, fd_array[0 .. 2]
> being in the same cache line)

Looks mostly good to me.

> +   * read mostly part
> +   */
>  	atomic_t count;
>  	struct fdtable *fdt;
>  	struct fdtable fdtab;
> -	fd_set close_on_exec_init;
> -	fd_set open_fds_init;
> +  /*
> +   * written part on a separate cache line in SMP
> +   */
> +	spinlock_t file_lock ____cacheline_aligned_in_smp;
> +	int next_fd;
> +	embedded_fd_set close_on_exec_init;
> +	embedded_fd_set open_fds_init;

You didn't describe that change, but unless it's clear the separate cache lines
are a win I would not do it and save memory again. Was this split based on
actual measurements or more theoretical considerations? 

-Andi