From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752639AbaHLMrz (ORCPT ); Tue, 12 Aug 2014 08:47:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:21152 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752591AbaHLMry (ORCPT ); Tue, 12 Aug 2014 08:47:54 -0400 Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells In-Reply-To: References: <53CE8862.7020201@adfin.com> <28626.1406648278@warthog.procyon.org.uk> To: Milosz Tanski Cc: dhowells@redhat.com, "linux-cachefs@redhat.com" , "linux-fsdevel@vger.kernel.org" , LKML , NeilBrown , Shantanu Goel Subject: Re: [PATCH 2/3] FS-Cache: Reduce cookie ref count if submit fails. MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <17619.1407847665.1@warthog.procyon.org.uk> Date: Tue, 12 Aug 2014 13:47:45 +0100 Message-ID: <17620.1407847665@warthog.procyon.org.uk> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Milosz Tanski wrote: > The honest answer is I don't know if it know if needs to be unlocked > before or after. I saw a same pattern with unlocking order inside of > __fscache_attr_changed in the failure case. Following the enomem label, I'm calling fscache_unuse_cookie() which does this without holding the lock in the same function. I don't think the lock is required because: (1) We hold a ref on cookie->n_active so the cookie cannot go away until we release it, so calling __fscache_unuse_cookie() without the lock held should be fine. (2) wake_up_atomic_t() does not access cookie->n_active. The address is merely needed as a key for the waiters to match on. David From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Howells Subject: Re: [PATCH 2/3] FS-Cache: Reduce cookie ref count if submit fails. Date: Tue, 12 Aug 2014 13:47:45 +0100 Message-ID: <17620.1407847665@warthog.procyon.org.uk> References: <53CE8862.7020201@adfin.com> <28626.1406648278@warthog.procyon.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Shantanu Goel , NeilBrown , LKML , "linux-cachefs@redhat.com" , "linux-fsdevel@vger.kernel.org" To: Milosz Tanski Return-path: In-Reply-To: Content-ID: <17619.1407847665.1@warthog.procyon.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cachefs-bounces@redhat.com Errors-To: linux-cachefs-bounces@redhat.com List-Id: linux-fsdevel.vger.kernel.org Milosz Tanski wrote: > The honest answer is I don't know if it know if needs to be unlocked > before or after. I saw a same pattern with unlocking order inside of > __fscache_attr_changed in the failure case. Following the enomem label, I'm calling fscache_unuse_cookie() which does this without holding the lock in the same function. I don't think the lock is required because: (1) We hold a ref on cookie->n_active so the cookie cannot go away until we release it, so calling __fscache_unuse_cookie() without the lock held should be fine. (2) wake_up_atomic_t() does not access cookie->n_active. The address is merely needed as a key for the waiters to match on. David