Redundant uses of might_sleep_if()

Redundant uses of might_sleep_if()

[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]

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/

Re: Redundant uses of might_sleep_if()

[Andrew Morton]

ioana alexandrescu <ioanamitu@yahoo.com> wrote:
>
> 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?

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.