Re: [PATCH 6/6] ipc/sem.c: make semctl(,,{GETNCNT,GETZCNT}) standard compliant

[Davidlohr Bueso]

On Sat, 2014-05-10 at 12:03 +0200, Manfred Spraul wrote:
> Per definition, a task waits on exactly one semaphore:
> The semaphore from the first operation in the sop array that cannot proceed.
> 
> The Linux implementation never followed the standard, it tried to count all
> semaphores that might be the reason why a task sleeps.
> 
> This patch fixes that.
> 
> Note:
> The implementation assumes that GETNCNT and GETZCNT are rare operations,
> therefore the code counts them only on demand.
> (If they wouldn't be rare, then the non-compliance would have
> been found earlier)
> 
> Signed-off-by: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>
> ---
>  ipc/sem.c | 37 ++++++++++++++++---------------------
>  1 file changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 22a4c12..5e8bcde 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -993,38 +993,33 @@ static void do_smart_update(struct sem_array *sma, struct sembuf *sops, int nsop
>  }
>  
>  /*
> - * check_qop: Test how often a queued operation sleeps on the semaphore semnum
> + * check_qop: Test if a queued operation sleeps on the semaphore semnum
>   */
>  static int check_qop(struct sem_array *sma, int semnum, struct sem_queue *q,
>  			bool count_zero)
>  {
> -	struct sembuf *sops = q->sops;
> -	int nsops = q->nsops;
> -	int i, semcnt;
> +	struct sembuf *sop = q->blocking;
>  
> -	semcnt = 0;
> +	BUG_ON(sop->sem_flg & IPC_NOWAIT);
> +	BUG_ON(sop->sem_op > 0);

Hmm in light of Linus' recent criticism about randomly sprinkling
BUG_ONs in the kernel I'm not sure we want this. Yes, all those calls
are correct from a logical pov and should never occur, however, would
WARN be more suitable instead? I don't know. 

Andrew, any thoughts?

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/