[PATCH 0/3] mmc_test: show test results via sysfs

[PATCH 0/3] mmc_test: show test results via sysfs

[Andy Shevchenko]

There is a patchset which brings possibility to get test results via sysfs. It
helps to do tests non-interactively.

Andy Shevchenko (3):
  mmc_test: use API to check card type
  mmc_test: change simple_strtol() to strict_strtol()
  mmc_test: collect data and show it via sysfs by demand

 drivers/mmc/card/mmc_test.c |  130 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 122 insertions(+), 8 deletions(-)

[PATCH 1/3] mmc_test: use API to check card type

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 drivers/mmc/card/mmc_test.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 7aaa4b2..359fae9 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -1960,7 +1960,7 @@ static int mmc_test_probe(struct mmc_card *card)
 {
 	int ret;
 
-	if ((card->type != MMC_TYPE_MMC) && (card->type != MMC_TYPE_SD))
+	if (!mmc_card_mmc(card) && !mmc_card_sd(card))
 		return -ENODEV;
 
 	ret = device_create_file(&card->dev, &dev_attr_test);
-- 
1.6.3.3

[PATCH 2/3] mmc_test: change simple_strtol() to strict_strtol()

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 drivers/mmc/card/mmc_test.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 359fae9..5423ac9 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -1836,9 +1836,10 @@ static const struct mmc_test_case mmc_test_cases[] = {
 
 static DEFINE_MUTEX(mmc_test_lock);
 
-static void mmc_test_run(struct mmc_test_card *test, int testcase)
+static void mmc_test_run(struct mmc_test_card *test, long testcase)
 {
-	int i, ret;
+	long i;
+	int ret;
 
 	printk(KERN_INFO "%s: Starting tests of card %s...\n",
 		mmc_hostname(test->card->host), mmc_card_id(test->card));
@@ -1849,7 +1850,7 @@ static void mmc_test_run(struct mmc_test_card *test, int testcase)
 		if (testcase && ((i + 1) != testcase))
 			continue;
 
-		printk(KERN_INFO "%s: Test case %d. %s...\n",
+		printk(KERN_INFO "%s: Test case %ld. %s...\n",
 			mmc_hostname(test->card->host), i + 1,
 			mmc_test_cases[i].name);
 
@@ -1920,9 +1921,10 @@ static ssize_t mmc_test_store(struct device *dev,
 {
 	struct mmc_card *card = mmc_dev_to_card(dev);
 	struct mmc_test_card *test;
-	int testcase;
+	long testcase;
 
-	testcase = simple_strtol(buf, NULL, 10);
+	if (strict_strtol(buf, 10, &testcase))
+		return -EINVAL;
 
 	test = kzalloc(sizeof(struct mmc_test_card), GFP_KERNEL);
 	if (!test)
-- 
1.6.3.3

[PATCH 3/3] mmc_test: collect data and show it via sysfs by demand

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 drivers/mmc/card/mmc_test.c |  116 ++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 5423ac9..3cfef37 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -16,6 +16,7 @@
 
 #include <linux/scatterlist.h>
 #include <linux/swap.h>
+#include <linux/list.h>
 
 #define RESULT_OK		0
 #define RESULT_FAIL		1
@@ -66,6 +67,28 @@ struct mmc_test_area {
 };
 
 /**
+ * struct mmc_test_transfer_result - container of the transfer results for
+ * performance tests.
+ */
+struct mmc_test_transfer_result {
+	struct list_head link;
+	unsigned int count;
+	unsigned int sectors;
+	struct timespec ts;
+	unsigned int rate;
+};
+
+/**
+ * struct mmc_test_general_result - container of the results for tests.
+ */
+struct mmc_test_general_result {
+	struct list_head link;
+	int testcase;
+	int result;
+	struct list_head transfer;
+};
+
+/**
  * struct mmc_test_card - test information.
  * @card: card under test
  * @scratch: transfer buffer
@@ -81,7 +104,8 @@ struct mmc_test_card {
 #ifdef CONFIG_HIGHMEM
 	struct page	*highmem;
 #endif
-	struct mmc_test_area area;
+	struct mmc_test_area	area;
+	struct list_head	*result;
 };
 
 /*******************************************************************/
@@ -420,6 +444,10 @@ static void mmc_test_print_rate(struct mmc_test_card *test, uint64_t bytes,
 {
 	unsigned int rate, sectors = bytes >> 9;
 	struct timespec ts;
+	struct mmc_test_transfer_result *transfer;
+	struct mmc_test_general_result *result =
+		list_entry(test->result->prev, struct mmc_test_general_result,
+			link);
 
 	ts = timespec_sub(*ts2, *ts1);
 
@@ -430,6 +458,15 @@ static void mmc_test_print_rate(struct mmc_test_card *test, uint64_t bytes,
 			 mmc_hostname(test->card->host), sectors, sectors >> 1,
 			 (sectors == 1 ? ".5" : ""), (unsigned long)ts.tv_sec,
 			 (unsigned long)ts.tv_nsec, rate / 1000, rate / 1024);
+
+	transfer = kmalloc(sizeof(struct mmc_test_transfer_result), GFP_KERNEL);
+
+	transfer->count = 0;
+	transfer->sectors = sectors;
+	memcpy(&transfer->ts, &ts, sizeof(struct timespec));
+	transfer->rate = rate;
+
+	list_add_tail(&transfer->link, &result->transfer);
 }
 
 /*
@@ -442,6 +479,10 @@ static void mmc_test_print_avg_rate(struct mmc_test_card *test, uint64_t bytes,
 	unsigned int rate, sectors = bytes >> 9;
 	uint64_t tot = bytes * count;
 	struct timespec ts;
+	struct mmc_test_transfer_result *transfer;
+	struct mmc_test_general_result *result =
+		list_entry(test->result->prev, struct mmc_test_general_result,
+			link);
 
 	ts = timespec_sub(*ts2, *ts1);
 
@@ -453,6 +494,15 @@ static void mmc_test_print_avg_rate(struct mmc_test_card *test, uint64_t bytes,
 			 sectors >> 1, (sectors == 1 ? ".5" : ""),
 			 (unsigned long)ts.tv_sec, (unsigned long)ts.tv_nsec,
 			 rate / 1000, rate / 1024);
+
+	transfer = kmalloc(sizeof(struct mmc_test_transfer_result), GFP_KERNEL);
+
+	transfer->count = count;
+	transfer->sectors = sectors;
+	memcpy(&transfer->ts, &ts, sizeof(struct timespec));
+	transfer->rate = rate;
+
+	list_add_tail(&transfer->link, &result->transfer);
 }
 
 /*
@@ -1847,6 +1897,8 @@ static void mmc_test_run(struct mmc_test_card *test, long testcase)
 	mmc_claim_host(test->card->host);
 
 	for (i = 0;i < ARRAY_SIZE(mmc_test_cases);i++) {
+		struct mmc_test_general_result *result;
+
 		if (testcase && ((i + 1) != testcase))
 			continue;
 
@@ -1865,6 +1917,11 @@ static void mmc_test_run(struct mmc_test_card *test, long testcase)
 			}
 		}
 
+		result = kzalloc(sizeof(struct mmc_test_general_result),
+			GFP_KERNEL);
+		INIT_LIST_HEAD(&result->transfer);
+		list_add_tail(&result->link, test->result);
+
 		ret = mmc_test_cases[i].run(test);
 		switch (ret) {
 		case RESULT_OK:
@@ -1890,6 +1947,10 @@ static void mmc_test_run(struct mmc_test_card *test, long testcase)
 				mmc_hostname(test->card->host), ret);
 		}
 
+		/* Save the result */
+		result->testcase = i;
+		result->result = ret;
+
 		if (mmc_test_cases[i].cleanup) {
 			ret = mmc_test_cases[i].cleanup(test);
 			if (ret) {
@@ -1907,13 +1968,57 @@ static void mmc_test_run(struct mmc_test_card *test, long testcase)
 		mmc_hostname(test->card->host));
 }
 
+static struct list_head mmc_test_result;
+
+static void mmc_test_result_free(struct list_head *result)
+{
+	struct mmc_test_general_result *gr, *grs;
+
+	list_for_each_entry_safe(gr, grs, result, link) {
+		struct mmc_test_transfer_result *tr, *trs;
+
+		list_for_each_entry_safe(tr, trs, &gr->transfer, link)
+			kfree(tr);
+		kfree(gr);
+	}
+}
+
 static ssize_t mmc_test_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {
+	struct mmc_test_general_result *result;
+	char *p = buf;
+	size_t len = PAGE_SIZE;
+
 	mutex_lock(&mmc_test_lock);
+
+	list_for_each_entry(result, &mmc_test_result, link) {
+		struct mmc_test_transfer_result *transfer;
+		int ret;
+
+		ret = snprintf(p, len, "Test %d: %d\n", result->testcase + 1,
+			result->result);
+		if (ret < 0)
+			return ret;
+		p += ret;
+		len -= ret;
+
+		list_for_each_entry(transfer, &result->transfer, link) {
+			ret = snprintf(p, len, "%u %d %lu.%09lu %u\n",
+				transfer->count, transfer->sectors,
+				(unsigned long)transfer->ts.tv_sec,
+				(unsigned long)transfer->ts.tv_nsec,
+				transfer->rate);
+			if (ret < 0)
+				return ret;
+			p += ret;
+			len -= ret;
+		}
+	}
+
 	mutex_unlock(&mmc_test_lock);
 
-	return 0;
+	return PAGE_SIZE - len;
 }
 
 static ssize_t mmc_test_store(struct device *dev,
@@ -1932,6 +2037,10 @@ static ssize_t mmc_test_store(struct device *dev,
 
 	test->card = card;
 
+	mmc_test_result_free(&mmc_test_result);
+	INIT_LIST_HEAD(&mmc_test_result);
+	test->result = &mmc_test_result;
+
 	test->buffer = kzalloc(BUFFER_SIZE, GFP_KERNEL);
 #ifdef CONFIG_HIGHMEM
 	test->highmem = alloc_pages(GFP_KERNEL | __GFP_HIGHMEM, BUFFER_ORDER);
@@ -1969,6 +2078,8 @@ static int mmc_test_probe(struct mmc_card *card)
 	if (ret)
 		return ret;
 
+	INIT_LIST_HEAD(&mmc_test_result);
+
 	dev_info(&card->dev, "Card claimed for testing.\n");
 
 	return 0;
@@ -1976,6 +2087,7 @@ static int mmc_test_probe(struct mmc_card *card)
 
 static void mmc_test_remove(struct mmc_card *card)
 {
+	mmc_test_result_free(&mmc_test_result);
 	device_remove_file(&card->dev, &dev_attr_test);
 }
 
-- 
1.6.3.3

Re: [PATCH 2/3] mmc_test: change simple_strtol() to strict_strtol()

[Roger Quadros]

Hi,

On 09/01/2010 09:26 AM, ext Andy Shevchenko wrote:

No patch summary?
> Signed-off-by: Andy Shevchenko<ext-andriy.shevchenko@nokia.com>
> ---
>   drivers/mmc/card/mmc_test.c |   12 +++++++-----
>   1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
> index 359fae9..5423ac9 100644
> --- a/drivers/mmc/card/mmc_test.c
> +++ b/drivers/mmc/card/mmc_test.c
> @@ -1836,9 +1836,10 @@ static const struct mmc_test_case mmc_test_cases[] = {
>
>   static DEFINE_MUTEX(mmc_test_lock);
>
> -static void mmc_test_run(struct mmc_test_card *test, int testcase)
> +static void mmc_test_run(struct mmc_test_card *test, long testcase)
>   {
> -	int i, ret;
> +	long i;

Why this change? isn't unsigned int sufficient for the mmc test cases?

> +	int ret;
>
>   	printk(KERN_INFO "%s: Starting tests of card %s...\n",
>   		mmc_hostname(test->card->host), mmc_card_id(test->card));
> @@ -1849,7 +1850,7 @@ static void mmc_test_run(struct mmc_test_card *test, int testcase)
>   		if (testcase&&  ((i + 1) != testcase))
>   			continue;
>
> -		printk(KERN_INFO "%s: Test case %d. %s...\n",
> +		printk(KERN_INFO "%s: Test case %ld. %s...\n",
>   			mmc_hostname(test->card->host), i + 1,
>   			mmc_test_cases[i].name);
>
> @@ -1920,9 +1921,10 @@ static ssize_t mmc_test_store(struct device *dev,
>   {
>   	struct mmc_card *card = mmc_dev_to_card(dev);
>   	struct mmc_test_card *test;
> -	int testcase;
> +	long testcase;
>
> -	testcase = simple_strtol(buf, NULL, 10);
> +	if (strict_strtol(buf, 10,&testcase))
space required                    ^ here

> +		return -EINVAL;
>
>   	test = kzalloc(sizeof(struct mmc_test_card), GFP_KERNEL);
>   	if (!test)


-- 
regards,
-roger

Re: [PATCH 2/3] mmc_test: change simple_strtol() to strict_strtol()

[Roger Quadros]

Hi,

On 09/01/2010 09:26 AM, ext Andy Shevchenko wrote:

No patch summary?
> Signed-off-by: Andy Shevchenko<ext-andriy.shevchenko@nokia.com>
> ---
>   drivers/mmc/card/mmc_test.c |   12 +++++++-----
>   1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
> index 359fae9..5423ac9 100644
> --- a/drivers/mmc/card/mmc_test.c
> +++ b/drivers/mmc/card/mmc_test.c
> @@ -1836,9 +1836,10 @@ static const struct mmc_test_case mmc_test_cases[] = {
>
>   static DEFINE_MUTEX(mmc_test_lock);
>
> -static void mmc_test_run(struct mmc_test_card *test, int testcase)
> +static void mmc_test_run(struct mmc_test_card *test, long testcase)
>   {
> -	int i, ret;
> +	long i;

Why this change? isn't unsigned int sufficient for the mmc test cases?

> +	int ret;
>
>   	printk(KERN_INFO "%s: Starting tests of card %s...\n",
>   		mmc_hostname(test->card->host), mmc_card_id(test->card));
> @@ -1849,7 +1850,7 @@ static void mmc_test_run(struct mmc_test_card *test, int testcase)
>   		if (testcase&&  ((i + 1) != testcase))
>   			continue;
>
> -		printk(KERN_INFO "%s: Test case %d. %s...\n",
> +		printk(KERN_INFO "%s: Test case %ld. %s...\n",
>   			mmc_hostname(test->card->host), i + 1,
>   			mmc_test_cases[i].name);
>
> @@ -1920,9 +1921,10 @@ static ssize_t mmc_test_store(struct device *dev,
>   {
>   	struct mmc_card *card = mmc_dev_to_card(dev);
>   	struct mmc_test_card *test;
> -	int testcase;
> +	long testcase;
>
> -	testcase = simple_strtol(buf, NULL, 10);
> +	if (strict_strtol(buf, 10,&testcase))
space required                    ^ here

> +		return -EINVAL;
>
>   	test = kzalloc(sizeof(struct mmc_test_card), GFP_KERNEL);
>   	if (!test)


-- 
regards,
-roger

Re: [PATCH 2/3] mmc_test: change simple_strtol() to strict_strtol()

[Andy Shevchenko]

On Wed, 2010-09-01 at 10:25 +0200, Quadros Roger (Nokia-MS/Helsinki)
wrote:

> No patch summary?
Will be in next version.

> > -static void mmc_test_run(struct mmc_test_card *test, int testcase)
> > +static void mmc_test_run(struct mmc_test_card *test, long testcase)
> >   {
> > -	int i, ret;
> > +	long i;
> 
> Why this change? isn't unsigned int sufficient for the mmc test cases?
You are right. This certain change is redundant. I'll skip it.

> > -	testcase = simple_strtol(buf, NULL, 10);
> > +	if (strict_strtol(buf, 10,&testcase))
> space required                    ^ here
It's odd. I have space in my patch.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] mmc_test: show test results via sysfs

[Chris Ball]

Hi Andy,

On Wed, Sep 01, 2010 at 09:26:44AM +0300, Andy Shevchenko wrote:
> There is a patchset which brings possibility to get test results via sysfs. It
> helps to do tests non-interactively.

Thanks for the patchset.  It doesn't apply to 2.6.36-rc3 without some
(trivial) merge conflicts -- could you rebase with your next version?

Tested-by: Chris Ball <cjb@laptop.org>

Unrelated to this patch, I'm hitting:

  drivers/mmc/core/core.c:174:
    BUG_ON(mrq->data->blocks * mrq->data->blksz > host->max_req_size);

when running the transfer tests under 2.6.36-rc3.  The request that
fires the BUG() has blocks=8192 * blksz=512 > max_req_size=524288,
on a Ricoh Co Ltd R5C822/1180:0822 in a Dell M6300 laptop.  Anyone
have any experience with that bug, before I dig deeper?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 0/3] mmc_test: show test results via sysfs

[Chris Ball]

On Wed, Sep 01, 2010 at 10:46:44PM +0100, Chris Ball wrote:
> Unrelated to this patch, I'm hitting:
> 
>   drivers/mmc/core/core.c:174:
>     BUG_ON(mrq->data->blocks * mrq->data->blksz > host->max_req_size);

Ah, I see this was discussed on linux-mmc@, and Adrian's going to come
up with a patch for it.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child