From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752787AbbKKWLV (ORCPT ); Wed, 11 Nov 2015 17:11:21 -0500 Received: from mail-io0-f175.google.com ([209.85.223.175]:35659 "EHLO mail-io0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751522AbbKKWLT (ORCPT ); Wed, 11 Nov 2015 17:11:19 -0500 Subject: Re: [PATCH] null_blk: Register as a LightNVM device To: =?UTF-8?Q?Matias_Bj=c3=b8rling?= , linux-block@vger.kernel.org, linux-kernel@vger.kernel.org References: <1447236398-9421-1-git-send-email-m@bjorling.me> <5643B2C6.9010400@kernel.dk> Cc: axboe@fb.com From: Jens Axboe Message-ID: <5643BD05.8000007@kernel.dk> Date: Wed, 11 Nov 2015 15:11:17 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <5643B2C6.9010400@kernel.dk> Content-Type: multipart/mixed; boundary="------------070008050202070206010203" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is a multi-part message in MIME format. --------------070008050202070206010203 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 11/11/2015 02:27 PM, Jens Axboe wrote: > On 11/11/2015 03:06 AM, Matias Bjørling wrote: >> Add support for registering as a LightNVM device. This allows us to >> evaluate the performance of the LightNVM library. >> >> In /drivers/Makefile, LightNVM is moved above block device drivers >> to make sure that the LightNVM media managers have been initialized >> before drivers under /drivers/block are initialized. > > Generally looks ok. One question: > >> +static void *null_lnvm_create_dma_pool(struct request_queue *q, char >> *name) >> +{ >> + mempool_t *virtmem_pool; >> + >> + ppa_cache = kmem_cache_create(name, PAGE_SIZE, 0, 0, NULL); >> + if (!ppa_cache) { >> + pr_err("null_nvm: Unable to create kmem cache\n"); >> + return NULL; >> + } >> + >> + virtmem_pool = mempool_create_slab_pool(64, ppa_cache); >> + if (!virtmem_pool) { >> + pr_err("null_nvm: Unable to create virtual memory pool\n"); >> + return NULL; >> + } >> + >> + return virtmem_pool; >> +} > > Why create a slab cache if it's pages? Why not just have the mempool > alloc/free alloc single pages? Ala attached. Also fixes a leak of not freeing the ppa_cache slab cache. Did you try and load/reload the module? I'm thinking it would have crashed. -- Jens Axboe --------------070008050202070206010203 Content-Type: text/plain; charset=UTF-8; name="null-lightnvm" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="null-lightnvm" ZGlmZiAtLWdpdCBhL2RyaXZlcnMvYmxvY2svbnVsbF9ibGsuYyBiL2RyaXZlcnMvYmxvY2sv bnVsbF9ibGsuYwppbmRleCAxZWZhZWY2ZTU0ZTkuLjMxZDVkZWJjMDIwZSAxMDA2NDQKLS0t IGEvZHJpdmVycy9ibG9jay9udWxsX2Jsay5jCisrKyBiL2RyaXZlcnMvYmxvY2svbnVsbF9i bGsuYwpAQCAtNDQ2LDggKzQ0Niw2IEBAIHN0YXRpYyB2b2lkIG51bGxfZGVsX2RldihzdHJ1 Y3QgbnVsbGIgKm51bGxiKQogCiAjaWZkZWYgQ09ORklHX05WTQogCi1zdGF0aWMgc3RydWN0 IGttZW1fY2FjaGUgKnBwYV9jYWNoZTsKLQogc3RhdGljIHZvaWQgbnVsbF9sbnZtX2VuZF9p byhzdHJ1Y3QgcmVxdWVzdCAqcnEsIGludCBlcnJvcikKIHsKIAlzdHJ1Y3QgbnZtX3JxICpy cWQgPSBycS0+ZW5kX2lvX2RhdGE7CkBAIC01MjMsMTMgKzUyMSw3IEBAIHN0YXRpYyB2b2lk ICpudWxsX2xudm1fY3JlYXRlX2RtYV9wb29sKHN0cnVjdCByZXF1ZXN0X3F1ZXVlICpxLCBj aGFyICpuYW1lKQogewogCW1lbXBvb2xfdCAqdmlydG1lbV9wb29sOwogCi0JcHBhX2NhY2hl ID0ga21lbV9jYWNoZV9jcmVhdGUobmFtZSwgUEFHRV9TSVpFLCAwLCAwLCBOVUxMKTsKLQlp ZiAoIXBwYV9jYWNoZSkgewotCQlwcl9lcnIoIm51bGxfbnZtOiBVbmFibGUgdG8gY3JlYXRl IGttZW0gY2FjaGVcbiIpOwotCQlyZXR1cm4gTlVMTDsKLQl9Ci0KLQl2aXJ0bWVtX3Bvb2wg PSBtZW1wb29sX2NyZWF0ZV9zbGFiX3Bvb2woNjQsIHBwYV9jYWNoZSk7CisJdmlydG1lbV9w b29sID0gbWVtcG9vbF9jcmVhdGVfcGFnZV9wb29sKDY0LCAwKTsKIAlpZiAoIXZpcnRtZW1f cG9vbCkgewogCQlwcl9lcnIoIm51bGxfbnZtOiBVbmFibGUgdG8gY3JlYXRlIHZpcnR1YWwg bWVtb3J5IHBvb2xcbiIpOwogCQlyZXR1cm4gTlVMTDsKQEAgLTU0MCwxMCArNTMyLDggQEAg c3RhdGljIHZvaWQgKm51bGxfbG52bV9jcmVhdGVfZG1hX3Bvb2woc3RydWN0IHJlcXVlc3Rf cXVldWUgKnEsIGNoYXIgKm5hbWUpCiAKIHN0YXRpYyB2b2lkIG51bGxfbG52bV9kZXN0cm95 X2RtYV9wb29sKHZvaWQgKnBvb2wpCiB7Ci0JbWVtcG9vbF90ICp2aXJ0bWVtX3Bvb2wgPSBw b29sOworCW1lbXBvb2xfZGVzdHJveShwb29sKTsKIAotCW1lbXBvb2xfZGVzdHJveSh2aXJ0 bWVtX3Bvb2wpOwotfQogCiBzdGF0aWMgdm9pZCAqbnVsbF9sbnZtX2Rldl9kbWFfYWxsb2Mo c3RydWN0IHJlcXVlc3RfcXVldWUgKnEsIHZvaWQgKnBvb2wsCiAJCQkJZ2ZwX3QgbWVtX2Zs YWdzLCBkbWFfYWRkcl90ICpkbWFfaGFuZGxlcikK --------------070008050202070206010203--