From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) Subject: Simplfying copy_siginfo_to_user Date: Sat, 22 Jul 2017 15:25:55 -0500 Message-ID: <8760ek5ics.fsf_-_@xmission.com> References: <87o9shg7t7.fsf_-_@xmission.com> <20170718140651.15973-7-ebiederm@xmission.com> <878tjlbqpt.fsf@xmission.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <878tjlbqpt.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> (Eric W. Biederman's message of "Tue, 18 Jul 2017 12:27:26 -0500") List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Linus Torvalds Cc: "linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Andrei Vagin , Greg KH , Linux Containers , Pavel Emelyanov , Oleg Nesterov , Linux Kernel Mailing List , Al Viro , Andy Lutomirski , Linux API , Cyrill Gorcunov , Michael Kerrisk , Thomas Gleixner , Willy Tarreau , Andrey Vagin List-Id: containers.vger.kernel.org ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) writes: > Linus Torvalds writes: > >> On Tue, Jul 18, 2017 at 7:06 AM, Eric W. Biederman >> wrote: >>> struct siginfo is a union and the kernel since 2.4 has been hiding a union >>> tag in the high 16bits of si_code using the values: >>> __SI_KILL >>> __SI_TIMER >>> __SI_POLL >>> __SI_FAULT >>> __SI_CHLD >>> __SI_RT >>> __SI_MESGQ >>> __SI_SYS >>> >>> While this looks plausible on the surface, in practice this situation has >>> not worked well. >> >> So on the whole I think we just need to do this, but the part I really >> hate about this series is still this the siginfo_layout() part. >> >> I can well believe that it is needed for the compat case. siginfo is a >> piece of crap crazy type, and re-ordering fields for compat is >> something we are always going to have to do. >> >> But for the native case, the *only* reason we do not just copy the >> siginfo as-is seems to be that it's just too big, due to other bad >> design decisions in siginfo ("let's make sure it's big enough by >> allocating 512 bytes for it). >> >> And afaik, absolutely nobody uses more than about 36 bytes of that >> 512-byte _sifields union (and that one use is SIGILL with three >> pointers and three integers and some padding. >> >> So why don't we just say "screw this idiotic layout crap, and just >> unconditionally copy that much smaller maximum of bytes"? >> >> Leave that layout thing purely for compat handling. > > I completely agree. So I just did some measurements to see what the performance impact is of doing the simple and obvious thing of always copying the entire siginfo around. There is a fair amount of variation in my timings but for the whole change I see about a 20ns increase in time taken to send a signal with siginfo from the current process to the current process. AKA timing kill(getpid(),...). I played with some clever changes such as limiting the copy to 48 bytes, disabling the memset and the like but I could not get a strong enough signal to say that any one change removed the extra or a clear part of it 20ns. Do we care about those 20ns for signal deliver? I suspect from my previous numbers that if Andy can get signal delivery to use sysret it will more than make up for the small increase in cost here. Eric From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out01.mta.xmission.com ([166.70.13.231]:42446 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750989AbdGVUeK (ORCPT ); Sat, 22 Jul 2017 16:34:10 -0400 From: ebiederm@xmission.com (Eric W. Biederman) References: <87o9shg7t7.fsf_-_@xmission.com> <20170718140651.15973-7-ebiederm@xmission.com> <878tjlbqpt.fsf@xmission.com> Date: Sat, 22 Jul 2017 15:25:55 -0500 In-Reply-To: <878tjlbqpt.fsf@xmission.com> (Eric W. Biederman's message of "Tue, 18 Jul 2017 12:27:26 -0500") Message-ID: <8760ek5ics.fsf_-_@xmission.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Simplfying copy_siginfo_to_user Sender: linux-arch-owner@vger.kernel.org List-ID: To: Linus Torvalds Cc: Linux Kernel Mailing List , Andy Lutomirski , Al Viro , Oleg Nesterov , Andrei Vagin , Thomas Gleixner , Greg KH , Andrey Vagin , Serge Hallyn , Pavel Emelyanov , Cyrill Gorcunov , Peter Zijlstra , Willy Tarreau , "linux-arch@vger.kernel.org" , Linux API , Linux Containers , Michael Kerrisk Message-ID: <20170722202555.HlLY0pfZenNiF1bWSNfXjReqLPna6YkhGUd5XD_SvUw@z> ebiederm@xmission.com (Eric W. Biederman) writes: > Linus Torvalds writes: > >> On Tue, Jul 18, 2017 at 7:06 AM, Eric W. Biederman >> wrote: >>> struct siginfo is a union and the kernel since 2.4 has been hiding a union >>> tag in the high 16bits of si_code using the values: >>> __SI_KILL >>> __SI_TIMER >>> __SI_POLL >>> __SI_FAULT >>> __SI_CHLD >>> __SI_RT >>> __SI_MESGQ >>> __SI_SYS >>> >>> While this looks plausible on the surface, in practice this situation has >>> not worked well. >> >> So on the whole I think we just need to do this, but the part I really >> hate about this series is still this the siginfo_layout() part. >> >> I can well believe that it is needed for the compat case. siginfo is a >> piece of crap crazy type, and re-ordering fields for compat is >> something we are always going to have to do. >> >> But for the native case, the *only* reason we do not just copy the >> siginfo as-is seems to be that it's just too big, due to other bad >> design decisions in siginfo ("let's make sure it's big enough by >> allocating 512 bytes for it). >> >> And afaik, absolutely nobody uses more than about 36 bytes of that >> 512-byte _sifields union (and that one use is SIGILL with three >> pointers and three integers and some padding. >> >> So why don't we just say "screw this idiotic layout crap, and just >> unconditionally copy that much smaller maximum of bytes"? >> >> Leave that layout thing purely for compat handling. > > I completely agree. So I just did some measurements to see what the performance impact is of doing the simple and obvious thing of always copying the entire siginfo around. There is a fair amount of variation in my timings but for the whole change I see about a 20ns increase in time taken to send a signal with siginfo from the current process to the current process. AKA timing kill(getpid(),...). I played with some clever changes such as limiting the copy to 48 bytes, disabling the memset and the like but I could not get a strong enough signal to say that any one change removed the extra or a clear part of it 20ns. Do we care about those 20ns for signal deliver? I suspect from my previous numbers that if Andy can get signal delivery to use sysret it will more than make up for the small increase in cost here. Eric From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753602AbdGVUe2 (ORCPT ); Sat, 22 Jul 2017 16:34:28 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:42446 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750989AbdGVUeK (ORCPT ); Sat, 22 Jul 2017 16:34:10 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Linus Torvalds Cc: Linux Kernel Mailing List , Andy Lutomirski , Al Viro , Oleg Nesterov , Andrei Vagin , Thomas Gleixner , Greg KH , Andrey Vagin , Serge Hallyn , Pavel Emelyanov , Cyrill Gorcunov , Peter Zijlstra , Willy Tarreau , "linux-arch\@vger.kernel.org" , Linux API , Linux Containers , Michael Kerrisk References: <87o9shg7t7.fsf_-_@xmission.com> <20170718140651.15973-7-ebiederm@xmission.com> <878tjlbqpt.fsf@xmission.com> Date: Sat, 22 Jul 2017 15:25:55 -0500 In-Reply-To: <878tjlbqpt.fsf@xmission.com> (Eric W. Biederman's message of "Tue, 18 Jul 2017 12:27:26 -0500") Message-ID: <8760ek5ics.fsf_-_@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1dZ165-0005wJ-Dm;;;mid=<8760ek5ics.fsf_-_@xmission.com>;;;hst=in01.mta.xmission.com;;;ip=67.3.213.87;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/9eMySxvRbiabHhYRzKOmA9ARa1Bnr6Gs= X-SA-Exim-Connect-IP: 67.3.213.87 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.5 XMGappySubj_01 Very gappy subject * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa05 1397; Body=1 Fuz1=1 Fuz2=1] * 1.0 T_XMDrugObfuBody_08 obfuscated drug references X-Spam-DCC: XMission; sa05 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: *;Linus Torvalds X-Spam-Relay-Country: X-Spam-Timing: total 5693 ms - load_scoreonly_sql: 0.07 (0.0%), signal_user_changed: 4.1 (0.1%), b_tie_ro: 2.8 (0.0%), parse: 1.45 (0.0%), extract_message_metadata: 23 (0.4%), get_uri_detail_list: 3.2 (0.1%), tests_pri_-1000: 11 (0.2%), tests_pri_-950: 1.65 (0.0%), tests_pri_-900: 1.41 (0.0%), tests_pri_-400: 30 (0.5%), check_bayes: 28 (0.5%), b_tokenize: 12 (0.2%), b_tok_get_all: 9 (0.2%), b_comp_prob: 3.1 (0.1%), b_tok_touch_all: 2.9 (0.1%), b_finish: 0.73 (0.0%), tests_pri_0: 580 (10.2%), check_dkim_signature: 0.61 (0.0%), check_dkim_adsp: 3.5 (0.1%), tests_pri_500: 5035 (88.5%), poll_dns_idle: 5028 (88.3%), rewrite_mail: 0.00 (0.0%) Subject: Simplfying copy_siginfo_to_user X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ebiederm@xmission.com (Eric W. Biederman) writes: > Linus Torvalds writes: > >> On Tue, Jul 18, 2017 at 7:06 AM, Eric W. Biederman >> wrote: >>> struct siginfo is a union and the kernel since 2.4 has been hiding a union >>> tag in the high 16bits of si_code using the values: >>> __SI_KILL >>> __SI_TIMER >>> __SI_POLL >>> __SI_FAULT >>> __SI_CHLD >>> __SI_RT >>> __SI_MESGQ >>> __SI_SYS >>> >>> While this looks plausible on the surface, in practice this situation has >>> not worked well. >> >> So on the whole I think we just need to do this, but the part I really >> hate about this series is still this the siginfo_layout() part. >> >> I can well believe that it is needed for the compat case. siginfo is a >> piece of crap crazy type, and re-ordering fields for compat is >> something we are always going to have to do. >> >> But for the native case, the *only* reason we do not just copy the >> siginfo as-is seems to be that it's just too big, due to other bad >> design decisions in siginfo ("let's make sure it's big enough by >> allocating 512 bytes for it). >> >> And afaik, absolutely nobody uses more than about 36 bytes of that >> 512-byte _sifields union (and that one use is SIGILL with three >> pointers and three integers and some padding. >> >> So why don't we just say "screw this idiotic layout crap, and just >> unconditionally copy that much smaller maximum of bytes"? >> >> Leave that layout thing purely for compat handling. > > I completely agree. So I just did some measurements to see what the performance impact is of doing the simple and obvious thing of always copying the entire siginfo around. There is a fair amount of variation in my timings but for the whole change I see about a 20ns increase in time taken to send a signal with siginfo from the current process to the current process. AKA timing kill(getpid(),...). I played with some clever changes such as limiting the copy to 48 bytes, disabling the memset and the like but I could not get a strong enough signal to say that any one change removed the extra or a clear part of it 20ns. Do we care about those 20ns for signal deliver? I suspect from my previous numbers that if Andy can get signal delivery to use sysret it will more than make up for the small increase in cost here. Eric