* Re: blk_integrity_revalidate() clears BDI_CAP_STABLE_WRITES
From: Martin K. Petersen @ 2017-02-22 4:41 UTC (permalink / raw)
To: Ilya Dryomov
Cc: Martin K. Petersen, Ceph Development, linux-block, Dan Williams,
Christoph Hellwig
In-Reply-To: <CAOi1vP8M91kiF-FnT=EoJgTxfstMEsC-nhLY5KtRkrKROjWPWA@mail.gmail.com>
>>>>> "Ilya" == Ilya Dryomov <idryomov@gmail.com> writes:
Ilya,
Ilya> could you please explain blk_integrity_revalidate() and
Ilya> its GENHD_FL_UP check in particular? We have the queue,
Ilya> bi->profile can't be NULL after blk_integrity_register(), and
Ilya> since the latter "must" be used for registering the profile with
Ilya> the block layer, wouldn't the following be sufficient for
Ilya> blk_integrity users?
IIrc, the FL_UP check fixed a registration problem in the nvme driver.
The rationale behind revalidate was that we need to handle devices which
lose the integrity capability at runtime (i.e. a integrity-enabled DM
device is extended with a non-cable drive forcing the feature to be
turned off). The clearing of the integrity profile is more important in
that case than zapping the stable pages flag. But that was the original
reason for not just ORing BDI_CAP_STABLE_WRITES.
I don't have a huge problem with keeping stable pages on if a device
suddenly stops being integrity capable. However, I'd like to understand
your use case a bit better.
Ilya> The alternative seems to be to set up a bogus
Ilya> blk_integrity_profile (nop_profile won't do -- this one would have
Ilya> to be truly bogus w/ NULL *_fn) under BLK_DEV_INTEGRITY ifdefs and
Ilya> hope that nothing breaks.
Can you point me to the relevant code on your end?
Thanks,
Martin
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
From: Christoph Hellwig @ 2017-02-22 7:13 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-block, linux-nvme, Scott Bauer, Rafael Antognolli,
Jens Axboe, Christoph Hellwig
In-Reply-To: <1487703556-19913-5-git-send-email-jonathan.derrick@intel.com>
> + if (!lock_held)
> + mutex_lock(&dev->dev_lock);
No conditional locking, please. I guess I causesd this by asking you
to remove __opal_lock_unlock, but it seems we'd either need to keep it
in the end.
Except for that the series looks fine to me.
Jens: given that 1-3 are the important fixes how about you pick those
up ASAP? They all also had my Reviewed-by for previous postings.
^ permalink raw reply
* Re: sense handling improvements
From: Christoph Hellwig @ 2017-02-22 7:19 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: Christoph Hellwig, axboe, linux-block, linux-scsi
In-Reply-To: <yq18tp6by2n.fsf@oracle.com>
On Wed, Feb 15, 2017 at 10:42:56PM -0500, Martin K. Petersen wrote:
> >>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:
>
> Christoph> this series is on top of the scsi_request changes in Jens'
> Christoph> tree and further improves the handling of the sense buffer.
>
> Very nice cleanup!
>
> Christoph> The first patch prevents any possibily of reusing stale sense
> Christoph> codes in sense headers, and is a bug fix that we should
> Christoph> probably get into the block tree ASAP.
>
> Christoph> The rest cleans up handling of the parsed sense data and
> Christoph> could go in either through the block tree, or a SCSI branch
> Christoph> on top of the block tree.
>
> I can bring them in after Linus' initial block pull.
Both the block and SCSI trees are now merged by Linus, and Jens didn't
pick up patch one from this series yet - maybe it's best to send the
whole series through the SCSI tree in this case.
^ permalink raw reply
* Re: [PATCH 04/13] block: Move bdi_unregister() to del_gendisk()
From: Jan Kara @ 2017-02-22 8:51 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jan Kara, Jens Axboe, linux-block@vger.kernel.org,
Christoph Hellwig, Dan Williams, Thiago Jung Bauermann,
Lekshmi Pillai, Tejun Heo, NeilBrown, Omar Sandoval
In-Reply-To: <1D08B61A9CF0974AA09887BE32D889DA0AC71A@ULS-OP-MBXIP03.sdcorp.global.sandisk.com>
On Tue 21-02-17 19:53:29, Bart Van Assche wrote:
> On 02/21/2017 09:55 AM, Jan Kara wrote:
> > diff --git a/block/blk-core.c b/block/blk-core.c
> > index 47104f6a398b..9a901dcfdd5c 100644
> > --- a/block/blk-core.c
> > +++ b/block/blk-core.c
> > @@ -580,8 +580,6 @@ void blk_cleanup_queue(struct request_queue *q)
> > q->queue_lock = &q->__queue_lock;
> > spin_unlock_irq(lock);
> >
> > - bdi_unregister(q->backing_dev_info);
> > -
> > /* @q is and will stay empty, shutdown and put */
> > blk_put_queue(q);
> > }
> > diff --git a/block/genhd.c b/block/genhd.c
> > index f6c4d4400759..68c613edb93a 100644
> > --- a/block/genhd.c
> > +++ b/block/genhd.c
> > @@ -660,6 +660,13 @@ void del_gendisk(struct gendisk *disk)
> > disk->flags &= ~GENHD_FL_UP;
> >
> > sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");
> > + /*
> > + * Unregister bdi before releasing device numbers (as they can get
> > + * reused and we'd get clashes in sysfs) but after bdev inodes are
> > + * unhashed and thus will be soon destroyed as bdev inode's reference
> > + * to wb_writeback can block bdi_unregister().
> > + */
> > + bdi_unregister(disk->queue->backing_dev_info);
> > blk_unregister_queue(disk);
> > blk_unregister_region(disk_devt(disk), disk->minors);
>
> This change looks suspicious to me. There are drivers that create a
> block layer queue but neither call device_add_disk() nor del_gendisk(),
> e.g. drivers/scsi/st.c. Although bdi_init() will be called for the
> queues created by these drivers, this patch will cause the
> bdi_unregister() call to be skipped for these drivers.
Well, the thing is that bdi_unregister() is the counterpart to
bdi_register(). Unless you call bdi_register(), which happens only in
device_add_disk() (and some filesystems which create their private bdis),
there's no point in calling bdi_unregister(). Counterpart to bdi_init() is
bdi_exit() and that gets called always once bdi reference count drops to 0.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* [PATCH 0/4] blk-mq: cleanup on all kinds of kobjects
From: Ming Lei @ 2017-02-22 10:13 UTC (permalink / raw)
To: Jens Axboe, linux-kernel, linux-block, Christoph Hellwig,
Omar Sandoval
Cc: Ming Lei
This patchset cleans up on kojects of request_queue.mq_kobj,
sw queue's kobject and hw queue's kobject.
The 1st patch initialized kobject of request_queue and sw queue
in blk_mq_init_allocated_queue(), so we can avoid to initialize
these kobjects several times, and this patch fixes one kerne
warning reported from Omar Sandoval.
The 2nd patch makes lifetime consitent between mq request queue/sw
queue and their kobjects.
The 3rd patch makes lifetime consitent between hw queue and their
kobjects.
The last patch is a followup of 3rd patch.
Thanks,
Ming
Ming Lei (4):
blk-mq: initialize mq kobjects in blk_mq_init_allocated_queue()
blk-mq: make lifetime consitent between q/ctx and its kobject
blk-mq: make lifetime consistent between hctx and its kobject
blk-mq: free hctx->cpumask in release handler of hctx's kobject
block/blk-mq-sysfs.c | 40 +++++++++++++++++++++++++---------------
block/blk-mq.c | 28 ++++++++++------------------
block/blk-mq.h | 2 ++
3 files changed, 37 insertions(+), 33 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/4] blk-mq: initialize mq kobjects in blk_mq_init_allocated_queue()
From: Ming Lei @ 2017-02-22 10:13 UTC (permalink / raw)
To: Jens Axboe, linux-kernel, linux-block, Christoph Hellwig,
Omar Sandoval
Cc: Ming Lei
In-Reply-To: <1487758442-5855-1-git-send-email-tom.leiming@gmail.com>
Both q->mq_kobj and sw queues' kobjects should have been initialized
once, instead of doing that each add_disk context.
Also this patch removes clearing of ctx in blk_mq_init_cpu_queues()
because percpu allocator fills zero to allocated variable.
This patch fixes one issue[1] reported from Omar.
[1] kernel wearning when doing unbind/bind on one scsi-mq device
[ 19.347924] kobject (ffff8800791ea0b8): tried to init an initialized object, something is seriously wrong.
[ 19.349781] CPU: 1 PID: 84 Comm: kworker/u8:1 Not tainted 4.10.0-rc7-00210-g53f39eeaa263 #34
[ 19.350686] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.1-20161122_114906-anatol 04/01/2014
[ 19.350920] Workqueue: events_unbound async_run_entry_fn
[ 19.350920] Call Trace:
[ 19.350920] dump_stack+0x63/0x83
[ 19.350920] kobject_init+0x77/0x90
[ 19.350920] blk_mq_register_dev+0x40/0x130
[ 19.350920] blk_register_queue+0xb6/0x190
[ 19.350920] device_add_disk+0x1ec/0x4b0
[ 19.350920] sd_probe_async+0x10d/0x1c0 [sd_mod]
[ 19.350920] async_run_entry_fn+0x48/0x150
[ 19.350920] process_one_work+0x1d0/0x480
[ 19.350920] worker_thread+0x48/0x4e0
[ 19.350920] kthread+0x101/0x140
[ 19.350920] ? process_one_work+0x480/0x480
[ 19.350920] ? kthread_create_on_node+0x60/0x60
[ 19.350920] ret_from_fork+0x2c/0x40
Cc: Omar Sandoval <osandov@osandov.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
block/blk-mq-sysfs.c | 4 +---
block/blk-mq.c | 4 +++-
block/blk-mq.h | 1 +
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 295e69670c39..124305407c80 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -277,7 +277,7 @@ void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx)
kobject_init(&hctx->kobj, &blk_mq_hw_ktype);
}
-static void blk_mq_sysfs_init(struct request_queue *q)
+void blk_mq_sysfs_init(struct request_queue *q)
{
struct blk_mq_ctx *ctx;
int cpu;
@@ -297,8 +297,6 @@ int blk_mq_register_dev(struct device *dev, struct request_queue *q)
blk_mq_disable_hotplug();
- blk_mq_sysfs_init(q);
-
ret = kobject_add(&q->mq_kobj, kobject_get(&dev->kobj), "%s", "mq");
if (ret < 0)
goto out;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index b29e7dc7b309..ade1d7cde37e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1941,7 +1941,6 @@ static void blk_mq_init_cpu_queues(struct request_queue *q,
struct blk_mq_ctx *__ctx = per_cpu_ptr(q->queue_ctx, i);
struct blk_mq_hw_ctx *hctx;
- memset(__ctx, 0, sizeof(*__ctx));
__ctx->cpu = i;
spin_lock_init(&__ctx->lock);
INIT_LIST_HEAD(&__ctx->rq_list);
@@ -2248,6 +2247,9 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
if (!q->queue_ctx)
goto err_exit;
+ /* init q->mq_kobj and sw queues' kobjects */
+ blk_mq_sysfs_init(q);
+
q->queue_hw_ctx = kzalloc_node(nr_cpu_ids * sizeof(*(q->queue_hw_ctx)),
GFP_KERNEL, set->numa_node);
if (!q->queue_hw_ctx)
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 24b2256186f3..445343425f6d 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -77,6 +77,7 @@ static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
/*
* sysfs helpers
*/
+extern void blk_mq_sysfs_init(struct request_queue *q);
extern int blk_mq_sysfs_register(struct request_queue *q);
extern void blk_mq_sysfs_unregister(struct request_queue *q);
extern void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx);
--
2.7.4
^ permalink raw reply related
* [PATCH 2/4] blk-mq: make lifetime consitent between q/ctx and its kobject
From: Ming Lei @ 2017-02-22 10:14 UTC (permalink / raw)
To: Jens Axboe, linux-kernel, linux-block, Christoph Hellwig,
Omar Sandoval
Cc: Ming Lei
In-Reply-To: <1487758442-5855-1-git-send-email-tom.leiming@gmail.com>
Currently from kobject view, both q->mq_kobj and ctx->kobj can
be released during one cycle of blk_mq_register_dev() and
blk_mq_unregister_dev(). Actually, sw queue's lifetime is
same with its request queue's, which is covered by request_queue->kobj.
So we don't need to call kobject_put() for the two kinds of
kobject in __blk_mq_unregister_dev(), instead we do that
in release handler of request queue.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
block/blk-mq-sysfs.c | 20 +++++++++++++-------
block/blk-mq.c | 7 ++++++-
block/blk-mq.h | 1 +
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 124305407c80..77fb238af2be 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -242,15 +242,11 @@ static int blk_mq_register_hctx(struct blk_mq_hw_ctx *hctx)
static void __blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- struct blk_mq_ctx *ctx;
- int i, j;
+ int i;
queue_for_each_hw_ctx(q, hctx, i) {
blk_mq_unregister_hctx(hctx);
- hctx_for_each_ctx(hctx, ctx, j)
- kobject_put(&ctx->kobj);
-
kobject_put(&hctx->kobj);
}
@@ -258,8 +254,6 @@ static void __blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
kobject_uevent(&q->mq_kobj, KOBJ_REMOVE);
kobject_del(&q->mq_kobj);
- kobject_put(&q->mq_kobj);
-
kobject_put(&dev->kobj);
q->mq_sysfs_init_done = false;
@@ -277,6 +271,18 @@ void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx)
kobject_init(&hctx->kobj, &blk_mq_hw_ktype);
}
+void blk_mq_sysfs_deinit(struct request_queue *q)
+{
+ struct blk_mq_ctx *ctx;
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ ctx = per_cpu_ptr(q->queue_ctx, cpu);
+ kobject_put(&ctx->kobj);
+ }
+ kobject_put(&q->mq_kobj);
+}
+
void blk_mq_sysfs_init(struct request_queue *q)
{
struct blk_mq_ctx *ctx;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index ade1d7cde37e..bc45736d70ba 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2160,7 +2160,12 @@ void blk_mq_release(struct request_queue *q)
kfree(q->queue_hw_ctx);
- /* ctx kobj stays in queue_ctx */
+ /*
+ * release .mq_kobj and sw queue's kobject now because
+ * both share lifetime with request queue.
+ */
+ blk_mq_sysfs_deinit(q);
+
free_percpu(q->queue_ctx);
}
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 445343425f6d..2cd73b762351 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -78,6 +78,7 @@ static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
* sysfs helpers
*/
extern void blk_mq_sysfs_init(struct request_queue *q);
+extern void blk_mq_sysfs_deinit(struct request_queue *q);
extern int blk_mq_sysfs_register(struct request_queue *q);
extern void blk_mq_sysfs_unregister(struct request_queue *q);
extern void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx);
--
2.7.4
^ permalink raw reply related
* [PATCH 3/4] blk-mq: make lifetime consistent between hctx and its kobject
From: Ming Lei @ 2017-02-22 10:14 UTC (permalink / raw)
To: Jens Axboe, linux-kernel, linux-block, Christoph Hellwig,
Omar Sandoval
Cc: Ming Lei
In-Reply-To: <1487758442-5855-1-git-send-email-tom.leiming@gmail.com>
This patch removes kobject_put() over hctx in __blk_mq_unregister_dev(),
and trys to keep lifetime consistent between hctx and hctx's kobject.
Now blk_mq_sysfs_register() and blk_mq_sysfs_unregister() become
totally symmetrical, and kobject's refcounter drops to zero just
when the hctx is freed.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
block/blk-mq-sysfs.c | 15 ++++++++++-----
block/blk-mq.c | 5 +----
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 77fb238af2be..cb19ec16a7fc 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -17,6 +17,14 @@ static void blk_mq_sysfs_release(struct kobject *kobj)
{
}
+static void blk_mq_hw_sysfs_release(struct kobject *kobj)
+{
+ struct blk_mq_hw_ctx *hctx = container_of(kobj, struct blk_mq_hw_ctx,
+ kobj);
+ kfree(hctx->ctxs);
+ kfree(hctx);
+}
+
struct blk_mq_ctx_sysfs_entry {
struct attribute attr;
ssize_t (*show)(struct blk_mq_ctx *, char *);
@@ -200,7 +208,7 @@ static struct kobj_type blk_mq_ctx_ktype = {
static struct kobj_type blk_mq_hw_ktype = {
.sysfs_ops = &blk_mq_hw_sysfs_ops,
.default_attrs = default_hw_ctx_attrs,
- .release = blk_mq_sysfs_release,
+ .release = blk_mq_hw_sysfs_release,
};
static void blk_mq_unregister_hctx(struct blk_mq_hw_ctx *hctx)
@@ -244,12 +252,9 @@ static void __blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
struct blk_mq_hw_ctx *hctx;
int i;
- queue_for_each_hw_ctx(q, hctx, i) {
+ queue_for_each_hw_ctx(q, hctx, i)
blk_mq_unregister_hctx(hctx);
- kobject_put(&hctx->kobj);
- }
-
blk_mq_debugfs_unregister_hctxs(q);
kobject_uevent(&q->mq_kobj, KOBJ_REMOVE);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index bc45736d70ba..993bb4d4d909 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2152,8 +2152,7 @@ void blk_mq_release(struct request_queue *q)
queue_for_each_hw_ctx(q, hctx, i) {
if (!hctx)
continue;
- kfree(hctx->ctxs);
- kfree(hctx);
+ kobject_put(&hctx->kobj);
}
q->mq_map = NULL;
@@ -2232,8 +2231,6 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
blk_mq_exit_hctx(q, set, hctx, j);
free_cpumask_var(hctx->cpumask);
kobject_put(&hctx->kobj);
- kfree(hctx->ctxs);
- kfree(hctx);
hctxs[j] = NULL;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 4/4] blk-mq: free hctx->cpumask in release handler of hctx's kobject
From: Ming Lei @ 2017-02-22 10:14 UTC (permalink / raw)
To: Jens Axboe, linux-kernel, linux-block, Christoph Hellwig,
Omar Sandoval
Cc: Ming Lei
In-Reply-To: <1487758442-5855-1-git-send-email-tom.leiming@gmail.com>
It is obviously that hctx->cpumask is per hctx, and both
share same lifetime, so this patch moves freeing of hctx->cpumask
into release handler of hctx's kobject.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
block/blk-mq-sysfs.c | 1 +
block/blk-mq.c | 12 ------------
2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index cb19ec16a7fc..d745ab81033a 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -21,6 +21,7 @@ static void blk_mq_hw_sysfs_release(struct kobject *kobj)
{
struct blk_mq_hw_ctx *hctx = container_of(kobj, struct blk_mq_hw_ctx,
kobj);
+ free_cpumask_var(hctx->cpumask);
kfree(hctx->ctxs);
kfree(hctx);
}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 993bb4d4d909..db97f53e99d0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1851,16 +1851,6 @@ static void blk_mq_exit_hw_queues(struct request_queue *q,
}
}
-static void blk_mq_free_hw_queues(struct request_queue *q,
- struct blk_mq_tag_set *set)
-{
- struct blk_mq_hw_ctx *hctx;
- unsigned int i;
-
- queue_for_each_hw_ctx(q, hctx, i)
- free_cpumask_var(hctx->cpumask);
-}
-
static int blk_mq_init_hctx(struct request_queue *q,
struct blk_mq_tag_set *set,
struct blk_mq_hw_ctx *hctx, unsigned hctx_idx)
@@ -2229,7 +2219,6 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
if (hctx->tags)
blk_mq_free_map_and_requests(set, j);
blk_mq_exit_hctx(q, set, hctx, j);
- free_cpumask_var(hctx->cpumask);
kobject_put(&hctx->kobj);
hctxs[j] = NULL;
@@ -2342,7 +2331,6 @@ void blk_mq_free_queue(struct request_queue *q)
blk_mq_del_queue_tag_set(q);
blk_mq_exit_hw_queues(q, set, set->nr_hw_queues);
- blk_mq_free_hw_queues(q, set);
}
/* Basically redo blk_mq_init_queue with queue frozen */
--
2.7.4
^ permalink raw reply related
* Re: Manual driver binding and unbinding broken for SCSI
From: Ming Lei @ 2017-02-22 10:21 UTC (permalink / raw)
To: Jan Kara
Cc: Omar Sandoval, James Bottomley, Dan Williams, Martin K. Petersen,
Jens Axboe, Linux SCSI List, linux-block
In-Reply-To: <20170221171441.GA15875@quack2.suse.cz>
On Wed, Feb 22, 2017 at 1:14 AM, Jan Kara <jack@suse.cz> wrote:
> On Sun 19-02-17 18:19:58, Omar Sandoval wrote:
>> On Fri, Feb 17, 2017 at 04:43:56PM -0800, James Bottomley wrote:
>> > This seems to be related to a 0day test we got on the block tree,
>> > details here:
>> >
>> > http://marc.info/?t=148624068800001
>> >
>> > I root caused the above to something not being released when it should
>> > be, so it looks like you have the same problem. It seems to be a
>> > recent commit in the block tree, so could you bisect it since you have
>> > a nice reproducer?
>>
>> These appear to actually be two separate issues.
>>
>> The unbind followed by bind crash only happens with scsi-mq. It reproes
>> since at least 4.0.
>>
>> The unbind followed by a new device coming up crash happens both with
>> and without scsi-mq. The earliest version I was able to check for that
>> was 4.6, which did reproduce.
>>
>> I'll see if I can get some more info on these two issues separately.
>
> Actually, the second issue is only a warning right? And if I understand the
> issue correctly, it should be fixed by either Dan's patches in linux-block
> or my patch 4 in the series which matches your test results. So that is
> dealt with. I have no idea about the first issue though.
Looks the 1st one is one old issue in blk-mq, and I have sent one patchset
to address it:
http://marc.info/?l=linux-kernel&m=148775847517071&w=2
Omar, feel free to give a test.
thanks,
Ming Lei
^ permalink raw reply
* Re: [PATCH 0/13 v2] block: Fix block device shutdown related races
From: Jan Kara @ 2017-02-22 10:24 UTC (permalink / raw)
To: Jens Axboe
Cc: Jan Kara, linux-block, Christoph Hellwig, Dan Williams,
Thiago Jung Bauermann, Lekshmi Pillai, Tejun Heo, NeilBrown,
Omar Sandoval
In-Reply-To: <7734fd93-e4fd-65a7-bf32-08012de83c1e@kernel.dk>
[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]
On Tue 21-02-17 10:19:28, Jens Axboe wrote:
> On 02/21/2017 10:09 AM, Jan Kara wrote:
> > Hello,
> >
> > this is a second revision of the patch set to fix several different races and
> > issues I've found when testing device shutdown and reuse. The first three
> > patches are fixes to problems in my previous series fixing BDI lifetime issues.
> > Patch 4 fixes issues with reuse of BDI name with scsi devices. With it I cannot
> > reproduce the BDI name reuse issues using Omar's stress test using scsi_debug
> > so it can be used as a replacement of Dan's patches. Patches 5-11 fix oops that
> > is triggered by __blkdev_put() calling inode_detach_wb() too early (the problem
> > reported by Thiago). Patches 12 and 13 fix oops due to a bug in gendisk code
> > where get_gendisk() can return already freed gendisk structure (again triggered
> > by Omar's stress test).
> >
> > People, please have a look at patches. They are mostly simple however the
> > interactions are rather complex so I may have missed something. Also I'm
> > happy for any additional testing these patches can get - I've stressed them
> > with Omar's script, tested memcg writeback, tested static (not udev managed)
> > device inodes.
> >
> > Jens, I think at least patches 1-3 should go in together with my fixes you
> > already have in your tree (or shortly after them). It is up to you whether
> > you decide to delay my first fixes or pick these up quickly. Patch 4 is
> > (IMHO a cleaner) replacement of Dan's patches so consider whether you want
> > to use it instead of those patches.
>
> I have applied 1-3 to my for-linus branch, which will go in after
> the initial pull request has been pulled by Linus. Consider fixing up
> #4 so it applies, I like it.
OK, attached is patch 4 rebased on top of Linus' tree from today which
already has linux-block changes pulled in. I've left put_disk_devt() in
blk_cleanup_queue() to maintain the logic in the original patch (now commit
0dba1314d4f8) that request_queue and gendisk each hold one devt reference.
The bdi_unregister() call that is moved to del_gendisk() by this patch is
now protected by the gendisk reference instead of the request_queue one
so it still maintains the property that devt reference protects bdi
registration-unregistration lifetime (as much as that is not needed anymore
after this patch).
I have also updated the comment in the code and the changelog - they were
somewhat stale after changes to the whole series Tejun suggested.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[-- Attachment #2: 0001-block-Move-bdi_unregister-to-del_gendisk.patch --]
[-- Type: text/x-patch, Size: 2245 bytes --]
>From 9abe9565c83af6b653b159a7bf5b895aff638c65 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Wed, 8 Feb 2017 08:05:56 +0100
Subject: [PATCH] block: Move bdi_unregister() to del_gendisk()
Commit 6cd18e711dd8 "block: destroy bdi before blockdev is
unregistered." moved bdi unregistration (at that time through
bdi_destroy()) from blk_release_queue() to blk_cleanup_queue() because
it needs to happen before blk_unregister_region() call in del_gendisk()
for MD. SCSI though will free up the device number from sd_remove()
called through a maze of callbacks from device_del() in
__scsi_remove_device() before blk_cleanup_queue() and thus similar races
as described in 6cd18e711dd8 can happen for SCSI as well as reported by
Omar [1].
Moving bdi_unregister() to del_gendisk() works for MD and fixes the
problem for SCSI since del_gendisk() gets called from sd_remove() before
freeing the device number.
This also makes device_add_disk() (calling bdi_register_owner()) more
symmetric with del_gendisk().
[1] http://marc.info/?l=linux-block&m=148554717109098&w=2
Tested-by: Lekshmi Pillai <lekshmicpillai@in.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Jan Kara <jack@suse.cz>
---
block/blk-core.c | 1 -
block/genhd.c | 5 +++++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index b9e857f4afe8..1086dac8724c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -578,7 +578,6 @@ void blk_cleanup_queue(struct request_queue *q)
q->queue_lock = &q->__queue_lock;
spin_unlock_irq(lock);
- bdi_unregister(q->backing_dev_info);
put_disk_devt(q->disk_devt);
/* @q is and will stay empty, shutdown and put */
diff --git a/block/genhd.c b/block/genhd.c
index 2f444b87a5f2..b26a5ea115d0 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -681,6 +681,11 @@ void del_gendisk(struct gendisk *disk)
disk->flags &= ~GENHD_FL_UP;
sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");
+ /*
+ * Unregister bdi before releasing device numbers (as they can get
+ * reused and we'd get clashes in sysfs).
+ */
+ bdi_unregister(disk->queue->backing_dev_info);
blk_unregister_queue(disk);
blk_unregister_region(disk_devt(disk), disk->minors);
--
2.10.2
^ permalink raw reply related
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
From: Adrian Hunter @ 2017-02-22 13:29 UTC (permalink / raw)
To: Linus Walleij, linux-mmc, Ulf Hansson, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann
In-Reply-To: <20170209153403.9730-7-linus.walleij@linaro.org>
On 09/02/17 17:33, Linus Walleij wrote:
> The waitqueue in the host context is there to signal back from
> mmc_request_done() through mmc_wait_data_done() that the hardware
> is done with a command, and when the wait is over, the core
> will typically submit the next asynchronous request that is pending
> just waiting for the hardware to be available.
>
> This is in the way for letting the mmc_request_done() trigger the
> report up to the block layer that a block request is finished.
>
> Re-jig this as a first step, remvoving the waitqueue and introducing
> a work that will run after a completed asynchronous request,
> finalizing that request, including retransmissions, and eventually
> reporting back with a completion and a status code to the
> asynchronous issue method.
>
> This had the upside that we can remove the MMC_BLK_NEW_REQUEST
> status code and the "new_request" state in the request queue
> that is only there to make the state machine spin out
> the first time we send a request.
>
> Introduce a workqueue in the host for handling just this, and
> then a work and completion in the asynchronous request to deal
> with this mechanism.
>
> This is a central change that let us do many other changes since
> we have broken the submit and complete code paths in two, and we
> can potentially remove the NULL flushing of the asynchronous
> pipeline and report block requests as finished directly from
> the worker.
This needs more thought. The completion should go straight to the mmc block
driver from the ->done() callback. And from there straight back to the
block layer if recovery is not needed. We want to stop using
mmc_start_areq() altogether because we never want to wait - we always want
to issue (if possible) and return.
The core API to use is __mmc_start_req() but the block driver should
populate mrq->done with its own handler. i.e. change __mmc_start_req()
- mrq->done = mmc_wait_done;
+ if (!mrq->done)
+ mrq->done = mmc_wait_done;
mrq->done() would complete the request (e.g. via blk_complete_request()) if
it has no errors (and doesn't need polling), and wake up the queue thread to
finish up everything else and start the next request.
For the blk-mq port, the queue thread should also be retained, partly
because it solves some synchronization problems, but mostly because, at this
stage, we anyway don't have solutions for all the different ways the driver
can block.
(as listed here https://marc.info/?l=linux-mmc&m=148336571720463&w=2 )
^ permalink raw reply
* Re: sense handling improvements
From: Martin K. Petersen @ 2017-02-22 13:52 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, axboe, linux-block, linux-scsi
In-Reply-To: <20170222071915.GA18084@lst.de>
>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:
>> I can bring them in after Linus' initial block pull.
Christoph> Both the block and SCSI trees are now merged by Linus, and
Christoph> Jens didn't pick up patch one from this series yet - maybe
Christoph> it's best to send the whole series through the SCSI tree in
Christoph> this case.
Will do.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
From: Jon Derrick @ 2017-02-22 14:35 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-block, linux-nvme, Scott Bauer, Rafael Antognolli,
Jens Axboe
In-Reply-To: <20170222071342.GA17472@lst.de>
On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>> + if (!lock_held)
>> + mutex_lock(&dev->dev_lock);
>
> No conditional locking, please. I guess I causesd this by asking you
> to remove __opal_lock_unlock, but it seems we'd either need to keep it
> in the end.
>
> Except for that the series looks fine to me.
>
> Jens: given that 1-3 are the important fixes how about you pick those
> up ASAP? They all also had my Reviewed-by for previous postings.
>
Thanks
I'll respin just 4/4 shortly with __opal_lock_unlock
^ permalink raw reply
* Re: blk_integrity_revalidate() clears BDI_CAP_STABLE_WRITES
From: Ilya Dryomov @ 2017-02-22 14:41 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Ceph Development, linux-block, Dan Williams, Christoph Hellwig
In-Reply-To: <yq1efyq25y2.fsf@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 2941 bytes --]
On Wed, Feb 22, 2017 at 5:41 AM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>>>>>> "Ilya" == Ilya Dryomov <idryomov@gmail.com> writes:
>
> Ilya,
>
> Ilya> could you please explain blk_integrity_revalidate() and
> Ilya> its GENHD_FL_UP check in particular? We have the queue,
> Ilya> bi->profile can't be NULL after blk_integrity_register(), and
> Ilya> since the latter "must" be used for registering the profile with
> Ilya> the block layer, wouldn't the following be sufficient for
> Ilya> blk_integrity users?
>
> IIrc, the FL_UP check fixed a registration problem in the nvme driver.
Did it have something to do with a NULL disk->queue?
>
> The rationale behind revalidate was that we need to handle devices which
> lose the integrity capability at runtime (i.e. a integrity-enabled DM
> device is extended with a non-cable drive forcing the feature to be
> turned off). The clearing of the integrity profile is more important in
> that case than zapping the stable pages flag. But that was the original
> reason for not just ORing BDI_CAP_STABLE_WRITES.
>
> I don't have a huge problem with keeping stable pages on if a device
> suddenly stops being integrity capable. However, I'd like to understand
> your use case a bit better.
Well, blk_integrity_revalidate() doesn't clear the profile, it just
clears the stable pages flag. Whoever calls blk_integrity_unregister()
to clear the profile can also clear the stable pages flag -- why not
let blk_integrity_unregister() clear the flag like I suggested?
>
> Ilya> The alternative seems to be to set up a bogus
> Ilya> blk_integrity_profile (nop_profile won't do -- this one would have
> Ilya> to be truly bogus w/ NULL *_fn) under BLK_DEV_INTEGRITY ifdefs and
> Ilya> hope that nothing breaks.
>
> Can you point me to the relevant code on your end?
This code doesn't exist yet -- it's just something I thought I'd have
to do if my patch or something along those lines isn't accepted. There
is the nop_profile with stub *_fn callbacks, which is used by nvme and
nvdimm to make bio_integrity_enabled() return true, so that per-interval
buffers are allocated in bio_integrity_prep(). "I want some space for
per-interval metadata, but no integrity checksums" is use case there.
In the rbd case, we don't want that. We just want to set the stable
pages flag and make sure it's not reset by blk_integrity code. So, if
blk_integrity_revalidate() isn't fixed, rbd will need to set up a bogus
profile with NULL *_fn callbacks to make bio_integrity_enabled() return
false. This is obviously fragile, because blk_get_integrity() will no
longer return NULL...
We are setting BDI_CAP_STABLE_WRITES in rbd_init_disk(), see commit
bae818ee1577 ("rbd: require stable pages if message data CRCs are
enabled"). The CRCs are generated in write_partial_message_data() and
verified in read_partial_msg_data().
I'm attaching the patch I had in mind.
Thanks,
Ilya
[-- Attachment #2: 0001-block-get-rid-of-blk_integrity_revalidate.patch --]
[-- Type: text/x-patch, Size: 4240 bytes --]
From 24a0cf8bc437a65b3986e9ab25cf2f05e6bbd5d0 Mon Sep 17 00:00:00 2001
From: Ilya Dryomov <idryomov@gmail.com>
Date: Mon, 20 Feb 2017 18:50:50 +0100
Subject: [PATCH] block: get rid of blk_integrity_revalidate()
Commit 25520d55cdb6 ("block: Inline blk_integrity in struct gendisk")
introduced blk_integrity_revalidate(), which clears the stable pages
bit if no blk_integrity profile is registered. It's called from
revalidate_disk() and rescan_partitions(), making it impossible to set
BDI_CAP_STABLE_WRITES for drivers that support partitions and don't use
blk_integrity. One such driver is rbd, where the ceph messenger is
responsible for generating/verifying CRCs.
Since blk_integrity_{un,}register() "must" be used for (un)registering
the integrity profile with the block layer, move BDI_CAP_STABLE_WRITES
setting there. This way drivers that call blk_integrity_register() and
use integrity infrastructure won't interfere with drivers that don't
but still want stable pages.
Fixes: 25520d55cdb6 ("block: Inline blk_integrity in struct gendisk")
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
block/blk-integrity.c | 19 ++-----------------
block/partition-generic.c | 1 -
fs/block_dev.c | 1 -
include/linux/genhd.h | 2 --
4 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index d69c5c79f98e..319f2e4f4a8b 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -417,7 +417,7 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template
bi->tuple_size = template->tuple_size;
bi->tag_size = template->tag_size;
- blk_integrity_revalidate(disk);
+ disk->queue->backing_dev_info.capabilities |= BDI_CAP_STABLE_WRITES;
}
EXPORT_SYMBOL(blk_integrity_register);
@@ -430,26 +430,11 @@ EXPORT_SYMBOL(blk_integrity_register);
*/
void blk_integrity_unregister(struct gendisk *disk)
{
- blk_integrity_revalidate(disk);
+ disk->queue->backing_dev_info.capabilities &= ~BDI_CAP_STABLE_WRITES;
memset(&disk->queue->integrity, 0, sizeof(struct blk_integrity));
}
EXPORT_SYMBOL(blk_integrity_unregister);
-void blk_integrity_revalidate(struct gendisk *disk)
-{
- struct blk_integrity *bi = &disk->queue->integrity;
-
- if (!(disk->flags & GENHD_FL_UP))
- return;
-
- if (bi->profile)
- disk->queue->backing_dev_info.capabilities |=
- BDI_CAP_STABLE_WRITES;
- else
- disk->queue->backing_dev_info.capabilities &=
- ~BDI_CAP_STABLE_WRITES;
-}
-
void blk_integrity_add(struct gendisk *disk)
{
if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 7afb9907821f..0171a2faad68 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -497,7 +497,6 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
if (disk->fops->revalidate_disk)
disk->fops->revalidate_disk(disk);
- blk_integrity_revalidate(disk);
check_disk_size_change(disk, bdev);
bdev->bd_invalidated = 0;
if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 3c47614a4b32..b94e2a4974a1 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1422,7 +1422,6 @@ int revalidate_disk(struct gendisk *disk)
if (disk->fops->revalidate_disk)
ret = disk->fops->revalidate_disk(disk);
- blk_integrity_revalidate(disk);
bdev = bdget_disk(disk, 0);
if (!bdev)
return ret;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 76f39754e7b0..76d6a1cd4153 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -722,11 +722,9 @@ static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
#if defined(CONFIG_BLK_DEV_INTEGRITY)
extern void blk_integrity_add(struct gendisk *);
extern void blk_integrity_del(struct gendisk *);
-extern void blk_integrity_revalidate(struct gendisk *);
#else /* CONFIG_BLK_DEV_INTEGRITY */
static inline void blk_integrity_add(struct gendisk *disk) { }
static inline void blk_integrity_del(struct gendisk *disk) { }
-static inline void blk_integrity_revalidate(struct gendisk *disk) { }
#endif /* CONFIG_BLK_DEV_INTEGRITY */
#else /* CONFIG_BLOCK */
--
2.4.3
^ permalink raw reply related
* [PATCH] block/sed: Embed function data into the function sequence
From: Jon Derrick @ 2017-02-22 14:55 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
By embedding the function data with the function sequence, we can
eliminate the external function data and state variable code. It also
made obvious some other small cleanups.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
This is 4/4 of [1]:
Reverted opal_lock_unlock conditional locking and reintroduced
__opal_lock_unlock
[1]
http://lists.infradead.org/pipermail/linux-nvme/2017-February/008380.html
block/sed-opal.c | 418 ++++++++++++++++++++++---------------------------------
1 file changed, 163 insertions(+), 255 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4fc4d7b..8935575 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -34,7 +34,11 @@
#define IO_BUFFER_LENGTH 2048
#define MAX_TOKS 64
-typedef int (*opal_step)(struct opal_dev *dev);
+struct opal_step {
+ int (*fn)(struct opal_dev *dev, void *data);
+ void *data;
+};
+typedef int (cont_fn)(struct opal_dev *dev);
enum opal_atom_width {
OPAL_WIDTH_TINY,
@@ -80,9 +84,7 @@ struct opal_dev {
void *data;
sec_send_recv *send_recv;
- const opal_step *funcs;
- void **func_data;
- int state;
+ const struct opal_step *steps;
struct mutex dev_lock;
u16 comid;
u32 hsn;
@@ -213,8 +215,6 @@ struct opal_dev {
{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x08, 0x03 },
};
-typedef int (cont_fn)(struct opal_dev *dev);
-
static int end_opal_session_error(struct opal_dev *dev);
struct opal_suspend_data {
@@ -375,18 +375,18 @@ static void check_geometry(struct opal_dev *dev, const void *data)
static int next(struct opal_dev *dev)
{
- opal_step func;
- int error = 0;
+ const struct opal_step *step;
+ int state = 0, error = 0;
do {
- func = dev->funcs[dev->state];
- if (!func)
+ step = &dev->steps[state];
+ if (!step->fn)
break;
- error = func(dev);
+ error = step->fn(dev, step->data);
if (error) {
pr_err("Error on step function: %d with error %d: %s\n",
- dev->state, error,
+ state, error,
opal_error_to_human(error));
/* For each OPAL command we do a discovery0 then we
@@ -396,10 +396,10 @@ static int next(struct opal_dev *dev)
* session. Therefore we shouldn't attempt to terminate
* a session, as one has not yet been created.
*/
- if (dev->state > 1)
+ if (state > 1)
return end_opal_session_error(dev);
}
- dev->state++;
+ state++;
} while (!error);
return error;
@@ -483,7 +483,7 @@ static int opal_discovery0_end(struct opal_dev *dev)
return 0;
}
-static int opal_discovery0(struct opal_dev *dev)
+static int opal_discovery0(struct opal_dev *dev, void *data)
{
int ret;
@@ -1018,7 +1018,7 @@ static int finalize_and_send(struct opal_dev *dev, cont_fn cont)
return opal_send_recv(dev, cont);
}
-static int gen_key(struct opal_dev *dev)
+static int gen_key(struct opal_dev *dev, void *data)
{
const u8 *method;
u8 uid[OPAL_UID_LENGTH];
@@ -1072,15 +1072,14 @@ static int get_active_key_cont(struct opal_dev *dev)
return 0;
}
-static int get_active_key(struct opal_dev *dev)
+static int get_active_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
int err = 0;
- u8 *lr;
+ u8 *lr = data;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- lr = dev->func_data[dev->state];
err = build_locking_range(uid, sizeof(uid), *lr);
if (err)
@@ -1163,17 +1162,16 @@ static inline int enable_global_lr(struct opal_dev *dev, u8 *uid,
return err;
}
-static int setup_locking_range(struct opal_dev *dev)
+static int setup_locking_range(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- struct opal_user_lr_setup *setup;
+ struct opal_user_lr_setup *setup = data;
u8 lr;
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- setup = dev->func_data[dev->state];
lr = setup->session.opal_key.lr;
err = build_locking_range(uid, sizeof(uid), lr);
if (err)
@@ -1286,20 +1284,19 @@ static int start_generic_opal_session(struct opal_dev *dev,
return finalize_and_send(dev, start_opal_session_cont);
}
-static int start_anybodyASP_opal_session(struct opal_dev *dev)
+static int start_anybodyASP_opal_session(struct opal_dev *dev, void *data)
{
return start_generic_opal_session(dev, OPAL_ANYBODY_UID,
OPAL_ADMINSP_UID, NULL, 0);
}
-static int start_SIDASP_opal_session(struct opal_dev *dev)
+static int start_SIDASP_opal_session(struct opal_dev *dev, void *data)
{
int ret;
const u8 *key = dev->prev_data;
- struct opal_key *okey;
if (!key) {
- okey = dev->func_data[dev->state];
+ const struct opal_key *okey = data;
ret = start_generic_opal_session(dev, OPAL_SID_UID,
OPAL_ADMINSP_UID,
okey->key,
@@ -1314,22 +1311,21 @@ static int start_SIDASP_opal_session(struct opal_dev *dev)
return ret;
}
-static inline int start_admin1LSP_opal_session(struct opal_dev *dev)
+static int start_admin1LSP_opal_session(struct opal_dev *dev, void *data)
{
- struct opal_key *key = dev->func_data[dev->state];
-
+ struct opal_key *key = data;
return start_generic_opal_session(dev, OPAL_ADMIN1_UID,
OPAL_LOCKINGSP_UID,
key->key, key->key_len);
}
-static int start_auth_opal_session(struct opal_dev *dev)
+static int start_auth_opal_session(struct opal_dev *dev, void *data)
{
+ struct opal_session_info *session = data;
u8 lk_ul_user[OPAL_UID_LENGTH];
+ size_t keylen = session->opal_key.key_len;
int err = 0;
- struct opal_session_info *session = dev->func_data[dev->state];
- size_t keylen = session->opal_key.key_len;
u8 *key = session->opal_key.key;
u32 hsn = GENERIC_HOST_SESSION_NUM;
@@ -1379,7 +1375,7 @@ static int start_auth_opal_session(struct opal_dev *dev)
return finalize_and_send(dev, start_opal_session_cont);
}
-static int revert_tper(struct opal_dev *dev)
+static int revert_tper(struct opal_dev *dev, void *data)
{
int err = 0;
@@ -1401,9 +1397,9 @@ static int revert_tper(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int internal_activate_user(struct opal_dev *dev)
+static int internal_activate_user(struct opal_dev *dev, void *data)
{
- struct opal_session_info *session = dev->func_data[dev->state];
+ struct opal_session_info *session = data;
u8 uid[OPAL_UID_LENGTH];
int err = 0;
@@ -1436,15 +1432,14 @@ static int internal_activate_user(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int erase_locking_range(struct opal_dev *dev)
+static int erase_locking_range(struct opal_dev *dev, void *data)
{
- struct opal_session_info *session;
+ struct opal_session_info *session = data;
u8 uid[OPAL_UID_LENGTH];
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- session = dev->func_data[dev->state];
if (build_locking_range(uid, sizeof(uid), session->opal_key.lr) < 0)
return -ERANGE;
@@ -1463,9 +1458,9 @@ static int erase_locking_range(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_mbr_done(struct opal_dev *dev)
+static int set_mbr_done(struct opal_dev *dev, void *data)
{
- u8 mbr_done_tf = *(u8 *)dev->func_data[dev->state];
+ u8 *mbr_done_tf = data;
int err = 0;
clear_opal_cmd(dev);
@@ -1481,7 +1476,7 @@ static int set_mbr_done(struct opal_dev *dev)
add_token_u8(&err, dev, OPAL_STARTLIST);
add_token_u8(&err, dev, OPAL_STARTNAME);
add_token_u8(&err, dev, 2); /* Done */
- add_token_u8(&err, dev, mbr_done_tf); /* Done T or F */
+ add_token_u8(&err, dev, *mbr_done_tf); /* Done T or F */
add_token_u8(&err, dev, OPAL_ENDNAME);
add_token_u8(&err, dev, OPAL_ENDLIST);
add_token_u8(&err, dev, OPAL_ENDNAME);
@@ -1495,9 +1490,9 @@ static int set_mbr_done(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_mbr_enable_disable(struct opal_dev *dev)
+static int set_mbr_enable_disable(struct opal_dev *dev, void *data)
{
- u8 mbr_en_dis = *(u8 *)dev->func_data[dev->state];
+ u8 *mbr_en_dis = data;
int err = 0;
clear_opal_cmd(dev);
@@ -1513,7 +1508,7 @@ static int set_mbr_enable_disable(struct opal_dev *dev)
add_token_u8(&err, dev, OPAL_STARTLIST);
add_token_u8(&err, dev, OPAL_STARTNAME);
add_token_u8(&err, dev, 1);
- add_token_u8(&err, dev, mbr_en_dis);
+ add_token_u8(&err, dev, *mbr_en_dis);
add_token_u8(&err, dev, OPAL_ENDNAME);
add_token_u8(&err, dev, OPAL_ENDLIST);
add_token_u8(&err, dev, OPAL_ENDNAME);
@@ -1554,11 +1549,10 @@ static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
return err;
}
-static int set_new_pw(struct opal_dev *dev)
+static int set_new_pw(struct opal_dev *dev, void *data)
{
u8 cpin_uid[OPAL_UID_LENGTH];
- struct opal_session_info *usr = dev->func_data[dev->state];
-
+ struct opal_session_info *usr = data;
memcpy(cpin_uid, opaluid[OPAL_C_PIN_ADMIN1], OPAL_UID_LENGTH);
@@ -1579,10 +1573,10 @@ static int set_new_pw(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_sid_cpin_pin(struct opal_dev *dev)
+static int set_sid_cpin_pin(struct opal_dev *dev, void *data)
{
u8 cpin_uid[OPAL_UID_LENGTH];
- struct opal_key *key = dev->func_data[dev->state];
+ struct opal_key *key = data;
memcpy(cpin_uid, opaluid[OPAL_C_PIN_SID], OPAL_UID_LENGTH);
@@ -1593,18 +1587,16 @@ static int set_sid_cpin_pin(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int add_user_to_lr(struct opal_dev *dev)
+static int add_user_to_lr(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
u8 user_uid[OPAL_UID_LENGTH];
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- lkul = dev->func_data[dev->state];
-
memcpy(lr_buffer, opaluid[OPAL_LOCKINGRANGE_ACE_RDLOCKED],
OPAL_UID_LENGTH);
@@ -1671,11 +1663,11 @@ static int add_user_to_lr(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int lock_unlock_locking_range(struct opal_dev *dev)
+static int lock_unlock_locking_range(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
const u8 *method;
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
u8 read_locked = 1, write_locked = 1;
int err = 0;
@@ -1683,7 +1675,6 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
set_comid(dev, dev->comid);
method = opalmethod[OPAL_SET];
- lkul = dev->func_data[dev->state];
if (build_locking_range(lr_buffer, sizeof(lr_buffer),
lkul->session.opal_key.lr) < 0)
return -ERANGE;
@@ -1735,19 +1726,18 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
}
-static int lock_unlock_locking_range_sum(struct opal_dev *dev)
+static int lock_unlock_locking_range_sum(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
u8 read_locked = 1, write_locked = 1;
const u8 *method;
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
int ret;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
method = opalmethod[OPAL_SET];
- lkul = dev->func_data[dev->state];
if (build_locking_range(lr_buffer, sizeof(lr_buffer),
lkul->session.opal_key.lr) < 0)
return -ERANGE;
@@ -1778,9 +1768,9 @@ static int lock_unlock_locking_range_sum(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int activate_lsp(struct opal_dev *dev)
+static int activate_lsp(struct opal_dev *dev, void *data)
{
- struct opal_lr_act *opal_act;
+ struct opal_lr_act *opal_act = data;
u8 user_lr[OPAL_UID_LENGTH];
u8 uint_3 = 0x83;
int err = 0, i;
@@ -1788,8 +1778,6 @@ static int activate_lsp(struct opal_dev *dev)
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- opal_act = dev->func_data[dev->state];
-
add_token_u8(&err, dev, OPAL_CALL);
add_token_bytestring(&err, dev, opaluid[OPAL_LOCKINGSP_UID],
OPAL_UID_LENGTH);
@@ -1854,7 +1842,7 @@ static int get_lsp_lifecycle_cont(struct opal_dev *dev)
}
/* Determine if we're in the Manufactured Inactive or Active state */
-static int get_lsp_lifecycle(struct opal_dev *dev)
+static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
{
int err = 0;
@@ -1915,14 +1903,13 @@ static int get_msid_cpin_pin_cont(struct opal_dev *dev)
return 0;
}
-static int get_msid_cpin_pin(struct opal_dev *dev)
+static int get_msid_cpin_pin(struct opal_dev *dev, void *data)
{
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
-
add_token_u8(&err, dev, OPAL_CALL);
add_token_bytestring(&err, dev, opaluid[OPAL_C_PIN_MSID],
OPAL_UID_LENGTH);
@@ -1952,58 +1939,48 @@ static int get_msid_cpin_pin(struct opal_dev *dev)
return finalize_and_send(dev, get_msid_cpin_pin_cont);
}
-static int build_end_opal_session(struct opal_dev *dev)
+static int end_opal_session(struct opal_dev *dev, void *data)
{
int err = 0;
clear_opal_cmd(dev);
-
set_comid(dev, dev->comid);
add_token_u8(&err, dev, OPAL_ENDOFSESSION);
- return err;
-}
-
-static int end_opal_session(struct opal_dev *dev)
-{
- int ret = build_end_opal_session(dev);
- if (ret < 0)
- return ret;
+ if (err < 0)
+ return err;
return finalize_and_send(dev, end_session_cont);
}
static int end_opal_session_error(struct opal_dev *dev)
{
- const opal_step error_end_session[] = {
- end_opal_session,
- NULL,
+ const struct opal_step error_end_session[] = {
+ { end_opal_session, },
+ { NULL, }
};
- dev->funcs = error_end_session;
- dev->state = 0;
+ dev->steps = error_end_session;
return next(dev);
}
static inline void setup_opal_dev(struct opal_dev *dev,
- const opal_step *funcs)
+ const struct opal_step *steps)
{
- dev->state = 0;
- dev->funcs = funcs;
+ dev->steps = steps;
dev->tsn = 0;
dev->hsn = 0;
- dev->func_data = NULL;
dev->prev_data = NULL;
}
static int check_opal_support(struct opal_dev *dev)
{
- static const opal_step funcs[] = {
- opal_discovery0,
- NULL
+ const struct opal_step steps[] = {
+ { opal_discovery0, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, funcs);
+ setup_opal_dev(dev, steps);
ret = next(dev);
dev->supported = !ret;
mutex_unlock(&dev->dev_lock);
@@ -2034,24 +2011,18 @@ struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv)
static int opal_secure_erase_locking_range(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- void *data[3] = { NULL };
- static const opal_step erase_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- get_active_key,
- gen_key,
- end_opal_session,
- NULL,
+ const struct opal_step erase_steps[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, opal_session },
+ { get_active_key, &opal_session->opal_key.lr },
+ { gen_key, },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, erase_funcs);
-
- dev->func_data = data;
- dev->func_data[1] = opal_session;
- dev->func_data[2] = &opal_session->opal_key.lr;
-
+ setup_opal_dev(dev, erase_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2060,23 +2031,17 @@ static int opal_secure_erase_locking_range(struct opal_dev *dev,
static int opal_erase_locking_range(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- void *data[3] = { NULL };
- static const opal_step erase_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- erase_locking_range,
- end_opal_session,
- NULL,
+ const struct opal_step erase_steps[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, opal_session },
+ { erase_locking_range, opal_session },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, erase_funcs);
-
- dev->func_data = data;
- dev->func_data[1] = opal_session;
- dev->func_data[2] = opal_session;
-
+ setup_opal_dev(dev, erase_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2085,16 +2050,15 @@ static int opal_erase_locking_range(struct opal_dev *dev,
static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
struct opal_mbr_data *opal_mbr)
{
- void *func_data[6] = { NULL };
- static const opal_step mbr_funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- set_mbr_done,
- end_opal_session,
- start_admin1LSP_opal_session,
- set_mbr_enable_disable,
- end_opal_session,
- NULL,
+ const struct opal_step mbr_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_done, &opal_mbr->enable_disable },
+ { end_opal_session, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_enable_disable, &opal_mbr->enable_disable },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2103,12 +2067,7 @@ static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
return -EINVAL;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, mbr_funcs);
- dev->func_data = func_data;
- dev->func_data[1] = &opal_mbr->key;
- dev->func_data[2] = &opal_mbr->enable_disable;
- dev->func_data[4] = &opal_mbr->key;
- dev->func_data[5] = &opal_mbr->enable_disable;
+ setup_opal_dev(dev, mbr_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2135,13 +2094,12 @@ static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
static int opal_add_user_to_lr(struct opal_dev *dev,
struct opal_lock_unlock *lk_unlk)
{
- void *func_data[3] = { NULL };
- static const opal_step funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- add_user_to_lr,
- end_opal_session,
- NULL
+ const struct opal_step steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &lk_unlk->session.opal_key },
+ { add_user_to_lr, lk_unlk },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2163,10 +2121,7 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
}
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, funcs);
- dev->func_data = func_data;
- dev->func_data[1] = &lk_unlk->session.opal_key;
- dev->func_data[2] = lk_unlk;
+ setup_opal_dev(dev, steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2174,55 +2129,46 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
static int opal_reverttper(struct opal_dev *dev, struct opal_key *opal)
{
- void *data[2] = { NULL };
- static const opal_step revert_funcs[] = {
- opal_discovery0,
- start_SIDASP_opal_session,
- revert_tper, /* controller will terminate session */
- NULL,
+ const struct opal_step revert_steps[] = {
+ { opal_discovery0, },
+ { start_SIDASP_opal_session, opal },
+ { revert_tper, }, /* controller will terminate session */
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, revert_funcs);
- dev->func_data = data;
- dev->func_data[1] = opal;
+ setup_opal_dev(dev, revert_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
}
-static int __opal_lock_unlock_sum(struct opal_dev *dev)
+static int __opal_lock_unlock(struct opal_dev *dev,
+ struct opal_lock_unlock *lk_unlk)
{
- static const opal_step ulk_funcs_sum[] = {
- opal_discovery0,
- start_auth_opal_session,
- lock_unlock_locking_range_sum,
- end_opal_session,
- NULL
+ const struct opal_step unlock_steps[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &lk_unlk->session },
+ { lock_unlock_locking_range, lk_unlk },
+ { end_opal_session, },
+ { NULL, }
};
-
- dev->funcs = ulk_funcs_sum;
- return next(dev);
-}
-
-static int __opal_lock_unlock(struct opal_dev *dev)
-{
- static const opal_step _unlock_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- lock_unlock_locking_range,
- end_opal_session,
- NULL
+ const struct opal_step unlock_sum_steps[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &lk_unlk->session },
+ { lock_unlock_locking_range_sum, lk_unlk },
+ { end_opal_session, },
+ { NULL, }
};
- dev->funcs = _unlock_funcs;
+ dev->steps = lk_unlk->session.sum ? unlock_sum_steps : unlock_steps;
return next(dev);
}
-static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
+static int opal_lock_unlock(struct opal_dev *dev,
+ struct opal_lock_unlock *lk_unlk)
{
- void *func_data[3] = { NULL };
int ret;
if (lk_unlk->session.who < OPAL_ADMIN1 ||
@@ -2230,43 +2176,30 @@ static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_un
return -EINVAL;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, NULL);
- dev->func_data = func_data;
- dev->func_data[1] = &lk_unlk->session;
- dev->func_data[2] = lk_unlk;
-
- if (lk_unlk->session.sum)
- ret = __opal_lock_unlock_sum(dev);
- else
- ret = __opal_lock_unlock(dev);
-
+ ret = __opal_lock_unlock(dev, lk_unlk);
mutex_unlock(&dev->dev_lock);
return ret;
}
static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
{
- static const opal_step owner_funcs[] = {
- opal_discovery0,
- start_anybodyASP_opal_session,
- get_msid_cpin_pin,
- end_opal_session,
- start_SIDASP_opal_session,
- set_sid_cpin_pin,
- end_opal_session,
- NULL
+ const struct opal_step owner_steps[] = {
+ { opal_discovery0, },
+ { start_anybodyASP_opal_session, },
+ { get_msid_cpin_pin, },
+ { end_opal_session, },
+ { start_SIDASP_opal_session, opal },
+ { set_sid_cpin_pin, opal },
+ { end_opal_session, },
+ { NULL, }
};
- void *data[6] = { NULL };
int ret;
if (!dev)
return -ENODEV;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, owner_funcs);
- dev->func_data = data;
- dev->func_data[4] = opal;
- dev->func_data[5] = opal;
+ setup_opal_dev(dev, owner_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2274,14 +2207,13 @@ static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_act)
{
- void *data[4] = { NULL };
- static const opal_step active_funcs[] = {
- opal_discovery0,
- start_SIDASP_opal_session, /* Open session as SID auth */
- get_lsp_lifecycle,
- activate_lsp,
- end_opal_session,
- NULL
+ const struct opal_step active_steps[] = {
+ { opal_discovery0, },
+ { start_SIDASP_opal_session, &opal_lr_act->key },
+ { get_lsp_lifecycle, },
+ { activate_lsp, opal_lr_act },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2289,10 +2221,7 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
return -EINVAL;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, active_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_lr_act->key;
- dev->func_data[3] = opal_lr_act;
+ setup_opal_dev(dev, active_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2301,21 +2230,17 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
static int opal_setup_locking_range(struct opal_dev *dev,
struct opal_user_lr_setup *opal_lrs)
{
- void *data[3] = { NULL };
- static const opal_step lr_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- setup_locking_range,
- end_opal_session,
- NULL,
+ const struct opal_step lr_steps[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &opal_lrs->session },
+ { setup_locking_range, opal_lrs },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, lr_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_lrs->session;
- dev->func_data[2] = opal_lrs;
+ setup_opal_dev(dev, lr_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2323,14 +2248,13 @@ static int opal_setup_locking_range(struct opal_dev *dev,
static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
{
- static const opal_step pw_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- set_new_pw,
- end_opal_session,
- NULL
+ const struct opal_step pw_steps[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &opal_pw->session },
+ { set_new_pw, &opal_pw->new_user_pw },
+ { end_opal_session, },
+ { NULL }
};
- void *data[3] = { NULL };
int ret;
if (opal_pw->session.who < OPAL_ADMIN1 ||
@@ -2340,11 +2264,7 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
return -EINVAL;
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, pw_funcs);
- dev->func_data = data;
- dev->func_data[1] = (void *) &opal_pw->session;
- dev->func_data[2] = (void *) &opal_pw->new_user_pw;
-
+ setup_opal_dev(dev, pw_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2353,14 +2273,13 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
static int opal_activate_user(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- static const opal_step act_funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- internal_activate_user,
- end_opal_session,
- NULL
+ const struct opal_step act_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_session->opal_key },
+ { internal_activate_user, opal_session },
+ { end_opal_session, },
+ { NULL, }
};
- void *data[3] = { NULL };
int ret;
/* We can't activate Admin1 it's active as manufactured */
@@ -2371,10 +2290,7 @@ static int opal_activate_user(struct opal_dev *dev,
}
mutex_lock(&dev->dev_lock);
- setup_opal_dev(dev, act_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_session->opal_key;
- dev->func_data[2] = opal_session;
+ setup_opal_dev(dev, act_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2383,7 +2299,6 @@ static int opal_activate_user(struct opal_dev *dev,
bool opal_unlock_from_suspend(struct opal_dev *dev)
{
struct opal_suspend_data *suspend;
- void *func_data[3] = { NULL };
bool was_failure = false;
int ret = 0;
@@ -2394,19 +2309,12 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, NULL);
- dev->func_data = func_data;
list_for_each_entry(suspend, &dev->unlk_lst, node) {
- dev->state = 0;
- dev->func_data[1] = &suspend->unlk.session;
- dev->func_data[2] = &suspend->unlk;
dev->tsn = 0;
dev->hsn = 0;
- if (suspend->unlk.session.sum)
- ret = __opal_lock_unlock_sum(dev);
- else
- ret = __opal_lock_unlock(dev);
+ ret = __opal_lock_unlock(dev, &suspend->unlk);
if (ret) {
pr_warn("Failed to unlock LR %hhu with sum %d\n",
suspend->unlk.session.opal_key.lr,
@@ -2433,7 +2341,7 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
return -ENOTSUPP;
}
- p = memdup_user(arg, _IOC_SIZE(cmd));
+ p = memdup_user(arg, _IOC_SIZE(cmd));
if (IS_ERR(p))
return PTR_ERR(p);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
From: Jens Axboe @ 2017-02-22 16:10 UTC (permalink / raw)
To: Christoph Hellwig, Jon Derrick
Cc: linux-block, linux-nvme, Scott Bauer, Rafael Antognolli
In-Reply-To: <20170222071342.GA17472@lst.de>
On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>> + if (!lock_held)
>> + mutex_lock(&dev->dev_lock);
>
> No conditional locking, please. I guess I causesd this by asking you
> to remove __opal_lock_unlock, but it seems we'd either need to keep it
> in the end.
>
> Except for that the series looks fine to me.
>
> Jens: given that 1-3 are the important fixes how about you pick those
> up ASAP? They all also had my Reviewed-by for previous postings.
I picked up 1-3, and re-added your reviewed by. #4 should be sorted
before -rc1, though.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
From: Scott Bauer @ 2017-02-22 16:13 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, Jon Derrick, linux-block, linux-nvme,
Rafael Antognolli
In-Reply-To: <22a94611-3cbd-792f-8347-3b1c1f200944@fb.com>
On Wed, Feb 22, 2017 at 09:10:31AM -0700, Jens Axboe wrote:
> On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
> >> + if (!lock_held)
> >> + mutex_lock(&dev->dev_lock);
> >
> > No conditional locking, please. I guess I causesd this by asking you
> > to remove __opal_lock_unlock, but it seems we'd either need to keep it
> > in the end.
> >
> > Except for that the series looks fine to me.
> >
> > Jens: given that 1-3 are the important fixes how about you pick those
> > up ASAP? They all also had my Reviewed-by for previous postings.
>
> I picked up 1-3, and re-added your reviewed by. #4 should be sorted
> before -rc1, though.
>
#4 Is good to go as well. It was resent this morning under
[PATCH] block/sed: Embed function data into the function sequence
And contains the changes Christoph requested, I'll re-add my sign-off.
Once that gets In I can rebase mine and get them out today too.
^ permalink raw reply
* Re: [PATCH] block/sed: Embed function data into the function sequence
From: Scott Bauer @ 2017-02-22 16:15 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-block, linux-nvme, Rafael Antognolli, Jens Axboe,
Christoph Hellwig
In-Reply-To: <1487775313-10188-1-git-send-email-jonathan.derrick@intel.com>
On Wed, Feb 22, 2017 at 07:55:13AM -0700, Jon Derrick wrote:
> By embedding the function data with the function sequence, we can
> eliminate the external function data and state variable code. It also
> made obvious some other small cleanups.
>
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
Reviewed-by: Scott Bauer <scott.bauer@intel.com>
Christoph's review is here (I don't know if its kosher for me to put it in my message)
http://lists.infradead.org/pipermail/linux-nvme/2017-February/008405.html
^ permalink raw reply
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
From: Jens Axboe @ 2017-02-22 16:47 UTC (permalink / raw)
To: Scott Bauer
Cc: Christoph Hellwig, Jon Derrick, linux-block, linux-nvme,
Rafael Antognolli
In-Reply-To: <20170222161331.GA2696@sbauer-Z170X-UD5>
On 02/22/2017 09:13 AM, Scott Bauer wrote:
> On Wed, Feb 22, 2017 at 09:10:31AM -0700, Jens Axboe wrote:
>> On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>>>> + if (!lock_held)
>>>> + mutex_lock(&dev->dev_lock);
>>>
>>> No conditional locking, please. I guess I causesd this by asking you
>>> to remove __opal_lock_unlock, but it seems we'd either need to keep it
>>> in the end.
>>>
>>> Except for that the series looks fine to me.
>>>
>>> Jens: given that 1-3 are the important fixes how about you pick those
>>> up ASAP? They all also had my Reviewed-by for previous postings.
>>
>> I picked up 1-3, and re-added your reviewed by. #4 should be sorted
>> before -rc1, though.
>>
>
> #4 Is good to go as well. It was resent this morning under
> [PATCH] block/sed: Embed function data into the function sequence
> And contains the changes Christoph requested, I'll re-add my sign-off.
> Once that gets In I can rebase mine and get them out today too.
I see, I found it now. Guys, let's get this process streamlined a bit
more. This whole thing has been a flurry of patches and patchseries,
posted by either you or Jon. Previous patch series was 1-4 patches
posted by you, and then patch #4 is replaced by a single patch from Jon,
posted outside of that thread. Honestly, I feel like this should have
been pushed to 4.12 instead, it clearly wasn't ready before the merge
window.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
From: Keith Busch @ 2017-02-22 17:22 UTC (permalink / raw)
To: Jens Axboe
Cc: Scott Bauer, linux-block, Rafael Antognolli, linux-nvme,
Christoph Hellwig, Jon Derrick
In-Reply-To: <51275275-9cff-ce94-f760-3572161dde79@fb.com>
On Wed, Feb 22, 2017 at 09:47:53AM -0700, Jens Axboe wrote:
> I see, I found it now. Guys, let's get this process streamlined a bit
> more. This whole thing has been a flurry of patches and patchseries,
> posted by either you or Jon. Previous patch series was 1-4 patches
> posted by you, and then patch #4 is replaced by a single patch from Jon,
> posted outside of that thread. Honestly, I feel like this should have
> been pushed to 4.12 instead, it clearly wasn't ready before the merge
> window.
Sorry, I'll run interference to help ensure future patch series are
complete and coherent in one thread.
^ permalink raw reply
* Final opal patches for rc1
From: Scott Bauer @ 2017-02-22 17:15 UTC (permalink / raw)
To: linux-block; +Cc: axboe, keith.busch, linux-nvme, hch, jonathan.derrick
These are the final changes for rc1.
Patch 1 contains some code to clean up an opal structure if something changes
at runtime. (already reviewed by Christoph)
Patch 2 contains changes to the nvme driver to use the above patch. If a new
FW is loaded and we no longer support opal we clean up. This patch is based off
code from Keith and Christoph.
Patch 3 is a one line change to propagate an original error messaage to user
land instead of the return status of the session termination code.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply
* [PATCH 1/3] block/sed-opal: Introduce free_opal_dev to free the structure and clean up state
From: Scott Bauer @ 2017-02-22 17:15 UTC (permalink / raw)
To: linux-block
Cc: keith.busch, linux-nvme, axboe, jonathan.derrick, hch,
Scott Bauer
In-Reply-To: <1487783708-16285-1-git-send-email-scott.bauer@intel.com>
Before we free the opal structure we need to clean up any saved
locking ranges that the user had told us to unlock from a suspend.
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
block/sed-opal.c | 30 ++++++++++++++++++++++++++++++
include/linux/sed-opal.h | 5 +++++
2 files changed, 35 insertions(+)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 8935575..020bf3e 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1987,6 +1987,28 @@ static int check_opal_support(struct opal_dev *dev)
return ret;
}
+static void clean_opal_dev(struct opal_dev *dev)
+{
+
+ struct opal_suspend_data *suspend, *next;
+
+ mutex_lock(&dev->dev_lock);
+ list_for_each_entry_safe(suspend, next, &dev->unlk_lst, node) {
+ list_del(&suspend->node);
+ kfree(suspend);
+ }
+ mutex_unlock(&dev->dev_lock);
+}
+
+void free_opal_dev(struct opal_dev *dev)
+{
+ if (!dev)
+ return;
+ clean_opal_dev(dev);
+ kfree(dev);
+}
+EXPORT_SYMBOL(free_opal_dev);
+
struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv)
{
struct opal_dev *dev;
@@ -2141,6 +2163,14 @@ static int opal_reverttper(struct opal_dev *dev, struct opal_key *opal)
setup_opal_dev(dev, revert_steps);
ret = next(dev);
mutex_unlock(&dev->dev_lock);
+
+ /*
+ * If we successfully reverted lets clean
+ * any saved locking ranges.
+ */
+ if (!ret)
+ clean_opal_dev(dev);
+
return ret;
}
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index deee23d..04b124f 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -27,6 +27,7 @@ typedef int (sec_send_recv)(void *data, u16 spsp, u8 secp, void *buffer,
size_t len, bool send);
#ifdef CONFIG_BLK_SED_OPAL
+void free_opal_dev(struct opal_dev *dev);
bool opal_unlock_from_suspend(struct opal_dev *dev);
struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv);
int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *ioctl_ptr);
@@ -51,6 +52,10 @@ static inline bool is_sed_ioctl(unsigned int cmd)
return false;
}
#else
+static inline void free_opal_dev(struct opal_dev *dev)
+{
+}
+
static inline bool is_sed_ioctl(unsigned int cmd)
{
return false;
--
2.7.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related
* [PATCH 2/3] nvme/pci: re-check security protocol support after reset
From: Scott Bauer @ 2017-02-22 17:15 UTC (permalink / raw)
To: linux-block
Cc: keith.busch, linux-nvme, axboe, jonathan.derrick, hch,
Scott Bauer
In-Reply-To: <1487783708-16285-1-git-send-email-scott.bauer@intel.com>
A device may change capabilities after each reset, e.g. due to a firmware
upgrade. We should thus check for Security Send/Receive and OPAL support
after each reset.
Based on patches from Christoph and Keith.
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
drivers/nvme/host/pci.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ddc51ad..f660fc2 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1739,7 +1739,7 @@ static void nvme_pci_free_ctrl(struct nvme_ctrl *ctrl)
if (dev->ctrl.admin_q)
blk_put_queue(dev->ctrl.admin_q);
kfree(dev->queues);
- kfree(dev->ctrl.opal_dev);
+ free_opal_dev(dev->ctrl.opal_dev);
kfree(dev);
}
@@ -1789,14 +1789,17 @@ static void nvme_reset_work(struct work_struct *work)
if (result)
goto out;
- if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
- dev->ctrl.opal_dev =
- init_opal_dev(&dev->ctrl, &nvme_sec_submit);
+ if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) {
+ if (!dev->ctrl.opal_dev)
+ dev->ctrl.opal_dev =
+ init_opal_dev(&dev->ctrl, &nvme_sec_submit);
+ else if (was_suspend)
+ opal_unlock_from_suspend(dev->ctrl.opal_dev);
+ } else {
+ free_opal_dev(dev->ctrl.opal_dev);
+ dev->ctrl.opal_dev = NULL;
}
- if (was_suspend)
- opal_unlock_from_suspend(dev->ctrl.opal_dev);
-
result = nvme_setup_io_queues(dev);
if (result)
goto out;
--
2.7.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related
* [PATCH 3/3] block/sed-opal: Propagate original error message to userland.
From: Scott Bauer @ 2017-02-22 17:15 UTC (permalink / raw)
To: linux-block
Cc: keith.busch, linux-nvme, axboe, jonathan.derrick, hch,
Scott Bauer
In-Reply-To: <1487783708-16285-1-git-send-email-scott.bauer@intel.com>
During an error on a comannd, ex: user provides wrong pw to unlock
range, we will gracefully terminate the opal session. We want to
propagate the original error to userland instead of the result of
the session termination, which is almost always a success.
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
block/sed-opal.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 020bf3e..1e18dca 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -396,8 +396,11 @@ static int next(struct opal_dev *dev)
* session. Therefore we shouldn't attempt to terminate
* a session, as one has not yet been created.
*/
- if (state > 1)
- return end_opal_session_error(dev);
+ if (state > 1) {
+ end_opal_session_error(dev);
+ return error;
+ }
+
}
state++;
} while (!error);
--
2.7.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox