From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpweb146.aruba.it (smtpweb146.aruba.it [62.149.158.146]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 45A69261B92 for ; Fri, 10 Apr 2026 06:39:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=62.149.158.146 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775803203; cv=none; b=IydBkOGilRVLqtr56jc5wfjJ+uicmNChspxm3rNRe4G1PiHXPxn4V8XkEHTVsKgLA2UWQVnWCAOQpUAEWWmFu3eYpGr0miRSdTO06/4CooJi4JTJzJc+f1OujE/9aWMKY99f/4FU4rk7GtdVewUO20jYczd04GPcvlBAdnwP1qM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775803203; c=relaxed/simple; bh=2YnwoqpbILyVk6ORBJRIUYMZK71jvJMmsVREULUmZEw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=caSPz9ka7VmKqP1wjlWwc1vSfLXEVMH5ZFc8YfZyLOaf+OOaXqrQJcF1AwcIaodxsVsRepP00imX+EEyZJBz/zU4SFWAK6OXe88UVNMuuKfG2UNQCAxA3MukMBLmplnsOEMx8l3YiI8x8YgcZEYbEJLFXEuLMVufR2AprZ23Ck8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=enneenne.com; spf=pass smtp.mailfrom=enneenne.com; dkim=pass (2048-bit key) header.d=aruba.it header.i=@aruba.it header.b=gpSA5vC0; arc=none smtp.client-ip=62.149.158.146 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=enneenne.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=enneenne.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=aruba.it header.i=@aruba.it header.b="gpSA5vC0" Received: from [192.168.1.58] ([79.0.204.227]) by Aruba SMTP with ESMTPSA id B5U2wHtErrbJKB5U2wTYHa; Fri, 10 Apr 2026 08:36:50 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=aruba.it; s=a1; t=1775803010; bh=2YnwoqpbILyVk6ORBJRIUYMZK71jvJMmsVREULUmZEw=; h=Date:MIME-Version:Subject:To:From:Content-Type; b=gpSA5vC0ZOm5XQ7XnHxsUSD3UqHa5TESan4ucr1PolKxdaQzidG3TPufQAkP9YHAN jmm06xuF7BI9Ip/7JYt4A9m364tixAV8ZpAx8wrYKevorLHythoalosEtzlH35tSKF 5hFN8RPGaUeN1SsAO4ASBgzka5rfTnR5eJsmOtxxKwL7xA+MhItdvxaSEhOB2/K59w ZX25uZ0AXIsfZVUuZJFRGmQeenXMysdHT8tkZH95CEi1XVPx0JvFS3EsOdY7CTr5af nTDb/PZzUHEA2aL+CsuzuYLIhHVQYeqCXjOj+fPAokQfDf24y6opgV7Py3bboZXtUf R25UAkaWJX/FA== Message-ID: <58db49fe-1c8b-48af-b366-2e3f6da2fbbc@enneenne.com> Date: Fri, 10 Apr 2026 08:36:49 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 2/3] pps: convert pps_device lock to raw_spinlock for PREEMPT_RT Content-Language: en-US To: Michael Byczkowski Cc: Andrew Morton , linux-kernel@vger.kernel.org References: <83318241-44C3-48BE-829D-5C5F82A78A74@by-online.de> <44669820-2D7D-44BF-B518-583B672946F4@by-online.de> From: Rodolfo Giometti In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CMAE-Envelope: MS4xfPUxasEHp8qmb1cLWioJ+NGPKHXW67TWohW0ZEnDLUOc/LaPnR37FeoRo8h3DCxJNKYQsIcgbEX/HDrP69z7PR3kNeXb0EHjb+w9NJjOk62Wm9Z2mY5s RvmNR96aSdZeS7vZEkmQVblN9mCDRBOGZm6NyYhNj/2WHr6/eHum0bfDZmwHsJ5d4OrhfekqHaI7NJ9AvBkGojV5qKaEegcRuLzSuZHRjJEkWjDTfZAGlljE V9cuI1V9Se2LoTSgcG/VNs2hWF1hBcYEMCu/Tf0TSRc= On 4/9/26 17:28, Michael Byczkowski wrote: > On PREEMPT_RT, spinlock_t becomes a sleeping mutex, which allows > pps_event() to be preempted mid-update by other RT threads. This > introduces jitter in the PPS event recording path. > > Convert pps_device.lock to raw_spinlock_t so the timestamp and > sequence number updates remain non-preemptible on RT. > > Move wake_up_interruptible_all() and kill_fasync() outside the > raw_spinlock critical section, as these acquire sleeping locks > on PREEMPT_RT. The lock now protects only the timestamp, sequence > number, and last_ev updates. This is safe because PPS_FETCH waiters > use wait_event_interruptible_timeout() and will re-check after > waking, and kill_fasync() does not require PPS data to be locked > (thanks to Nikolaus Buchwitz for reporting). > > On non-RT kernels, raw_spinlock_t compiles to identical code as > spinlock_t — no behavioral change. > > Signed-off-by: Michael Byczkowski > Tested-by: Michael Byczkowski > --- > drivers/pps/kapi.c | 20 ++++++++++++-------- > drivers/pps/pps.c | 16 ++++++++-------- > include/linux/pps_kernel.h | 2 +- > 3 files changed, 21 insertions(+), 17 deletions(-) > > diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c > index 1bf0335a1b41..97898b0eeff4 100644 > --- a/drivers/pps/kapi.c > +++ b/drivers/pps/kapi.c > @@ -102,7 +102,7 @@ struct pps_device *pps_register_source(struct pps_source_info *info, > pps->info.echo = pps_echo_client_default; > > init_waitqueue_head(&pps->queue); > - spin_lock_init(&pps->lock); > + raw_spin_lock_init(&pps->lock); > > /* Create the char device */ > err = pps_register_cdev(pps); > @@ -167,7 +167,7 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event, > > timespec_to_pps_ktime(&ts_real, ts->ts_real); > > - spin_lock_irqsave(&pps->lock, flags); > + raw_spin_lock_irqsave(&pps->lock, flags); > > /* Must call the echo function? */ > if ((pps->params.mode & (PPS_ECHOASSERT | PPS_ECHOCLEAR))) > @@ -204,16 +204,20 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event, > captured = ~0; > } > > - pps_kc_event(pps, ts, event); > +pps_kc_event(pps, ts, event); What is this??? > > - /* Wake up if captured something */ > - if (captured) { > + if (captured) > pps->last_ev++; > - wake_up_interruptible_all(&pps->queue); > > + raw_spin_unlock_irqrestore(&pps->lock, flags); > + > + /* > + * Wake up after releasing the lock: wake_up_interruptible_all() > + * and kill_fasync() acquire sleeping locks on PREEMPT_RT. > + */ > + if (captured) { > + wake_up_interruptible_all(&pps->queue); > kill_fasync(&pps->async_queue, SIGIO, POLL_IN); > } > - > - spin_unlock_irqrestore(&pps->lock, flags); > } > EXPORT_SYMBOL(pps_event); > diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c > index c6b8b6478276..ad96425208a1 100644 > --- a/drivers/pps/pps.c > +++ b/drivers/pps/pps.c > @@ -103,12 +103,12 @@ static long pps_cdev_ioctl(struct file *file, > case PPS_GETPARAMS: > dev_dbg(&pps->dev, "PPS_GETPARAMS\n"); > > - spin_lock_irq(&pps->lock); > + raw_spin_lock_irq(&pps->lock); > > /* Get the current parameters */ > params = pps->params; > > - spin_unlock_irq(&pps->lock); > + raw_spin_unlock_irq(&pps->lock); > > err = copy_to_user(uarg, ¶ms, sizeof(struct pps_kparams)); > if (err) > @@ -139,7 +139,7 @@ static long pps_cdev_ioctl(struct file *file, > return -EINVAL; > } > > - spin_lock_irq(&pps->lock); > + raw_spin_lock_irq(&pps->lock); > > /* Save the new parameters */ > pps->params = params; > @@ -163,7 +163,7 @@ static long pps_cdev_ioctl(struct file *file, > pps->params.assert_off_tu.flags = 0; > pps->params.clear_off_tu.flags = 0; > > - spin_unlock_irq(&pps->lock); > + raw_spin_unlock_irq(&pps->lock); > > break; > > @@ -190,7 +190,7 @@ static long pps_cdev_ioctl(struct file *file, > return err; > > /* Return the fetched timestamp and save last fetched event */ > - spin_lock_irq(&pps->lock); > + raw_spin_lock_irq(&pps->lock); > > pps->last_fetched_ev = pps->last_ev; > > @@ -200,7 +200,7 @@ static long pps_cdev_ioctl(struct file *file, > fdata.info.clear_tu = pps->clear_tu; > fdata.info.current_mode = pps->current_mode; > > - spin_unlock_irq(&pps->lock); > + raw_spin_unlock_irq(&pps->lock); > > err = copy_to_user(uarg, &fdata, sizeof(struct pps_fdata)); > if (err) > @@ -278,7 +278,7 @@ static long pps_cdev_compat_ioctl(struct file *file, > return err; > > /* Return the fetched timestamp and save last fetched event */ > - spin_lock_irq(&pps->lock); > + raw_spin_lock_irq(&pps->lock); > > pps->last_fetched_ev = pps->last_ev; > > @@ -291,7 +291,7 @@ static long pps_cdev_compat_ioctl(struct file *file, > memcpy(&compat.info.clear_tu, &pps->clear_tu, > sizeof(struct pps_ktime_compat)); > > - spin_unlock_irq(&pps->lock); > + raw_spin_unlock_irq(&pps->lock); > > return copy_to_user(uarg, &compat, > sizeof(struct pps_fdata_compat)) ? -EFAULT : 0; > diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h > index aab0aebb529e..f2fe504071ed 100644 > --- a/include/linux/pps_kernel.h > +++ b/include/linux/pps_kernel.h > @@ -59,7 +59,7 @@ struct pps_device { > void const *lookup_cookie; /* For pps_lookup_dev() only */ > struct device dev; > struct fasync_struct *async_queue; /* fasync method */ > - spinlock_t lock; > + raw_spinlock_t lock; > }; > > /* -- GNU/Linux Solutions e-mail: giometti@enneenne.com Linux Device Driver giometti@linux.it Embedded Systems phone: +39 349 2432127 UNIX programming