Re: [patch 1/8] fdmap v2 - fdmap core

[Eric Dumazet <dada1@cosmosbay.com>]

Davide Libenzi a écrit :
> +struct fd_map {
> +	struct fd_map *next;
> +	struct rcu_head rcu;
> +	unsigned int base;
> +	unsigned int size;
> +	struct list_head slist;
> +	struct list_head *slots;
> +	unsigned int fdnext;
> +	unsigned long *map;
> +	void (*freecb)(void *, struct fd_map *);
> +	void *freecb_priv;
> +};


On x86_64 that would mean folowing offsets

struct fd_map {
	struct fd_map *next;  /* 0 */
	struct rcu_head rcu; /* 8 */
	unsigned int base;   /* 0x18 */
	unsigned int size;   /* 0x1c */
	struct list_head slist; /* 0x20 */
	struct list_head *slots; /* 0x30 */
	unsigned int fdnext; /* 0x38 */
	unsigned long *map; /* 0x40 */
	void (*freecb)(void *, struct fd_map *); /* 0x48 */
	void *freecb_priv; /* 0x50 */
}; /* size 0x58 */

As L1_CACHE_BYTES is 64, two cache lines are necessary to get base,size,map
And a change of fdnext dirties one cache line that should be kept read only to 
avoid false sharing.

I suggest to reorder to move rcu and next at the end (since they are seldom 
used). Also move fdnext on a separate cache line on SMP

struct fd_map {
	/*
	 * read mostly part
	 */
	unsigned int base;   /* 0x00 */
	unsigned int size;   /* 0x04 */
	struct list_head slist; /* 0x08 */
	struct list_head *slots; /* 0x18 */
	unsigned long *map; /* 0x28 */
	void (*freecb)(void *, struct fd_map *); /* 0x30 */
	void *freecb_priv; /* 0x38 */

	/*
	 * written part on a separate cache line in SMP
	 */
	unsigned int fdnext ____cacheline_aligned_in_smp; /* 0x40 */

	struct fd_map *next;  /* 0x48 */
	struct rcu_head rcu; /* 0x50 */
};

Thank you