Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752835AbbHCL44 (ORCPT ); Mon, 3 Aug 2015 07:56:56 -0400 Received: from mail-qg0-f42.google.com ([209.85.192.42]:33614 "EHLO mail-qg0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752063AbbHCL4y (ORCPT ); Mon, 3 Aug 2015 07:56:54 -0400 Date: Mon, 3 Aug 2015 07:56:44 -0400 From: Jerome Glisse To: Girish KS Cc: =?iso-8859-1?B?Suly9G1l?= 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 , ks.giri@samsung.com Subject: Re: [PATCH 05/15] HMM: introduce heterogeneous memory management v4. Message-ID: <20150803115643.GA2981@gmail.com> References: <1437159145-6548-1-git-send-email-jglisse@redhat.com> <1437159145-6548-6-git-send-email-jglisse@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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: 2068 Lines: 55 On Mon, Aug 03, 2015 at 01:20:13PM +0530, Girish KS wrote: > On 18-Jul-2015 12:47 am, "J?r?me 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(). Note that because we have the mmap sem in write mode we know it is safe to dereference mm->hmm and even to overwrite that field it if it is being destroy concurently. 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/