From: yangds.fnst@cn.fujitsu.com (Dongsheng Yang)
Subject: [PATCH v7 1/5] lightnvm: Support for Open-Channel SSDs
Date: Fri, 4 Sep 2015 15:06:41 +0800 [thread overview]
Message-ID: <55E94301.3010501@cn.fujitsu.com> (raw)
In-Reply-To: <55E6D41A.5060100@bjorling.me>
On 09/02/2015 06:48 PM, Matias Bj?rling wrote:
>>> +
>>> + /* register with device with a supported BM */
>>> + list_for_each_entry(bt, &nvm_bms, list) {
>>> + ret = bt->register_bm(dev);
>>> + if (ret < 0)
>>> + goto err; /* initialization failed */
>>> + if (ret > 0) {
>>> + dev->bm = bt;
>>> + break; /* successfully initialized */
>>> + }
>>> + }
>>
>> Why just search it from head to tail? Can user specific it
>> in nvm_create_target()?
>
> Hi Yang,
>
> Currently only the rrpc and a couple of out of tree block managers are
> built. The register_bm only tries to find a block manager that supports
> the device, when it finds it, that one is initialized. It is an open
> question on how we choose the right block manager, e.g. a proprietary
> and a open-source block manager is in place. Priorities might be a way
> to go? or mark certain block managers as a catch all?
>
> Hopefully we will get away with only a single or two block managers in
> the future, so we won't have one for each type of device.
>
>>> +
>>> + if (!ret) {
>>> + pr_info("nvm: no compatible bm was found.\n");
>>> + return 0;
>>> + }
>>
>> If we allow nvm_device registered with no bm, we would get
>> a NULL pointer reference problem in later using.
>>
>
> Yes, definitely.
So here is a suggestion, register_bm again
if we found nvm_dev->bm == NULL in create_target(). And if it is still
NULL after that. return an error "nvm: no compatible bm was found"
and stop target creating. Otherwise, there would be a NULL Pointer
reference problem.
That's a real problem I met in my testing and I did this change
in my local using. I hope that's useful to you.
Thanx
Yang
> In the care that happens, I envision it should be
> possible to register a block manager after a device is loaded, and then
> any outstanding devices (which does not have a registered block
> manager), will be probed again.
>
>> As mentioned above, why we have to choose bm for nvm in nvm_register?
>
> Without a block manager, we don't know the structure of the device and
> how to interact with it. I want to initialize that as soon as possible.
> So that layers on top can start interacting.
>
>>
>> Thanx
>> Yang
> .
>
WARNING: multiple messages have this Message-ID (diff)
From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
To: "Matias Bjørling" <m@bjorling.me>,
hch@infradead.org, axboe@fb.com, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org
Cc: jg@lightnvm.io, Stephen.Bates@pmcs.com, keith.busch@intel.com,
"Matias Bjørling" <mb@lightnvm.io>
Subject: Re: [PATCH v7 1/5] lightnvm: Support for Open-Channel SSDs
Date: Fri, 4 Sep 2015 15:06:41 +0800 [thread overview]
Message-ID: <55E94301.3010501@cn.fujitsu.com> (raw)
In-Reply-To: <55E6D41A.5060100@bjorling.me>
On 09/02/2015 06:48 PM, Matias Bjørling wrote:
>>> +
>>> + /* register with device with a supported BM */
>>> + list_for_each_entry(bt, &nvm_bms, list) {
>>> + ret = bt->register_bm(dev);
>>> + if (ret < 0)
>>> + goto err; /* initialization failed */
>>> + if (ret > 0) {
>>> + dev->bm = bt;
>>> + break; /* successfully initialized */
>>> + }
>>> + }
>>
>> Why just search it from head to tail? Can user specific it
>> in nvm_create_target()?
>
> Hi Yang,
>
> Currently only the rrpc and a couple of out of tree block managers are
> built. The register_bm only tries to find a block manager that supports
> the device, when it finds it, that one is initialized. It is an open
> question on how we choose the right block manager, e.g. a proprietary
> and a open-source block manager is in place. Priorities might be a way
> to go? or mark certain block managers as a catch all?
>
> Hopefully we will get away with only a single or two block managers in
> the future, so we won't have one for each type of device.
>
>>> +
>>> + if (!ret) {
>>> + pr_info("nvm: no compatible bm was found.\n");
>>> + return 0;
>>> + }
>>
>> If we allow nvm_device registered with no bm, we would get
>> a NULL pointer reference problem in later using.
>>
>
> Yes, definitely.
So here is a suggestion, register_bm again
if we found nvm_dev->bm == NULL in create_target(). And if it is still
NULL after that. return an error "nvm: no compatible bm was found"
and stop target creating. Otherwise, there would be a NULL Pointer
reference problem.
That's a real problem I met in my testing and I did this change
in my local using. I hope that's useful to you.
Thanx
Yang
> In the care that happens, I envision it should be
> possible to register a block manager after a device is loaded, and then
> any outstanding devices (which does not have a registered block
> manager), will be probed again.
>
>> As mentioned above, why we have to choose bm for nvm in nvm_register?
>
> Without a block manager, we don't know the structure of the device and
> how to interact with it. I want to initialize that as soon as possible.
> So that layers on top can start interacting.
>
>>
>> Thanx
>> Yang
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
To: "Matias Bjørling" <m@bjorling.me>,
hch@infradead.org, axboe@fb.com, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org
Cc: jg@lightnvm.io, Stephen.Bates@pmcs.com, keith.busch@intel.com,
"Matias Bjørling" <mb@lightnvm.io>
Subject: Re: [PATCH v7 1/5] lightnvm: Support for Open-Channel SSDs
Date: Fri, 4 Sep 2015 15:06:41 +0800 [thread overview]
Message-ID: <55E94301.3010501@cn.fujitsu.com> (raw)
In-Reply-To: <55E6D41A.5060100@bjorling.me>
On 09/02/2015 06:48 PM, Matias Bjørling wrote:
>>> +
>>> + /* register with device with a supported BM */
>>> + list_for_each_entry(bt, &nvm_bms, list) {
>>> + ret = bt->register_bm(dev);
>>> + if (ret < 0)
>>> + goto err; /* initialization failed */
>>> + if (ret > 0) {
>>> + dev->bm = bt;
>>> + break; /* successfully initialized */
>>> + }
>>> + }
>>
>> Why just search it from head to tail? Can user specific it
>> in nvm_create_target()?
>
> Hi Yang,
>
> Currently only the rrpc and a couple of out of tree block managers are
> built. The register_bm only tries to find a block manager that supports
> the device, when it finds it, that one is initialized. It is an open
> question on how we choose the right block manager, e.g. a proprietary
> and a open-source block manager is in place. Priorities might be a way
> to go? or mark certain block managers as a catch all?
>
> Hopefully we will get away with only a single or two block managers in
> the future, so we won't have one for each type of device.
>
>>> +
>>> + if (!ret) {
>>> + pr_info("nvm: no compatible bm was found.\n");
>>> + return 0;
>>> + }
>>
>> If we allow nvm_device registered with no bm, we would get
>> a NULL pointer reference problem in later using.
>>
>
> Yes, definitely.
So here is a suggestion, register_bm again
if we found nvm_dev->bm == NULL in create_target(). And if it is still
NULL after that. return an error "nvm: no compatible bm was found"
and stop target creating. Otherwise, there would be a NULL Pointer
reference problem.
That's a real problem I met in my testing and I did this change
in my local using. I hope that's useful to you.
Thanx
Yang
> In the care that happens, I envision it should be
> possible to register a block manager after a device is loaded, and then
> any outstanding devices (which does not have a registered block
> manager), will be probed again.
>
>> As mentioned above, why we have to choose bm for nvm in nvm_register?
>
> Without a block manager, we don't know the structure of the device and
> how to interact with it. I want to initialize that as soon as possible.
> So that layers on top can start interacting.
>
>>
>> Thanx
>> Yang
> .
>
next prev parent reply other threads:[~2015-09-04 7:06 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-07 14:29 [PATCH v7 0/5] Support for Open-Channel SSDs Matias Bjørling
2015-08-07 14:29 ` Matias Bjørling
2015-08-07 14:29 ` [PATCH v7 1/5] lightnvm: " Matias Bjørling
2015-08-07 14:29 ` Matias Bjørling
2015-08-07 14:29 ` Matias Bjørling
2015-09-02 3:50 ` Dongsheng Yang
2015-09-02 3:50 ` Dongsheng Yang
2015-09-02 3:50 ` Dongsheng Yang
2015-09-02 10:48 ` Matias Bjørling
2015-09-02 10:48 ` Matias Bjørling
2015-09-04 7:06 ` Dongsheng Yang [this message]
2015-09-04 7:06 ` Dongsheng Yang
2015-09-04 7:06 ` Dongsheng Yang
2015-09-04 8:05 ` Matias Bjørling
2015-09-04 8:05 ` Matias Bjørling
2015-09-04 8:27 ` Dongsheng Yang
2015-09-04 8:27 ` Dongsheng Yang
2015-09-04 8:49 ` Matias Bjørling
2015-09-04 8:49 ` Matias Bjørling
2015-08-07 14:29 ` [PATCH v7 2/5] lightnvm: Hybrid Open-Channel SSD RRPC target Matias Bjørling
2015-08-07 14:29 ` Matias Bjørling
2015-08-07 14:29 ` Matias Bjørling
2015-08-07 14:29 ` [PATCH v7 3/5] lightnvm: Hybrid Open-Channel SSD block manager Matias Bjørling
2015-08-07 14:29 ` Matias Bjørling
2015-08-07 14:29 ` [PATCH v7 4/5] null_nvm: Lightnvm test driver Matias Bjørling
2015-08-07 14:29 ` Matias Bjørling
2015-08-07 14:29 ` [PATCH v7 5/5] nvme: LightNVM support Matias Bjørling
2015-08-07 14:29 ` Matias Bjørling
2015-09-02 3:50 ` [PATCH v7 0/5] Support for Open-Channel SSDs Dongsheng Yang
2015-09-02 3:50 ` Dongsheng Yang
2015-09-02 3:50 ` Dongsheng Yang
2015-09-02 10:59 ` Matias Bjørling
2015-09-02 10:59 ` Matias Bjørling
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55E94301.3010501@cn.fujitsu.com \
--to=yangds.fnst@cn.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.