Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756095AbbHDORN (ORCPT ); Tue, 4 Aug 2015 10:17:13 -0400 Received: from mail-qk0-f172.google.com ([209.85.220.172]:36649 "EHLO mail-qk0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753935AbbHDORL (ORCPT ); Tue, 4 Aug 2015 10:17:11 -0400 Date: Tue, 4 Aug 2015 10:17:02 -0400 From: Jerome Glisse To: GIRISH K S Cc: Girish KS , J??e Glisse , Christophe Harle , Mark Hairgrove , Dave Airlie , Arvind Gopalakrishnan , Jatin Kumar , "joro@8bytes.org" , Greg Stoner , "akpm@linux-foundation.org" , Cameron Buschardt , Rik van Riel , Paul Blinzer , Lucien Dunning , Johannes Weiner , Haggai Eran , Michael Mantor , Laurent Morichetti , Larry Woodman , John Hubbard , Brendan Conoboy , John Bridgman , Subhash Gutti , Roland Dreier , Duncan Poole , "linux-mm@kvack.org" , Alexander Deucher , Linus Torvalds , Andrea Arcangeli , Leonid Shamis , Sherry Cheung , Linux Kernel Mailing List , Shachar Raindel , Liran Liss , Ben Sander , Joe Donohue , Mel Gorman , "H. Peter Anvin" , Peter Zijlstra Subject: Re: Re: [PATCH 05/15] HMM: introduce heterogeneous memory management v4. Message-ID: <20150804141701.GA3511@gmail.com> References: <359230388.367691438604474011.JavaMail.weblogic@epmlwas08c> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <359230388.367691438604474011.JavaMail.weblogic@epmlwas08c> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2593 Lines: 62 On Mon, Aug 03, 2015 at 12:21:14PM +0000, GIRISH K S wrote: > On Mon, Aug 03, 2015 at 01:20:13PM +0530, Girish KS wrote: > > On 18-Jul-2015 12:47 am, "J��e Glisse" wrote: > > > > > [...] > > > > +int hmm_mirror_register(struct hmm_mirror *mirror) > > > +{ > > > + struct mm_struct *mm = current->mm; > > > + struct hmm *hmm = NULL; > > > + int ret = 0; > > > + > > > + /* Sanity checks. */ > > > + BUG_ON(!mirror); > > > + BUG_ON(!mirror->device); > > > + BUG_ON(!mm); > > > + > > > + /* > > > + * Initialize the mirror struct fields, the mlist init and del > > dance is > > > + * necessary to make the error path easier for driver and for hmm. > > > + */ > > > + kref_init(&mirror->kref); > > > + INIT_HLIST_NODE(&mirror->mlist); > > > + INIT_LIST_HEAD(&mirror->dlist); > > > + spin_lock(&mirror->device->lock); > > > + list_add(&mirror->dlist, &mirror->device->mirrors); > > > + spin_unlock(&mirror->device->lock); > > > + > > > + down_write(&mm->mmap_sem); > > > + > > > + hmm = mm->hmm ? hmm_ref(hmm) : NULL; > > > > Instead of hmm mm->hmm would be the right param to be passed. Here even > > though mm->hmm is true hmm_ref returns NULL. Because hmm is not updated > > after initialization in the beginning. > > ENOPARSE ? While this can be simplified to hmm = hmm_ref(mm->hmm); I do not > see what you mean. The mm struct might already have a valid hmm field set, > and that valid hmm struct might also already be in the process of being > destroy. So hmm_ref() might either return the same hmm pointer if the hmm > object is not about to be release or NULL. But at this point there is no > certainty on the return value of hmm_ref(). > > I didn't mean hmm = hmm_ref(mm->hmm);. I ll try to put it in a better way. > The hmm local variable is initialized to NULL in the start of the function > (struct hmm *hmm = NULL;), and this is not modified till it is passed to > hmm_ref. So hmm_ref would always return a NULL irrespective of mm->hmm is > NULL or valid address. > So the statement hmm = mm->hmm ? hmm_ref(hmm) : NULL; should be replaced > as hmm = mm->hmm ? hmm_ref(mm->hmm) : NULL;. Oh yeah typo probably outcome of many patch reorg i did. Cheers, Jérôme -- 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/