From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933140AbcLNPxu (ORCPT ); Wed, 14 Dec 2016 10:53:50 -0500 Received: from mail-oi0-f44.google.com ([209.85.218.44]:34854 "EHLO mail-oi0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932871AbcLNPxt (ORCPT ); Wed, 14 Dec 2016 10:53:49 -0500 MIME-Version: 1.0 In-Reply-To: <20161214143802.GD6279@linux-x5ow.site> References: <148143770485.10950.13227732273892953675.stgit@dwillia2-desk3.amr.corp.intel.com> <148143771052.10950.7622110904173815243.stgit@dwillia2-desk3.amr.corp.intel.com> <20161214143802.GD6279@linux-x5ow.site> From: Dan Williams Date: Wed, 14 Dec 2016 07:53:47 -0800 Message-ID: Subject: Re: [PATCH 1/8] dax: add region-available-size attribute To: Johannes Thumshirn Cc: "linux-nvdimm@lists.01.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 14, 2016 at 6:38 AM, Johannes Thumshirn wrote: > Hi Dan, > > On Sat, Dec 10, 2016 at 10:28:30PM -0800, Dan Williams wrote: >> In preparation for a facility that enables dax regions to be >> sub-divided, introduce a 'dax/available_size' attribute. This attribute >> appears under the parent device that registered the device-dax region, >> and it assumes that the device-dax-core owns the driver-data for that >> device. >> >> 'dax/available_size' adjusts dynamically as dax-device instances are >> registered and unregistered. >> >> As a side effect of using __request_region() to reserve capacity from >> the dax_region we now track pointers to those returned resources rather >> than duplicating the passed in resource array. >> >> Signed-off-by: Dan Williams >> --- > > [...] > >> +static const struct attribute_group *dax_region_attribute_groups[] = { >> + &dax_region_attribute_group, >> + NULL, >> }; >> >> static struct inode *dax_alloc_inode(struct super_block *sb) >> @@ -200,12 +251,27 @@ void dax_region_put(struct dax_region *dax_region) >> } >> EXPORT_SYMBOL_GPL(dax_region_put); >> >> + > > Stray extra newline? > > [...] > >> struct dax_region *alloc_dax_region(struct device *parent, int region_id, >> struct resource *res, unsigned int align, void *addr, >> unsigned long pfn_flags) >> { >> struct dax_region *dax_region; >> >> + if (dev_get_drvdata(parent)) { >> + dev_WARN(parent, "dax core found drvdata already in use\n"); >> + return NULL; >> + } >> + > > My first thought was, it might be interesting to see who already claimed > the drvdata. Then I figured, how are multiple sub-regions of a dax-device > supposed to work? What am I missing here? This is a check similar to the -EBUSY return you would get from request_mem_region(). In fact if all dax drivers are correctly calling request_mem_region() before alloc_dax_region() then it would be impossible for this check to ever fire. It's already impossible because there's only one dax driver upstream (dax_pmem). It's not really benefiting the kernel at all until we have multiple dax drivers, I'll remove it.