Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932772Ab2JWJIu (ORCPT ); Tue, 23 Oct 2012 05:08:50 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:45740 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756666Ab2JWJIr (ORCPT ); Tue, 23 Oct 2012 05:08:47 -0400 MIME-Version: 1.0 In-Reply-To: References: <1350894794-1494-3-git-send-email-ming.lei@canonical.com> Date: Tue, 23 Oct 2012 17:08:45 +0800 Message-ID: Subject: Re: [RFC PATCH v2 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio() From: Ming Lei To: Alan Stern Cc: linux-kernel@vger.kernel.org, Oliver Neukum , Minchan Kim , Greg Kroah-Hartman , "Rafael J. Wysocki" , Jens Axboe , "David S. Miller" , Andrew Morton , netdev@vger.kernel.org, linux-usb@vger.kernel.org, linux-pm@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1941 Lines: 56 On Mon, Oct 22, 2012 at 10:33 PM, Alan Stern wrote: > > Tail recursion should be implemented as a loop, not as an explicit > recursion. That is, the function should be: > > void pm_runtime_set_memalloc_noio(struct device *dev, bool enable) > { > do { > dev->power.memalloc_noio_resume = enable; > > if (!enable) { > /* > * Don't clear the parent's flag if any of the > * parent's children have their flag set. > */ > if (device_for_each_child(dev->parent, NULL, > dev_memalloc_noio)) > return; > } > dev = dev->parent; > } while (dev); > } OK, will take the non-recursion implementation for saving kernel stack space. > > except that you need to add locking, for two reasons: > > There's a race. What happens if another child sets the flag > between the time device_for_each_child() runs and the next loop > iteration? Yes, I know the race, and not adding a lock because the function is mostly called in .probe() or .remove() callback and its parent's device lock is held to avoid this race. Considered that it may be called in async probe() (scsi disk), one lock is needed, the simplest way is to add a global lock. Any suggestion? > > Even without a race, access to bitfields is not SMP-safe > without locking. You mean one ancestor device might not be in active when one of its descendants is being probed or removed? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/