Re: [PATCH 2/4] xen/events: document behaviour when scanning the start word for events

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 08/15/2013 08:21 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> The original comment on the scanning of the start word on the 2nd pass
> did not reflect the actual behaviour (the code was incorrectly masking
> bit_idx instead of the pending word itself).
>
> The documented behaviour is not actually required since if event were
> pending in the MSBs, they would be immediately scanned anyway as we go
> through the loop again.
>
> Update the documentation to reflect this (instead of trying to change
> the behaviour).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>   drivers/xen/events.c |   17 ++++++++++++-----
>   1 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index a58ac43..f866f50 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -1379,14 +1379,21 @@ static void __xen_evtchn_do_upcall(void)
>   
>   			pending_bits = active_evtchns(cpu, s, word_idx);
>   			bit_idx = 0; /* usually scan entire word from start */
> +			/*
> +			 * We scan the starting word in two parts.
> +			 *
> +			 * 1st time: start in the middle, scanning the
> +			 * MSBs.

I think MSB stands for most significant *byte*. 'msb' is for *bit*. Or 
just say
'upper bits'.

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

> +			 *
> +			 * 2nd time: scan the whole word (not just the
> +			 * parts skipped in the first pass) -- if an
> +			 * event in the previously scanned bits is
> +			 * pending again it would just be scanned on
> +			 * the next loop anyway.
> +			 */
>   			if (word_idx == start_word_idx) {
> -				/* We scan the starting word in two parts */
>   				if (i == 0)
> -					/* 1st time: start in the middle */
>   					bit_idx = start_bit_idx;
> -				else
> -					/* 2nd time: mask bits done already */
> -					bit_idx &= (1UL << start_bit_idx) - 1;
>   			}
>   
>   			do {