From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750890AbdA1ACA (ORCPT ); Fri, 27 Jan 2017 19:02:00 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:42645 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750724AbdA1ABw (ORCPT ); Fri, 27 Jan 2017 19:01:52 -0500 Subject: Re: [PATCH 1/1] percpu-refcount: fix reference leak during percpu-atomic transition To: linux-kernel@vger.kernel.org, Christoph Lameter , Tejun Heo References: <1485550748-28075-1-git-send-email-dougmill@linux.vnet.ibm.com> Cc: linux-block@vger.kernel.org, Guilherme Piccoli From: Douglas Miller Date: Fri, 27 Jan 2017 15:39:35 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1485550748-28075-1-git-send-email-dougmill@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17012721-0016-0000-0000-000005FE9906 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006509; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000201; SDB=6.00813616; UDB=6.00396926; IPR=6.00591032; BA=6.00005093; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00014078; XFM=3.00000011; UTC=2017-01-27 21:39:39 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17012721-0017-0000-0000-000036E1ADFF Message-Id: <6877752c-2935-a27f-5390-95aa4990b2a3@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-01-27_16:,, 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-1612050000 definitions=main-1701270209 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I'd like to request this be flagged for "stable". Thanks, Doug On 01/27/2017 02:59 PM, Douglas Miller wrote: > percpu_ref_tryget() and percpu_ref_tryget_live() should return > "true" IFF they acquire a reference. But the return value from > atomic_long_inc_not_zero() is a long and may have high bits set, > e.g. PERCPU_COUNT_BIAS, and the return value of the tryget routines > is bool so the reference may actually be acquired but the routines > return "false" which results in a reference leak since the caller > assumes it does not need to do a corresponding percpu_ref_put(). > > This was seen when performing CPU hotplug during I/O, as hangs in > blk_mq_freeze_queue_wait where percpu_ref_kill (blk_mq_freeze_queue_start) > raced with percpu_ref_tryget (blk_mq_timeout_work). > Sample stack trace: > > __switch_to+0x2c0/0x450 > __schedule+0x2f8/0x970 > schedule+0x48/0xc0 > blk_mq_freeze_queue_wait+0x94/0x120 > blk_mq_queue_reinit_work+0xb8/0x180 > blk_mq_queue_reinit_prepare+0x84/0xa0 > cpuhp_invoke_callback+0x17c/0x600 > cpuhp_up_callbacks+0x58/0x150 > _cpu_up+0xf0/0x1c0 > do_cpu_up+0x120/0x150 > cpu_subsys_online+0x64/0xe0 > device_online+0xb4/0x120 > online_store+0xb4/0xc0 > dev_attr_store+0x68/0xa0 > sysfs_kf_write+0x80/0xb0 > kernfs_fop_write+0x17c/0x250 > __vfs_write+0x6c/0x1e0 > vfs_write+0xd0/0x270 > SyS_write+0x6c/0x110 > system_call+0x38/0xe0 > > Examination of the queue showed a single reference (no PERCPU_COUNT_BIAS, > and __PERCPU_REF_DEAD, __PERCPU_REF_ATOMIC set) and no requests. > However, conditions at the time of the race are count of PERCPU_COUNT_BIAS + 0 > and __PERCPU_REF_DEAD and __PERCPU_REF_ATOMIC set. > > The fix is to make the tryget routines return an actual boolean instead > of the atomic long result truncated to a bool. > > Fixes: e625305b3907 percpu-refcount: make percpu_ref based on longs instead of ints > Link: https://bugzilla.kernel.org/show_bug.cgi?id=190751 > Signed-off-by: Douglas Miller > --- > include/linux/percpu-refcount.h | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h > index 1c7eec0..88dc96b 100644 > --- a/include/linux/percpu-refcount.h > +++ b/include/linux/percpu-refcount.h > @@ -212,7 +212,7 @@ static inline bool percpu_ref_tryget(struct percpu_ref *ref) > this_cpu_inc(*percpu_count); > ret = true; > } else { > - ret = atomic_long_inc_not_zero(&ref->count); > + ret = (atomic_long_inc_not_zero(&ref->count) != 0); > } > > rcu_read_unlock_sched(); > @@ -246,7 +246,7 @@ static inline bool percpu_ref_tryget_live(struct percpu_ref *ref) > this_cpu_inc(*percpu_count); > ret = true; > } else if (!(ref->percpu_count_ptr & __PERCPU_REF_DEAD)) { > - ret = atomic_long_inc_not_zero(&ref->count); > + ret = (atomic_long_inc_not_zero(&ref->count) != 0); > } > > rcu_read_unlock_sched();