From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753254AbbCYUWS (ORCPT ); Wed, 25 Mar 2015 16:22:18 -0400 Received: from mga09.intel.com ([134.134.136.24]:28344 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753214AbbCYUWQ (ORCPT ); Wed, 25 Mar 2015 16:22:16 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,466,1422950400"; d="scan'208";a="704180305" Message-ID: <1427314913.14654.1.camel@theros.lm.intel.com> Subject: Re: [PATCH 1/3] pmem: Initial version of persistent memory driver From: Ross Zwisler To: Christoph Hellwig Cc: linux-nvdimm@ml01.01.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org, boaz@plexistor.com, axboe@kernel.dk Date: Wed, 25 Mar 2015 14:21:53 -0600 In-Reply-To: <1427299449-26722-2-git-send-email-hch@lst.de> References: <1427299449-26722-1-git-send-email-hch@lst.de> <1427299449-26722-2-git-send-email-hch@lst.de> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4 (3.10.4-4.fc20.rez) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2015-03-25 at 17:04 +0100, Christoph Hellwig wrote: > From: Ross Zwisler > > PMEM is a new driver that presents a reserved range of memory as a > block device. This is useful for developing with NV-DIMMs, and > can be used with volatile memory as a development platform. > > Signed-off-by: Ross Zwisler > [hch: convert to use a platform_device for discovery, fix partition > support] Overall I really like this approach. It makes things simpler, removes unneeded code and most importantly removes the ability for the user to have a configuration where the PMEM / memmap reservation via the command line don't match the parameters given to pmem. What needed to be fixed with the partition support? I used to have real numbers for first_minor and passed into alloc_disk(), but simplified it based on code found in this commit in the nvme driver: 469071a37afc NVMe: Dynamically allocate partition numbers This has worked fine for me - is there some test case in which it breaks? > +static int pmem_probe(struct platform_device *pdev) > +{ > + struct pmem_device *pmem; > + struct gendisk *disk; > + struct resource *res; > + int idx, err; > + > + if (WARN_ON(pdev->num_resources > 1)) > + return -ENXIO; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -ENXIO; > + > + pmem = kzalloc(sizeof(*pmem), GFP_KERNEL); > + if (unlikely(!pmem)) > + return -ENOMEM; > + > + pmem->phys_addr = res->start; > + pmem->size = resource_size(res); > + > + err = pmem_mapmem(pmem); > + if (unlikely(err)) > + goto out_free_dev; > + > + err = -ENOMEM; > + pmem->pmem_queue = blk_alloc_queue(GFP_KERNEL); > + if (unlikely(!pmem->pmem_queue)) > + goto out_unmap; > + > + blk_queue_make_request(pmem->pmem_queue, pmem_make_request); > + blk_queue_max_hw_sectors(pmem->pmem_queue, 1024); > + blk_queue_bounce_limit(pmem->pmem_queue, BLK_BOUNCE_ANY); > + > + disk = alloc_disk(PMEM_MINORS); > + if (unlikely(!disk)) > + goto out_free_queue; > + > + idx = atomic_inc_return(&pmem_index) - 1; > + > + disk->major = pmem_major; > + disk->first_minor = PMEM_MINORS * idx; > + disk->fops = &pmem_fops; > + disk->private_data = pmem; > + disk->queue = pmem->pmem_queue; > + disk->flags = GENHD_FL_EXT_DEVT; > + sprintf(disk->disk_name, "pmem%d", idx); > + disk->driverfs_dev = &pdev->dev; > + set_capacity(disk, pmem->size >> SECTOR_SHIFT); > + pmem->pmem_disk = disk; > + > + add_disk(disk); > + > + platform_set_drvdata(pdev, pmem); > + return 0; > + > +out_free_queue: > + blk_cleanup_queue(pmem->pmem_queue); > +out_unmap: > + pmem_unmapmem(pmem); > +out_free_dev: > + kfree(pmem); > +out: This label is no longer used, and can be removed.