![]() | |||||||||||
Messages in this thread |
David S. Miller a écrit :
> From: Andi Kleen <ak@suse.de>
> Date: Thu, 18 Aug 2005 03:05:25 +0200
>
>
>>I would just set the ra pointer to a single global structure if the
>>allocation fails. Then you can avoid all the other checks. It will
>>slow down things and trash some state, but not fail and nobody
>>should expect good performance after out of memory anyways. The only
>>check still needed would be on freeing.
>
>
> I would think twice about that due to repeatability concerns. Yes, we
> should care less when memory is so low, but if we can avoid this kind
> of scenerio easily we should.
>
> Having said that, I would like to recommend looking into a scheme
> where the path leading to the filp allocation states whether the
> read-ahead bits are needed or not. This has two benefits:
>
> 1) Repeatability, and error signalling at the correct place
> should the memory allocation fail.
>
> 2) We can avoid the pointer dereference overhead. The read-ahead
> state is always at (filp + 1). Macro'ized or static inline
> function'ized interfaces for this access can make it look
> clean and perhaps even implement debugging of the case where
> we try to get at the read-ahead state for a non-read-ahead
> filp.
>
> I do really think that would be a better approach. A quick glance
> shows that it should be easy to propagate the "need_read_ahead"
> state, just by passing a boolean to get_unused_fd() via
> sock_map_fd().
Thanks David and Andi
Hum... get_unused_fd() allocates a file descriptor, not a 'struct file *'
Maybe you meant get_empty_filp(void), that we might change to get_empty_filp(int need_read_ahead)
After reading your suggestions, I understand we still need two slabs.
One (filp_cachep) without the readahead data, the other one (filp_ra_cachep) with it.
static inline struct file_ra_state *get_ra_state(struct file *f)
{
#ifdef CONFIG_DEBUG_READAHEAD
BUG_ON(filp_ra_cachep != GET_PAGE_CACHE(virt_to_page(f)));
#endif
return (struct file_ra_state *)(f+1);
}
struct file *get_empty_filp(int need_read_ahead)
{
struct file *f;
...
f = kmem_cache_alloc(need_read_ahead ? filp_ra_cachep : filp_cachep, GFP_KERNEL);
if (f == NULL)
goto fail;
memset(f, 0, need_read_ahead ? sizeof(struct file) : sizeof(struct file) + sizeof(struct file_ra_state));
...
}
file_free() then call kfree() that should find the right kmem_cache_t *
static inline void file_free(struct file *f)
{
kfree(f);
}
I will provide a new version of the patch in a later mail
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/
| ||||||||||
| Last update: 2005-08-18 09:17 [from the cache] ©2003-2008 | |||||||||||