Re: [PATCH] reorder struct files_struct

From: Eric Dumazet
Date: Thu Sep 15 2005 - 01:18:14 EST


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