Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752441AbdGMOho (ORCPT ); Thu, 13 Jul 2017 10:37:44 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:42527 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751267AbdGMOhn (ORCPT ); Thu, 13 Jul 2017 10:37:43 -0400 Date: Thu, 13 Jul 2017 20:07:31 +0530 From: "Naveen N. Rao" To: Peter Zijlstra Cc: Arnaldo Carvalho de Melo , Jiri Olsa , Ingo Molnar , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] kernel/events: Add option to notify through signals on wakeup References: <20170712114627.hm4pj6kybxgdre3b@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170712114627.hm4pj6kybxgdre3b@hirez.programming.kicks-ass.net> User-Agent: NeoMutt/20170609 (1.8.3) X-TM-AS-MML: disable x-cbid: 17071314-0044-0000-0000-0000027FA8D6 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17071314-0045-0000-0000-0000071048C9 Message-Id: <20170713143730.viuzwijalb2smrxm@naverao1-tp.localdomain> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-07-13_08:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000 definitions=main-1707130230 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2805 Lines: 95 On 2017/07/12 01:46PM, Peter Zijlstra wrote: > On Mon, Jun 19, 2017 at 08:01:07PM +0530, Naveen N. Rao wrote: > > > diff --git a/kernel/events/core.c b/kernel/events/core.c > > index 6c4e523dc1e2..812fcfc767f4 100644 > > --- a/kernel/events/core.c > > +++ b/kernel/events/core.c > > @@ -2679,7 +2679,8 @@ static int _perf_event_refresh(struct perf_event *event, int refresh) > > /* > > * not supported on inherited events > > That comment wants updating to explain the signal crud.. Ok sure. Will do. > > > */ > > - if (event->attr.inherit || !is_sampling_event(event)) > > + if (event->attr.inherit || event->attr.signal_on_wakeup || > > + !is_sampling_event(event)) > > return -EINVAL; > > > > atomic_add(refresh, &event->event_limit); > > > @@ -7362,12 +7362,15 @@ static int __perf_event_overflow(struct perf_event *event, > > * events > > */ > > > > + if (!event->attr.signal_on_wakeup) { > > + int events = atomic_read(&event->event_limit); > > + event->pending_kill = POLL_IN; > > + if (events && atomic_dec_and_test(&event->event_limit)) { > > + ret = 1; > > + event->pending_kill = POLL_HUP; > > > > + perf_event_disable_inatomic(event); > > + } > > } > > So even without event_limit (IOC_REFRESH) this would've generated > SIGIO:POLL_IN. Yes, this still does if signal_on_wakeup isn't set. However, if signal_on_wakeup is set, we follow the ring buffer notification which will generate a POLL_IN controlled by {wakeup_events, wakeup_watermark}. > > > > > READ_ONCE(event->overflow_handler)(event, data, regs); > > @@ -10408,6 +10411,7 @@ perf_event_exit_event(struct perf_event *child_event, > > perf_group_detach(child_event); > > list_del_event(child_event, child_ctx); > > child_event->state = PERF_EVENT_STATE_EXIT; /* is_event_hup() */ > > + child_event->pending_kill = POLL_HUP; > > This looks like an undocumented change.. Yes, sorry - I should have added a note. This comes from Jiri's feedback that if user chooses to have a signal delivered on ring buffer wakeup, we should also send a HUP on exit. > > > raw_spin_unlock_irq(&child_ctx->lock); > > > > /* > > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c > > index 2831480c63a2..4e7c728569a8 100644 > > --- a/kernel/events/ring_buffer.c > > +++ b/kernel/events/ring_buffer.c > > @@ -21,6 +21,9 @@ static void perf_output_wakeup(struct perf_output_handle *handle) > > { > > atomic_set(&handle->rb->poll, POLLIN); > > > > + if (handle->event->attr.signal_on_wakeup) > > + handle->event->pending_kill = POLL_IN; > > + > > And this is the bit that generates SIGIO:POLL_IN on wakeup. Yes. Thanks, Naveen > > > handle->event->pending_wakeup = 1; > > irq_work_queue(&handle->event->pending); > > } > > -- > > 2.13.1 > > >