Re: [PATCH] reorder struct files_struct

[Eric Dumazet <dada1@cosmosbay.com>]

Dipankar Sarma a écrit :
> On Thu, Sep 15, 2005 at 01:19:47AM +0200, Eric Dumazet wrote:
> 
>>Dipankar Sarma a écrit :
>>
>>>On Thu, Sep 15, 2005 at 12:42:03AM +0200, Eric Dumazet wrote:
>>>
>>>
>>>>Dipankar Sarma a écrit :
>>
>>>>If yes, the whole embedded struct fdtable is readonly.
>>>
>>>
>>>But not close_on_exec_init or open_fds_init. We would update them
>>>on open/close.
>>
>>Yes, sure, but those fields are not part of the embedded struct fdtable
> 
> 
> Those fdsets would share a cache line with fdt, fdtable which would
> be invalidated on open/close. So, what is the point in moving
> file_lock ?
> 

The point is that we gain nothing in this case for 32 bits platforms, but we 
gain something on 64 bits platform. And for apps using more than 
NR_OPEN_DEFAULT files we definitly win on both 32bits/64bits.

Maybe moving file_lock just before close_on_exec_init would be a better 
choice, so that 32bits platform small apps touch one cache line instead of two.

sizeof(struct fdtable) = 40/0x28 on 32bits, 72/0x48 on 64 bits

struct files_struct {

/* mostly read */
	atomic_t count; /* offset 0x00 */
	struct fdtable *fdt; /* offset 0x04 (0x08 on 64 bits) */
	struct fdtable fdtab; /* offset 0x08 (0x10 on 64 bits)*/

/* read/written for apps using small number of files */
	fd_set close_on_exec_init; /* offset 0x30 (0x58 on 64 bits) */
	fd_set open_fds_init; /* offset 0x34 (0x60 on 64 bits) */
	struct file * fd_array[NR_OPEN_DEFAULT]; /* offset 0x38 (0x68 on 64 bits */
	spinlock_t file_lock; /* 0xB8 (0x268 on 64 bits) */
	}; /* size = 0xBC (0x270 on 64 bits) */

Moving next_fd from 'struct fdtable' to 'struct files_struct' is also a win 
for 64bits platforms since sizeof(struct fdtable) become 64 : a nice power of 
two, so 64 bytes are allocated instead of 128.

Eric