Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   David Laight <David.Laight@ACULAB.COM>
To:     'Oleg Nesterov' <oleg@redhat.com>
CC:     'Linus Torvalds' <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Deepa Dinamani <deepa.kernel@gmail.com>,
        "Linux List Kernel Mailing" <linux-kernel@vger.kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Davidlohr Bueso <dbueso@suse.de>, Jens Axboe <axboe@kernel.dk>,
        Davidlohr Bueso <dave@stgolabs.net>,
        "e@80x24.org" <e@80x24.org>, Jason Baron <jbaron@akamai.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        "linux-aio@kvack.org" <linux-aio@kvack.org>,
        "omar.kilani@gmail.com" <omar.kilani@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        stable <stable@vger.kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        "Eric W. Biederman" <ebiederm@xmission.com>
Subject: RE: [PATCH] signal: remove the wrong signal_pending() check in
 restore_user_sigmask()
Thread-Topic: [PATCH] signal: remove the wrong signal_pending() check in
 restore_user_sigmask()
Thread-Index: AQHVGxwzwFf0q/qAAkiR7PRGfFAGAqaMwPEw///5P4CAABTbsA==
Date:   Wed, 5 Jun 2019 09:58:25 +0000
Message-ID: <29dd2937475b4407b617e2516f9cdd05@AcuMS.aculab.com>
References: <20190522032144.10995-1-deepa.kernel@gmail.com>
 <20190529161157.GA27659@redhat.com> <20190604134117.GA29963@redhat.com>
 <CAHk-=wjSOh5zmApq2qsNjmY-GMn4CWe9YwdcKPjT+nVoGiDKOQ@mail.gmail.com>
 <263d0e478ee447d9aa10baab0d8673a5@AcuMS.aculab.com>
 <20190605092516.GC32406@redhat.com>
In-Reply-To: <20190605092516.GC32406@redhat.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

From: Oleg Nesterov [mailto:oleg@redhat.com]
> Sent: 05 June 2019 10:25
> On 06/05, David Laight wrote:
> >
> > epoll() would have:
> > 	if (restore_user_sigmask(xxx.sigmask, &sigsaved, !ret || ret == -EINTR))
> > 		ret = -EINTR;
> 
> I don't think so but lets discuss this later.

I certainly think there should be some comments at least
about when/whether signal handlers get called and that
being separate from the return value.

The system call restart stuff does seem strange.
ISTR that was originally added for SIG_SUSPEND (^Z) so that those
signals wouldn't be seen by the appication.
But that makes it a property of the signal, not the system call.

> > I also think it could be simplified if code that loaded the 'user sigmask'
> > saved the old one in 'current->saved_sigmask' (and saved that it had done it).
> > You'd not need 'sigsaved' nor pass the user sigmask address into
> > the restore function.
> 
> Heh. apparently you do not read my emails ;)
> 
> This is what I proposed in my very 1st email, and I even showed the patch
> and the code with the patch applied twice. Let me do this again.

I did read that one, I've even quoted it in the past :-)
It's just not been mentioned recently.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)