[PATCH] mmc: print message if a card supports secure erase/trim

[PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

It's an interesting detail, so inform the user about it.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 drivers/mmc/core/bus.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 8a1f124..4a60028 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -347,6 +347,10 @@ int mmc_add_card(struct mmc_card *card)
 			uhs_bus_speed_mode, type, card->rca);
 	}
 
+	if (mmc_can_secure_erase_trim(card))
+		pr_info("%s: card supports secure erase/trim\n",
+			mmc_hostname(card->host));
+
 #ifdef CONFIG_DEBUG_FS
 	mmc_add_card_debugfs(card);
 #endif
-- 
2.1.0

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Richard Weinberger]

On Tue, Jan 27, 2015 at 12:48 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> It's an interesting detail, so inform the user about it.
>
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
> ---
>  drivers/mmc/core/bus.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 8a1f124..4a60028 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -347,6 +347,10 @@ int mmc_add_card(struct mmc_card *card)
>                         uhs_bus_speed_mode, type, card->rca);
>         }
>
> +       if (mmc_can_secure_erase_trim(card))
> +               pr_info("%s: card supports secure erase/trim\n",
> +                       mmc_hostname(card->host));
> +

Please make this a sysfs attribute.
Exposing such properties as log messages is horrible.

-- 
Thanks,
//richard

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 12:55 schrieb Richard Weinberger:
> On Tue, Jan 27, 2015 at 12:48 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>> It's an interesting detail, so inform the user about it.
>>
>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>> ---
>>   drivers/mmc/core/bus.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>> index 8a1f124..4a60028 100644
>> --- a/drivers/mmc/core/bus.c
>> +++ b/drivers/mmc/core/bus.c
>> @@ -347,6 +347,10 @@ int mmc_add_card(struct mmc_card *card)
>>                          uhs_bus_speed_mode, type, card->rca);
>>          }
>>
>> +       if (mmc_can_secure_erase_trim(card))
>> +               pr_info("%s: card supports secure erase/trim\n",
>> +                       mmc_hostname(card->host));
>> +
>
> Please make this a sysfs attribute.
> Exposing such properties as log messages is horrible.

Look at the source at the message which is printed just before and 
decide which one you find more informational / useful.

Alexander Holler

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Richard Weinberger]

Am 27.01.2015 um 13:02 schrieb Alexander Holler:
> Am 27.01.2015 um 12:55 schrieb Richard Weinberger:
>> On Tue, Jan 27, 2015 at 12:48 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>>> It's an interesting detail, so inform the user about it.
>>>
>>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>>> ---
>>>   drivers/mmc/core/bus.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>>> index 8a1f124..4a60028 100644
>>> --- a/drivers/mmc/core/bus.c
>>> +++ b/drivers/mmc/core/bus.c
>>> @@ -347,6 +347,10 @@ int mmc_add_card(struct mmc_card *card)
>>>                          uhs_bus_speed_mode, type, card->rca);
>>>          }
>>>
>>> +       if (mmc_can_secure_erase_trim(card))
>>> +               pr_info("%s: card supports secure erase/trim\n",
>>> +                       mmc_hostname(card->host));
>>> +
>>
>> Please make this a sysfs attribute.
>> Exposing such properties as log messages is horrible.
> 
> Look at the source at the message which is printed just before and decide which one you find more informational / useful.

-ENOPARSE

Thanks,
//richard

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Borislav Petkov]

On Tue, Jan 27, 2015 at 01:02:49PM +0100, Alexander Holler wrote:
> Look at the source at the message which is printed just before and decide
> which one you find more informational / useful.

I find such a message absolutely useless. Also, if it flies off and
printk buffer gets overwritten, it serves you sh*t. IOW, listen to what
Richard tells you.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 13:08 schrieb Borislav Petkov:
> On Tue, Jan 27, 2015 at 01:02:49PM +0100, Alexander Holler wrote:
>> Look at the source at the message which is printed just before and decide
>> which one you find more informational / useful.
>
> I find such a message absolutely useless. Also, if it flies off and
> printk buffer gets overwritten, it serves you sh*t. IOW, listen to what
> Richard tells you.

Ok. I shut/give up.

Maybe you could be so nice to inform me where the similiar detail is 
exposed in sysfs for SSDs.

Also it's unlikely I will send any further patch. There are still 21 
from me where I've exposed sysfs attributes for NAND devices which are 
still not applied and most NAND devices still don't have a sysfs entry.

So why should I waste my time again?

Alexander Holler

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Arend van Spriel]

On 01/27/15 12:48, Alexander Holler wrote:
> It's an interesting detail, so inform the user about it.
>
> Signed-off-by: Alexander Holler<holler@ahsoftware.de>
> ---
>   drivers/mmc/core/bus.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 8a1f124..4a60028 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -347,6 +347,10 @@ int mmc_add_card(struct mmc_card *card)
>   			uhs_bus_speed_mode, type, card->rca);
>   	}
>
> +	if (mmc_can_secure_erase_trim(card))
> +		pr_info("%s: card supports secure erase/trim\n",
> +			mmc_hostname(card->host));
> +

If you want to expose this info you could also consider doing it in the 
debugfs folder that is created below.

>   #ifdef CONFIG_DEBUG_FS
>   	mmc_add_card_debugfs(card);
>   #endif

Regards,
Arend

[1] http://lxr.free-electrons.com/ident?i=mmc_add_card_debugfs

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Arend van Spriel]

On 01/27/15 12:48, Alexander Holler wrote:
> It's an interesting detail, so inform the user about it.
>
> Signed-off-by: Alexander Holler<holler@ahsoftware.de>
> ---
>   drivers/mmc/core/bus.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 8a1f124..4a60028 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -347,6 +347,10 @@ int mmc_add_card(struct mmc_card *card)
>   			uhs_bus_speed_mode, type, card->rca);
>   	}
>
> +	if (mmc_can_secure_erase_trim(card))
> +		pr_info("%s: card supports secure erase/trim\n",
> +			mmc_hostname(card->host));
> +

If you want to expose this info you could also consider doing it in the 
debugfs folder that is created below.

>   #ifdef CONFIG_DEBUG_FS
>   	mmc_add_card_debugfs(card);
>   #endif

Regards,
Arend

[1] http://lxr.free-electrons.com/ident?i=mmc_add_card_debugfs

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Richard Weinberger]

Am 27.01.2015 um 13:15 schrieb Alexander Holler:
> Am 27.01.2015 um 13:08 schrieb Borislav Petkov:
>> On Tue, Jan 27, 2015 at 01:02:49PM +0100, Alexander Holler wrote:
>>> Look at the source at the message which is printed just before and decide
>>> which one you find more informational / useful.
>>
>> I find such a message absolutely useless. Also, if it flies off and
>> printk buffer gets overwritten, it serves you sh*t. IOW, listen to what
>> Richard tells you.
> 
> Ok. I shut/give up.

There is no need to be sulky.

> Maybe you could be so nice to inform me where the similiar detail is exposed in sysfs for SSDs.

If it is not already exposed and you can come up with a sane use case send a patch.

> Also it's unlikely I will send any further patch. There are still 21 from me where I've exposed sysfs attributes for NAND devices which are still not applied and most NAND devices
> still don't have a sysfs entry.
> 
> So why should I waste my time again?

If you give up after the first review round there is nothing I can do for you.
Postings like https://lkml.org/lkml/2014/5/30/162 are also not really beneficial
to get a patch merged.

Thanks,
//richard

P.s. If you see every not-merged patch as a waste of time, please stop developing software as
it is obviously not an enjoyment for you.

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 13:31 schrieb Richard Weinberger:

>> Also it's unlikely I will send any further patch. There are still 21 from me where I've exposed sysfs attributes for NAND devices which are still not applied and most NAND devices
>> still don't have a sysfs entry.
>>
>> So why should I waste my time again?
>
> If you give up after the first review round there is nothing I can do for you.
> Postings like https://lkml.org/lkml/2014/5/30/162 are also not really beneficial
> to get a patch merged.

Yes. Sorry, but I had so many bad experiences with maintainers. They 
told me that using functions means hiding stuff, that I should bisect 
even when I was absolutely sure which patch was wrong (and everxyone 
could see it by just reading the source and by reading my comments, I've 
got blamed because I prefer a controlled BUG_ON() (which might even just 
kill the involved tasks, depending on config settings) instead of a 
desastrous memory corruption which (hard-) reseted the machine (and 
might have done even more bad things) and so on.

Regards,

Alexander Holler

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Borislav Petkov]

On Tue, Jan 27, 2015 at 01:44:15PM +0100, Alexander Holler wrote:
> Yes. Sorry, but I had so many bad experiences with maintainers.

Well, I can't comment on your experience for the simple reason that I
haven't followed those discussions. In their defence, though, you have
to know that those people get to review your code, apply it and then
deal with it long after you've gone on about doing other things.

So, even if you don't always agree with them, you need to realize that
those are the people responsible for that code and if they request a
reasonable change in your submission, you have to do it. It is that
simple.

 [ Unless you have a good argument against it but that's a different
   story and this is what is called the "review" process. ]

But this is how basically linux kernel development is done. It is not
about simply doing something but about doing it right. And as long as
you get *reasonable* change requests for your patch, you go and change
it.

And yes, not only *you* but *we* all have to redo our patches. And it
is not unheard of people working on a single feature for months if not
years, doing changes, doing changes again, testing variants and so on
and so on. Simply reading lkml would give you lots of examples.

And yet we still do it because it is simply great fun and one gets to
learn tons of new things when working with the community (don't listen
to all that bullshit about the kernel community being unfriendly - the
real problem there is most of the time somewhere else).

But instead of having fun, you're frustrated. And people should not be
frustrated because that is generally bad for their health. So maybe you
should sit down, take a quiet minute and think about what you actually
want to do. And if kernel development doesn't give you any fun, then
you're probably on the right track with not wanting to do it anymore.

If it were me, I'd stop doing it too if it is not fun anymore - life's
too short to be frustrated and not have fun. It is cheesy but it is
true.

I sincerely hope that helps in any way.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 15:21 schrieb Borislav Petkov:
> On Tue, Jan 27, 2015 at 01:44:15PM +0100, Alexander Holler wrote:
>> Yes. Sorry, but I had so many bad experiences with maintainers.
>
> Well, I can't comment on your experience for the simple reason that I
> haven't followed those discussions. In their defence, though, you have
> to know that those people get to review your code, apply it and then
> deal with it long after you've gone on about doing other things.
>
> So, even if you don't always agree with them, you need to realize that
> those are the people responsible for that code and if they request a
> reasonable change in your submission, you have to do it. It is that
> simple.

Totally wrong. I'm free to just stop discussion. That's the difference 
between the old type of os communites and corporates where people do 
what they've got instructed from above without discussion. I offered 
something and wasn't paid to do something.

It was worth a try but I don't have to build a framework to achieve 
something simple just because a maintainer requests it.

>   [ Unless you have a good argument against it but that's a different
>     story and this is what is called the "review" process. ]

I want to see the mentionend information and want that it is seen and do 
not want to hide it away deep in the sysfs where it actually must be 
searched.

Regards,

Alexander Holler

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Steven Rostedt]

On Tue, Jan 27, 2015 at 05:55:08PM +0100, Alexander Holler wrote:
> >
> >So, even if you don't always agree with them, you need to realize that
> >those are the people responsible for that code and if they request a
> >reasonable change in your submission, you have to do it. It is that
> >simple.
> 
> Totally wrong. I'm free to just stop discussion. That's the difference
> between the old type of os communites and corporates where people do what
> they've got instructed from above without discussion. I offered something
> and wasn't paid to do something.

Yes, you are free to stop this discussion and not do the changes that were
asked. That is your prerogative. If you don't want to do what the maintainer
asked you to do, you have every right to ignore it. The change will just
not go in.

The question is, does someone else want this change too? If not, then this
change will go away and be a footnote of lkml archives. If people would
like this change. The maintainer could easily add it the way he wants it
added. The reason maintainers usually do not do such things is because
it is more polite to have the original submitter do the change and get credit
in the change logs. If the maintainer does the work, the best the original
submitter could get is some mention in the change log. Some people care
about getting the commit credit, so maintainers try to respect that.

> 
> It was worth a try but I don't have to build a framework to achieve
> something simple just because a maintainer requests it.

You are again correct. But as Boris stated, it is the maintainer that must
maintain and defend the code. If someone asks why the code is done some way,
it is the maintainer that must respond. Saying, I just took someone's patch
is not acceptable. The maintainer is responsible for all code that they
maintain regardless if they wrote it or not.

> 
> >  [ Unless you have a good argument against it but that's a different
> >    story and this is what is called the "review" process. ]
> 
> I want to see the mentionend information and want that it is seen and do not
> want to hide it away deep in the sysfs where it actually must be searched.

For most people a message in dmesg is not very useful. What you are asking for
is to print a characteristic of a device. Something that someone might want to
check much later in boot, where, as Boris stated, the dmesg could have been
flushed (by systemd, which loves to write stuff to the kernel buffers), and
there's no way to find out this information. The print message is long gone.
Having a static location like sysfs is the proper place, because user space
tools can always access it.

Is this something a tool would like to find out? If so, parsing dmesg is not
the way to go. Looking it up in sysfs is.

You are free to end this discussion. We are not forcing you to. But it is
obvious that the maintainer does not agree with your approach, so you can
either write a patch that he agrees with or not. If you do not, then you
will not get the feature you want in the kernel. No big deal either way.

-- Steve

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 18:24 schrieb Steven Rostedt:

> For most people a message in dmesg is not very useful. What you are asking for
> is to print a characteristic of a device. Something that someone might want to
> check much later in boot, where, as Boris stated, the dmesg could have been
> flushed (by systemd, which loves to write stuff to the kernel buffers), and
> there's no way to find out this information. The print message is long gone.
> Having a static location like sysfs is the proper place, because user space
> tools can always access it.
>
> Is this something a tool would like to find out? If so, parsing dmesg is not
> the way to go. Looking it up in sysfs is.

Oh, systemd.

Anyway, I like(d) Linux because it didn't had a splash screen and used 
to spit out all types of information on the screen where it could be 
easily seen or found (in contrast other OS which try to hide all 
technical details from users).

Of course, times are changing, including the amount of stuff printed on 
screen. But I still find it much much easier to grep on the output of 
dmesg than to search through thousands files in sysfs. Even if that can 
be done with grep too (kind of). But it's much more complicated because 
grep doesn't connect the file name with the content, so you need more 
complicated stuff to combine both in order to search for and find 
something in sysfs.

Regards,

Alexander Holler

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Steven Rostedt]

On Tue, 27 Jan 2015 18:38:36 +0100
Alexander Holler <holler@ahsoftware.de> wrote:

> Anyway, I like(d) Linux because it didn't had a splash screen and used 
> to spit out all types of information on the screen where it could be 
> easily seen or found (in contrast other OS which try to hide all 
> technical details from users).

Yes, I like those days too, but as you say, times are changing, and we
must adapt.

> 
> Of course, times are changing, including the amount of stuff printed on 
> screen. But I still find it much much easier to grep on the output of 
> dmesg than to search through thousands files in sysfs. Even if that can 
> be done with grep too (kind of). But it's much more complicated because 
> grep doesn't connect the file name with the content, so you need more 
> complicated stuff to combine both in order to search for and find 
> something in sysfs.

Come on, it's not that more complex. If you know the name of the file,
just do:

find /sys -print -name <name> -exec cat {} \;

And you'll get the data you want.

Basically, what you are saying is "printk is more convenient for me and
I do not care about the other cases that make much more sense with
sysfs". The kernel does not work that way.

-- Steve

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Arend van Spriel]

On 01/27/15 18:38, Alexander Holler wrote:
> Am 27.01.2015 um 18:24 schrieb Steven Rostedt:
>
>> For most people a message in dmesg is not very useful. What you are
>> asking for
>> is to print a characteristic of a device. Something that someone might
>> want to
>> check much later in boot, where, as Boris stated, the dmesg could have
>> been
>> flushed (by systemd, which loves to write stuff to the kernel
>> buffers), and
>> there's no way to find out this information. The print message is long
>> gone.
>> Having a static location like sysfs is the proper place, because user
>> space
>> tools can always access it.
>>
>> Is this something a tool would like to find out? If so, parsing dmesg
>> is not
>> the way to go. Looking it up in sysfs is.
>
> Oh, systemd.
>
> Anyway, I like(d) Linux because it didn't had a splash screen and used
> to spit out all types of information on the screen where it could be
> easily seen or found (in contrast other OS which try to hide all
> technical details from users).

What ends up in kernel log is still a fraction of what is going on in 
the kernel.

> Of course, times are changing, including the amount of stuff printed on
> screen. But I still find it much much easier to grep on the output of
> dmesg than to search through thousands files in sysfs. Even if that can
> be done with grep too (kind of). But it's much more complicated because
> grep doesn't connect the file name with the content, so you need more
> complicated stuff to combine both in order to search for and find
> something in sysfs.

Ever used rgrep or grep -R. Anyway, if this is you use-case what about 
the gazillion other pieces of info in the kernel. When moving in that 
direction you can be sure dmesg will flush out.

Regards,
Arend

> Regards,
>
> Alexander Holler
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Arend van Spriel]

On 01/27/15 18:38, Alexander Holler wrote:
> Am 27.01.2015 um 18:24 schrieb Steven Rostedt:
>
>> For most people a message in dmesg is not very useful. What you are
>> asking for
>> is to print a characteristic of a device. Something that someone might
>> want to
>> check much later in boot, where, as Boris stated, the dmesg could have
>> been
>> flushed (by systemd, which loves to write stuff to the kernel
>> buffers), and
>> there's no way to find out this information. The print message is long
>> gone.
>> Having a static location like sysfs is the proper place, because user
>> space
>> tools can always access it.
>>
>> Is this something a tool would like to find out? If so, parsing dmesg
>> is not
>> the way to go. Looking it up in sysfs is.
>
> Oh, systemd.
>
> Anyway, I like(d) Linux because it didn't had a splash screen and used
> to spit out all types of information on the screen where it could be
> easily seen or found (in contrast other OS which try to hide all
> technical details from users).

What ends up in kernel log is still a fraction of what is going on in 
the kernel.

> Of course, times are changing, including the amount of stuff printed on
> screen. But I still find it much much easier to grep on the output of
> dmesg than to search through thousands files in sysfs. Even if that can
> be done with grep too (kind of). But it's much more complicated because
> grep doesn't connect the file name with the content, so you need more
> complicated stuff to combine both in order to search for and find
> something in sysfs.

Ever used rgrep or grep -R. Anyway, if this is you use-case what about 
the gazillion other pieces of info in the kernel. When moving in that 
direction you can be sure dmesg will flush out.

Regards,
Arend

> Regards,
>
> Alexander Holler
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 18:53 schrieb Arend van Spriel:

> Ever used rgrep or grep -R. Anyway, if this is you use-case what about
> the gazillion other pieces of info in the kernel. When moving in that
> direction you can be sure dmesg will flush out.

Sorry, never heard about that -R and wasn't aware that the kernel does a 
lot of things. ;)

I don't want that ftrace stuff appears in dmesg, but I like to do

dmesg | grep -i mmc (or whatever is of interest)

instead of

grep -Rsi mmc /sys/<a hundred tabs to search a matching direcory> or 
something like

find /sys -iname '*mmc*' -exec <more silly complications> \;

Alexander Holler

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Richard Weinberger]

Am 27.01.2015 um 19:04 schrieb Alexander Holler:
> Am 27.01.2015 um 18:53 schrieb Arend van Spriel:
> 
>> Ever used rgrep or grep -R. Anyway, if this is you use-case what about
>> the gazillion other pieces of info in the kernel. When moving in that
>> direction you can be sure dmesg will flush out.
> 
> Sorry, never heard about that -R and wasn't aware that the kernel does a lot of things. ;)
> 
> I don't want that ftrace stuff appears in dmesg, but I like to do
> 
> dmesg | grep -i mmc (or whatever is of interest)
> 
> instead of
> 
> grep -Rsi mmc /sys/<a hundred tabs to search a matching direcory> or something like
> 
> find /sys -iname '*mmc*' -exec <more silly complications> \;

You don't have to use grep or find.
Add a sysfs attribute, document it in Documentation/ABI and done.
By documenting stuff you'll make the world a better place.

Thanks,
//richard

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Borislav Petkov]

On Tue, Jan 27, 2015 at 07:04:13PM +0100, Alexander Holler wrote:
> dmesg | grep -i mmc (or whatever is of interest)

And what happens if someone has done "dmesg -c" before you?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 18:48 schrieb Steven Rostedt:
> On Tue, 27 Jan 2015 18:38:36 +0100
> Alexander Holler <holler@ahsoftware.de> wrote:
>
>> Anyway, I like(d) Linux because it didn't had a splash screen and used
>> to spit out all types of information on the screen where it could be
>> easily seen or found (in contrast other OS which try to hide all
>> technical details from users).
>
> Yes, I like those days too, but as you say, times are changing, and we
> must adapt.
>
>>
>> Of course, times are changing, including the amount of stuff printed on
>> screen. But I still find it much much easier to grep on the output of
>> dmesg than to search through thousands files in sysfs. Even if that can
>> be done with grep too (kind of). But it's much more complicated because
>> grep doesn't connect the file name with the content, so you need more
>> complicated stuff to combine both in order to search for and find
>> something in sysfs.
>
> Come on, it's not that more complex. If you know the name of the file,
> just do:
>
> find /sys -print -name <name> -exec cat {} \;
>
> And you'll get the data you want.
>
> Basically, what you are saying is "printk is more convenient for me and
> I do not care about the other cases that make much more sense with
> sysfs". The kernel does not work that way.

No. First I don't know the name of one of the thousands file in sysfs, 
just like I don't know all the possible kernel messages.

And second I still believe that KISS is the right way and frameworks 
aren't the right choice for everything.

But because I still don't refuse to learn, I will attach the output of

find /sys -type f -print -exec cat {} \;

to future bug reports instead of the output of dmesg.

Alexander Holler

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Steven Rostedt]

On Tue, 27 Jan 2015 19:13:29 +0100
Alexander Holler <holler@ahsoftware.de> wrote:

> > Basically, what you are saying is "printk is more convenient for me and
> > I do not care about the other cases that make much more sense with
> > sysfs". The kernel does not work that way.
> 
> No. First I don't know the name of one of the thousands file in sysfs, 
> just like I don't know all the possible kernel messages.

But as Richard mentioned, it should be documented in Documentation/ABI,
and the information you want should be right there. Once you know it,
you will know it for good.

> 
> And second I still believe that KISS is the right way and frameworks 
> aren't the right choice for everything.

But having tools parse dmesg is not KISS. It's backwards. /sys
filesystem is really simple to use. It's not that difficult. And it was
created for just this purpose!

> 
> But because I still don't refuse to learn, I will attach the output of
> 
> find /sys -type f -print -exec cat {} \;
> 
> to future bug reports instead of the output of dmesg.
> 

That is completely different. dmesg will contain back traces which are
not in /sys filesystem, nor do they belong there. dmesg also shows
order of events, which may be needed in debugging, again, sysfs is
about properties and states of devices. How would sysfs be used for
debugging?

You want to add a way to tell a property of a device. That is *exactly*
what sysfs was made for. Not dmesg.

-- Steve

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 19:21 schrieb Steven Rostedt:

> You want to add a way to tell a property of a device. That is *exactly*
> what sysfs was made for. Not dmesg.

Err, dmesg existed long before sysfs.

Anyway, it's ok. You're all right and I should request project 
sponsoring for offering a simple piece of (imho) interesting information.

Happy frameworking and sorry for disturbing.

Alexander Holler

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Steven Rostedt]

On Tue, 27 Jan 2015 19:33:52 +0100
Alexander Holler <holler@ahsoftware.de> wrote:

> Am 27.01.2015 um 19:21 schrieb Steven Rostedt:
> 
> > You want to add a way to tell a property of a device. That is *exactly*
> > what sysfs was made for. Not dmesg.
> 
> Err, dmesg existed long before sysfs.

Yep, and dmesg shows you what is happening in the kernel. sysfs shows
you the state and properties of the kernel (or devices). That's what it
was created for, because dmesg wasn't the place to put it.

> 
> Anyway, it's ok. You're all right and I should request project 
> sponsoring for offering a simple piece of (imho) interesting information.

I'm sure if it's interesting enough, someone will eventually add it.

Hey, if you end up doing it, it could help with your resume.

> 
> Happy frameworking and sorry for disturbing.

No problem. Enjoy,

-- Steve

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Joe Perches]

On Tue, 2015-01-27 at 13:06 +0100, Richard Weinberger wrote:
> Am 27.01.2015 um 13:02 schrieb Alexander Holler:
> > Am 27.01.2015 um 12:55 schrieb Richard Weinberger:
> >> On Tue, Jan 27, 2015 at 12:48 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> >>> It's an interesting detail, so inform the user about it.
[]
> >>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
[]
> >>> @@ -347,6 +347,10 @@ int mmc_add_card(struct mmc_card *card)
> >>>                          uhs_bus_speed_mode, type, card->rca);
> >>>          }
> >>>
> >>> +       if (mmc_can_secure_erase_trim(card))
> >>> +               pr_info("%s: card supports secure erase/trim\n",
> >>> +                       mmc_hostname(card->host));
> >>> +
> >>
> >> Please make this a sysfs attribute.
> >> Exposing such properties as log messages is horrible.
> > 
> > Look at the source at the message which is printed just before and decide which one you find more informational / useful.
> 
> -ENOPARSE

Because you didn't look at the code?

Alexander, while I kind of agree with Richard about various
properties being rightfully exposed in sysfs, it might have
made more short-term sense to extend the original line instead
of printing another new one.

Something like:
---
 drivers/mmc/core/bus.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index c5ef100..e282780 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -259,6 +259,7 @@ int mmc_add_card(struct mmc_card *card)
 	int ret;
 	const char *type;
 	const char *uhs_bus_speed_mode = "";
+	const char *secure_trim_erase = "";
 	static const char *const uhs_speeds[] = {
 		[UHS_SDR12_BUS_SPEED] = "SDR12 ",
 		[UHS_SDR25_BUS_SPEED] = "SDR25 ",
@@ -300,21 +301,26 @@ int mmc_add_card(struct mmc_card *card)
 		(card->sd_bus_speed < ARRAY_SIZE(uhs_speeds)))
 		uhs_bus_speed_mode = uhs_speeds[card->sd_bus_speed];
 
+	if (mmc_can_secure_erase_trim(card))
+		secure_trim_erase = " (secure erase/trim)";
+
 	if (mmc_host_is_spi(card->host)) {
-		pr_info("%s: new %s%s%s card on SPI\n",
+		pr_info("%s: new %s%s%s%s card on SPI\n",
 			mmc_hostname(card->host),
 			mmc_card_hs(card) ? "high speed " : "",
 			mmc_card_ddr52(card) ? "DDR " : "",
-			type);
+			type,
+			secure_trim_erase);
 	} else {
-		pr_info("%s: new %s%s%s%s%s card at address %04x\n",
+		pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
 			mmc_hostname(card->host),
 			mmc_card_uhs(card) ? "ultra high speed " :
 			(mmc_card_hs(card) ? "high speed " : ""),
 			mmc_card_hs400(card) ? "HS400 " :
 			(mmc_card_hs200(card) ? "HS200 " : ""),
 			mmc_card_ddr52(card) ? "DDR " : "",
-			uhs_bus_speed_mode, type, card->rca);
+			uhs_bus_speed_mode, type, secure_trim_erase,
+			card->rca);
 	}
 
 #ifdef CONFIG_DEBUG_FS

Re: [PATCH] mmc: print message if a card supports secure erase/trim

[Alexander Holler]

Am 27.01.2015 um 19:42 schrieb Steven Rostedt:

> Hey, if you end up doing it, it could help with your resume.

Oh, young padovan, I'm writing SW since 30a and it just would make it 
even longer, which already seems to be a problem for many HR people.

Alexander Holer