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

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

[Andy Shevchenko]

There are methods to check card type. Let's use them instead of direct checking
type bits.

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 b992725..cd8edf4 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -1977,7 +1977,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

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

[Andy Shevchenko]

It's better to use strict_strtol() to convert user's input and strictly check
it. At least it forbids to interpret wrong input as a 0 and prevents to run all
tests.

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

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index cd8edf4..6bffb33 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -1937,9 +1937,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

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

[Andy Shevchenko]

TODO:
 - implement show() method based on seq_file API

Changes since v5:
 - we can't use BUG_ON at exit() method because it quite normal case when the
   module is going to be removed with card plugged in, that's why we just free
   memory there
 - rebase against recent linux-next tree

Changes since v4:
 - BUG_ON at exit if the list of the results isn't empty

Changes since v3:
 - fix multi-line commentary style
 - protect mmc_test_free_result() by mutex
 - apply known values to newly created mmc_test_general_result structure before
   attaching it to the list
 - call list_del() before free mmc_test_transfer_result structure
 - check truncated output in show() method, return -ENOBUFS if so
 - avoid deadlock in show() method if snprintf() fails

Changes since v2:
 - move card memeber to mmc_test_general_result structrure and thus throw away
   mmc_test_overall_result which was overkill
 - check result of kmalloc and kzalloc
 - keep pointer to the actual mmc_test_general_result structure in the
   mmc_test_card

Changes since v1:
 - structrure members are described
 - save transfer results code is split to separate function
 - in mmc_test_print_rate() use count = 1 instead of 0
 - mmc_test_result keeps overall results (across all tested cards), however
   it's still global variable
 - list_head variables inside structures have a suffix _lst now

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

We have the file created under sysfs already and we could use it to out test
results.

This patch applied on top of patches published here [1]

[1] http://lkml.org/lkml/2010/8/18/164

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

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 6bffb33..58d746b 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -17,6 +17,7 @@
 
 #include <linux/scatterlist.h>
 #include <linux/swap.h>		/* For nr_free_buffer_pages() */
+#include <linux/list.h>
 
 #define RESULT_OK		0
 #define RESULT_FAIL		1
@@ -73,12 +74,45 @@ struct mmc_test_area {
 };
 
 /**
+ * struct mmc_test_transfer_result - transfer results for performance tests.
+ * @link: double-linked list
+ * @count: amount of group of sectors to check
+ * @sectors: amount of sectors to check in one group
+ * @ts: time values of transfer
+ * @rate: calculated transfer rate
+ */
+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 - results for tests.
+ * @link: double-linked list
+ * @card: card under test
+ * @testcase: number of test case
+ * @result: result of test run
+ * @tr_lst: transfer measurements if any as mmc_test_transfer_result
+ */
+struct mmc_test_general_result {
+	struct list_head link;
+	struct mmc_card *card;
+	int testcase;
+	int result;
+	struct list_head tr_lst;
+};
+
+/**
  * struct mmc_test_card - test information.
  * @card: card under test
  * @scratch: transfer buffer
  * @buffer: transfer buffer
  * @highmem: buffer for highmem tests
  * @area: information for performance tests
+ * @gr: pointer to results of current testcase
  */
 struct mmc_test_card {
 	struct mmc_card	*card;
@@ -88,7 +122,8 @@ struct mmc_test_card {
 #ifdef CONFIG_HIGHMEM
 	struct page	*highmem;
 #endif
-	struct mmc_test_area area;
+	struct mmc_test_area		area;
+	struct mmc_test_general_result	*gr;
 };
 
 /*******************************************************************/
@@ -421,6 +456,30 @@ static unsigned int mmc_test_rate(uint64_t bytes, struct timespec *ts)
 }
 
 /*
+ * Save transfer results for future usage
+ */
+static void mmc_test_save_transfer_result(struct mmc_test_card *test,
+	unsigned int count, unsigned int sectors, struct timespec ts,
+	unsigned int rate)
+{
+	struct mmc_test_transfer_result *tr;
+
+	if (!test->gr)
+		return;
+
+	tr = kmalloc(sizeof(struct mmc_test_transfer_result), GFP_KERNEL);
+	if (!tr)
+		return;
+
+	tr->count = count;
+	tr->sectors = sectors;
+	tr->ts = ts;
+	tr->rate = rate;
+
+	list_add_tail(&tr->link, &test->gr->tr_lst);
+}
+
+/*
  * Print the transfer rate.
  */
 static void mmc_test_print_rate(struct mmc_test_card *test, uint64_t bytes,
@@ -438,6 +497,8 @@ 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);
+
+	mmc_test_save_transfer_result(test, 1, sectors, ts, rate);
 }
 
 /*
@@ -461,6 +522,8 @@ 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);
+
+	mmc_test_save_transfer_result(test, count, sectors, ts, rate);
 }
 
 /*
@@ -1853,6 +1916,8 @@ static const struct mmc_test_case mmc_test_cases[] = {
 
 static DEFINE_MUTEX(mmc_test_lock);
 
+static LIST_HEAD(mmc_test_result);
+
 static void mmc_test_run(struct mmc_test_card *test, int testcase)
 {
 	int i, ret;
@@ -1863,6 +1928,8 @@ static void mmc_test_run(struct mmc_test_card *test, int testcase)
 	mmc_claim_host(test->card->host);
 
 	for (i = 0;i < ARRAY_SIZE(mmc_test_cases);i++) {
+		struct mmc_test_general_result *gr;
+
 		if (testcase && ((i + 1) != testcase))
 			continue;
 
@@ -1881,6 +1948,25 @@ static void mmc_test_run(struct mmc_test_card *test, int testcase)
 			}
 		}
 
+		gr = kzalloc(sizeof(struct mmc_test_general_result),
+			GFP_KERNEL);
+		if (gr) {
+			INIT_LIST_HEAD(&gr->tr_lst);
+
+			/* Assign data what we know already */
+			gr->card = test->card;
+			gr->testcase = i;
+
+			/* Append container to global one */
+			list_add_tail(&gr->link, &mmc_test_result);
+
+			/*
+			 * Save the pointer to created container in our private
+			 * structure.
+			 */
+			test->gr = gr;
+		}
+
 		ret = mmc_test_cases[i].run(test);
 		switch (ret) {
 		case RESULT_OK:
@@ -1906,6 +1992,10 @@ static void mmc_test_run(struct mmc_test_card *test, int testcase)
 				mmc_hostname(test->card->host), ret);
 		}
 
+		/* Save the result */
+		if (gr)
+			gr->result = ret;
+
 		if (mmc_test_cases[i].cleanup) {
 			ret = mmc_test_cases[i].cleanup(test);
 			if (ret) {
@@ -1923,13 +2013,80 @@ static void mmc_test_run(struct mmc_test_card *test, int testcase)
 		mmc_hostname(test->card->host));
 }
 
+static void mmc_test_free_result(struct mmc_card *card)
+{
+	struct mmc_test_general_result *gr, *grs;
+
+	mutex_lock(&mmc_test_lock);
+
+	list_for_each_entry_safe(gr, grs, &mmc_test_result, link) {
+		struct mmc_test_transfer_result *tr, *trs;
+
+		if (card && gr->card != card)
+			continue;
+
+		list_for_each_entry_safe(tr, trs, &gr->tr_lst, link) {
+			list_del(&tr->link);
+			kfree(tr);
+		}
+
+		list_del(&gr->link);
+		kfree(gr);
+	}
+
+	mutex_unlock(&mmc_test_lock);
+}
+
 static ssize_t mmc_test_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {
+	struct mmc_card *card = mmc_dev_to_card(dev);
+	struct mmc_test_general_result *gr;
+	char *p = buf;
+	size_t len = PAGE_SIZE;
+	int ret;
+
 	mutex_lock(&mmc_test_lock);
+
+	list_for_each_entry(gr, &mmc_test_result, link) {
+		struct mmc_test_transfer_result *tr;
+
+		if (gr->card != card)
+			continue;
+
+		ret = snprintf(p, len, "Test %d: %d\n", gr->testcase + 1,
+			gr->result);
+		if (ret < 0)
+			goto err;
+		if (ret >= len) {
+			ret = -ENOBUFS;
+			goto err;
+		}
+		p += ret;
+		len -= ret;
+
+		list_for_each_entry(tr, &gr->tr_lst, link) {
+			ret = snprintf(p, len, "%u %d %lu.%09lu %u\n",
+				tr->count, tr->sectors,
+				(unsigned long)tr->ts.tv_sec,
+				(unsigned long)tr->ts.tv_nsec,
+				tr->rate);
+			if (ret < 0)
+				goto err;
+			if (ret >= len) {
+				ret = -ENOBUFS;
+				goto err;
+			}
+			p += ret;
+			len -= ret;
+		}
+	}
+
+	ret = PAGE_SIZE - len;
+err:
 	mutex_unlock(&mmc_test_lock);
 
-	return 0;
+	return ret;
 }
 
 static ssize_t mmc_test_store(struct device *dev,
@@ -1946,6 +2103,12 @@ static ssize_t mmc_test_store(struct device *dev,
 	if (!test)
 		return -ENOMEM;
 
+	/*
+	 * Remove all test cases associated with given card. Thus we have only
+	 * actual data of the last run.
+	 */
+	mmc_test_free_result(card);
+
 	test->card = card;
 
 	test->buffer = kzalloc(BUFFER_SIZE, GFP_KERNEL);
@@ -1992,6 +2155,7 @@ static int mmc_test_probe(struct mmc_card *card)
 
 static void mmc_test_remove(struct mmc_card *card)
 {
+	mmc_test_free_result(card);
 	device_remove_file(&card->dev, &dev_attr_test);
 }
 
@@ -2010,6 +2174,9 @@ static int __init mmc_test_init(void)
 
 static void __exit mmc_test_exit(void)
 {
+	/* Clear stalled data if card is still plugged */
+	mmc_test_free_result(NULL);
+
 	mmc_unregister_driver(&mmc_driver);
 }
 
-- 
1.6.3.3

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

[Andrew Morton]

On Tue,  7 Sep 2010 15:35:26 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> TODO:
>  - implement show() method based on seq_file API
> 
> Changes since v5:
>  - we can't use BUG_ON at exit() method because it quite normal case when the
>    module is going to be removed with card plugged in, that's why we just free
>    memory there
>  - rebase against recent linux-next tree
> 
> Changes since v4:
>  - BUG_ON at exit if the list of the results isn't empty
> 
> Changes since v3:
>  - fix multi-line commentary style
>  - protect mmc_test_free_result() by mutex
>  - apply known values to newly created mmc_test_general_result structure before
>    attaching it to the list
>  - call list_del() before free mmc_test_transfer_result structure
>  - check truncated output in show() method, return -ENOBUFS if so
>  - avoid deadlock in show() method if snprintf() fails
> 
> Changes since v2:
>  - move card memeber to mmc_test_general_result structrure and thus throw away
>    mmc_test_overall_result which was overkill
>  - check result of kmalloc and kzalloc
>  - keep pointer to the actual mmc_test_general_result structure in the
>    mmc_test_card
> 
> Changes since v1:
>  - structrure members are described
>  - save transfer results code is split to separate function
>  - in mmc_test_print_rate() use count = 1 instead of 0
>  - mmc_test_result keeps overall results (across all tested cards), however
>    it's still global variable
>  - list_head variables inside structures have a suffix _lst now

While useful and important, none of the above is relevant for a
mainline commit (IMP) so I always remove it.

> Here is a patch which brings possibility to get test results via sysfs. It
> helps to do tests non-interactively.
>
> We have the file created under sysfs already and we could use it to out test
> results.

That's the patch changelog.

> This patch applied on top of patches published here [1]
> 
> [1] http://lkml.org/lkml/2010/8/18/164

And that's irrelevant for a mainline commit.


So what we end up with is extremely thin.  Something about adding
something to sysfs.

This is not enough!  You're proposing an addition to the kernel->user
ABI.  Please fully describe this interface so that we can understand
and review it.  What are the names of these sysfs files?  What do they
do?  Provide us with example output in the changelog so we can see for
ourselves.

Please consider documenting the thing in a permanent documentation
file.  (I don't believe that Documentation/ABI/ is appropriate, given
mmc_test's scope).


afacit the only useful info we have about mmc_test is

        help
          Development driver that performs a series of reads and writes
          to a memory card in order to expose certain well known bugs
          in host controllers. The tests are executed by writing to the
          "test" file in sysfs under each card. Note that whatever is
          on your card will be overwritten by these tests.

          This driver is only of interest to those developing or
          testing a host driver. Most people should say N here.

which I guess is good enough, given the smallness and sophistication of
its users.  It could be improved on though!

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

[Chris Ball]

On Tue, Sep 07, 2010 at 03:27:53PM -0700, Andrew Morton wrote:
> So what we end up with is extremely thin.  Something about adding
> something to sysfs.
> 
> This is not enough!  You're proposing an addition to the kernel->user
> ABI.  Please fully describe this interface so that we can understand
> and review it.  What are the names of these sysfs files?  What do they
> do?  Provide us with example output in the changelog so we can see for
> ourselves.

Hoping Andy doesn't mind me jumping in, here's an attempt at a better
changelog:

---

Prior to this patch, the "test" file under each card's sysfs node was
write-only, and results were obtained by looking at dmesg.  This patch
improves programmatic access to the test results, making them available
by reading back from the same "test" file:

[root@host mmc0:e624]# echo 6 > test
[root@host mmc0:e624]# cat test
Test 6: 2


> Please consider documenting the thing in a permanent documentation
> file.  (I don't believe that Documentation/ABI/ is appropriate, given
> mmc_test's scope).

I think we should do this by modifying the Kconfig text as well:

diff --git a/drivers/mmc/card/Kconfig b/drivers/mmc/card/Kconfig
index 3f2a912..ddd7e42 100644
--- a/drivers/mmc/card/Kconfig
+++ b/drivers/mmc/card/Kconfig
@@ -45,8 +45,9 @@ config MMC_TEST
 	  Development driver that performs a series of reads and writes
 	  to a memory card in order to expose certain well known bugs
 	  in host controllers. The tests are executed by writing to the
-	  "test" file in sysfs under each card. Note that whatever is
-	  on your card will be overwritten by these tests.
+	  "test" file in sysfs under each card, and results can be read
+	  back from the same file. Note that whatever is on your card
+	  will be overwritten by these tests.
 
 	  This driver is only of interest to those developing or
 	  testing a host driver. Most people should say N here.

Separately, Andy, I think we should translate the general result return
code for the user, i.e. print OK/FAIL/UNSUP_HOST/UNSUP_CARD for return
values 0-3.  Would you mind adding that?  (Unfortunately, it invalidates
the usage example I just gave above, so that should change too..)

Thanks,

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

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

[Andy Shevchenko]

On Wed, Sep 8, 2010 at 1:58 AM, Chris Ball <cjb@laptop.org> wrote:
> Hoping Andy doesn't mind me jumping in, here's an attempt at a better
> changelog:
Thanks. I will take into account this.

> Prior to this patch, the "test" file under each card's sysfs node was
> write-only, and results were obtained by looking at dmesg.  This patch
> improves programmatic access to the test results, making them available
> by reading back from the same "test" file:
>
> [root@host mmc0:e624]# echo 6 > test
> [root@host mmc0:e624]# cat test
> Test 6: 2
It even better to get this sample for performance tests as well.

> I think we should do this by modifying the Kconfig text as well:
At least. And add documentation and link to it in Kconfig.

> Separately, Andy, I think we should translate the general result return
> code for the user, i.e. print OK/FAIL/UNSUP_HOST/UNSUP_CARD for return
> values 0-3.  Would you mind adding that?  (Unfortunately, it invalidates
> the usage example I just gave above, so that should change too..)
I thought about it. But for the scripts may be better to keep numeric
value as well.
I will think again hot to implement better.

Thanks.

-- 
With Best Regards,
Andy Shevchenko

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

[Greg KH]

On Tue, Sep 07, 2010 at 11:58:43PM +0100, Chris Ball wrote:
> On Tue, Sep 07, 2010 at 03:27:53PM -0700, Andrew Morton wrote:
> > So what we end up with is extremely thin.  Something about adding
> > something to sysfs.
> > 
> > This is not enough!  You're proposing an addition to the kernel->user
> > ABI.  Please fully describe this interface so that we can understand
> > and review it.  What are the names of these sysfs files?  What do they
> > do?  Provide us with example output in the changelog so we can see for
> > ourselves.
> 
> Hoping Andy doesn't mind me jumping in, here's an attempt at a better
> changelog:
> 
> ---
> 
> Prior to this patch, the "test" file under each card's sysfs node was
> write-only, and results were obtained by looking at dmesg.  This patch
> improves programmatic access to the test results, making them available
> by reading back from the same "test" file:
> 
> [root@host mmc0:e624]# echo 6 > test
> [root@host mmc0:e624]# cat test
> Test 6: 2

Ick, no.

Why is this in sysfs at all anyway?

Why not put it in debugfs?

And for every sysfs file, you need to have a Documenation/ABI/ entry.

Remember, sysfs is "one value per file".  Does that work with this file?
At first glance, it doesn't look like it.

Please use debugfs instead.

thanks,

greg k-h

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

[Andy Shevchenko]

On Thu, Sep 9, 2010 at 6:59 AM, Greg KH <greg@kroah.com> wrote:
> Why is this in sysfs at all anyway?
I don't know. The author of module should be asked.

> Why not put it in debugfs?
I thought about this. I even have somewhere a draft patch.

> And for every sysfs file, you need to have a Documenation/ABI/ entry.
>
> Remember, sysfs is "one value per file".  Does that work with this file?
> At first glance, it doesn't look like it.
>
> Please use debugfs instead.
I guess to optimize a work I may do another patch which shifts from
sysfs to debugfs.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Wed, Sep 8, 2010 at 1:27 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> Here is a patch which brings possibility to get test results via sysfs. It
>> helps to do tests non-interactively.
>>
>> We have the file created under sysfs already and we could use it to out test
>> results.
>
> So what we end up with is extremely thin.  Something about adding
> something to sysfs.
Yeah, agree with you. How to proceed with commit messages?
Should I make newer version of this certain patch with updated commit message?

> This is not enough!  You're proposing an addition to the kernel->user
> ABI.  Please fully describe this interface so that we can understand
> and review it.  What are the names of these sysfs files?  What do they
> do?
The sysfs file already exists for me as for feature contributor. I
think I could issue separate
patch to update documentation  part.

> Provide us with example output in the changelog so we can see for
> ourselves.
Will do.

> Please consider documenting the thing in a permanent documentation
> file.  (I don't believe that Documentation/ABI/ is appropriate, given
> mmc_test's scope).
Adrian pointed me once to some standalone documentation about this stuff.
I will talk to him how we could use this and apply to kernel.

> afacit the only useful info we have about mmc_test is
>
>        help
>          Development driver that performs a series of reads and writes
>          to a memory card in order to expose certain well known bugs
>          in host controllers. The tests are executed by writing to the
>          "test" file in sysfs under each card. Note that whatever is
>          on your card will be overwritten by these tests.
>
>          This driver is only of interest to those developing or
>          testing a host driver. Most people should say N here.
>
> which I guess is good enough, given the smallness and sophistication of
> its users.  It could be improved on though!
Ok.
Thanks.

-- 
With Best Regards,
Andy Shevchenko

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

[Andrew Morton]

On Wed, 8 Sep 2010 12:08:58 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, Sep 8, 2010 at 1:27 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> >> Here is a patch which brings possibility to get test results via sysfs. It
> >> helps to do tests non-interactively.
> >>
> >> We have the file created under sysfs already and we could use it to out test
> >> results.
> >
> > So what we end up with is extremely thin. __Something about adding
> > something to sysfs.
> Yeah, agree with you. How to proceed with commit messages?
> Should I make newer version of this certain patch with updated commit message?
> 

I used the words which Chris sent:


: Make it possibile to get test results via sysfs.  It helps to do tests
: non-interactively.
: 
: We have the file created under sysfs already and we could use it to out test
: results.
: 
: Prior to this patch, the "test" file under each card's sysfs node was
: write-only, and results were obtained by looking at dmesg.  This patch
: improves programmatic access to the test results, making them available by
: reading back from the same "test" file:
: 
: [root@host mmc0:e624]# echo 6 > test
: [root@host mmc0:e624]# cat test
: Test 6: 2
: 
: [cjb@laptop.org: changelog improvements]
: Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
: Cc: Chris Ball <cjb@laptop.org>
: Cc: <linux-mmc@vger.kernel.org>

which is good enough.  You can send me replacement changelog text any
old time - it's not problem.  I'm forever editing changelogs, mainly
adding acked-by's etc.  

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

[hong zhang]

Andy,

Would you please give any command to check card type?

---henry

--- On Tue, 9/7/10, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Subject: [PATCHv6 3/3] mmc_test: collect data and show it via sysfs by demand
> To: linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
> Cc: "Hunter Adrian" <adrian.hunter@nokia.com>, "Chris Ball" <cjb@laptop.org>, "Andrew Morton" <akpm@linux-foundation.org>, "Andy Shevchenko" <ext-andriy.shevchenko@nokia.com>
> Date: Tuesday, September 7, 2010, 7:35 AM
> TODO:
>  - implement show() method based on seq_file API
> 
> Changes since v5:
>  - we can't use BUG_ON at exit() method because it quite
> normal case when the
>    module is going to be removed with card
> plugged in, that's why we just free
>    memory there
>  - rebase against recent linux-next tree
> 
> Changes since v4:
>  - BUG_ON at exit if the list of the results isn't empty
> 
> Changes since v3:
>  - fix multi-line commentary style
>  - protect mmc_test_free_result() by mutex
>  - apply known values to newly created
> mmc_test_general_result structure before
>    attaching it to the list
>  - call list_del() before free mmc_test_transfer_result
> structure
>  - check truncated output in show() method, return -ENOBUFS
> if so
>  - avoid deadlock in show() method if snprintf() fails
> 
> Changes since v2:
>  - move card memeber to mmc_test_general_result structrure
> and thus throw away
>    mmc_test_overall_result which was
> overkill
>  - check result of kmalloc and kzalloc
>  - keep pointer to the actual mmc_test_general_result
> structure in the
>    mmc_test_card
> 
> Changes since v1:
>  - structrure members are described
>  - save transfer results code is split to separate
> function
>  - in mmc_test_print_rate() use count = 1 instead of 0
>  - mmc_test_result keeps overall results (across all tested
> cards), however
>    it's still global variable
>  - list_head variables inside structures have a suffix _lst
> now
> 
> Here is a patch which brings possibility to get test
> results via sysfs. It
> helps to do tests non-interactively.
> 
> We have the file created under sysfs already and we could
> use it to out test
> results.
> 
> This patch applied on top of patches published here [1]
> 
> [1] http://lkml.org/lkml/2010/8/18/164
> 
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
> ---
>  drivers/mmc/card/mmc_test.c |  171
> ++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 169 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/card/mmc_test.c
> b/drivers/mmc/card/mmc_test.c
> index 6bffb33..58d746b 100644
> --- a/drivers/mmc/card/mmc_test.c
> +++ b/drivers/mmc/card/mmc_test.c
> @@ -17,6 +17,7 @@
>  
>  #include <linux/scatterlist.h>
>  #include <linux/swap.h>   
>     /* For nr_free_buffer_pages() */
> +#include <linux/list.h>
>  
>  #define RESULT_OK        0
>  #define RESULT_FAIL       
> 1
> @@ -73,12 +74,45 @@ struct mmc_test_area {
>  };
>  
>  /**
> + * struct mmc_test_transfer_result - transfer results for
> performance tests.
> + * @link: double-linked list
> + * @count: amount of group of sectors to check
> + * @sectors: amount of sectors to check in one group
> + * @ts: time values of transfer
> + * @rate: calculated transfer rate
> + */
> +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 - results for tests.
> + * @link: double-linked list
> + * @card: card under test
> + * @testcase: number of test case
> + * @result: result of test run
> + * @tr_lst: transfer measurements if any as
> mmc_test_transfer_result
> + */
> +struct mmc_test_general_result {
> +    struct list_head link;
> +    struct mmc_card *card;
> +    int testcase;
> +    int result;
> +    struct list_head tr_lst;
> +};
> +
> +/**
>   * struct mmc_test_card - test information.
>   * @card: card under test
>   * @scratch: transfer buffer
>   * @buffer: transfer buffer
>   * @highmem: buffer for highmem tests
>   * @area: information for performance tests
> + * @gr: pointer to results of current testcase
>   */
>  struct mmc_test_card {
>      struct mmc_card   
> *card;
> @@ -88,7 +122,8 @@ struct mmc_test_card {
>  #ifdef CONFIG_HIGHMEM
>      struct page   
> *highmem;
>  #endif
> -    struct mmc_test_area area;
> +    struct mmc_test_area   
>     area;
> +    struct
> mmc_test_general_result    *gr;
>  };
>  
> 
> /*******************************************************************/
> @@ -421,6 +456,30 @@ static unsigned int
> mmc_test_rate(uint64_t bytes, struct timespec *ts)
>  }
>  
>  /*
> + * Save transfer results for future usage
> + */
> +static void mmc_test_save_transfer_result(struct
> mmc_test_card *test,
> +    unsigned int count, unsigned int
> sectors, struct timespec ts,
> +    unsigned int rate)
> +{
> +    struct mmc_test_transfer_result *tr;
> +
> +    if (!test->gr)
> +        return;
> +
> +    tr = kmalloc(sizeof(struct
> mmc_test_transfer_result), GFP_KERNEL);
> +    if (!tr)
> +        return;
> +
> +    tr->count = count;
> +    tr->sectors = sectors;
> +    tr->ts = ts;
> +    tr->rate = rate;
> +
> +    list_add_tail(&tr->link,
> &test->gr->tr_lst);
> +}
> +
> +/*
>   * Print the transfer rate.
>   */
>  static void mmc_test_print_rate(struct mmc_test_card
> *test, uint64_t bytes,
> @@ -438,6 +497,8 @@ 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);
> +
> +    mmc_test_save_transfer_result(test, 1,
> sectors, ts, rate);
>  }
>  
>  /*
> @@ -461,6 +522,8 @@ 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);
> +
> +    mmc_test_save_transfer_result(test,
> count, sectors, ts, rate);
>  }
>  
>  /*
> @@ -1853,6 +1916,8 @@ static const struct mmc_test_case
> mmc_test_cases[] = {
>  
>  static DEFINE_MUTEX(mmc_test_lock);
>  
> +static LIST_HEAD(mmc_test_result);
> +
>  static void mmc_test_run(struct mmc_test_card *test, int
> testcase)
>  {
>      int i, ret;
> @@ -1863,6 +1928,8 @@ static void mmc_test_run(struct
> mmc_test_card *test, int testcase)
>     
> mmc_claim_host(test->card->host);
>  
>      for (i = 0;i <
> ARRAY_SIZE(mmc_test_cases);i++) {
> +        struct
> mmc_test_general_result *gr;
> +
>          if (testcase
> && ((i + 1) != testcase))
>             
> continue;
>  
> @@ -1881,6 +1948,25 @@ static void mmc_test_run(struct
> mmc_test_card *test, int testcase)
>             
> }
>          }
>  
> +        gr =
> kzalloc(sizeof(struct mmc_test_general_result),
> +           
> GFP_KERNEL);
> +        if (gr) {
> +           
> INIT_LIST_HEAD(&gr->tr_lst);
> +
> +           
> /* Assign data what we know already */
> +           
> gr->card = test->card;
> +           
> gr->testcase = i;
> +
> +           
> /* Append container to global one */
> +           
> list_add_tail(&gr->link, &mmc_test_result);
> +
> +           
> /*
> +       
>      * Save the pointer to created
> container in our private
> +       
>      * structure.
> +       
>      */
> +           
> test->gr = gr;
> +        }
> +
>          ret =
> mmc_test_cases[i].run(test);
>          switch (ret) {
>          case RESULT_OK:
> @@ -1906,6 +1992,10 @@ static void mmc_test_run(struct
> mmc_test_card *test, int testcase)
>             
>     mmc_hostname(test->card->host),
> ret);
>          }
>  
> +        /* Save the result
> */
> +        if (gr)
> +           
> gr->result = ret;
> +
>          if
> (mmc_test_cases[i].cleanup) {
>             
> ret = mmc_test_cases[i].cleanup(test);
>             
> if (ret) {
> @@ -1923,13 +2013,80 @@ static void mmc_test_run(struct
> mmc_test_card *test, int testcase)
>         
> mmc_hostname(test->card->host));
>  }
>  
> +static void mmc_test_free_result(struct mmc_card *card)
> +{
> +    struct mmc_test_general_result *gr,
> *grs;
> +
> +    mutex_lock(&mmc_test_lock);
> +
> +    list_for_each_entry_safe(gr, grs,
> &mmc_test_result, link) {
> +        struct
> mmc_test_transfer_result *tr, *trs;
> +
> +        if (card &&
> gr->card != card)
> +           
> continue;
> +
> +       
> list_for_each_entry_safe(tr, trs, &gr->tr_lst, link)
> {
> +           
> list_del(&tr->link);
> +           
> kfree(tr);
> +        }
> +
> +       
> list_del(&gr->link);
> +        kfree(gr);
> +    }
> +
> +    mutex_unlock(&mmc_test_lock);
> +}
> +
>  static ssize_t mmc_test_show(struct device *dev,
>      struct device_attribute *attr, char
> *buf)
>  {
> +    struct mmc_card *card =
> mmc_dev_to_card(dev);
> +    struct mmc_test_general_result *gr;
> +    char *p = buf;
> +    size_t len = PAGE_SIZE;
> +    int ret;
> +
>      mutex_lock(&mmc_test_lock);
> +
> +    list_for_each_entry(gr,
> &mmc_test_result, link) {
> +        struct
> mmc_test_transfer_result *tr;
> +
> +        if (gr->card !=
> card)
> +           
> continue;
> +
> +        ret = snprintf(p,
> len, "Test %d: %d\n", gr->testcase + 1,
> +           
> gr->result);
> +        if (ret < 0)
> +           
> goto err;
> +        if (ret >= len)
> {
> +           
> ret = -ENOBUFS;
> +           
> goto err;
> +        }
> +        p += ret;
> +        len -= ret;
> +
> +       
> list_for_each_entry(tr, &gr->tr_lst, link) {
> +           
> ret = snprintf(p, len, "%u %d %lu.%09lu %u\n",
> +           
>     tr->count, tr->sectors,
> +           
>     (unsigned long)tr->ts.tv_sec,
> +           
>     (unsigned long)tr->ts.tv_nsec,
> +           
>     tr->rate);
> +           
> if (ret < 0)
> +           
>     goto err;
> +           
> if (ret >= len) {
> +           
>     ret = -ENOBUFS;
> +           
>     goto err;
> +           
> }
> +            p
> += ret;
> +           
> len -= ret;
> +        }
> +    }
> +
> +    ret = PAGE_SIZE - len;
> +err:
>      mutex_unlock(&mmc_test_lock);
>  
> -    return 0;
> +    return ret;
>  }
>  
>  static ssize_t mmc_test_store(struct device *dev,
> @@ -1946,6 +2103,12 @@ static ssize_t mmc_test_store(struct
> device *dev,
>      if (!test)
>          return -ENOMEM;
>  
> +    /*
> +     * Remove all test cases
> associated with given card. Thus we have only
> +     * actual data of the last
> run.
> +     */
> +    mmc_test_free_result(card);
> +
>      test->card = card;
>  
>      test->buffer = kzalloc(BUFFER_SIZE,
> GFP_KERNEL);
> @@ -1992,6 +2155,7 @@ static int mmc_test_probe(struct
> mmc_card *card)
>  
>  static void mmc_test_remove(struct mmc_card *card)
>  {
> +    mmc_test_free_result(card);
>      device_remove_file(&card->dev,
> &dev_attr_test);
>  }
>  
> @@ -2010,6 +2174,9 @@ static int __init
> mmc_test_init(void)
>  
>  static void __exit mmc_test_exit(void)
>  {
> +    /* Clear stalled data if card is still
> plugged */
> +    mmc_test_free_result(NULL);
> +
>     
> mmc_unregister_driver(&mmc_driver);
>  }
>  
> -- 
> 1.6.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>