Received-SPF: pass (google.com: domain of linux-kernel+bounces-155514-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 23 Apr 2024 12:04:15 -0400
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Vincent Donnefort <vdonnefort@google.com>, mhiramat@kernel.org,
        linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
        mathieu.desnoyers@efficios.com, kernel-team@android.com,
        rdunlap@infradead.org, linux-mm@kvack.org
Subject: Re: [PATCH v20 2/5] ring-buffer: Introducing ring-buffer mapping
 functions
Message-ID: <6apbwt27afeercymrjsulrgyv4c2pj2tzqdllukf4czsbfifqo@6p23uqdjqtu7>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@oracle.com>, 
	Steven Rostedt <rostedt@goodmis.org>, Vincent Donnefort <vdonnefort@google.com>, mhiramat@kernel.org, 
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, 
	mathieu.desnoyers@efficios.com, kernel-team@android.com, rdunlap@infradead.org, 
	linux-mm@kvack.org
References: <20240406173649.3210836-1-vdonnefort@google.com>
 <20240406173649.3210836-3-vdonnefort@google.com>
 <20240410134352.66d35fbc@gandalf.local.home>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240410134352.66d35fbc@gandalf.local.home>
User-Agent: NeoMutt/20231103
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?HfVwcDFGpXLR6jroOobFFiufmp+Vq/Finv7BvHzmI/lvA2EKV3vVaUXi+rcM?=
 =?us-ascii?Q?MLlsIUvquqG2bXVRyIs3+RuCRakM8Hukd08TImnD46I87Ma8nPyLNDnTMSEQ?=
 =?us-ascii?Q?r31H0tQB5N3DWWVsKjO03qO2kf6Ta8mN99dOEhLYM34dJVpGxixq4PGFxaAW?=
 =?us-ascii?Q?6o8Dc/LiHdO8IAcHMKT5uBdDoMeiBwkfepKUf9Ea5Mr8xOxJBR0wTnoiZ52h?=
 =?us-ascii?Q?up03jWD8ERta9ONpwwsCGJTNhpoVZwQ0SGek2DLJJBpUcNTsP2u6Tb8Gq774?=
 =?us-ascii?Q?iAuL71h87PWQUdA2ffZSJAtS3NIhkgj4W4YFD666Ahm+zhHSMIJdp+PbRjEI?=
 =?us-ascii?Q?eEdfQLMbSlKtzTh3ku0p1FTNnjHg7H6kegF3bBMWdBhjYb2uWmgkKZ8G3GSZ?=
 =?us-ascii?Q?lw1IibsVABWrbWzePTrHlwKJV7o73/wYZ1fFZ6ocCgbgWZhVMsayGPePvdZd?=
 =?us-ascii?Q?AivpML+o8K1TuTF1utiyhoVsL8wWdoYwqt8FFdSaRToVQKgc7R1lbMCsf+Wq?=
 =?us-ascii?Q?QkAFK2IfNlsm6Ug6gXYEAtHII18ZGVVjlkaJHyiyLVzjZWTaIeGQXRuyWsQt?=
 =?us-ascii?Q?g2EhvH4k+TMvjic41z1Kn3ke9ak1EMQgifVCh4t29Ts0pkAF6zsfJVeFs9zG?=
 =?us-ascii?Q?qYyJQe7rVtxGTsMh/AdtvDHquzaSpawBx/Gw1tJJD/jy/Djit+L9jw5A3Veb?=
 =?us-ascii?Q?48LWYlr5C1z9i4KjEnaT6XvsNeDp2vLEGUPfZx4EXQmVZ1GIGbx1gURoc9KG?=
 =?us-ascii?Q?tJ9FtWD8z4YigznN4aOLyZY2o7hLNFaO506ilNt8e7ppmRDiDUPfEO586dqK?=
 =?us-ascii?Q?UEeGhnPuS/PTGkSWoBVC7ZmLkjDPfAMl5m8SxpYcWFq1nZ+vuJfNXJ+UUfYn?=
 =?us-ascii?Q?bcEQOFE68phXa81+aPGaM6+gc3Gc2KxI5+kX7/n+HpPN2xqq/LcZVKSr4KBR?=
 =?us-ascii?Q?LDuW4ntu2uYVKGVe25bJbRR2sF4fq/7Ljo0sbM1gOMFh+KUay4nLO+HhjZFb?=
 =?us-ascii?Q?dVCoE6VHWEOhRJMMgI/ZroqzwcETbANO9KOF8XVmh7ZW/tyQ16epMjGaOC1T?=
 =?us-ascii?Q?A+XiZyrliadCqe/pp2182xsE3P/un9IAal9tzAhcSHkkvrRf04zqX8MeNgVZ?=
 =?us-ascii?Q?bhd5YOqputbQhg0E7MSDD8UbluDtzw8kIlPMnQ0X83cgpnGEBry3AJAm6yis?=
 =?us-ascii?Q?KEQU0MUKT0WT9TYcCRPi88eOe5WQri7AxCo9YFvOxqAvF160hZ5nfZ13jDsV?=
 =?us-ascii?Q?BVDxeKhRNocX6vnN5WZhM2J8/QtIs0giECsUSjypsQnx/y2sk5EJPOle/5C0?=
 =?us-ascii?Q?KXI2BaLwBNP0zZimqHvZCiM+V5yQfm9f6jwZ3MgskDmxWjfgi2xkwVDxiMSY?=
 =?us-ascii?Q?nirr4tVk6sb1hnVH4cDtEuLJw2N+1+phi2pDCNIcnPjJs/3IbpXAmN+Gz2qa?=
 =?us-ascii?Q?B/2VsA/+kC42gKxumaymXPdNCBapd3BdCxzSLDUIdvQkbwAr+UbreJMy/jX5?=
 =?us-ascii?Q?2XnX1m17sYBRMKwY1Yu4WjRpV1tNJOLQNLAyH9BfEKSvYTotEcJvKIbUoJFG?=
 =?us-ascii?Q?FgqSMVxauHmXKoBFKQ+JqX7biBzkzww9Gj30qvcB?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	DpOOemRz+Stl37JzvpNiqLLxwlQfU7D1IoeVJPDHfeGEg8tBxXzw3KwK8OW7NPyDu3cUcTFlZhMJw0vvrLnr53WvPZtxrTmAgJKnv8MYLNPwM/ha6DmHiuia+lEQ3BCbT0jnqTYFbdiE6qnklSKyFHU2uczTWTkPYGGgQgSnWQ9P5C6LlYC6gF8fHZoeN41YhQqhGpN7vGO/ZwJ5Fz5QuRSxqlN/Urc1mt1D8jLbYKdt++Zg9ggvull6Pxxp1gvj/eWzwyhNPxPFDk6meTxiNjXhj2w4MPahVGgplXOsKiP4vLPNVKGonW4biZxR4BYCE0yZVRBOU+BOWkIuPDtgXP/dw5XAaZrVc+buM/I4tNVge3BENUAxtVQhdZ+MvBGf3AHHdxqbAEpFOALyAmdgmltiNrXwKoEmygctHUuPS39hO3rO+0no/kHRYwg+09r1BMPV1ghYw2Q/XxD9t5EhIerlkFX3tg8y75tDhRZY5DTNFVPyVodS8AvJy+CIUDMoZ7GdvjtMv35xNpK/yFCm2JWeNhiDtjFFgm9bqgX1s3VkKVtofnyu8yuM/1+T5H1eAbk48XyPbAhgLbtSd7qHFL4SOAuQkXlgfF8o0LH4azY=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6dffd1a2-4a7e-49c9-647e-08dc63af071d
X-MS-Exchange-CrossTenant-AuthSource: DS0PR10MB7933.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Apr 2024 16:04:17.0614
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: UIWPL8wpCyHeJWeBmb4Ow+p9YLq7DsCfed9qIYfdu1V9o8gtF68+c1L/jL2T+RaGZqqrpcc9e5C82eIJMqJ4yQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR10MB6263
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-04-23_12,2024-04-23_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 suspectscore=0 mlxscore=0
 mlxlogscore=946 bulkscore=0 malwarescore=0 spamscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2404230037
X-Proofpoint-GUID: enlZ7Klzyz2zqMbWVyfbSd6Pct6a2-5v
X-Proofpoint-ORIG-GUID: enlZ7Klzyz2zqMbWVyfbSd6Pct6a2-5v

* Steven Rostedt <rostedt@goodmis.org> [240410 13:41]:
> On Sat,  6 Apr 2024 18:36:46 +0100
> Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> > +int ring_buffer_map(struct trace_buffer *buffer, int cpu,
> > +		    struct vm_area_struct *vma)
> > +{
> > +	struct ring_buffer_per_cpu *cpu_buffer;
> > +	unsigned long flags, *subbuf_ids;
> > +	int err = 0;
> > +
> > +	if (!cpumask_test_cpu(cpu, buffer->cpumask))
> > +		return -EINVAL;
> > +
> > +	cpu_buffer = buffer->buffers[cpu];
> > +
> > +	mutex_lock(&cpu_buffer->mapping_lock);
> > +
> > +	if (cpu_buffer->mapped) {
> > +		err = __rb_map_vma(cpu_buffer, vma);
> > +		if (!err)
> > +			err = __rb_inc_dec_mapped(cpu_buffer, true);
> > +		mutex_unlock(&cpu_buffer->mapping_lock);
> > +		return err;
> > +	}
> > +
> > +	/* prevent another thread from changing buffer/sub-buffer sizes */
> > +	mutex_lock(&buffer->mutex);
> > +
> > +	err = rb_alloc_meta_page(cpu_buffer);
> > +	if (err)
> > +		goto unlock;
> > +
> > +	/* subbuf_ids include the reader while nr_pages does not */
> > +	subbuf_ids = kcalloc(cpu_buffer->nr_pages + 1, sizeof(*subbuf_ids), GFP_KERNEL);
> > +	if (!subbuf_ids) {
> > +		rb_free_meta_page(cpu_buffer);
> > +		err = -ENOMEM;
> > +		goto unlock;
> > +	}
> > +
> > +	atomic_inc(&cpu_buffer->resize_disabled);
> > +
> > +	/*
> > +	 * Lock all readers to block any subbuf swap until the subbuf IDs are
> > +	 * assigned.
> > +	 */
> > +	raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
> > +	rb_setup_ids_meta_page(cpu_buffer, subbuf_ids);
> > +	raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> > +
> > +	err = __rb_map_vma(cpu_buffer, vma);
> > +	if (!err) {
> > +		raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
> > +		cpu_buffer->mapped = 1;
> > +		raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> > +	} else {
> > +		kfree(cpu_buffer->subbuf_ids);
> > +		cpu_buffer->subbuf_ids = NULL;
> > +		rb_free_meta_page(cpu_buffer);
> > +	}
> > +unlock:
> 
> Nit: For all labels, please add a space before them. Otherwise, diffs will
> show "unlock" as the function and not "ring_buffer_map", making it harder
> to find where the change is.
> 

Isn't the inclusion of a space before labels outdate since 'git diff'
superseds 'diff' and fixed this issue?

Thanks,
Liam