Re: [PATCH 3/5] nvme.h: add Write Zeroes definitions

[chaitany kulkarni]

I'll resend entire series with v2 prefix and add whitespaces fix along
with other comments.

On Thu, Nov 17, 2016 at 2:30 AM, Christoph Hellwig <h...@infradead.org> wrote:
> On Tue, Nov 15, 2016 at 11:06:58PM -0800, chaitany kulkarni wrote:
>> Add the command structure, optional command set support (ONCS) bit and
>> a new error code for the Write Zeroes command.
>>
>> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulka...@hgst.com>
>
> Unlike the other patches this one looks white space damaged by a mail
> client (and it also arrived out of sequence).
>
> With the whitespaces fixed up this looks good:
>
> Reviewed-by: Christoph Hellwig <h...@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/5] block: add support for REQ_OP_WRITE_ZEROES

[chaitany kulkarni]

Sounds good, I'll update the whole series and resend it with v2 prefix.



On Thu, Nov 17, 2016 at 11:46 PM, Christoph Hellwig  wrote:
> On Thu, Nov 17, 2016 at 02:17:11PM -0800, Chaitanya Kulkarni wrote:
>> From: Chaitanya Kulkarni 
>>
>> This adds a new block layer operation to zero out a range of
>> LBAs. This allows to implement zeroing for devices that don't use
>> either discard with a predictable zero pattern or WRITE SAME of zeroes.
>> The prominent example of that is NVMe with the Write Zeroes command,
>> but in the future this should also help with improving the way
>> zeroing discards work.
>>
>> Signed-off-by: Chaitanya Kulkarni 
>
> I think you'll need to resend the whole series so that nvme can set
> the maximum discard sectors value.
>
>> @@ -575,9 +575,10 @@ static inline bool wbt_should_throttle(struct rq_wb 
>> *rwb, struct bio *bio)
>>   const int op = bio_op(bio);
>>
>>   /*
>> -  * If not a WRITE (or a discard), do nothing
>> +  * If not a WRITE (or a discard or write zeroes), do nothing
>>*/
>> - if (!(op == REQ_OP_WRITE || op == REQ_OP_DISCARD))
>> + if (!(op == REQ_OP_WRITE || op == REQ_OP_DISCARD ||
>> + op == REQ_OP_WRITE_ZEROES))
>>   return false;
>
> Jens: should we really throttle for discard or write zeroes here?
> Those aren't really writeback driven..
>
>> +static inline unsigned int bdev_write_zeroes(struct block_device *bdev)
>> +{
>> + struct request_queue *q = bdev_get_queue(bdev);
>> +
>> + if (q)
>> + return q->limits.max_write_zeroes_sectors;
>> +
>> + return 0;
>
> If this returns a sector value I'd name it bdev_write_zeroes_sectors.
>
> Otherwise this looks great.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] block: add support for REQ_OP_WRITE_ZEROES

[chaitany kulkarni]

Incorporated the comments and sent new patch.

On Wed, Nov 16, 2016 at 6:48 PM, Martin K. Petersen
 wrote:
>> "Keith" == Keith Busch  writes:
>
> Keith> Your maximum bi_size exceeds the 2-bytes an NVMe Write Zeroes
> Keith> command provides for the block count. Instead of having a simple
> Keith> queue flag for write zeroes support, have it take a max sectors
> Keith> value instead.
>
> Yes, please. Make it a queue limit and make sure it is properly stacked.
>
> The rest looks sensible to me. I'll wire up the SCSI pieces.
>
> --
> Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Unittest framework based on nvme-cli.

[chaitany kulkarni]

Hi,

Introducing nvme-cli based unit test framework. All the test cases use
commands implemented in nvme-cli.

The goal is to have simple, lightweight, and easily expandable
framework which we can used to
develop various categories of unit tests based on nvme-cli and improve
overall development.

Over the period of time since release of the nvme-cli various test
cases are developed which are now integral part of our device deriver
testing.

These test cases are evolved around nvme-cli and can be used for
nvme-cli testing.

I would like to take this opportunity and share first set of test
cases which has most frequently used generic NVMe features from cli :-

1. nvme_attach_detach_ns_test.py
2. nvme_compare_test.py
3. nvme_create_max_ns_test.py
4. nvme_error_log_test.py
5. nvme_flush_test.py
6. nvme_format_test.py
7. nvme_get_features_test.py
8. nvme_read_write_test.py
9. nvme_smart_log_test.py
10. nvme_writeuncor_test.py
11. nvme_writezeros_test.py

Please have a look at README for an overview and process of adding new
test case.
Framework also has a sample skeleton which can be used readily to
write new testcard.

Assumptions for current implementation :-
1. nvme-cli is already installed on the system.
2. Only one test case can be executed at any given time.
3. Each test case has logical PRE, RUN and POST sections.
4. It assumes that driver is loaded and default namespace
"/dev/nvme0n1" is present.

(It is easy to add driver load/unload in pre and post sections of test
cases in as per requirement.)

I’d like to know what features, test cases most people would want as a
part of this framework.
Any suggestions are welcome, I'd like to implement them.

On approval I would like to submit more test cases to enhance the framework.

Regards,
Chaitanya

On Mon, Oct 31, 2016 at 11:01 PM, Chaitanya Kulkarni
 wrote:
>
> From: Chaitanya Kulkarni 
>
> Signed-off-by: Chaitanya Kulkarni 
> ---
>  Makefile|   5 +-
>  tests/Makefile  |  48 +
>  tests/README|  84 
>  tests/TODO  |  14 ++
>  tests/config.json   |   5 +
>  tests/nvme_attach_detach_ns_test.py |  90 
>  tests/nvme_compare_test.py  |  79 
>  tests/nvme_create_max_ns_test.py|  97 +
>  tests/nvme_error_log_test.py|  86 
>  tests/nvme_flush_test.py|  61 ++
>  tests/nvme_format_test.py   | 145 +
>  tests/nvme_get_features_test.py | 103 ++
>  tests/nvme_read_write_test.py   |  72 +++
>  tests/nvme_simple_template_test.py  |  55 +
>  tests/nvme_smart_log_test.py|  86 
>  tests/nvme_test.py  | 395 
> 
>  tests/nvme_test_io.py   |  99 +
>  tests/nvme_test_logger.py   |  52 +
>  tests/nvme_writeuncor_test.py   |  76 +++
>  tests/nvme_writezeros_test.py   | 102 ++
>  20 files changed, 1753 insertions(+), 1 deletion(-)
>  create mode 100644 tests/Makefile
>  create mode 100644 tests/README
>  create mode 100644 tests/TODO
>  create mode 100644 tests/config.json
>  create mode 100644 tests/nvme_attach_detach_ns_test.py
>  create mode 100644 tests/nvme_compare_test.py
>  create mode 100644 tests/nvme_create_max_ns_test.py
>  create mode 100644 tests/nvme_error_log_test.py
>  create mode 100644 tests/nvme_flush_test.py
>  create mode 100644 tests/nvme_format_test.py
>  create mode 100644 tests/nvme_get_features_test.py
>  create mode 100644 tests/nvme_read_write_test.py
>  create mode 100644 tests/nvme_simple_template_test.py
>  create mode 100644 tests/nvme_smart_log_test.py
>  create mode 100644 tests/nvme_test.py
>  create mode 100644 tests/nvme_test_io.py
>  create mode 100644 tests/nvme_test_logger.py
>  create mode 100644 tests/nvme_writeuncor_test.py
>  create mode 100644 tests/nvme_writezeros_test.py
>
> diff --git a/Makefile b/Makefile
> index 117cbbe..33c7190 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -46,6 +46,9 @@ nvme.o: nvme.c nvme.h nvme-print.h nvme-ioctl.h argconfig.h 
> suffix.h nvme-lightn
>  doc: $(NVME)
> $(MAKE) -C Documentation
>
> +test:
> +   $(MAKE) -C tests/ run
> +
>  all: doc
>
>  clean:
> @@ -136,4 +139,4 @@ rpm: dist
> $(RPMBUILD) -ta nvme-$(NVME_VERSION).tar.gz
>
>  .PHONY: default doc all clean clobber install-man install-bin install
> -.PHONY: dist pkg dist-orig deb deb-light rpm FORCE
> +.PHONY: dist pkg dist-orig deb deb-light rpm FORCE test
> diff --git a/tests/Makefile b/tests/Makefile
> new file mode 100644
> index 000..c0f9f31
> --- /dev/null
> +++ b/tests/Makefile
> @@ -0,0 +1,48 @@
> +###
> +#
> +#Makefile : Allows user to run testcases, generate documentation, and
> +#   perform static