Skip to contentSkip to conversation

Redundant uses of might_sleep_if()

DISCUSSION ioana alexandrescu 2 participants 2 messages Jan 30, 2004

lore
Share & tools
lore
#0 lore raw

In kernel 2.6.1 it appears that the only necessary uses of might_sleep_if()are in __alloc_pages(), and perhaps, in cache_alloc_debugcheck_before() (see notes).

Other uses of might_sleep_if() appear to be redundant:

Pte_chain_alloc()-->might_sleep_if(), but also Pte_chain_alloc-->kmem_cache_alloc
-->__cache_alloc -->__cache_alloc() -->cache_alloc_debugcheck_before() -->might_sleep_if()

skb_share_check()-->might_sleep_if(), but also skb_share_check()-->skb_clone()
-->kmem_cache_alloc()[as above]

skb_unshare()-->might_sleep_if(), but also skb_unshare()-->skb_copy()
-->kmem_cache_alloc()[as above]

Other paths through skb_unshare, same result.

QUERY: Should these redundant uses be patched out?

Note 1: all present uses of might_sleep_if(cond) resolve to the equivalent of might_sleep_if(gfp_mask
& __GFP_WAIT) - which suggests an encapsulating
macro:

#define might_sleep_if_wait(flags) might_sleep_if\
(flags & __GFP_WAIT)

Note 2: preliminary analysis suggests that even
cache_alloc_debugcheck_before()-->might_sleep_if()
is, strictly speaking, unnecessary since the same check in performed in __alloc_pages(). Of course the duplicated check doesn't cost much.

Carl Spalletta
--
See New Jersey and die!

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free web site building tool. Try it!
http://webhosting.yahoo.com/ps/sb/

#1 lore raw
Quoted 18 lines · ioana alexandrescu

In kernel 2.6.1 it appears that the only necessary uses of might_sleep_if()are in __alloc_pages(), and perhaps, in cache_alloc_debugcheck_before() (see notes).

Other uses of might_sleep_if() appear to be redundant:

Pte_chain_alloc()-->might_sleep_if(), but also
Pte_chain_alloc-->kmem_cache_alloc
-->__cache_alloc -->__cache_alloc()
-->cache_alloc_debugcheck_before()
-->might_sleep_if()

skb_share_check()-->might_sleep_if(), but also
skb_share_check()-->skb_clone()
-->kmem_cache_alloc()[as above]

Quoted 8 lines

skb_unshare()-->might_sleep_if(), but also
skb_unshare()-->skb_copy()
-->kmem_cache_alloc()[as above]

Other paths through skb_unshare, same result.

QUERY: Should these redundant uses be patched out?

Nope.

Take the case of pte_chain_alloc(). Most of the time, it won't call kmem_cache_alloc() at all. But sometimes it will. But we want to run the might_sleep() check *every* time someone calls pte_chain_alloc(), not just some of the times.