From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755995AbYEQRSw (ORCPT ); Sat, 17 May 2008 13:18:52 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752049AbYEQRSm (ORCPT ); Sat, 17 May 2008 13:18:42 -0400 Received: from smtp1.linux-foundation.org ([140.211.169.13]:51348 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751780AbYEQRSl (ORCPT ); Sat, 17 May 2008 13:18:41 -0400 Date: Sat, 17 May 2008 10:11:37 -0700 (PDT) From: Linus Torvalds To: Oleg Nesterov cc: Andrew Morton , Austin Clements , Ingo Molnar , john stultz , Michael Kerrisk , Roland McGrath , Thomas Gleixner , linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] posix timers: use SIGQUEUE_CANCELLED when the timer is destroyed In-Reply-To: <20080517153140.GA9534@tv-sign.ru> Message-ID: References: <20080517151422.GA9502@tv-sign.ru> <20080517153140.GA9534@tv-sign.ru> User-Agent: Alpine 1.10 (LFD 962 2008-03-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 17 May 2008, Oleg Nesterov wrote: > On 05/17, Oleg Nesterov wrote: > > > > This is a user visible change. With this patch sys_timer_delete() discards > > the pending signal which was generated by the timer. > > If this change is undesirable, we can (for example) do > > --- kernel/posix-timers.c > +++ kernel/posix-timers.c > @@ -885,6 +885,7 @@ itimer_delete(struct k_itimer *timer) > timer->it_process = NULL; > > unlock_timer(timer, flags); > + tmr->sigq->flags |= SIGQUEUE_CANCELLED; > release_posix_timer(timer, IT_ID_SET); > } > > instead, and still fix the "BUG 10460". The only reason I like that better is that it makes me nervous when one re-initializes the whole flags field. So your original 3/3 patch - q->flags &= ~SIGQUEUE_PREALLOC; + q->flags = SIGQUEUE_CANCELLED; /* clears SIGQUEUE_PREALLOC */ just makes me go "Hmm, what about all the other flag bits?" Now, admittedly, there are currently (with your patch) just two SIGQUEUE_xyz bits, so by just doing that single assignment, you really only modify the two bits you want to modify. But maybe that will change. So I'd prefer to either write it as q->flags &= ~SIGQUEUE_PREALLOC; q->flags |= SIGQUEUE_CANCELLED; or to use bitfields, or to do something else to make it safe in the presense of multiple bits. Your alternate patch obviously doesn't have that issue, since it just sets the single bit. But apart from that issue, I have absolutely no preferences either way. You're effectively the maintainer in this area, you get to choose. Linus