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