Re: [PATCH 11/11] nvme: add support for streams and directives

[Christoph Hellwig <hch@infradead.org>]

> We default to allocating 4 streams per name space, but it is
> configurable with the 'streams_per_ns' module option.

What's your strategy for multi-namespace devices?  You won't even get
your 4 streams for a handful namespaces with devices today or the near
future.  Should we cut down the number of streams per namespace?  Or
just not set aside streams for namespace exclusive use?  Or so far
you simply don't care about the multi-ns case?

> +static void nvme_write_hint_work(struct work_struct *work)
> +{
	struct nvme_ns *ns = container_of(work, struct nvme_ns, write_hint_work);

nasty 81 character line :)

> +	nr_streams = streams_per_ns;
> +	if (nr_streams > ns->ctrl->nssa)
> +		nr_streams = ns->ctrl->nssa;

min() ?

> +static enum rw_hint nvme_get_write_stream(struct nvme_ns *ns,
> +					   struct request *req)
> +{
> +	enum rw_hint streamid = req->cmd_flags & REQ_WRITE_LIFE_MASK;
> +
> +	if (req_op(req) != REQ_OP_WRITE ||
> +	    !(ns->ctrl->oacs & NVME_CTRL_OACS_DIRECTIVES))
> +		return WRITE_LIFE_NONE;

How about moving this to the caller?

> +	/*
> +	 * If we support streams and this request is a write with a valid
> +	 * hint, then flag it as such. If we haven't allocated streams on
> +	 * this ns before, do so lazily.
> +	 */
> +	stream = nvme_get_write_stream(ns, req);
> +	if (stream != WRITE_LIFE_NONE) {
> +		if (ns->nr_streams) {
> +			control |= NVME_RW_DTYPE_STREAMS;
> +			dsmgmt |= (stream << 16);
> +		} else
> +			nvme_configure_streams(ns);
> +	}

.. and instead pass control and dsmgmt to nvme_get_write_stream by
reference to isolate the functionality there.  And move the
nvme_configure_streams call into it as well.


Last but not least this will need some tweaks in the deallocate
code due to:

"If the host issues a Dataset Management command to deallocate logical
 blocks that are associated with a stream, it should specify a starting
 LBA and length that is aligned to and in multiples of the Stream
 Granularity Size"