linux-kernel - Re: [PATCH 1/3] pmem: Initial version of persistent memory driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1427314913.14654.1.camel@theros.lm.intel.com>
Date:	Wed, 25 Mar 2015 14:21:53 -0600
From:	Ross Zwisler <ross.zwisler@...ux.intel.com>
To:	Christoph Hellwig <hch@....de>
Cc:	linux-nvdimm@...1.01.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, x86@...nel.org, boaz@...xistor.com,
	axboe@...nel.dk
Subject: Re: [PATCH 1/3] pmem: Initial version of persistent memory driver

On Wed, 2015-03-25 at 17:04 +0100, Christoph Hellwig wrote:
> From: Ross Zwisler <ross.zwisler@...ux.intel.com>
> 
> 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 <ross.zwisler@...ux.intel.com>
> [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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/