From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 886C0C46467 for ; Wed, 4 Jan 2023 08:28:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233876AbjADI21 (ORCPT ); Wed, 4 Jan 2023 03:28:27 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59002 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234032AbjADI2R (ORCPT ); Wed, 4 Jan 2023 03:28:17 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3967F186CC; Wed, 4 Jan 2023 00:28:17 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id CA0B7615C3; Wed, 4 Jan 2023 08:28:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B9058C433EF; Wed, 4 Jan 2023 08:28:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1672820896; bh=AJFMrKxnl4eUbyyWptRKXOFJIxojbdYw+u9qBEVdUYo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jrihKAEdmgJjdnSdToPubNpXoq+SgITHs9Q3VoE3EfHYEnOycNp5EQPTE2O0P4Qoo OQz8vGvvpExcVVakdOutHyJzDgL36UXQVCYhwJnsOtB+qbaz8Fa8EzK4QzFSCkQiag lw41t3MPXYkToOPPIZTK8o80QgiLq6ugst8uTAws= Date: Wed, 4 Jan 2023 09:28:13 +0100 From: Greg Kroah-Hartman To: Deepak R Varma Cc: Jiri Slaby , "Maciej W. Rozycki" , linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, Saurabh Singh Sengar , Praveen Kumar Subject: Re: [PATCH v4 1/2] tty: serial: dz: convert atomic_* to refcount_* APIs for map_guard Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 03, 2023 at 03:35:15PM +0530, Deepak R Varma wrote: > On Tue, Jan 03, 2023 at 09:59:52AM +0100, Jiri Slaby wrote: > > On 26. 12. 22, 7:21, Deepak R Varma wrote: > > > The refcount_* APIs are designed to address known issues with the > > > atomic_t APIs for reference counting. They provide following distinct > > > advantages > > > - protect the reference counters from overflow/underflow > > > - avoid use-after-free errors > > > - provide improved memory ordering guarantee schemes > > > - neater and safer. > > > > Really? (see below) > > > > > --- a/drivers/tty/serial/dz.c > > > +++ b/drivers/tty/serial/dz.c > > ... > > > @@ -687,23 +686,19 @@ static int dz_map_port(struct uart_port *uport) > > > static int dz_request_port(struct uart_port *uport) > > > { > > > struct dz_mux *mux = to_dport(uport)->mux; > > > - int map_guard; > > > int ret; > > > > > > - map_guard = atomic_add_return(1, &mux->map_guard); > > > - if (map_guard == 1) { > > > - if (!request_mem_region(uport->mapbase, dec_kn_slot_size, > > > - "dz")) { > > > - atomic_add(-1, &mux->map_guard); > > > - printk(KERN_ERR > > > - "dz: Unable to reserve MMIO resource\n"); > > > + refcount_inc(&mux->map_guard); > > > + if (refcount_read(&mux->map_guard) == 1) { > > > > This is now racy, right? > > Hello Jiri, > Thank you for the feedback. You are correct. I have split a single instruction > in two (or more?) instructions potentially resulting in race conditions. I > looked through the refcount_* APIs but did not find a direct match. > > > Can you please comment if the the following variation will avoid race condition? > > if (!refcount_add_not_zero(1, &mux->map_guard)) { > refcount_inc(&mux->map_guard); > ... > } What do you think? The onus is on you to prove the conversion is correct, otherwise, why do the conversion at all? Actually, why do this at all, what is the goal here? And how was this tested? thanks, greg k-h