From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756222AbdCGUuO (ORCPT ); Tue, 7 Mar 2017 15:50:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42512 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755923AbdCGUuM (ORCPT ); Tue, 7 Mar 2017 15:50:12 -0500 Subject: Re: [PATCH] locking/rwsem: Make rwsem_is_contended() track status of OSQ To: Peter Zijlstra References: <1488902628-14948-1-git-send-email-longman@redhat.com> <20170307174504.GC3312@twins.programming.kicks-ass.net> Cc: Ingo Molnar , linux-kernel@vger.kernel.org From: Waiman Long Organization: Red Hat Message-ID: Date: Tue, 7 Mar 2017 13:21:44 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170307174504.GC3312@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 07 Mar 2017 18:21:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/07/2017 12:45 PM, Peter Zijlstra wrote: > On Tue, Mar 07, 2017 at 11:03:48AM -0500, Waiman Long wrote: >> It was found that the current rwsem_is_contended() function did not >> look at the status of the OSQ and hence would miss waiters on OSQ. So >> that function is now modified to look at the OSQ as well. > Ideally I'd kill the entire function. > > if (need_resched() || > rwsem_is_contended(&fs_info->commit_root_sem)) { > if (wakeup) > caching_ctl->progress = last; > btrfs_release_path(path); > up_read(&fs_info->commit_root_sem); > mutex_unlock(&caching_ctl->mutex); > cond_resched(); > mutex_lock(&caching_ctl->mutex); > down_read(&fs_info->commit_root_sem); > goto next; > } > > is the only user of it in the entire tree and it makes no bloody sense > what so ever. rwsem is a preemptible lock after all. That works for me too. I do realize that there is only one user in the kernel, but I am hesitant to change it as I am not familiar with that piece of code. Cheers, Longman