[openib-general] Re: Re: [Andrew Morton] inappropriate use of in_atomic()

[Libor Michalek]

On Sun, Mar 27, 2005 at 05:58:47PM +0200, Michael S. Tsirkin wrote:
> 
> My understanding is you must give kmap_atomic the proper parameter:
> KM_IRQ0/KM_SOFTIRQ0/KM_USR0, to avoid conflicts with other callers of
> kmap on the same CPU.
> 
> Something like this then?

  If you're going to check for in_irq() why not just use kmap() when
you are not in an interrupt? I think the benefit of using kmap_atomic
all the time is that you don't need to check if you are in an
interrupt, you just need to make sure the local interrupts are
disabled in case you are not in an interrupt. Once they are disabled
I think you can use KM_IRQ0 for all cases. With interrupts disabled
you should never get a collision on the KM_IRQ0 page as long as the
map/unmap occur before the interrupt is enabled, which it would in
the SDP case.

-Libor


> Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>
> 
> Index: ulp/sdp/sdp_iocb.h
> ===================================================================
> --- ulp/sdp/sdp_iocb.h	(revision 2050)
> +++ ulp/sdp/sdp_iocb.h	(working copy)
> @@ -133,10 +133,12 @@
>   */
>  static inline void *sdp_kmap(struct page *page)
>  {
> -	if (in_atomic() || irqs_disabled())
> +	if (in_irq())
>  		return kmap_atomic(page, KM_IRQ0);
> +	else if (in_softirq())
> +		return kmap_atomic(page, KM_SOFTIRQ0);
>  	else
> -		return kmap(page);
> +		return kmap_atomic(page, KM_USR0);
>  }
>  
>  /*
> @@ -144,10 +146,12 @@
>   */
>  static inline void sdp_kunmap(struct page *page)
>  {
> -	if (in_atomic() || irqs_disabled())
> +	if (in_irq())
>  		kunmap_atomic(page, KM_IRQ0);
> +	else if (in_softirq())
> +		kunmap_atomic(page, KM_SOFTIRQ0);
>  	else
> -		kunmap(page);
> +		kunmap_atomic(page, KM_USR0);
>  }
>  
>  #endif /* _SDP_IOCB_H */
> -- 
> MST - Michael S. Tsirkin