Re: [PATCH v3 0/2] CIFS: Info-level log support, print message when attempting mount.

2018-10-07 Thread Rodrigo Freire
Thanks Steve! Sorry for overlooking it, whoops.

One more question; what’s your/community opinion on rewriting the existing 
pr_notice to the new cifs_info?

I could happily retrofit it.

Have a great week!

- RF
Sent from a mobile device

> On 7 Oct 2018, at 15:59, Steve French  wrote:
> 
> Merged into cifs-2.6.git for-next
> 
> Made a trivial tab/space correction in the patch (pointed out by
> checkpatch) and then added a trivial followon patch to address a
> comment/style (trivial)
> issue pointed out by checkpatch and to add a little more detailed
> comments about generally when to use each debug function.  If any
> objections let me know.
> 
> 
>> On Sun, Oct 7, 2018 at 10:21 AM Rodrigo Freire  wrote:
>> 
>> Hi Steve,
>> From our conversation over v2, I came out with this v3 patch, which I broke
>> in two commits:
>> 
>> * The first commit in cifs_debug.h, creating the cifs_info() function.
>>  - The aim of this commit is to allow to the developer to be able to print
>>informational-level data without having to use pr_info, pr_notice etc,
>>in line with other filesystems.
>>. One interesting and noteworthy feature of cifs_info() is that it is
>>  transparent to the CIFS_DEBUG config state, either in "y" or "n".
>>. Also, by using KERN_INFO level, it can be safely filtered by a
>>  maintainer / administrator, without cluttering their log monitors,
>>  since this is a low level alert.
>>  - I took the liberty to not add it inside the existing pr_debug, because
>>of the eventual need of some developer to print stuff that should be
>>printed not only in CIFS_DEBUG mode (as there are plenty of pr_notice
>>scattered over the code).
>>  - Also, this is not a debug but a info message, so i liked cifs_info()
>>more ;-)
>>  - I saw plenty of pr_notice() in CIFS code, but I resisted the urge to
>>convert them to cifs_info().
>> 
>> * The second commit contains the code printing a cifs_info() when attempting
>>  a CIFS mount operation.
>> 
>> Appreciate your review.
>> 
>> V3: Created a new cifs_info() function, moved the mount attempt message to
>>cifs_info
>> 
>> V2: Created a loop to select the right cifs_dbg message to be printed,
>>considering the current system's scenario, in order to avoid a
>>duplicate message or stripping out important information in
>>debug.
>> 
>> Rodrigo Freire (2):
>>  CIFS: Adds information-level logging function
>>  CIFS: Print message when attempting a mount
>> 
>> fs/cifs/cifs_debug.h | 16 
>> fs/cifs/cifsfs.c |  7 ++-
>> fs/cifs/transport.c  |  2 +-
>> 3 files changed, 23 insertions(+), 2 deletions(-)
>> 
>> --
>> 1.8.3.1
>> 
> 
> 
> -- 
> Thanks,
> 
> Steve
> <0001-cifs-minor-clarification-in-comments.patch>


Re: [PATCH v3 0/2] CIFS: Info-level log support, print message when attempting mount.

2018-10-07 Thread Rodrigo Freire
Thanks Steve! Sorry for overlooking it, whoops.

One more question; what’s your/community opinion on rewriting the existing 
pr_notice to the new cifs_info?

I could happily retrofit it.

Have a great week!

- RF
Sent from a mobile device

> On 7 Oct 2018, at 15:59, Steve French  wrote:
> 
> Merged into cifs-2.6.git for-next
> 
> Made a trivial tab/space correction in the patch (pointed out by
> checkpatch) and then added a trivial followon patch to address a
> comment/style (trivial)
> issue pointed out by checkpatch and to add a little more detailed
> comments about generally when to use each debug function.  If any
> objections let me know.
> 
> 
>> On Sun, Oct 7, 2018 at 10:21 AM Rodrigo Freire  wrote:
>> 
>> Hi Steve,
>> From our conversation over v2, I came out with this v3 patch, which I broke
>> in two commits:
>> 
>> * The first commit in cifs_debug.h, creating the cifs_info() function.
>>  - The aim of this commit is to allow to the developer to be able to print
>>informational-level data without having to use pr_info, pr_notice etc,
>>in line with other filesystems.
>>. One interesting and noteworthy feature of cifs_info() is that it is
>>  transparent to the CIFS_DEBUG config state, either in "y" or "n".
>>. Also, by using KERN_INFO level, it can be safely filtered by a
>>  maintainer / administrator, without cluttering their log monitors,
>>  since this is a low level alert.
>>  - I took the liberty to not add it inside the existing pr_debug, because
>>of the eventual need of some developer to print stuff that should be
>>printed not only in CIFS_DEBUG mode (as there are plenty of pr_notice
>>scattered over the code).
>>  - Also, this is not a debug but a info message, so i liked cifs_info()
>>more ;-)
>>  - I saw plenty of pr_notice() in CIFS code, but I resisted the urge to
>>convert them to cifs_info().
>> 
>> * The second commit contains the code printing a cifs_info() when attempting
>>  a CIFS mount operation.
>> 
>> Appreciate your review.
>> 
>> V3: Created a new cifs_info() function, moved the mount attempt message to
>>cifs_info
>> 
>> V2: Created a loop to select the right cifs_dbg message to be printed,
>>considering the current system's scenario, in order to avoid a
>>duplicate message or stripping out important information in
>>debug.
>> 
>> Rodrigo Freire (2):
>>  CIFS: Adds information-level logging function
>>  CIFS: Print message when attempting a mount
>> 
>> fs/cifs/cifs_debug.h | 16 
>> fs/cifs/cifsfs.c |  7 ++-
>> fs/cifs/transport.c  |  2 +-
>> 3 files changed, 23 insertions(+), 2 deletions(-)
>> 
>> --
>> 1.8.3.1
>> 
> 
> 
> -- 
> Thanks,
> 
> Steve
> <0001-cifs-minor-clarification-in-comments.patch>


[PATCH v3 1/2] CIFS: Adds information-level logging function

2018-10-07 Thread Rodrigo Freire
Currently, CIFS lacks a internal logging function that prints out data
when CIFS_DEBUG=n. When CIFS_DEBUG=y, the only message level for CIFS
events are KERN_ERR or KERN_DEBUG.

This patch creates cifs_info(), which is useful for printing
non-critical event messges, at either CIFS_DEBUG state.

Signed-off-by: Rodrigo Freire 
---
 fs/cifs/cifs_debug.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h
index f4f3f08..72db298 100644
--- a/fs/cifs/cifs_debug.h
+++ b/fs/cifs/cifs_debug.h
@@ -47,6 +47,17 @@
  */
 #ifdef CONFIG_CIFS_DEBUG
 
+/* Information level messages, minor events */
+#define cifs_info_func(ratefunc, fmt, ...) \
+do {   \
+   pr_info_ ## ratefunc("CIFS: " fmt, ##__VA_ARGS__);  \
+} while (0)
+
+#define cifs_info(fmt, ...)\
+do {   \
+   cifs_info_func(ratelimited, fmt, ##__VA_ARGS__);\
+} while (0)
+
 /* information message: e.g., configuration, major event */
 #define cifs_dbg_func(ratefunc, type, fmt, ...)\
 do {   \
@@ -81,6 +92,11 @@
if (0)  \
pr_debug(fmt, ##__VA_ARGS__);   \
 } while (0)
+
+#define cifs_info(fmt, ...)\
+do {   \
+   pr_info("CIFS: "fmt, ##__VA_ARGS__);\
+} while (0)
 #endif
 
 #endif /* _H_CIFS_DEBUG */
-- 
1.8.3.1



[PATCH v3 0/2] CIFS: Info-level log support, print message when attempting mount.

2018-10-07 Thread Rodrigo Freire
Hi Steve,
>From our conversation over v2, I came out with this v3 patch, which I broke
in two commits:

* The first commit in cifs_debug.h, creating the cifs_info() function.
  - The aim of this commit is to allow to the developer to be able to print
informational-level data without having to use pr_info, pr_notice etc,
in line with other filesystems.
. One interesting and noteworthy feature of cifs_info() is that it is
  transparent to the CIFS_DEBUG config state, either in "y" or "n".
. Also, by using KERN_INFO level, it can be safely filtered by a
  maintainer / administrator, without cluttering their log monitors,
  since this is a low level alert.
  - I took the liberty to not add it inside the existing pr_debug, because
of the eventual need of some developer to print stuff that should be
printed not only in CIFS_DEBUG mode (as there are plenty of pr_notice
scattered over the code).
  - Also, this is not a debug but a info message, so i liked cifs_info() 
more ;-)
  - I saw plenty of pr_notice() in CIFS code, but I resisted the urge to
convert them to cifs_info().

* The second commit contains the code printing a cifs_info() when attempting
  a CIFS mount operation.

Appreciate your review.

V3: Created a new cifs_info() function, moved the mount attempt message to
cifs_info

V2: Created a loop to select the right cifs_dbg message to be printed,
considering the current system's scenario, in order to avoid a
duplicate message or stripping out important information in
    debug.

Rodrigo Freire (2):
  CIFS: Adds information-level logging function
  CIFS: Print message when attempting a mount

 fs/cifs/cifs_debug.h | 16 
 fs/cifs/cifsfs.c |  7 ++-
 fs/cifs/transport.c  |  2 +-
 3 files changed, 23 insertions(+), 2 deletions(-)

-- 
1.8.3.1



[PATCH v3 2/2] CIFS: Print message when attempting a mount

2018-10-07 Thread Rodrigo Freire
Currently, no messages are printed when mounting a CIFS filesystem and
no debug configuration is enabled.

However, a CIFS mount information is valuable when troubleshooting
and/or forensic analyzing a system and finding out if was a CIFS
endpoint mount attempted.

Other filesystems such as XFS, EXT* does issue a printk() when mounting
their filesystems.

A terse log message is printed only if cifsFYI is not enabled. Otherwise,
the default full debug message is printed.

In order to not clutter and classify correctly the event messages, these
are logged as KERN_INFO level.

Sample mount operations:

[root@corinthians ~]# mount -o user=administrator //172.25.250.18/c$ /mnt
(non-existent system)

[root@corinthians ~]# mount -o user=administrator //172.25.250.19/c$ /mnt
(Valid system)

Kernel message log for the mount operations:

[  450.464543] CIFS: Attempting to mount //172.25.250.18/c$
[  456.478186] CIFS VFS: Error connecting to socket. Aborting operation.
[  456.478381] CIFS VFS: cifs_mount failed w/return code = -113
[  467.688866] CIFS: Attempting to mount //172.25.250.19/c$

Signed-off-by: Rodrigo Freire 
---
 fs/cifs/cifsfs.c| 7 ++-
 fs/cifs/transport.c | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7065426..ebdf25e 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -707,7 +707,12 @@ static int cifs_set_super(struct super_block *sb, void 
*data)
struct cifs_mnt_data mnt_data;
struct dentry *root;
 
-   cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
+   /* Prints in Kernel / CIFS log the attempted mount operation *
+*  IF CIFS_DEBUG && cifs_FYI */
+   if (cifsFYI)
+   cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
+   else
+   cifs_info("Attempting to mount %s\n", dev_name);
 
volume_info = cifs_get_volume_info((char *)data, dev_name, is_smb3);
if (IS_ERR(volume_info))
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 78f96fa..c2afd6a 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -128,7 +128,7 @@ void cifs_mid_q_entry_release(struct mid_q_entry *midEntry)
if (cifsFYI & CIFS_TIMER) {
pr_debug(" CIFS slow rsp: cmd %d mid %llu",
   midEntry->command, midEntry->mid);
-   pr_info(" A: 0x%lx S: 0x%lx R: 0x%lx\n",
+   cifs_info(" A: 0x%lx S: 0x%lx R: 0x%lx\n",
   now - midEntry->when_alloc,
   now - midEntry->when_sent,
   now - midEntry->when_received);
-- 
1.8.3.1



[PATCH v3 2/2] CIFS: Print message when attempting a mount

2018-10-07 Thread Rodrigo Freire
Currently, no messages are printed when mounting a CIFS filesystem and
no debug configuration is enabled.

However, a CIFS mount information is valuable when troubleshooting
and/or forensic analyzing a system and finding out if was a CIFS
endpoint mount attempted.

Other filesystems such as XFS, EXT* does issue a printk() when mounting
their filesystems.

A terse log message is printed only if cifsFYI is not enabled. Otherwise,
the default full debug message is printed.

In order to not clutter and classify correctly the event messages, these
are logged as KERN_INFO level.

Sample mount operations:

[root@corinthians ~]# mount -o user=administrator //172.25.250.18/c$ /mnt
(non-existent system)

[root@corinthians ~]# mount -o user=administrator //172.25.250.19/c$ /mnt
(Valid system)

Kernel message log for the mount operations:

[  450.464543] CIFS: Attempting to mount //172.25.250.18/c$
[  456.478186] CIFS VFS: Error connecting to socket. Aborting operation.
[  456.478381] CIFS VFS: cifs_mount failed w/return code = -113
[  467.688866] CIFS: Attempting to mount //172.25.250.19/c$

Signed-off-by: Rodrigo Freire 
---
 fs/cifs/cifsfs.c| 7 ++-
 fs/cifs/transport.c | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7065426..ebdf25e 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -707,7 +707,12 @@ static int cifs_set_super(struct super_block *sb, void 
*data)
struct cifs_mnt_data mnt_data;
struct dentry *root;
 
-   cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
+   /* Prints in Kernel / CIFS log the attempted mount operation *
+*  IF CIFS_DEBUG && cifs_FYI */
+   if (cifsFYI)
+   cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
+   else
+   cifs_info("Attempting to mount %s\n", dev_name);
 
volume_info = cifs_get_volume_info((char *)data, dev_name, is_smb3);
if (IS_ERR(volume_info))
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 78f96fa..c2afd6a 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -128,7 +128,7 @@ void cifs_mid_q_entry_release(struct mid_q_entry *midEntry)
if (cifsFYI & CIFS_TIMER) {
pr_debug(" CIFS slow rsp: cmd %d mid %llu",
   midEntry->command, midEntry->mid);
-   pr_info(" A: 0x%lx S: 0x%lx R: 0x%lx\n",
+   cifs_info(" A: 0x%lx S: 0x%lx R: 0x%lx\n",
   now - midEntry->when_alloc,
   now - midEntry->when_sent,
   now - midEntry->when_received);
-- 
1.8.3.1



[PATCH v3 1/2] CIFS: Adds information-level logging function

2018-10-07 Thread Rodrigo Freire
Currently, CIFS lacks a internal logging function that prints out data
when CIFS_DEBUG=n. When CIFS_DEBUG=y, the only message level for CIFS
events are KERN_ERR or KERN_DEBUG.

This patch creates cifs_info(), which is useful for printing
non-critical event messges, at either CIFS_DEBUG state.

Signed-off-by: Rodrigo Freire 
---
 fs/cifs/cifs_debug.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h
index f4f3f08..72db298 100644
--- a/fs/cifs/cifs_debug.h
+++ b/fs/cifs/cifs_debug.h
@@ -47,6 +47,17 @@
  */
 #ifdef CONFIG_CIFS_DEBUG
 
+/* Information level messages, minor events */
+#define cifs_info_func(ratefunc, fmt, ...) \
+do {   \
+   pr_info_ ## ratefunc("CIFS: " fmt, ##__VA_ARGS__);  \
+} while (0)
+
+#define cifs_info(fmt, ...)\
+do {   \
+   cifs_info_func(ratelimited, fmt, ##__VA_ARGS__);\
+} while (0)
+
 /* information message: e.g., configuration, major event */
 #define cifs_dbg_func(ratefunc, type, fmt, ...)\
 do {   \
@@ -81,6 +92,11 @@
if (0)  \
pr_debug(fmt, ##__VA_ARGS__);   \
 } while (0)
+
+#define cifs_info(fmt, ...)\
+do {   \
+   pr_info("CIFS: "fmt, ##__VA_ARGS__);\
+} while (0)
 #endif
 
 #endif /* _H_CIFS_DEBUG */
-- 
1.8.3.1



[PATCH v3 0/2] CIFS: Info-level log support, print message when attempting mount.

2018-10-07 Thread Rodrigo Freire
Hi Steve,
>From our conversation over v2, I came out with this v3 patch, which I broke
in two commits:

* The first commit in cifs_debug.h, creating the cifs_info() function.
  - The aim of this commit is to allow to the developer to be able to print
informational-level data without having to use pr_info, pr_notice etc,
in line with other filesystems.
. One interesting and noteworthy feature of cifs_info() is that it is
  transparent to the CIFS_DEBUG config state, either in "y" or "n".
. Also, by using KERN_INFO level, it can be safely filtered by a
  maintainer / administrator, without cluttering their log monitors,
  since this is a low level alert.
  - I took the liberty to not add it inside the existing pr_debug, because
of the eventual need of some developer to print stuff that should be
printed not only in CIFS_DEBUG mode (as there are plenty of pr_notice
scattered over the code).
  - Also, this is not a debug but a info message, so i liked cifs_info() 
more ;-)
  - I saw plenty of pr_notice() in CIFS code, but I resisted the urge to
convert them to cifs_info().

* The second commit contains the code printing a cifs_info() when attempting
  a CIFS mount operation.

Appreciate your review.

V3: Created a new cifs_info() function, moved the mount attempt message to
cifs_info

V2: Created a loop to select the right cifs_dbg message to be printed,
considering the current system's scenario, in order to avoid a
duplicate message or stripping out important information in
    debug.

Rodrigo Freire (2):
  CIFS: Adds information-level logging function
  CIFS: Print message when attempting a mount

 fs/cifs/cifs_debug.h | 16 
 fs/cifs/cifsfs.c |  7 ++-
 fs/cifs/transport.c  |  2 +-
 3 files changed, 23 insertions(+), 2 deletions(-)

-- 
1.8.3.1



Re: [PATCH v2] CIFS: Print message when attempting a mount

2018-10-06 Thread Rodrigo Freire
Hi Steve! o/ 

Appreciate your time and review. 

For a v3, what would you like more: a pr_info() straight away in the code
for the message, or, writing a new cifs_info() function wrapping
over pr_info, like we have in cifs_dbg (which translates to pr_debug or pr_err)?

Waiting for your comments. Thanks!

---
Rodrigo Freire - Principal Technical Account Manager
GLOBAL CUSTOMER SUCCESS - Partnering with you to help achieve your business 
goals
redhat.com | TRIED. TESTED. TRUSTED. | redhat.com/trusted 

- Original Message - 

> From: "Steve French" 
> To: rfre...@redhat.com
> Cc: "LKML" , "Steve French"
> , "CIFS" , "Pavel Shilovsky"
> 
> Sent: Saturday, October 6, 2018 4:09:30 PM
> Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount

> On Tue, Oct 2, 2018 at 4:53 PM Rodrigo Freire  wrote:
> >
> > Hi hi again Steve \o
> >
> > I do see potential for a ftrace rewrite for the cifs_dbg messages.

> Was looking at this on current kernels and debugging mount is probably
> fine for developers (or users) - plenty of debug messages get printed
> via the dynamic ftrace points. In practice some would prefer less
> noisy trace logs so the minimum set for some would be something like
> this (which already works with current cifs). If we want to add some
> default log messages on mount to dmesg instead of the trace-cmd log,
> maybe we should log them at a slightly less noisy level (as we see
> with xfs and btrfs) so they will be more log friendly

> root@smf-Thinkpad~/# trace-cmd record -e smb3_enter* -e smb3_exit*
> Hit Ctrl^C to stop recording

> root@smf-Thinkpad:~/# trace-cmd show
> # tracer: nop
> #
> # _-=> irqs-off
> # / _=> need-resched
> # | / _---=> hardirq/softirq
> # || / _--=> preempt-depth
> # ||| / delay
> # TASK-PID CPU#  TIMESTAMP FUNCTION
> # | | |  | |
> mount.cifs-14199 [002]  97642.728411: smb3_enter:
> cifs_mount: xid=20
> mount.cifs-14199 [002]  97642.728669: smb3_enter:
> cifs_get_smb_ses: xid=21
> mount.cifs-14199 [003]  97642.742336: smb3_exit_done:
> cifs_get_smb_ses: xid=21
> mount.cifs-14199 [003]  97642.742343: smb3_enter:
> cifs_setup_ipc: xid=22
> mount.cifs-14199 [003]  97642.742530: smb3_exit_done:
> cifs_setup_ipc: xid=22
> mount.cifs-14199 [003]  97642.742532: smb3_enter:
> cifs_get_tcon: xid=23
> mount.cifs-14199 [003]  97642.742674: smb3_exit_done:
> cifs_get_tcon: xid=23
> mount.cifs-14199 [003]  97642.743267: smb3_exit_done:
> cifs_mount: xid=20
> mount.cifs-14199 [003]  97642.743311: smb3_enter:
> cifs_root_iget: xid=24
> mount.cifs-14199 [003]  97642.743471: smb3_exit_done:
> cifs_root_iget: xid=24

> --
> Thanks,

> Steve


Re: [PATCH v2] CIFS: Print message when attempting a mount

2018-10-06 Thread Rodrigo Freire
Hi Steve! o/ 

Appreciate your time and review. 

For a v3, what would you like more: a pr_info() straight away in the code
for the message, or, writing a new cifs_info() function wrapping
over pr_info, like we have in cifs_dbg (which translates to pr_debug or pr_err)?

Waiting for your comments. Thanks!

---
Rodrigo Freire - Principal Technical Account Manager
GLOBAL CUSTOMER SUCCESS - Partnering with you to help achieve your business 
goals
redhat.com | TRIED. TESTED. TRUSTED. | redhat.com/trusted 

- Original Message - 

> From: "Steve French" 
> To: rfre...@redhat.com
> Cc: "LKML" , "Steve French"
> , "CIFS" , "Pavel Shilovsky"
> 
> Sent: Saturday, October 6, 2018 4:09:30 PM
> Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount

> On Tue, Oct 2, 2018 at 4:53 PM Rodrigo Freire  wrote:
> >
> > Hi hi again Steve \o
> >
> > I do see potential for a ftrace rewrite for the cifs_dbg messages.

> Was looking at this on current kernels and debugging mount is probably
> fine for developers (or users) - plenty of debug messages get printed
> via the dynamic ftrace points. In practice some would prefer less
> noisy trace logs so the minimum set for some would be something like
> this (which already works with current cifs). If we want to add some
> default log messages on mount to dmesg instead of the trace-cmd log,
> maybe we should log them at a slightly less noisy level (as we see
> with xfs and btrfs) so they will be more log friendly

> root@smf-Thinkpad~/# trace-cmd record -e smb3_enter* -e smb3_exit*
> Hit Ctrl^C to stop recording

> root@smf-Thinkpad:~/# trace-cmd show
> # tracer: nop
> #
> # _-=> irqs-off
> # / _=> need-resched
> # | / _---=> hardirq/softirq
> # || / _--=> preempt-depth
> # ||| / delay
> # TASK-PID CPU#  TIMESTAMP FUNCTION
> # | | |  | |
> mount.cifs-14199 [002]  97642.728411: smb3_enter:
> cifs_mount: xid=20
> mount.cifs-14199 [002]  97642.728669: smb3_enter:
> cifs_get_smb_ses: xid=21
> mount.cifs-14199 [003]  97642.742336: smb3_exit_done:
> cifs_get_smb_ses: xid=21
> mount.cifs-14199 [003]  97642.742343: smb3_enter:
> cifs_setup_ipc: xid=22
> mount.cifs-14199 [003]  97642.742530: smb3_exit_done:
> cifs_setup_ipc: xid=22
> mount.cifs-14199 [003]  97642.742532: smb3_enter:
> cifs_get_tcon: xid=23
> mount.cifs-14199 [003]  97642.742674: smb3_exit_done:
> cifs_get_tcon: xid=23
> mount.cifs-14199 [003]  97642.743267: smb3_exit_done:
> cifs_mount: xid=20
> mount.cifs-14199 [003]  97642.743311: smb3_enter:
> cifs_root_iget: xid=24
> mount.cifs-14199 [003]  97642.743471: smb3_exit_done:
> cifs_root_iget: xid=24

> --
> Thanks,

> Steve


Re: [PATCH v2] CIFS: Print message when attempting a mount

2018-10-02 Thread Rodrigo Freire
Hi hi again Steve \o

I do see potential for a ftrace rewrite for the cifs_dbg messages.

But for the original post, I am aiming for a message to be printed
and found in dmesg, helping to correlate and troubleshoot events in
production systems.

So given the debugging nature of ftrace, this is not of help for my
patch request.

So, ACK for v2, using cifs_dbg(VFS) which actually translates to a
pr_warn(), or request a V3 using pr_info() (which I am absolutely fine
with) or...?

Let me know.

I appreciate your time and review!

- RF.

- Original Message - 

> From: "Steve French" 
> To: rfre...@redhat.com
> Cc: "LKML" , "Steve French"
> , "CIFS" , "Pavel Shilovsky"
> 
> Sent: Tuesday, October 2, 2018 6:25:46 PM
> Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount

> It is an interesting question - my gut reaction is that messages that
> need more immediate attention should be logged as KERN_ERR (similar to
> cifs_dbg(VFS ...) but given how easy it is now to use dynamic tracing
> and better to read, if a developer would need it ... probably best to
> use ftrace (trace-cmd). Note that xfs has more than 570 (!) dynamic
> trace point callouts now vs. fewer than 30 for xfs_notice
> On Tue, Oct 2, 2018 at 4:20 PM Rodrigo Freire  wrote:
> >
> > Hi Steve o/
> >
> > I personally like more a pr_info() instead of a cifs_dbg (which wraps to a
> > pr_warn). But in order to keep in line with the general CIFS coding style
> > I stuck to cifs_dbg
> >
> > But I would happily rewrite the cifs_dbg to pr_info a v3: That would be
> > good enough too.
> >
> > Ah for what is worth my test/target systems are CentOS/Red Hat Enterprise
> > Linux.
> >
> > Thoughts?
> >
> > Thanks!
> >
> > - RF.
> >
> > - Original Message -
> > > From: "Steve French" 
> > > To: rfre...@redhat.com
> > > Cc: "LKML" , "Steve French"
> > > , "CIFS" , "Pavel
> > > Shilovsky"
> > > 
> > > Sent: Tuesday, October 2, 2018 5:35:49 PM
> > > Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount
> >
> > > I noticed that on at least the first system I looked at (Ubuntu 18.04)
> > > it defaults to KERN_WARNING (ie 4) so wouldn't have shown a KERN_INFO
> > > which is level 6 (as the mount example from ext4) by default
> > > or the xfs_notice (which is level 5)
> >
> > > https://elinux.org/Debugging_by_printing
> >
> > > On Tue, Oct 2, 2018 at 2:28 PM Rodrigo Freire  wrote:
> > > >
> > > > Hi Steve,
> > > >
> > > > - Original Message -
> > > > > From: "Steve French" 
> > > > > To: rfre...@redhat.com
> > > > > Cc: "LKML" , "Steve French"
> > > > > , "CIFS" , "Pavel
> > > > > Shilovsky"
> > > > > 
> > > > > Sent: Tuesday, October 2, 2018 4:17:02 PM
> > > > > Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount
> > > > >
> > > > > Are you sure that these aren't logged by the automounter (for ext4,
> > > > > xfs etc.). When I looked in my dmesg logs I didn't find matching log
> > > > > entries in the file systems themselves. Do you have an example?
> > > >
> > > > I'm positive about it. Check it out:
> > > >
> > > > [rfreire@rf ~]$ cd git/upstream/fs/ext4/
> > > > [rfreire@rf ext4]$
> > > > [rfreire@rf ext4]$
> > > > [rfreire@rf ext4]$ grep -r "mounted filesystem with"
> > > > super.c: ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "
> > > >
> > > >
> > > > [rfreire@rf ext4]$ dmesg | grep mount
> > > > [ 21.550897] EXT4-fs (dm-1): mounted filesystem with ordered data mode.
> > > > Opts: (null)
> > > > [ 22.216213] EXT4-fs (dm-1): re-mounted. Opts: discard
> > > > [ 22.598267] EXT4-fs (sda1): mounted filesystem with ordered data mode.
> > > > Opts: (null)
> > > > [ 22.605225] EXT4-fs (sdc): mounted filesystem without journal. Opts:
> > > > discard
> > > > [ 24.029161] EXT4-fs (dm-2): mounted filesystem with ordered data mode.
> > > > Opts: (null)
> > > > [ 24.04] EXT4-fs (dm-4): mounted filesystem without journal. Opts:
> > > > (null)
> > > >
> > > > XFS sample dmesg (from
> > > > https://www.reddit.com/r/a

Re: [PATCH v2] CIFS: Print message when attempting a mount

2018-10-02 Thread Rodrigo Freire
Hi hi again Steve \o

I do see potential for a ftrace rewrite for the cifs_dbg messages.

But for the original post, I am aiming for a message to be printed
and found in dmesg, helping to correlate and troubleshoot events in
production systems.

So given the debugging nature of ftrace, this is not of help for my
patch request.

So, ACK for v2, using cifs_dbg(VFS) which actually translates to a
pr_warn(), or request a V3 using pr_info() (which I am absolutely fine
with) or...?

Let me know.

I appreciate your time and review!

- RF.

- Original Message - 

> From: "Steve French" 
> To: rfre...@redhat.com
> Cc: "LKML" , "Steve French"
> , "CIFS" , "Pavel Shilovsky"
> 
> Sent: Tuesday, October 2, 2018 6:25:46 PM
> Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount

> It is an interesting question - my gut reaction is that messages that
> need more immediate attention should be logged as KERN_ERR (similar to
> cifs_dbg(VFS ...) but given how easy it is now to use dynamic tracing
> and better to read, if a developer would need it ... probably best to
> use ftrace (trace-cmd). Note that xfs has more than 570 (!) dynamic
> trace point callouts now vs. fewer than 30 for xfs_notice
> On Tue, Oct 2, 2018 at 4:20 PM Rodrigo Freire  wrote:
> >
> > Hi Steve o/
> >
> > I personally like more a pr_info() instead of a cifs_dbg (which wraps to a
> > pr_warn). But in order to keep in line with the general CIFS coding style
> > I stuck to cifs_dbg
> >
> > But I would happily rewrite the cifs_dbg to pr_info a v3: That would be
> > good enough too.
> >
> > Ah for what is worth my test/target systems are CentOS/Red Hat Enterprise
> > Linux.
> >
> > Thoughts?
> >
> > Thanks!
> >
> > - RF.
> >
> > - Original Message -
> > > From: "Steve French" 
> > > To: rfre...@redhat.com
> > > Cc: "LKML" , "Steve French"
> > > , "CIFS" , "Pavel
> > > Shilovsky"
> > > 
> > > Sent: Tuesday, October 2, 2018 5:35:49 PM
> > > Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount
> >
> > > I noticed that on at least the first system I looked at (Ubuntu 18.04)
> > > it defaults to KERN_WARNING (ie 4) so wouldn't have shown a KERN_INFO
> > > which is level 6 (as the mount example from ext4) by default
> > > or the xfs_notice (which is level 5)
> >
> > > https://elinux.org/Debugging_by_printing
> >
> > > On Tue, Oct 2, 2018 at 2:28 PM Rodrigo Freire  wrote:
> > > >
> > > > Hi Steve,
> > > >
> > > > - Original Message -
> > > > > From: "Steve French" 
> > > > > To: rfre...@redhat.com
> > > > > Cc: "LKML" , "Steve French"
> > > > > , "CIFS" , "Pavel
> > > > > Shilovsky"
> > > > > 
> > > > > Sent: Tuesday, October 2, 2018 4:17:02 PM
> > > > > Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount
> > > > >
> > > > > Are you sure that these aren't logged by the automounter (for ext4,
> > > > > xfs etc.). When I looked in my dmesg logs I didn't find matching log
> > > > > entries in the file systems themselves. Do you have an example?
> > > >
> > > > I'm positive about it. Check it out:
> > > >
> > > > [rfreire@rf ~]$ cd git/upstream/fs/ext4/
> > > > [rfreire@rf ext4]$
> > > > [rfreire@rf ext4]$
> > > > [rfreire@rf ext4]$ grep -r "mounted filesystem with"
> > > > super.c: ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "
> > > >
> > > >
> > > > [rfreire@rf ext4]$ dmesg | grep mount
> > > > [ 21.550897] EXT4-fs (dm-1): mounted filesystem with ordered data mode.
> > > > Opts: (null)
> > > > [ 22.216213] EXT4-fs (dm-1): re-mounted. Opts: discard
> > > > [ 22.598267] EXT4-fs (sda1): mounted filesystem with ordered data mode.
> > > > Opts: (null)
> > > > [ 22.605225] EXT4-fs (sdc): mounted filesystem without journal. Opts:
> > > > discard
> > > > [ 24.029161] EXT4-fs (dm-2): mounted filesystem with ordered data mode.
> > > > Opts: (null)
> > > > [ 24.04] EXT4-fs (dm-4): mounted filesystem without journal. Opts:
> > > > (null)
> > > >
> > > > XFS sample dmesg (from
> > > > https://www.reddit.com/r/a

Re: [PATCH v2] CIFS: Print message when attempting a mount

2018-10-02 Thread Rodrigo Freire
Hi Steve o/

I personally like more a pr_info() instead of a cifs_dbg (which wraps to a
pr_warn). But in order to keep in line with the general CIFS coding style
I stuck to cifs_dbg

But I would happily rewrite the cifs_dbg to pr_info a v3: That would be
good enough too.

Ah for what is worth my test/target systems are CentOS/Red Hat Enterprise
Linux.

Thoughts?

Thanks!

- RF.

- Original Message - 
> From: "Steve French" 
> To: rfre...@redhat.com
> Cc: "LKML" , "Steve French"
> , "CIFS" , "Pavel Shilovsky"
> 
> Sent: Tuesday, October 2, 2018 5:35:49 PM
> Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount

> I noticed that on at least the first system I looked at (Ubuntu 18.04)
> it defaults to KERN_WARNING (ie 4) so wouldn't have shown a KERN_INFO
> which is level 6 (as the mount example from ext4) by default
> or the xfs_notice (which is level 5)

> https://elinux.org/Debugging_by_printing

> On Tue, Oct 2, 2018 at 2:28 PM Rodrigo Freire  wrote:
> >
> > Hi Steve,
> >
> > - Original Message -
> > > From: "Steve French" 
> > > To: rfre...@redhat.com
> > > Cc: "LKML" , "Steve French"
> > > , "CIFS" , "Pavel
> > > Shilovsky"
> > > 
> > > Sent: Tuesday, October 2, 2018 4:17:02 PM
> > > Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount
> > >
> > > Are you sure that these aren't logged by the automounter (for ext4,
> > > xfs etc.). When I looked in my dmesg logs I didn't find matching log
> > > entries in the file systems themselves. Do you have an example?
> >
> > I'm positive about it. Check it out:
> >
> > [rfreire@rf ~]$ cd git/upstream/fs/ext4/
> > [rfreire@rf ext4]$
> > [rfreire@rf ext4]$
> > [rfreire@rf ext4]$ grep -r "mounted filesystem with"
> > super.c: ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "
> >
> >
> > [rfreire@rf ext4]$ dmesg | grep mount
> > [ 21.550897] EXT4-fs (dm-1): mounted filesystem with ordered data mode.
> > Opts: (null)
> > [ 22.216213] EXT4-fs (dm-1): re-mounted. Opts: discard
> > [ 22.598267] EXT4-fs (sda1): mounted filesystem with ordered data mode.
> > Opts: (null)
> > [ 22.605225] EXT4-fs (sdc): mounted filesystem without journal. Opts:
> > discard
> > [ 24.029161] EXT4-fs (dm-2): mounted filesystem with ordered data mode.
> > Opts: (null)
> > [ 24.04] EXT4-fs (dm-4): mounted filesystem without journal. Opts:
> > (null)
> >
> > XFS sample dmesg (from
> > https://www.reddit.com/r/archlinux/comments/40b9r9/xfs_partition_is_mounted_during_boot_and_then/):
> >
> > [ 2.764491] XFS (sdb1): Mounting V5 Filesystem
> > [ 3.200886] XFS (sdb1): Ending clean mount
> > [ 5.384218] XFS (sdb1): Unmounting Filesystem
> >
> > Relevant code:
> >
> > [rfreire@rf ~]$ cd ../xfs
> >
> > [rfreire@rf xfs]$ grep "Mounting V" *.c
> > xfs_log.c: xfs_notice(mp, "Mounting V%d Filesystem",
> >
> >
> > > On the idea of adding cifsFYI logging here - I slightly prefer using
> > > ftrace (trace-cmd, ie dynamic tracing) so there is less overhead and
> > > easier to turn on/off following the example of xfs, f2fs, nfs, nfsd
> > > etc.
> >
> > Remember that cifsFYI already exists; I just moved it inside a if clause
> > to print it only when running under debug. (they way it is originally).
> >
> > > On Tue, Oct 2, 2018 at 6:57 AM Rodrigo Freire  wrote:
> > > >
> > > > Currently, no messages are printed when mounting a CIFS filesystem and
> > > > no debug configuration is enabled.
> > > >
> > > > However, a CIFS mount information is valuable when troubleshooting
> > > > and/or forensic analyzing a system and finding out if was a CIFS
> > > > endpoint mount attempted.
> > > >
> > > > Other filesystems such as XFS, EXT* does issue a printk() when mounting
> > > > their filesystems.
> >
> >
> > > --
> > > Thanks,
> >
> > Thank You! o/

> --
> Thanks,

> Steve


Re: [PATCH v2] CIFS: Print message when attempting a mount

2018-10-02 Thread Rodrigo Freire
Hi Steve o/

I personally like more a pr_info() instead of a cifs_dbg (which wraps to a
pr_warn). But in order to keep in line with the general CIFS coding style
I stuck to cifs_dbg

But I would happily rewrite the cifs_dbg to pr_info a v3: That would be
good enough too.

Ah for what is worth my test/target systems are CentOS/Red Hat Enterprise
Linux.

Thoughts?

Thanks!

- RF.

- Original Message - 
> From: "Steve French" 
> To: rfre...@redhat.com
> Cc: "LKML" , "Steve French"
> , "CIFS" , "Pavel Shilovsky"
> 
> Sent: Tuesday, October 2, 2018 5:35:49 PM
> Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount

> I noticed that on at least the first system I looked at (Ubuntu 18.04)
> it defaults to KERN_WARNING (ie 4) so wouldn't have shown a KERN_INFO
> which is level 6 (as the mount example from ext4) by default
> or the xfs_notice (which is level 5)

> https://elinux.org/Debugging_by_printing

> On Tue, Oct 2, 2018 at 2:28 PM Rodrigo Freire  wrote:
> >
> > Hi Steve,
> >
> > - Original Message -
> > > From: "Steve French" 
> > > To: rfre...@redhat.com
> > > Cc: "LKML" , "Steve French"
> > > , "CIFS" , "Pavel
> > > Shilovsky"
> > > 
> > > Sent: Tuesday, October 2, 2018 4:17:02 PM
> > > Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount
> > >
> > > Are you sure that these aren't logged by the automounter (for ext4,
> > > xfs etc.). When I looked in my dmesg logs I didn't find matching log
> > > entries in the file systems themselves. Do you have an example?
> >
> > I'm positive about it. Check it out:
> >
> > [rfreire@rf ~]$ cd git/upstream/fs/ext4/
> > [rfreire@rf ext4]$
> > [rfreire@rf ext4]$
> > [rfreire@rf ext4]$ grep -r "mounted filesystem with"
> > super.c: ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "
> >
> >
> > [rfreire@rf ext4]$ dmesg | grep mount
> > [ 21.550897] EXT4-fs (dm-1): mounted filesystem with ordered data mode.
> > Opts: (null)
> > [ 22.216213] EXT4-fs (dm-1): re-mounted. Opts: discard
> > [ 22.598267] EXT4-fs (sda1): mounted filesystem with ordered data mode.
> > Opts: (null)
> > [ 22.605225] EXT4-fs (sdc): mounted filesystem without journal. Opts:
> > discard
> > [ 24.029161] EXT4-fs (dm-2): mounted filesystem with ordered data mode.
> > Opts: (null)
> > [ 24.04] EXT4-fs (dm-4): mounted filesystem without journal. Opts:
> > (null)
> >
> > XFS sample dmesg (from
> > https://www.reddit.com/r/archlinux/comments/40b9r9/xfs_partition_is_mounted_during_boot_and_then/):
> >
> > [ 2.764491] XFS (sdb1): Mounting V5 Filesystem
> > [ 3.200886] XFS (sdb1): Ending clean mount
> > [ 5.384218] XFS (sdb1): Unmounting Filesystem
> >
> > Relevant code:
> >
> > [rfreire@rf ~]$ cd ../xfs
> >
> > [rfreire@rf xfs]$ grep "Mounting V" *.c
> > xfs_log.c: xfs_notice(mp, "Mounting V%d Filesystem",
> >
> >
> > > On the idea of adding cifsFYI logging here - I slightly prefer using
> > > ftrace (trace-cmd, ie dynamic tracing) so there is less overhead and
> > > easier to turn on/off following the example of xfs, f2fs, nfs, nfsd
> > > etc.
> >
> > Remember that cifsFYI already exists; I just moved it inside a if clause
> > to print it only when running under debug. (they way it is originally).
> >
> > > On Tue, Oct 2, 2018 at 6:57 AM Rodrigo Freire  wrote:
> > > >
> > > > Currently, no messages are printed when mounting a CIFS filesystem and
> > > > no debug configuration is enabled.
> > > >
> > > > However, a CIFS mount information is valuable when troubleshooting
> > > > and/or forensic analyzing a system and finding out if was a CIFS
> > > > endpoint mount attempted.
> > > >
> > > > Other filesystems such as XFS, EXT* does issue a printk() when mounting
> > > > their filesystems.
> >
> >
> > > --
> > > Thanks,
> >
> > Thank You! o/

> --
> Thanks,

> Steve


Re: [PATCH v2] CIFS: Print message when attempting a mount

2018-10-02 Thread Rodrigo Freire
Hi Steve,

- Original Message - 
> From: "Steve French" 
> To: rfre...@redhat.com
> Cc: "LKML" , "Steve French"
> , "CIFS" , "Pavel Shilovsky"
> 
> Sent: Tuesday, October 2, 2018 4:17:02 PM
> Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount
>
> Are you sure that these aren't logged by the automounter (for ext4,
> xfs etc.). When I looked in my dmesg logs I didn't find matching log
> entries in the file systems themselves. Do you have an example?

I'm positive about it. Check it out:

[rfreire@rf ~]$ cd git/upstream/fs/ext4/
[rfreire@rf ext4]$ 
[rfreire@rf ext4]$ 
[rfreire@rf ext4]$ grep -r "mounted filesystem with"
super.c:ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "


[rfreire@rf ext4]$ dmesg | grep mount
[   21.550897] EXT4-fs (dm-1): mounted filesystem with ordered data mode. Opts: 
(null)
[   22.216213] EXT4-fs (dm-1): re-mounted. Opts: discard
[   22.598267] EXT4-fs (sda1): mounted filesystem with ordered data mode. Opts: 
(null)
[   22.605225] EXT4-fs (sdc): mounted filesystem without journal. Opts: discard
[   24.029161] EXT4-fs (dm-2): mounted filesystem with ordered data mode. Opts: 
(null)
[   24.04] EXT4-fs (dm-4): mounted filesystem without journal. Opts: (null)

XFS sample dmesg (from 
https://www.reddit.com/r/archlinux/comments/40b9r9/xfs_partition_is_mounted_during_boot_and_then/):

[ 2.764491] XFS (sdb1): Mounting V5 Filesystem
[ 3.200886] XFS (sdb1): Ending clean mount
[ 5.384218] XFS (sdb1): Unmounting Filesystem

Relevant code:

[rfreire@rf ~]$ cd ../xfs

[rfreire@rf xfs]$ grep "Mounting V" *.c
xfs_log.c:  xfs_notice(mp, "Mounting V%d Filesystem",


> On the idea of adding cifsFYI logging here - I slightly prefer using
> ftrace (trace-cmd, ie dynamic tracing) so there is less overhead and
> easier to turn on/off following the example of xfs, f2fs, nfs, nfsd
> etc.

Remember that cifsFYI already exists; I just moved it inside a if clause
to print it only when running under debug. (they way it is originally).

> On Tue, Oct 2, 2018 at 6:57 AM Rodrigo Freire  wrote:
> >
> > Currently, no messages are printed when mounting a CIFS filesystem and
> > no debug configuration is enabled.
> >
> > However, a CIFS mount information is valuable when troubleshooting
> > and/or forensic analyzing a system and finding out if was a CIFS
> > endpoint mount attempted.
> >
> > Other filesystems such as XFS, EXT* does issue a printk() when mounting
> > their filesystems.


> --
> Thanks,

Thank You! o/


Re: [PATCH v2] CIFS: Print message when attempting a mount

2018-10-02 Thread Rodrigo Freire
Hi Steve,

- Original Message - 
> From: "Steve French" 
> To: rfre...@redhat.com
> Cc: "LKML" , "Steve French"
> , "CIFS" , "Pavel Shilovsky"
> 
> Sent: Tuesday, October 2, 2018 4:17:02 PM
> Subject: Re: [PATCH v2] CIFS: Print message when attempting a mount
>
> Are you sure that these aren't logged by the automounter (for ext4,
> xfs etc.). When I looked in my dmesg logs I didn't find matching log
> entries in the file systems themselves. Do you have an example?

I'm positive about it. Check it out:

[rfreire@rf ~]$ cd git/upstream/fs/ext4/
[rfreire@rf ext4]$ 
[rfreire@rf ext4]$ 
[rfreire@rf ext4]$ grep -r "mounted filesystem with"
super.c:ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "


[rfreire@rf ext4]$ dmesg | grep mount
[   21.550897] EXT4-fs (dm-1): mounted filesystem with ordered data mode. Opts: 
(null)
[   22.216213] EXT4-fs (dm-1): re-mounted. Opts: discard
[   22.598267] EXT4-fs (sda1): mounted filesystem with ordered data mode. Opts: 
(null)
[   22.605225] EXT4-fs (sdc): mounted filesystem without journal. Opts: discard
[   24.029161] EXT4-fs (dm-2): mounted filesystem with ordered data mode. Opts: 
(null)
[   24.04] EXT4-fs (dm-4): mounted filesystem without journal. Opts: (null)

XFS sample dmesg (from 
https://www.reddit.com/r/archlinux/comments/40b9r9/xfs_partition_is_mounted_during_boot_and_then/):

[ 2.764491] XFS (sdb1): Mounting V5 Filesystem
[ 3.200886] XFS (sdb1): Ending clean mount
[ 5.384218] XFS (sdb1): Unmounting Filesystem

Relevant code:

[rfreire@rf ~]$ cd ../xfs

[rfreire@rf xfs]$ grep "Mounting V" *.c
xfs_log.c:  xfs_notice(mp, "Mounting V%d Filesystem",


> On the idea of adding cifsFYI logging here - I slightly prefer using
> ftrace (trace-cmd, ie dynamic tracing) so there is less overhead and
> easier to turn on/off following the example of xfs, f2fs, nfs, nfsd
> etc.

Remember that cifsFYI already exists; I just moved it inside a if clause
to print it only when running under debug. (they way it is originally).

> On Tue, Oct 2, 2018 at 6:57 AM Rodrigo Freire  wrote:
> >
> > Currently, no messages are printed when mounting a CIFS filesystem and
> > no debug configuration is enabled.
> >
> > However, a CIFS mount information is valuable when troubleshooting
> > and/or forensic analyzing a system and finding out if was a CIFS
> > endpoint mount attempted.
> >
> > Other filesystems such as XFS, EXT* does issue a printk() when mounting
> > their filesystems.


> --
> Thanks,

Thank You! o/


[PATCH v2] CIFS: Print message when attempting a mount

2018-10-02 Thread Rodrigo Freire
Currently, no messages are printed when mounting a CIFS filesystem and
no debug configuration is enabled.

However, a CIFS mount information is valuable when troubleshooting
and/or forensic analyzing a system and finding out if was a CIFS
endpoint mount attempted.

Other filesystems such as XFS, EXT* does issue a printk() when mounting
their filesystems.

A terse log message is printed only if cifsFYI is not enabled.

Sample mount operations:

[root@corinthians ~]# mount -o user=administrator //172.25.250.18/c$ /mnt
(non-existent system)

[root@corinthians ~]# mount -o user=administrator //172.25.250.19/c$ /mnt
(Valid system)

Kernel message log for the mount operations:

[  450.464543] CIFS VFS: Attempting to mount //172.25.250.18/c$
[  456.478186] CIFS VFS: Error connecting to socket. Aborting operation.
[  456.478381] CIFS VFS: cifs_mount failed w/return code = -113
[  467.688866] CIFS VFS: Attempting to mount //172.25.250.19/c$

v2: Created a loop to select the right cifs_dbg message to be printed,
considering the current system's scenario, in order to avoid a
duplicate message or stripping out important information in
debug.

Signed-off-by: Rodrigo Freire 
---
 fs/cifs/cifsfs.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7065426..7fde6bc 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -707,7 +707,11 @@ static int cifs_set_super(struct super_block *sb, void 
*data)
struct cifs_mnt_data mnt_data;
struct dentry *root;
 
-   cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
+   /* Prints in Kernel / CIFS log the attempted mount operation */
+   if (cifsFYI)
+   cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
+   else
+   cifs_dbg(VFS, "Attempting to mount %s\n", dev_name);
 
volume_info = cifs_get_volume_info((char *)data, dev_name, is_smb3);
if (IS_ERR(volume_info))
-- 
1.8.3.1



[PATCH v2] CIFS: Print message when attempting a mount

2018-10-02 Thread Rodrigo Freire
Currently, no messages are printed when mounting a CIFS filesystem and
no debug configuration is enabled.

However, a CIFS mount information is valuable when troubleshooting
and/or forensic analyzing a system and finding out if was a CIFS
endpoint mount attempted.

Other filesystems such as XFS, EXT* does issue a printk() when mounting
their filesystems.

A terse log message is printed only if cifsFYI is not enabled.

Sample mount operations:

[root@corinthians ~]# mount -o user=administrator //172.25.250.18/c$ /mnt
(non-existent system)

[root@corinthians ~]# mount -o user=administrator //172.25.250.19/c$ /mnt
(Valid system)

Kernel message log for the mount operations:

[  450.464543] CIFS VFS: Attempting to mount //172.25.250.18/c$
[  456.478186] CIFS VFS: Error connecting to socket. Aborting operation.
[  456.478381] CIFS VFS: cifs_mount failed w/return code = -113
[  467.688866] CIFS VFS: Attempting to mount //172.25.250.19/c$

v2: Created a loop to select the right cifs_dbg message to be printed,
considering the current system's scenario, in order to avoid a
duplicate message or stripping out important information in
debug.

Signed-off-by: Rodrigo Freire 
---
 fs/cifs/cifsfs.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7065426..7fde6bc 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -707,7 +707,11 @@ static int cifs_set_super(struct super_block *sb, void 
*data)
struct cifs_mnt_data mnt_data;
struct dentry *root;
 
-   cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
+   /* Prints in Kernel / CIFS log the attempted mount operation */
+   if (cifsFYI)
+   cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
+   else
+   cifs_dbg(VFS, "Attempting to mount %s\n", dev_name);
 
volume_info = cifs_get_volume_info((char *)data, dev_name, is_smb3);
if (IS_ERR(volume_info))
-- 
1.8.3.1



[PATCH] CIFS: Print message when attempting mount

2018-10-01 Thread Rodrigo Freire
By default, no messages are printed when mounting a CIFS filesystem.
This information is valuable when troubleshooting and/or forensic
analyzing a system and finding out if was a CIFS endpoint mount
attempted.
Other filesystems such as XFS, EXT* does issue a printk() when mounting
their filesystems.

Sample output:

[root@ll-rhel7 ~]# mount -o user=administrator //172.25.250.18/c$ /mnt
(non-existent system)

[root@ll-rhel7 ~]# mount -o user=administrator //172.25.250.19/c$ /mnt
(Valid system)

Kernel message log:

[  450.464543] CIFS VFS: Attempting to mount //172.25.250.18/c$
[  456.478186] CIFS VFS: Error connecting to socket. Aborting operation.
[  456.478381] CIFS VFS: cifs_mount failed w/return code = -113
[  467.688866] CIFS VFS: Attempting to mount //172.25.250.19/c$

Signed-off-by: Rodrigo Freire 
---
 fs/cifs/cifsfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7065426..3f5a31e 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -707,6 +707,7 @@ static int cifs_set_super(struct super_block *sb, void 
*data)
struct cifs_mnt_data mnt_data;
struct dentry *root;
 
+   cifs_dbg(VFS, "Attempting to mount %s\n", dev_name);
cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
 
volume_info = cifs_get_volume_info((char *)data, dev_name, is_smb3);
-- 
1.8.3.1



[PATCH] CIFS: Print message when attempting mount

2018-10-01 Thread Rodrigo Freire
By default, no messages are printed when mounting a CIFS filesystem.
This information is valuable when troubleshooting and/or forensic
analyzing a system and finding out if was a CIFS endpoint mount
attempted.
Other filesystems such as XFS, EXT* does issue a printk() when mounting
their filesystems.

Sample output:

[root@ll-rhel7 ~]# mount -o user=administrator //172.25.250.18/c$ /mnt
(non-existent system)

[root@ll-rhel7 ~]# mount -o user=administrator //172.25.250.19/c$ /mnt
(Valid system)

Kernel message log:

[  450.464543] CIFS VFS: Attempting to mount //172.25.250.18/c$
[  456.478186] CIFS VFS: Error connecting to socket. Aborting operation.
[  456.478381] CIFS VFS: cifs_mount failed w/return code = -113
[  467.688866] CIFS VFS: Attempting to mount //172.25.250.19/c$

Signed-off-by: Rodrigo Freire 
---
 fs/cifs/cifsfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7065426..3f5a31e 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -707,6 +707,7 @@ static int cifs_set_super(struct super_block *sb, void 
*data)
struct cifs_mnt_data mnt_data;
struct dentry *root;
 
+   cifs_dbg(VFS, "Attempting to mount %s\n", dev_name);
cifs_dbg(FYI, "Devname: %s flags: %d\n", dev_name, flags);
 
volume_info = cifs_get_volume_info((char *)data, dev_name, is_smb3);
-- 
1.8.3.1



[PATCH v2] mm, oom: Describe task memory unit, larger PID pad

2018-07-04 Thread Rodrigo Freire
The default page memory unit of OOM task dump events might not be
intuitive and potentially misleading for the non-initiated when
debugging OOM events: These are pages and not kBs. Add a small
printk prior to the task dump informing that the memory units are
actually memory _pages_.

Also extends PID field to align on up to 7 characters.
References: https://lkml.org/lkml/2018/7/3/1201

Signed-off-by: Rodrigo Freire 
Acked-by: David Rientjes 
Acked-by: Rafael Aquini 
---
 mm/oom_kill.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 84081e7..520a483 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -392,7 +392,8 @@ static void dump_tasks(struct mem_cgroup *memcg, const 
nodemask_t *nodemask)
struct task_struct *p;
struct task_struct *task;
 
-   pr_info("[ pid ]   uid  tgid total_vm  rss pgtables_bytes swapents 
oom_score_adj name\n");
+   pr_info("Tasks state (memory values in pages):\n");
+   pr_info("[  pid  ]   uid  tgid total_vm  rss pgtables_bytes 
swapents oom_score_adj name\n");
rcu_read_lock();
for_each_process(p) {
if (oom_unkillable_task(p, memcg, nodemask))
@@ -408,7 +409,7 @@ static void dump_tasks(struct mem_cgroup *memcg, const 
nodemask_t *nodemask)
continue;
}
 
-   pr_info("[%5d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n",
+   pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n",
task->pid, from_kuid(_user_ns, task_uid(task)),
task->tgid, task->mm->total_vm, get_mm_rss(task->mm),
mm_pgtables_bytes(task->mm),
-- 
1.8.3.1



[PATCH v2] mm, oom: Describe task memory unit, larger PID pad

2018-07-04 Thread Rodrigo Freire
The default page memory unit of OOM task dump events might not be
intuitive and potentially misleading for the non-initiated when
debugging OOM events: These are pages and not kBs. Add a small
printk prior to the task dump informing that the memory units are
actually memory _pages_.

Also extends PID field to align on up to 7 characters.
References: https://lkml.org/lkml/2018/7/3/1201

Signed-off-by: Rodrigo Freire 
Acked-by: David Rientjes 
Acked-by: Rafael Aquini 
---
 mm/oom_kill.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 84081e7..520a483 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -392,7 +392,8 @@ static void dump_tasks(struct mem_cgroup *memcg, const 
nodemask_t *nodemask)
struct task_struct *p;
struct task_struct *task;
 
-   pr_info("[ pid ]   uid  tgid total_vm  rss pgtables_bytes swapents 
oom_score_adj name\n");
+   pr_info("Tasks state (memory values in pages):\n");
+   pr_info("[  pid  ]   uid  tgid total_vm  rss pgtables_bytes 
swapents oom_score_adj name\n");
rcu_read_lock();
for_each_process(p) {
if (oom_unkillable_task(p, memcg, nodemask))
@@ -408,7 +409,7 @@ static void dump_tasks(struct mem_cgroup *memcg, const 
nodemask_t *nodemask)
continue;
}
 
-   pr_info("[%5d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n",
+   pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n",
task->pid, from_kuid(_user_ns, task_uid(task)),
task->tgid, task->mm->total_vm, get_mm_rss(task->mm),
mm_pgtables_bytes(task->mm),
-- 
1.8.3.1



Re: [PATCH] mm: be more informative in OOM task list

2018-07-02 Thread Rodrigo Freire
Hello Michal!

- Original Message - 
> From: "Michal Hocko" 
> To: "Rodrigo Freire" 
> Cc: linux...@kvack.org, linux-kernel@vger.kernel.org
> Sent: Monday, July 2, 2018 8:29:06 AM
> Subject: Re: [PATCH] mm: be more informative in OOM task list
>
> On Mon 02-07-18 07:22:13, Rodrigo Freire wrote:
> > Hello Michal,
> >
> > - Original Message -
> > > From: "Michal Hocko" 
> > > To: "Rodrigo Freire" 
> > > Cc: linux...@kvack.org, linux-kernel@vger.kernel.org
> > > Sent: Monday, July 2, 2018 6:30:43 AM
> > > Subject: Re: [PATCH] mm: be more informative in OOM task list
> > >
> > > On Sun 01-07-18 13:09:40, Rodrigo Freire wrote:
> > > > The default page memory unit of OOM task dump events might not be
> > > > intuitive for the non-initiated when debugging OOM events. Add
> > > > a small printk prior to the task dump informing that the memory
> > > > units are actually memory _pages_.
> > >
> > > Does this really help? I understand the the oom report might be not the
> > > easiest thing to grasp but wouldn't it be much better to actually add
> > > documentation with clarification of each part of it?
> >
> > That would be great: After a quick grep -ri for oom in Documentation,
> > I found several other files containing its own OOM behaviour modifier
> > configurations. But it indeed lacks a central and canonical Doc file
> > which documents the OOM Killer behavior and workflows.
> >
> > However, I still stand by my proposed patch: It is unobtrusive, infers
> > no performance issue and clarifying: I recently worked in a case (for
> > full disclosure: I am a far cry from a MM expert) where the sum of the
> > RSS pages made sense when interpreted as real kB pages. Reason: There
> > were processes sharing (a good amount of) memory regions, misleading
> > the interpretation and that misled not only me, but some other
> > colleagues a well: The pages was only sorted out after actually
> > inspecting the source code.
> >
> > This patch is user-friendly and can be a great time saver to others in
> > the community.
>
> Well, all other counters we print are in page units unless explicitly
> kB. 

Your statement is correct. And I thought about that too. And then the doubt:
* Maybe someone forgot to state that these values are in kB?

> So I am not sure we really need to do anything but document the
> output better. Maybe others will find it more important though.

The thing is, it also led some other colleagues (a few!) to think the
very same as me: That raised the flag and made me write the patch:
That was indeed misleading.
And you may not have a MM and OOM-versed specialist available all the 
time! ;-)

Still ask you to reconsider.

My best regards,

- RF.


Re: [PATCH] mm: be more informative in OOM task list

2018-07-02 Thread Rodrigo Freire
Hello Michal!

- Original Message - 
> From: "Michal Hocko" 
> To: "Rodrigo Freire" 
> Cc: linux...@kvack.org, linux-kernel@vger.kernel.org
> Sent: Monday, July 2, 2018 8:29:06 AM
> Subject: Re: [PATCH] mm: be more informative in OOM task list
>
> On Mon 02-07-18 07:22:13, Rodrigo Freire wrote:
> > Hello Michal,
> >
> > - Original Message -
> > > From: "Michal Hocko" 
> > > To: "Rodrigo Freire" 
> > > Cc: linux...@kvack.org, linux-kernel@vger.kernel.org
> > > Sent: Monday, July 2, 2018 6:30:43 AM
> > > Subject: Re: [PATCH] mm: be more informative in OOM task list
> > >
> > > On Sun 01-07-18 13:09:40, Rodrigo Freire wrote:
> > > > The default page memory unit of OOM task dump events might not be
> > > > intuitive for the non-initiated when debugging OOM events. Add
> > > > a small printk prior to the task dump informing that the memory
> > > > units are actually memory _pages_.
> > >
> > > Does this really help? I understand the the oom report might be not the
> > > easiest thing to grasp but wouldn't it be much better to actually add
> > > documentation with clarification of each part of it?
> >
> > That would be great: After a quick grep -ri for oom in Documentation,
> > I found several other files containing its own OOM behaviour modifier
> > configurations. But it indeed lacks a central and canonical Doc file
> > which documents the OOM Killer behavior and workflows.
> >
> > However, I still stand by my proposed patch: It is unobtrusive, infers
> > no performance issue and clarifying: I recently worked in a case (for
> > full disclosure: I am a far cry from a MM expert) where the sum of the
> > RSS pages made sense when interpreted as real kB pages. Reason: There
> > were processes sharing (a good amount of) memory regions, misleading
> > the interpretation and that misled not only me, but some other
> > colleagues a well: The pages was only sorted out after actually
> > inspecting the source code.
> >
> > This patch is user-friendly and can be a great time saver to others in
> > the community.
>
> Well, all other counters we print are in page units unless explicitly
> kB. 

Your statement is correct. And I thought about that too. And then the doubt:
* Maybe someone forgot to state that these values are in kB?

> So I am not sure we really need to do anything but document the
> output better. Maybe others will find it more important though.

The thing is, it also led some other colleagues (a few!) to think the
very same as me: That raised the flag and made me write the patch:
That was indeed misleading.
And you may not have a MM and OOM-versed specialist available all the 
time! ;-)

Still ask you to reconsider.

My best regards,

- RF.


Re: [PATCH] mm: be more informative in OOM task list

2018-07-02 Thread Rodrigo Freire
Hello Michal,

- Original Message - 
> From: "Michal Hocko" 
> To: "Rodrigo Freire" 
> Cc: linux...@kvack.org, linux-kernel@vger.kernel.org
> Sent: Monday, July 2, 2018 6:30:43 AM
> Subject: Re: [PATCH] mm: be more informative in OOM task list
>
> On Sun 01-07-18 13:09:40, Rodrigo Freire wrote:
> > The default page memory unit of OOM task dump events might not be
> > intuitive for the non-initiated when debugging OOM events. Add
> > a small printk prior to the task dump informing that the memory
> > units are actually memory _pages_.
>
> Does this really help? I understand the the oom report might be not the
> easiest thing to grasp but wouldn't it be much better to actually add
> documentation with clarification of each part of it?

That would be great: After a quick grep -ri for oom in Documentation,
I found several other files containing its own OOM behaviour modifier
configurations. But it indeed lacks a central and canonical Doc file
which documents the OOM Killer behavior and workflows.

However, I still stand by my proposed patch: It is unobtrusive, infers
no performance issue and clarifying: I recently worked in a case (for
full disclosure: I am a far cry from a MM expert) where the sum of the
RSS pages made sense when interpreted as real kB pages. Reason: There
were processes sharing (a good amount of) memory regions, misleading
the interpretation and that misled not only me, but some other
colleagues a well: The pages was only sorted out after actually
inspecting the source code.

This patch is user-friendly and can be a great time saver to others in
the community.

I kindly request the ACKed-by ;-)

Have a great week,

- RF.


Re: [PATCH] mm: be more informative in OOM task list

2018-07-02 Thread Rodrigo Freire
Hello Michal,

- Original Message - 
> From: "Michal Hocko" 
> To: "Rodrigo Freire" 
> Cc: linux...@kvack.org, linux-kernel@vger.kernel.org
> Sent: Monday, July 2, 2018 6:30:43 AM
> Subject: Re: [PATCH] mm: be more informative in OOM task list
>
> On Sun 01-07-18 13:09:40, Rodrigo Freire wrote:
> > The default page memory unit of OOM task dump events might not be
> > intuitive for the non-initiated when debugging OOM events. Add
> > a small printk prior to the task dump informing that the memory
> > units are actually memory _pages_.
>
> Does this really help? I understand the the oom report might be not the
> easiest thing to grasp but wouldn't it be much better to actually add
> documentation with clarification of each part of it?

That would be great: After a quick grep -ri for oom in Documentation,
I found several other files containing its own OOM behaviour modifier
configurations. But it indeed lacks a central and canonical Doc file
which documents the OOM Killer behavior and workflows.

However, I still stand by my proposed patch: It is unobtrusive, infers
no performance issue and clarifying: I recently worked in a case (for
full disclosure: I am a far cry from a MM expert) where the sum of the
RSS pages made sense when interpreted as real kB pages. Reason: There
were processes sharing (a good amount of) memory regions, misleading
the interpretation and that misled not only me, but some other
colleagues a well: The pages was only sorted out after actually
inspecting the source code.

This patch is user-friendly and can be a great time saver to others in
the community.

I kindly request the ACKed-by ;-)

Have a great week,

- RF.


[PATCH] mm: be more informative in OOM task list

2018-07-01 Thread Rodrigo Freire
The default page memory unit of OOM task dump events might not be
intuitive for the non-initiated when debugging OOM events. Add
a small printk prior to the task dump informing that the memory
units are actually memory _pages_.

Signed-off-by: Rodrigo Freire 
---
 mm/oom_kill.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 84081e7..b4d9557 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -392,6 +392,7 @@ static void dump_tasks(struct mem_cgroup *memcg, const 
nodemask_t *nodemask)
struct task_struct *p;
struct task_struct *task;
 
+   pr_info("Tasks state (memory values in pages):\n");
pr_info("[ pid ]   uid  tgid total_vm  rss pgtables_bytes swapents 
oom_score_adj name\n");
rcu_read_lock();
for_each_process(p) {
-- 
1.8.3.1



[PATCH] mm: be more informative in OOM task list

2018-07-01 Thread Rodrigo Freire
The default page memory unit of OOM task dump events might not be
intuitive for the non-initiated when debugging OOM events. Add
a small printk prior to the task dump informing that the memory
units are actually memory _pages_.

Signed-off-by: Rodrigo Freire 
---
 mm/oom_kill.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 84081e7..b4d9557 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -392,6 +392,7 @@ static void dump_tasks(struct mem_cgroup *memcg, const 
nodemask_t *nodemask)
struct task_struct *p;
struct task_struct *task;
 
+   pr_info("Tasks state (memory values in pages):\n");
pr_info("[ pid ]   uid  tgid total_vm  rss pgtables_bytes swapents 
oom_score_adj name\n");
rcu_read_lock();
for_each_process(p) {
-- 
1.8.3.1



[PATCH] iwlwifi: Document missing module options

2016-01-06 Thread Rodrigo Freire
This patch documents two missing module options in the internal
code comment block.

Signed-off-by: Rodrigo Freire 
---
 drivers/net/wireless/iwlwifi/iwl-modparams.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-modparams.h 
b/drivers/net/wireless/iwlwifi/iwl-modparams.h
index ac2b90d..1477277 100644
--- a/drivers/net/wireless/iwlwifi/iwl-modparams.h
+++ b/drivers/net/wireless/iwlwifi/iwl-modparams.h
@@ -102,6 +102,8 @@ enum iwl_disable_11n {
  * @power_level: power level, default = 1
  * @debug_level: levels are IWL_DL_*
  * @ant_coupling: antenna coupling in dB, default = 0
+ * @nvm_file: specifies a external NVM file
+ * @uapsd_disable: disable U-APSD, default = 1
  * @d0i3_disable: disable d0i3, default = 1,
  * @lar_disable: disable LAR (regulatory), default = 0
  * @fw_monitor: allow to use firmware monitor
-- 
1.7.1

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


[PATCH] iwlwifi: Document missing module options

2016-01-06 Thread Rodrigo Freire
This patch documents two missing module options in the internal
code comment block.

Signed-off-by: Rodrigo Freire <rfre...@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-modparams.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-modparams.h 
b/drivers/net/wireless/iwlwifi/iwl-modparams.h
index ac2b90d..1477277 100644
--- a/drivers/net/wireless/iwlwifi/iwl-modparams.h
+++ b/drivers/net/wireless/iwlwifi/iwl-modparams.h
@@ -102,6 +102,8 @@ enum iwl_disable_11n {
  * @power_level: power level, default = 1
  * @debug_level: levels are IWL_DL_*
  * @ant_coupling: antenna coupling in dB, default = 0
+ * @nvm_file: specifies a external NVM file
+ * @uapsd_disable: disable U-APSD, default = 1
  * @d0i3_disable: disable d0i3, default = 1,
  * @lar_disable: disable LAR (regulatory), default = 0
  * @fw_monitor: allow to use firmware monitor
-- 
1.7.1

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


[PATCH V2] Documentation: Describe the shared memory usage/accounting

2015-12-21 Thread Rodrigo Freire

The Shared Memory accounting support is present in Kernel since
commit 4b02108ac1b3 ("mm: oom analysis: add shmem vmstat") and in
userland free(1) since 2014. This patch updates the Documentation to
reflect this change.

Signed-off-by: Rodrigo Freire 
---
V2: Better wording as per Vlastimil Babka's suggestions
---
 Documentation/filesystems/proc.txt  |2 ++
 Documentation/filesystems/tmpfs.txt |8 
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/filesystems/proc.txt 
b/Documentation/filesystems/proc.txt
index 402ab99..8ca61a0 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -842,6 +842,7 @@ Dirty: 968 kB
 Writeback:   0 kB
 AnonPages:  861800 kB
 Mapped: 280372 kB
+Shmem: 644 kB
 Slab:   284364 kB
 SReclaimable:   159856 kB
 SUnreclaim: 124508 kB
@@ -898,6 +899,7 @@ MemAvailable: An estimate of how much memory is available 
for starting new
AnonPages: Non-file backed pages mapped into userspace page tables
 AnonHugePages: Non-file backed huge pages mapped into userspace page tables
   Mapped: files which have been mmaped, such as libraries
+   Shmem: Total memory used by shared memory (shmem) and tmpfs
 Slab: in-kernel data structures cache
 SReclaimable: Part of Slab, that might be reclaimed, such as caches
   SUnreclaim: Part of Slab, that cannot be reclaimed on memory pressure
diff --git a/Documentation/filesystems/tmpfs.txt 
b/Documentation/filesystems/tmpfs.txt
index 98ef551..d1abf2d 100644
--- a/Documentation/filesystems/tmpfs.txt
+++ b/Documentation/filesystems/tmpfs.txt
@@ -17,10 +17,10 @@ RAM, where you have to create an ordinary filesystem on 
top. Ramdisks
 cannot swap and you do not have the possibility to resize them. 
 
 Since tmpfs lives completely in the page cache and on swap, all tmpfs
-pages currently in memory will show up as cached. It will not show up
-as shared or something like that. Further on you can check the actual
-RAM+swap use of a tmpfs instance with df(1) and du(1).
-
+pages will be shown as "Shmem" in /proc/meminfo and "Shared" in
+free(1). Notice that these counters also include shared memory
+(shmem, see ipcs(1)). The most reliable way to get the count is
+using df(1) and du(1).
 
 tmpfs has the following uses:
 
-- 
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH V2] Documentation: Describe the shared memory usage/accounting

2015-12-21 Thread Rodrigo Freire

The Shared Memory accounting support is present in Kernel since
commit 4b02108ac1b3 ("mm: oom analysis: add shmem vmstat") and in
userland free(1) since 2014. This patch updates the Documentation to
reflect this change.

Signed-off-by: Rodrigo Freire <rfre...@redhat.com>
---
V2: Better wording as per Vlastimil Babka's suggestions
---
 Documentation/filesystems/proc.txt  |2 ++
 Documentation/filesystems/tmpfs.txt |8 
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/filesystems/proc.txt 
b/Documentation/filesystems/proc.txt
index 402ab99..8ca61a0 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -842,6 +842,7 @@ Dirty: 968 kB
 Writeback:   0 kB
 AnonPages:  861800 kB
 Mapped: 280372 kB
+Shmem: 644 kB
 Slab:   284364 kB
 SReclaimable:   159856 kB
 SUnreclaim: 124508 kB
@@ -898,6 +899,7 @@ MemAvailable: An estimate of how much memory is available 
for starting new
AnonPages: Non-file backed pages mapped into userspace page tables
 AnonHugePages: Non-file backed huge pages mapped into userspace page tables
   Mapped: files which have been mmaped, such as libraries
+   Shmem: Total memory used by shared memory (shmem) and tmpfs
 Slab: in-kernel data structures cache
 SReclaimable: Part of Slab, that might be reclaimed, such as caches
   SUnreclaim: Part of Slab, that cannot be reclaimed on memory pressure
diff --git a/Documentation/filesystems/tmpfs.txt 
b/Documentation/filesystems/tmpfs.txt
index 98ef551..d1abf2d 100644
--- a/Documentation/filesystems/tmpfs.txt
+++ b/Documentation/filesystems/tmpfs.txt
@@ -17,10 +17,10 @@ RAM, where you have to create an ordinary filesystem on 
top. Ramdisks
 cannot swap and you do not have the possibility to resize them. 
 
 Since tmpfs lives completely in the page cache and on swap, all tmpfs
-pages currently in memory will show up as cached. It will not show up
-as shared or something like that. Further on you can check the actual
-RAM+swap use of a tmpfs instance with df(1) and du(1).
-
+pages will be shown as "Shmem" in /proc/meminfo and "Shared" in
+free(1). Notice that these counters also include shared memory
+(shmem, see ipcs(1)). The most reliable way to get the count is
+using df(1) and du(1).
 
 tmpfs has the following uses:
 
-- 
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH RESEND] Documentation: Describe the shared memory usage/accounting

2015-12-05 Thread Rodrigo Freire

The Shared Memory accounting support is present in Kernel since 
commit 4b02108ac1b3 ("mm: oom analysis: add shmem vmstat") and in userland 
free(1) since 2014. This patch updates the Documentation to reflect 
this change. 

Signed-off-by: Rodrigo Freire  
--- 
--- a/Documentation/filesystems/proc.txt 
+++ b/Documentation/filesystems/proc.txt 
@@ -842,6 +842,7 @@ 
Writeback: 0 kB 
AnonPages: 861800 kB 
Mapped: 280372 kB 
+Shmem: 644 kB 
Slab: 284364 kB 
SReclaimable: 159856 kB 
SUnreclaim: 124508 kB 
@@ -898,6 +899,7 @@ 
AnonPages: Non-file backed pages mapped into userspace page tables 
AnonHugePages: Non-file backed huge pages mapped into userspace page tables 
Mapped: files which have been mmaped, such as libraries 
+ Shmem: Total memory used by shared memory (shmem) and tmpfs 
Slab: in-kernel data structures cache 
SReclaimable: Part of Slab, that might be reclaimed, such as caches 
SUnreclaim: Part of Slab, that cannot be reclaimed on memory pressure 
--- a/Documentation/filesystems/tmpfs.txt 
+++ b/Documentation/filesystems/tmpfs.txt 
@@ -17,10 +17,10 @@ 
cannot swap and you do not have the possibility to resize them. 

Since tmpfs lives completely in the page cache and on swap, all tmpfs 
-pages currently in memory will show up as cached. It will not show up 
-as shared or something like that. Further on you can check the actual 
-RAM+swap use of a tmpfs instance with df(1) and du(1). 
- 
+pages will be shown in /proc/meminfo as "Shmem" and "Shared" in 
+free(1). Notice that shared memory pages (see ipcs(1)) will be also 
+counted as shared memory. The most reliable way to get the count is 
+using df(1) and du(1). 

tmpfs has the following uses: 

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


[PATCH RESEND] Documentation: Describe the shared memory usage/accounting

2015-12-05 Thread Rodrigo Freire

The Shared Memory accounting support is present in Kernel since 
commit 4b02108ac1b3 ("mm: oom analysis: add shmem vmstat") and in userland 
free(1) since 2014. This patch updates the Documentation to reflect 
this change. 

Signed-off-by: Rodrigo Freire <rfre...@redhat.com> 
--- 
--- a/Documentation/filesystems/proc.txt 
+++ b/Documentation/filesystems/proc.txt 
@@ -842,6 +842,7 @@ 
Writeback: 0 kB 
AnonPages: 861800 kB 
Mapped: 280372 kB 
+Shmem: 644 kB 
Slab: 284364 kB 
SReclaimable: 159856 kB 
SUnreclaim: 124508 kB 
@@ -898,6 +899,7 @@ 
AnonPages: Non-file backed pages mapped into userspace page tables 
AnonHugePages: Non-file backed huge pages mapped into userspace page tables 
Mapped: files which have been mmaped, such as libraries 
+ Shmem: Total memory used by shared memory (shmem) and tmpfs 
Slab: in-kernel data structures cache 
SReclaimable: Part of Slab, that might be reclaimed, such as caches 
SUnreclaim: Part of Slab, that cannot be reclaimed on memory pressure 
--- a/Documentation/filesystems/tmpfs.txt 
+++ b/Documentation/filesystems/tmpfs.txt 
@@ -17,10 +17,10 @@ 
cannot swap and you do not have the possibility to resize them. 

Since tmpfs lives completely in the page cache and on swap, all tmpfs 
-pages currently in memory will show up as cached. It will not show up 
-as shared or something like that. Further on you can check the actual 
-RAM+swap use of a tmpfs instance with df(1) and du(1). 
- 
+pages will be shown in /proc/meminfo as "Shmem" and "Shared" in 
+free(1). Notice that shared memory pages (see ipcs(1)) will be also 
+counted as shared memory. The most reliable way to get the count is 
+using df(1) and du(1). 

tmpfs has the following uses: 

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


[PATCH] Documentation: Describe the shared memory usage/accounting

2015-11-19 Thread Rodrigo Freire

The Shared Memory accounting support is present in Kernel since 
commit 4b02108ac1b3 ("mm: oom analysis: add shmem vmstat") and in userland
free(1) since 2014. This patch updates the Documentation to reflect
this change.

Signed-off-by: Rodrigo Freire  
---
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -842,6 +842,7 @@
 Writeback:   0 kB
 AnonPages:  861800 kB
 Mapped: 280372 kB
+Shmem: 644 kB
 Slab:   284364 kB
 SReclaimable:   159856 kB
 SUnreclaim: 124508 kB
@@ -898,6 +899,7 @@
AnonPages: Non-file backed pages mapped into userspace page tables
 AnonHugePages: Non-file backed huge pages mapped into userspace page tables
   Mapped: files which have been mmaped, such as libraries
+   Shmem: Total memory used by shared memory (shmem) and tmpfs
 Slab: in-kernel data structures cache
 SReclaimable: Part of Slab, that might be reclaimed, such as caches
   SUnreclaim: Part of Slab, that cannot be reclaimed on memory pressure
--- a/Documentation/filesystems/tmpfs.txt
+++ b/Documentation/filesystems/tmpfs.txt
@@ -17,10 +17,10 @@
 cannot swap and you do not have the possibility to resize them. 
 
 Since tmpfs lives completely in the page cache and on swap, all tmpfs
-pages currently in memory will show up as cached. It will not show up
-as shared or something like that. Further on you can check the actual
-RAM+swap use of a tmpfs instance with df(1) and du(1).
-
+pages will be shown in /proc/meminfo as "Shmem" and "Shared" in
+free(1). Notice that shared memory pages (see ipcs(1)) will be also
+counted as shared memory. The most reliable way to get the count is
+using df(1) and du(1).
 
 tmpfs has the following uses:
 
---
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Documentation: Describe the shared memory usage/accounting

2015-11-19 Thread Rodrigo Freire

The Shared Memory accounting support is present in Kernel since 
commit 4b02108ac1b3 ("mm: oom analysis: add shmem vmstat") and in userland
free(1) since 2014. This patch updates the Documentation to reflect
this change.

Signed-off-by: Rodrigo Freire <rfre...@redhat.com> 
---
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -842,6 +842,7 @@
 Writeback:   0 kB
 AnonPages:  861800 kB
 Mapped: 280372 kB
+Shmem: 644 kB
 Slab:   284364 kB
 SReclaimable:   159856 kB
 SUnreclaim: 124508 kB
@@ -898,6 +899,7 @@
AnonPages: Non-file backed pages mapped into userspace page tables
 AnonHugePages: Non-file backed huge pages mapped into userspace page tables
   Mapped: files which have been mmaped, such as libraries
+   Shmem: Total memory used by shared memory (shmem) and tmpfs
 Slab: in-kernel data structures cache
 SReclaimable: Part of Slab, that might be reclaimed, such as caches
   SUnreclaim: Part of Slab, that cannot be reclaimed on memory pressure
--- a/Documentation/filesystems/tmpfs.txt
+++ b/Documentation/filesystems/tmpfs.txt
@@ -17,10 +17,10 @@
 cannot swap and you do not have the possibility to resize them. 
 
 Since tmpfs lives completely in the page cache and on swap, all tmpfs
-pages currently in memory will show up as cached. It will not show up
-as shared or something like that. Further on you can check the actual
-RAM+swap use of a tmpfs instance with df(1) and du(1).
-
+pages will be shown in /proc/meminfo as "Shmem" and "Shared" in
+free(1). Notice that shared memory pages (see ipcs(1)) will be also
+counted as shared memory. The most reliable way to get the count is
+using df(1) and du(1).
 
 tmpfs has the following uses:
 
---
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 V2] mtd: block2mtd: Present block2mtd timely on boot time

2015-02-11 Thread Rodrigo Freire
From: "Brian Norris"  
Sent: Wednesday, November 26, 2014 1:33:04 AM 
Subject: Re: [PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time 

> On Sun, Nov 09, 2014 at 07:18:05AM -0500, Rodrigo Freire wrote: 
> > > From: "Brian Norris"  
> > > Sent: Wednesday, November 5, 2014 6:23:03 PM 
> > 
> > > This still seems like a bad idea (using a block device + block2mtd + 
> > > JFFS2). Why are you doing this? See comments here: 
> > > http://www.linux-mtd.infradead.org/faq/jffs2.html#L_hdd_jffs2 
> > 
> > As Felix stated on a previous message to the thread, I am using JFFS2 over 
> > block2mtd where regular filesystems failed to do so well. There are several
> > [1] threads pointing this issue, and JFFS2 over block2mtd works like a 
> > charm 
> > on more harsh scenarios. 
> [...] 
> > [1] - http://bit.ly/1smGvwa 

> OK, so there are definitely problems with cheap SD card power cut 
> tolerance. That's not news. But that doesn't mean block2mtd + JFFS2 is a 
> good solution. In fact, when I add 'jffs2' to your Google search query 
> of 'raspberry pi corrupt sd card', the only mentions I see are those who 
> agree that this is not the right choice. 

> But anyway, we can look at supporting block2mtd (since you provided the 
> patches), even if we don't agree how it should be used. And in fact, I 
> might argue there are no good (production) uses for block2mtd, so I 
> suppose I don't have much stake in it :) 

Hi there Brian, 

This patchset primarily aims to fix a block2mtd behavior, and not introduce
new features (well, the device name and a timeout option are indeed new
options, but they're actually enhancements). block2mtd already exists, works
 nicely as boot root device on several architectures, but fails on BCM2835
arch. Our patchset only aims to get it fixed. We just want to block2mtd work
on BCM2835 the way it works on different architectures. So, this is a fix. 

As a side note, WRT the SD card corruption; it also happens on good quality
SD cards too. The main culprit for the corruption is bad mains / power supply
issues / abrupt poweroff. And there's also the wear leveling...

Thanks for the thorough review. 

Looking forward for the ACK ;-) 

My best regards, 

- RF. 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 V2] mtd: block2mtd: Present block2mtd timely on boot time

2015-02-11 Thread Rodrigo Freire
From: Brian Norris computersforpe...@gmail.com 
Sent: Wednesday, November 26, 2014 1:33:04 AM 
Subject: Re: [PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time 

 On Sun, Nov 09, 2014 at 07:18:05AM -0500, Rodrigo Freire wrote: 
   From: Brian Norris computersforpe...@gmail.com 
   Sent: Wednesday, November 5, 2014 6:23:03 PM 
  
   This still seems like a bad idea (using a block device + block2mtd + 
   JFFS2). Why are you doing this? See comments here: 
   http://www.linux-mtd.infradead.org/faq/jffs2.html#L_hdd_jffs2 
  
  As Felix stated on a previous message to the thread, I am using JFFS2 over 
  block2mtd where regular filesystems failed to do so well. There are several
  [1] threads pointing this issue, and JFFS2 over block2mtd works like a 
  charm 
  on more harsh scenarios. 
 [...] 
  [1] - http://bit.ly/1smGvwa 

 OK, so there are definitely problems with cheap SD card power cut 
 tolerance. That's not news. But that doesn't mean block2mtd + JFFS2 is a 
 good solution. In fact, when I add 'jffs2' to your Google search query 
 of 'raspberry pi corrupt sd card', the only mentions I see are those who 
 agree that this is not the right choice. 

 But anyway, we can look at supporting block2mtd (since you provided the 
 patches), even if we don't agree how it should be used. And in fact, I 
 might argue there are no good (production) uses for block2mtd, so I 
 suppose I don't have much stake in it :) 

Hi there Brian, 

This patchset primarily aims to fix a block2mtd behavior, and not introduce
new features (well, the device name and a timeout option are indeed new
options, but they're actually enhancements). block2mtd already exists, works
 nicely as boot root device on several architectures, but fails on BCM2835
arch. Our patchset only aims to get it fixed. We just want to block2mtd work
on BCM2835 the way it works on different architectures. So, this is a fix. 

As a side note, WRT the SD card corruption; it also happens on good quality
SD cards too. The main culprit for the corruption is bad mains / power supply
issues / abrupt poweroff. And there's also the wear leveling...

Thanks for the thorough review. 

Looking forward for the ACK ;-) 

My best regards, 

- RF. 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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 V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-11-26 Thread Rodrigo Freire
From: "Brian Norris" 
Sent: Wednesday, November 26, 2014 1:33:04 AM
Subject: Re: [PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

> On Sun, Nov 09, 2014 at 07:18:05AM -0500, Rodrigo Freire wrote:
> > > From: "Brian Norris" 
> > > Sent: Wednesday, November 5, 2014 6:23:03 PM
> >
> > > This still seems like a bad idea (using a block device + block2mtd +
> > > JFFS2). Why are you doing this? See comments here:
> > > http://www.linux-mtd.infradead.org/faq/jffs2.html#L_hdd_jffs2
> >
> > As Felix stated on a previous message to the thread, I am using JFFS2 over
> > block2mtd where regular filesystems failed to do so well. There are several
> > [1] threads pointing this issue, and JFFS2 over block2mtd works like a
> > charm
> > on more harsh scenarios.
> [...]
> > [1] - http://bit.ly/1smGvwa

> OK, so there are definitely problems with cheap SD card power cut
> tolerance. That's not news. But that doesn't mean block2mtd + JFFS2 is a
> good solution. In fact, when I add 'jffs2' to your Google search query
> of 'raspberry pi corrupt sd card', the only mentions I see are those who
> agree that this is not the right choice.

> But anyway, we can look at supporting block2mtd (since you provided the
> patches), even if we don't agree how it should be used. And in fact, I
> might argue there are no good (production) uses for block2mtd, so I
> suppose I don't have much stake in it :)

Hi there Brian,

This patchset primarily aims to fix a block2mtd behavior, and not introduce new 
features (well, the device name and a timeout option are indeed new options, 
but they're actually enhancements). block2mtd already exists, works nicely as 
boot root device on several architectures, but fails on BCM2835 arch. Our 
patchset only aims to get it fixed. We just want to block2mtd work on BCM2835 
the way it works on different architectures. So, this is a fix.

As a side note, WRT the SD card corruption; it also happens on good quality SD 
cards too. The main culprit for the corruption is bad mains / power supply 
issues / abrupt poweroff.

Thanks for the thorough review.

Looking forward for the ACK ;-)

My best regards,

- RF.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v3 3/3] mtd: block2mtd: Removes PAGE_MASK as a index to partition size

2014-11-26 Thread Rodrigo Freire
From: "Brian Norris" 
Sent: Wednesday, November 26, 2014 5:21:47 AM
Subject: Re: [PATCH v3 3/3] mtd: block2mtd: Removes PAGE_MASK as a index to
partition size

> On Sun, Nov 09, 2014 at 07:23:12AM -0500, Rodrigo Freire wrote:
> > PAGE_MASK is no longer needed with the new term.

> This isn't too descriptive. What you probably mean is that we assume the
> erase size is always larger than PAGE_SIZE.

> > This patch keeps the device size aligned with the erase_size.
> >
> > Signed-off-by: Felix Fietkau 
> > Signed-off-by: Rodrigo Freire 
> > Signed-off-by: Herton Krzesinski 
> > ---
> > V3: Separated on a single patch
> > --- a/drivers/mtd/devices/block2mtd.c 2014-11-07 17:40:58.688747820 -0200
> > +++ b/drivers/mtd/devices/block2mtd.c 2014-11-07 17:41:28.054750893 -0200
> > @@ -291,8 +291,7 @@ static struct block2mtd_dev *add_device(
> > goto err_destroy_mutex;
> >
> > dev->mtd.name = name;
> > -
> > - dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;
> > + dev->mtd.size = dev->blkdev->bd_inode->i_size & ~(erase_size - 1);

> You never guaranteed that erase_size is a power of two, so this doesn't
> necessarily mask the way you'd like...

> But also, why is this even necessary? I see that we should already have
> errored out if this was actually significant, since we have above:

> if ((long)dev->blkdev->bd_inode->i_size % erase_size) {
> pr_err("erasesize must be a divisor of device size\n");
> goto err_free_block2mtd;
> }

Hello Brian, and thanks for the review.

Honestly, I'd leave that untouched, but Jörn pointed that it could be a issue 
at https://lkml.org/lkml/2014/9/9/680

I'd happily let it go without this patch 3, unless Jörg wants to state 
otherwise.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v3 3/3] mtd: block2mtd: Removes PAGE_MASK as a index to partition size

2014-11-26 Thread Rodrigo Freire
From: Brian Norris computersforpe...@gmail.com
Sent: Wednesday, November 26, 2014 5:21:47 AM
Subject: Re: [PATCH v3 3/3] mtd: block2mtd: Removes PAGE_MASK as a index to
partition size

 On Sun, Nov 09, 2014 at 07:23:12AM -0500, Rodrigo Freire wrote:
  PAGE_MASK is no longer needed with the new term.

 This isn't too descriptive. What you probably mean is that we assume the
 erase size is always larger than PAGE_SIZE.

  This patch keeps the device size aligned with the erase_size.
 
  Signed-off-by: Felix Fietkau n...@openwrt.org
  Signed-off-by: Rodrigo Freire rfre...@redhat.com
  Signed-off-by: Herton Krzesinski her...@redhat.com
  ---
  V3: Separated on a single patch
  --- a/drivers/mtd/devices/block2mtd.c 2014-11-07 17:40:58.688747820 -0200
  +++ b/drivers/mtd/devices/block2mtd.c 2014-11-07 17:41:28.054750893 -0200
  @@ -291,8 +291,7 @@ static struct block2mtd_dev *add_device(
  goto err_destroy_mutex;
 
  dev-mtd.name = name;
  -
  - dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK;
  + dev-mtd.size = dev-blkdev-bd_inode-i_size  ~(erase_size - 1);

 You never guaranteed that erase_size is a power of two, so this doesn't
 necessarily mask the way you'd like...

 But also, why is this even necessary? I see that we should already have
 errored out if this was actually significant, since we have above:

 if ((long)dev-blkdev-bd_inode-i_size % erase_size) {
 pr_err(erasesize must be a divisor of device size\n);
 goto err_free_block2mtd;
 }

Hello Brian, and thanks for the review.

Honestly, I'd leave that untouched, but Jörn pointed that it could be a issue 
at https://lkml.org/lkml/2014/9/9/680

I'd happily let it go without this patch 3, unless Jörg wants to state 
otherwise.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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 V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-11-26 Thread Rodrigo Freire
From: Brian Norris computersforpe...@gmail.com
Sent: Wednesday, November 26, 2014 1:33:04 AM
Subject: Re: [PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

 On Sun, Nov 09, 2014 at 07:18:05AM -0500, Rodrigo Freire wrote:
   From: Brian Norris computersforpe...@gmail.com
   Sent: Wednesday, November 5, 2014 6:23:03 PM
 
   This still seems like a bad idea (using a block device + block2mtd +
   JFFS2). Why are you doing this? See comments here:
   http://www.linux-mtd.infradead.org/faq/jffs2.html#L_hdd_jffs2
 
  As Felix stated on a previous message to the thread, I am using JFFS2 over
  block2mtd where regular filesystems failed to do so well. There are several
  [1] threads pointing this issue, and JFFS2 over block2mtd works like a
  charm
  on more harsh scenarios.
 [...]
  [1] - http://bit.ly/1smGvwa

 OK, so there are definitely problems with cheap SD card power cut
 tolerance. That's not news. But that doesn't mean block2mtd + JFFS2 is a
 good solution. In fact, when I add 'jffs2' to your Google search query
 of 'raspberry pi corrupt sd card', the only mentions I see are those who
 agree that this is not the right choice.

 But anyway, we can look at supporting block2mtd (since you provided the
 patches), even if we don't agree how it should be used. And in fact, I
 might argue there are no good (production) uses for block2mtd, so I
 suppose I don't have much stake in it :)

Hi there Brian,

This patchset primarily aims to fix a block2mtd behavior, and not introduce new 
features (well, the device name and a timeout option are indeed new options, 
but they're actually enhancements). block2mtd already exists, works nicely as 
boot root device on several architectures, but fails on BCM2835 arch. Our 
patchset only aims to get it fixed. We just want to block2mtd work on BCM2835 
the way it works on different architectures. So, this is a fix.

As a side note, WRT the SD card corruption; it also happens on good quality SD 
cards too. The main culprit for the corruption is bad mains / power supply 
issues / abrupt poweroff.

Thanks for the thorough review.

Looking forward for the ACK ;-)

My best regards,

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


[PATCH v3 3/3] mtd: block2mtd: Removes PAGE_MASK as a index to partition size

2014-11-09 Thread Rodrigo Freire
From: Felix Fietkau 

mtd: block2mtd: Removes PAGE_MASK as a index to partition size

PAGE_MASK is no longer needed with the new term.
This patch keeps the device size aligned with the erase_size.

Signed-off-by: Felix Fietkau 
Signed-off-by: Rodrigo Freire 
Signed-off-by: Herton Krzesinski 
---
V3: Separated on a single patch
--- a/drivers/mtd/devices/block2mtd.c   2014-11-07 17:40:58.688747820 -0200
+++ b/drivers/mtd/devices/block2mtd.c   2014-11-07 17:41:28.054750893 -0200
@@ -291,8 +291,7 @@ static struct block2mtd_dev *add_device(
goto err_destroy_mutex;
 
dev->mtd.name = name;
-
-   dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;
+   dev->mtd.size = dev->blkdev->bd_inode->i_size & ~(erase_size - 1);
dev->mtd.erasesize = erase_size;
dev->mtd.writesize = 1;
dev->mtd.writebufsize = PAGE_SIZE;
---
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 2/3] mtd: block2mtd: Adds a mtd name and a block device timeout option

2014-11-09 Thread Rodrigo Freire
From: Felix Fietkau 

mtd: block2mtd: Adds a mtd name and a block device timeout option

This patch adds support to a block2mtd MTD name and allows to specify
a block device timeout when adding a block2mtd MTD drive.
Usage: block2mtd=[,[,[,]]]
The devices will be identified the following way:
[root@server01 ~]# cat /proc/mtd
dev:size   erasesize  name
mtd0: a000 0001 "rootfs"
mtd1: 26508 0001 "anothername"
mtd2: acd0 0001 "/dev/mmcblk0p2"
Notice that the device mtd2 was added without specifying a name.

Signed-off-by: Felix Fietkau 
Signed-off-by: Rodrigo Freire 
Signed-off-by: Herton Krzesinski 
---
V3: Separated on a single patch
--- a/drivers/mtd/devices/block2mtd.c   2014-11-07 17:16:11.711479312 -0200
+++ b/drivers/mtd/devices/block2mtd.c   2014-11-07 17:15:14.255464054 -0200
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -218,7 +219,7 @@ static void block2mtd_free_device(struct
 
 
 static struct block2mtd_dev *add_device(char *devname, int erase_size,
-   int timeout)
+   const char *mtdname, int timeout)
 {
 #ifndef MODULE
int i;
@@ -226,6 +227,7 @@ static struct block2mtd_dev *add_device(
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
struct block_device *bdev = ERR_PTR(-ENODEV);
struct block2mtd_dev *dev;
+   struct mtd_partition *part;
char *name;
 
if (!devname)
@@ -282,7 +284,9 @@ static struct block2mtd_dev *add_device(
 
/* Setup the MTD structure */
/* make the name contain the block device in */
-   name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
+   if (!mtdname)
+   mtdname = devname;
+   name = kstrdup(mtdname, GFP_KERNEL);
if (!name)
goto err_destroy_mutex;
 
@@ -301,7 +305,11 @@ static struct block2mtd_dev *add_device(
dev->mtd.priv = dev;
dev->mtd.owner = THIS_MODULE;
 
-   if (mtd_device_register(>mtd, NULL, 0)) {
+   part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL);
+   part->name = name;
+   part->offset = 0;
+   part->size = dev->mtd.size;
+   if (mtd_device_register(>mtd, part, 1)) {
/* Device didn't get added, so free the entry */
goto err_destroy_mutex;
}
@@ -309,8 +317,7 @@ static struct block2mtd_dev *add_device(
list_add(>list, _device_list);
pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n",
dev->mtd.index,
-   dev->mtd.name + strlen("block2mtd: "),
-   dev->mtd.erasesize >> 10, dev->mtd.erasesize);
+   mtdname, dev->mtd.erasesize >> 10, dev->mtd.erasesize);
return dev;
 
 err_destroy_mutex:
@@ -383,7 +390,7 @@ static int block2mtd_setup2(const char *
/* 80 for device, 12 for erase size, 80 for name, 8 for timeout */
char buf[80 + 12 + 80 + 8];
char *str = buf;
-   char *token[2];
+   char *token[4];
char *name;
size_t erase_size = PAGE_SIZE;
unsigned long timeout = MTD_DEFAULT_TIMEOUT;
@@ -397,7 +404,7 @@ static int block2mtd_setup2(const char *
strcpy(str, val);
kill_final_newline(str);
 
-   for (i = 0; i < 2; i++)
+   for (i = 0; i < 4; i++)
token[i] = strsep(, ",");
 
if (str) {
@@ -424,7 +431,13 @@ static int block2mtd_setup2(const char *
}
}
 
-   add_device(name, erase_size, timeout);
+   if (token[2] && (strlen(token[2]) + 1 > 80))
+   pr_err("mtd device name too long");
+
+
+   if (token[3] && kstrtoul(token[3], 0, ))
+   pr_err("invalid timeout");
+   add_device(name, erase_size, token[2], timeout);
 
return 0;
 }
@@ -458,7 +471,7 @@ static int block2mtd_setup(const char *v
 
 
 module_param_call(block2mtd, block2mtd_setup, NULL, NULL, 0200);
-MODULE_PARM_DESC(block2mtd, "Device to use. 
\"block2mtd=[,]\"");
+MODULE_PARM_DESC(block2mtd, "Device to use. 
\"block2mtd=[,[,[,]]]\"");
 
 static int __init block2mtd_init(void)
 {
---
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 1/3] mtd: block2mtd: Ensure that block2mtd is triggered after block devices are presented.

2014-11-09 Thread Rodrigo Freire
From: Felix Fietkau 

mtd: block2mtd: Ensure that block2mtd is triggered after block devices are 
presented.

Ensures that block2mtd is triggered after the block devices are enumerated
at boot time.
This issue is seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
block2mtd filesystems, probably because of the delay on enumerating a USB
MMC card reader.

Signed-off-by: Felix Fietkau 
Signed-off-by: Rodrigo Freire 
Signed-off-by: Herton Krzesinski 
---
V3: Separated on a single patch, fixes a compiling warning, cosmethic changes
V2: Uses kstrdup, removed PAGE_MASK.
--- a/drivers/mtd/devices/block2mtd.c   2014-11-07 16:40:14.638676860 -0200
+++ b/drivers/mtd/devices/block2mtd.c   2014-11-07 17:44:45.277769924 -0200
@@ -9,7 +9,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+/*
+ * When the first attempt at device initialization fails, we may need to
+ * wait a little bit and retry. This timeout, by default 3 seconds, gives
+ * device time to start up. Required on BCM2708 and a few other chipsets.
+ */
+#define MTD_DEFAULT_TIMEOUT3
+
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -209,10 +217,14 @@ static void block2mtd_free_device(struct
 }
 
 
-static struct block2mtd_dev *add_device(char *devname, int erase_size)
+static struct block2mtd_dev *add_device(char *devname, int erase_size,
+   int timeout)
 {
+#ifndef MODULE
+   int i;
+#endif
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
-   struct block_device *bdev;
+   struct block_device *bdev = ERR_PTR(-ENODEV);
struct block2mtd_dev *dev;
char *name;
 
@@ -225,15 +237,28 @@ static struct block2mtd_dev *add_device(
 
/* Get a handle on the device */
bdev = blkdev_get_by_path(devname, mode, dev);
-#ifndef MODULE
-   if (IS_ERR(bdev)) {
 
-   /* We might not have rootfs mounted at this point. Try
-  to resolve the device name by other means. */
+#ifndef MODULE
+/*
+ * We might not have the root device mounted at this point.
+ * Try to resolve the device name by other means.
+ */
+   for (i = 0; IS_ERR(bdev) && i <= timeout; i++) {
+   dev_t devt;
 
-   dev_t devt = name_to_dev_t(devname);
-   if (devt)
-   bdev = blkdev_get_by_dev(devt, mode, dev);
+   if (i)
+   /*
+* Calling wait_for_device_probe in the first loop
+* was not enough, sleep for a bit in subsequent
+* go-arounds.
+*/
+   msleep(1000);
+   wait_for_device_probe();
+
+   devt = name_to_dev_t(devname);
+   if (!devt)
+   continue;
+   bdev = blkdev_get_by_dev(devt, mode, dev);
}
 #endif
 
@@ -280,6 +305,7 @@ static struct block2mtd_dev *add_device(
/* Device didn't get added, so free the entry */
goto err_destroy_mutex;
}
+
list_add(>list, _device_list);
pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n",
dev->mtd.index,
@@ -348,16 +374,19 @@ static inline void kill_final_newline(ch
 
 #ifndef MODULE
 static int block2mtd_init_called = 0;
-static char block2mtd_paramline[80 + 12]; /* 80 for device, 12 for erase size 
*/
+/* 80 for device, 12 for erase size */
+static char block2mtd_paramline[80 + 12];
 #endif
 
 static int block2mtd_setup2(const char *val)
 {
-   char buf[80 + 12]; /* 80 for device, 12 for erase size */
+   /* 80 for device, 12 for erase size, 80 for name, 8 for timeout */
+   char buf[80 + 12 + 80 + 8];
char *str = buf;
char *token[2];
char *name;
size_t erase_size = PAGE_SIZE;
+   unsigned long timeout = MTD_DEFAULT_TIMEOUT;
int i, ret;
 
if (strnlen(val, sizeof(buf)) >= sizeof(buf)) {
@@ -395,7 +424,7 @@ static int block2mtd_setup2(const char *
}
}
 
-   add_device(name, erase_size);
+   add_device(name, erase_size, timeout);
 
return 0;
 }
@@ -463,8 +492,7 @@ static void block2mtd_exit(void)
}
 }
 
-
-module_init(block2mtd_init);
+late_initcall(block2mtd_init);
 module_exit(block2mtd_exit);
 
 MODULE_LICENSE("GPL");
---
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 0/3] mtd: block2mtd: wait for device enumeration, add name support

2014-11-09 Thread Rodrigo Freire
From: Felix Fietkau 

mtd: block2mtd: wait for device enumeration, add name support

Currently, a block MTD device is not presented timely on boot time, in
order to start mounting the filesystems, causing the system to not boot
or panic because of lack of rootfs. This patch ensures that block2mtd
is presented at the right time, so the filesystems can be mounted on boot
time.
This issue was seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
block2mtd filesystems.
This patchset also adds a MTD device name and a timeout option to the driver
and deprecates PAGE_MASK when calculating the device size.
Original patchset:
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444

V3: Split the changes on 3 different patches, fixes a compile warning
V2: Uses kstrdup, removed PAGE_MASK.

 drivers/mtd/devices/block2mtd.c |   57 --
 1 files changed, 42 insertions(+), 15 deletions(-)
 drivers/mtd/devices/block2mtd.c |   32 +++-
 1 files changed, 23 insertions(+), 9 deletions(-)
 drivers/mtd/devices/block2mtd.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
---
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-11-09 Thread Rodrigo Freire
Hi Brian,

> From: "Brian Norris" 
> Sent: Wednesday, November 5, 2014 6:23:03 PM

> On Wed, Sep 17, 2014 at 04:28:03PM -0400, Rodrigo Freire wrote:
> > Currently, a block MTD device is not presented to the system on time, in
> > order to start mounting the filesystems. This patch ensures that block2mtd
> > is presented at the right time, so filesystems can be mounted on boot time.
> > This issue was seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
> > block2mtd filesystems.

> This still seems like a bad idea (using a block device + block2mtd +
> JFFS2). Why are you doing this? See comments here:
> http://www.linux-mtd.infradead.org/faq/jffs2.html#L_hdd_jffs2

As Felix stated on a previous message to the thread, I am using JFFS2 over
block2mtd where regular filesystems failed to do so well. There are several
[1] threads pointing this issue, and JFFS2 over block2mtd works like a charm
on more harsh scenarios. The block2mtd behavior was not working correctly on
BCM2835 architecture (the kernel waited for the block device prior to its
actual enumeration by the kernel) and this patch ensures that block2mtd
kicks in _after_ the block devices was enumerated or after a user-defined
timeout. 
The patchset also enables block2mtd to define a MTD name (a MTD supports it
natively, the block2mtd had its name hard-coded to its block device name).

> You're stating right up front that this patch is doing several different
> things. Please split these up into separate commits which get their own
> description.

Done. I'll send a new split V3 patchset.

> You have several checkpatch warnings. Please fix them.

Thanks for pointing. Done.

> The addition of this name parameter should definitely be its own patch.

I agree. Done.

> This variable produces a warning when built as a module:

> drivers/mtd/devices/block2mtd.c: In function ‘add_device’:
> drivers/mtd/devices/block2mtd.c:228:6: warning: unused variable ‘i’
> [-Wunused-variable]
> int i;
> ^

Oooops. Fixed.

> > +#ifndef MODULE
> > +/*
> > +* We might not have the root device mounted at this point.
> > +* Try to resolve the device name by other means.
> > +*/

> These lines should start with a space, so the asterisks line up.

Fixed, and also fixed other non-aligned asterisks as well.

> > - dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;
> > + dev->mtd.size = dev->blkdev->bd_inode->i_size & ~(erase_size - 1);

> This deserves its own patch, or at least some explanation of why you're
> doing this. I guess you're seeing cases where the provided erasesize is
> not aligned with the size of the block device?

Jörg pointed it on https://lkml.org/lkml/2014/9/9/680. Now, we just keep
it aligned with erase_size. Separated on a 3rd patch.

> > dev->mtd.erasesize = erase_size;
> > dev->mtd.writesize = 1;
> > dev->mtd.writebufsize = PAGE_SIZE;
> > @@ -276,15 +301,19 @@ static struct block2mtd_dev *add_device(
> > dev->mtd.priv = dev;
> > dev->mtd.owner = THIS_MODULE;
> >
> > - if (mtd_device_register(>mtd, NULL, 0)) {
> > + part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL);
> > + part->name = name;
> > + part->offset = 0;
> > + part->size = dev->mtd.size;

> Why are you doing this? This also does not fit the description of this
> patch. And what's wrong with using the default partitioning options?
> Won't we (if not specified in some other way) default to an
> unpartitioned MTD, which covers the entire device?

This code was changed in order to support a MTD name.

Thanks for your dilligent review.

Best regards,

- RF.

[1] - http://bit.ly/1smGvwa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-11-09 Thread Rodrigo Freire
Hi Brian,

 From: Brian Norris computersforpe...@gmail.com
 Sent: Wednesday, November 5, 2014 6:23:03 PM

 On Wed, Sep 17, 2014 at 04:28:03PM -0400, Rodrigo Freire wrote:
  Currently, a block MTD device is not presented to the system on time, in
  order to start mounting the filesystems. This patch ensures that block2mtd
  is presented at the right time, so filesystems can be mounted on boot time.
  This issue was seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
  block2mtd filesystems.

 This still seems like a bad idea (using a block device + block2mtd +
 JFFS2). Why are you doing this? See comments here:
 http://www.linux-mtd.infradead.org/faq/jffs2.html#L_hdd_jffs2

As Felix stated on a previous message to the thread, I am using JFFS2 over
block2mtd where regular filesystems failed to do so well. There are several
[1] threads pointing this issue, and JFFS2 over block2mtd works like a charm
on more harsh scenarios. The block2mtd behavior was not working correctly on
BCM2835 architecture (the kernel waited for the block device prior to its
actual enumeration by the kernel) and this patch ensures that block2mtd
kicks in _after_ the block devices was enumerated or after a user-defined
timeout. 
The patchset also enables block2mtd to define a MTD name (a MTD supports it
natively, the block2mtd had its name hard-coded to its block device name).

 You're stating right up front that this patch is doing several different
 things. Please split these up into separate commits which get their own
 description.

Done. I'll send a new split V3 patchset.

 You have several checkpatch warnings. Please fix them.

Thanks for pointing. Done.

 The addition of this name parameter should definitely be its own patch.

I agree. Done.

 This variable produces a warning when built as a module:

 drivers/mtd/devices/block2mtd.c: In function ‘add_device’:
 drivers/mtd/devices/block2mtd.c:228:6: warning: unused variable ‘i’
 [-Wunused-variable]
 int i;
 ^

Oooops. Fixed.

  +#ifndef MODULE
  +/*
  +* We might not have the root device mounted at this point.
  +* Try to resolve the device name by other means.
  +*/

 These lines should start with a space, so the asterisks line up.

Fixed, and also fixed other non-aligned asterisks as well.

  - dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK;
  + dev-mtd.size = dev-blkdev-bd_inode-i_size  ~(erase_size - 1);

 This deserves its own patch, or at least some explanation of why you're
 doing this. I guess you're seeing cases where the provided erasesize is
 not aligned with the size of the block device?

Jörg pointed it on https://lkml.org/lkml/2014/9/9/680. Now, we just keep
it aligned with erase_size. Separated on a 3rd patch.

  dev-mtd.erasesize = erase_size;
  dev-mtd.writesize = 1;
  dev-mtd.writebufsize = PAGE_SIZE;
  @@ -276,15 +301,19 @@ static struct block2mtd_dev *add_device(
  dev-mtd.priv = dev;
  dev-mtd.owner = THIS_MODULE;
 
  - if (mtd_device_register(dev-mtd, NULL, 0)) {
  + part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL);
  + part-name = name;
  + part-offset = 0;
  + part-size = dev-mtd.size;

 Why are you doing this? This also does not fit the description of this
 patch. And what's wrong with using the default partitioning options?
 Won't we (if not specified in some other way) default to an
 unpartitioned MTD, which covers the entire device?

This code was changed in order to support a MTD name.

Thanks for your dilligent review.

Best regards,

- RF.

[1] - http://bit.ly/1smGvwa
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 0/3] mtd: block2mtd: wait for device enumeration, add name support

2014-11-09 Thread Rodrigo Freire
From: Felix Fietkau n...@openwrt.org

mtd: block2mtd: wait for device enumeration, add name support

Currently, a block MTD device is not presented timely on boot time, in
order to start mounting the filesystems, causing the system to not boot
or panic because of lack of rootfs. This patch ensures that block2mtd
is presented at the right time, so the filesystems can be mounted on boot
time.
This issue was seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
block2mtd filesystems.
This patchset also adds a MTD device name and a timeout option to the driver
and deprecates PAGE_MASK when calculating the device size.
Original patchset:
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444

V3: Split the changes on 3 different patches, fixes a compile warning
V2: Uses kstrdup, removed PAGE_MASK.

 drivers/mtd/devices/block2mtd.c |   57 --
 1 files changed, 42 insertions(+), 15 deletions(-)
 drivers/mtd/devices/block2mtd.c |   32 +++-
 1 files changed, 23 insertions(+), 9 deletions(-)
 drivers/mtd/devices/block2mtd.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
---
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 1/3] mtd: block2mtd: Ensure that block2mtd is triggered after block devices are presented.

2014-11-09 Thread Rodrigo Freire
From: Felix Fietkau n...@openwrt.org

mtd: block2mtd: Ensure that block2mtd is triggered after block devices are 
presented.

Ensures that block2mtd is triggered after the block devices are enumerated
at boot time.
This issue is seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
block2mtd filesystems, probably because of the delay on enumerating a USB
MMC card reader.

Signed-off-by: Felix Fietkau n...@openwrt.org
Signed-off-by: Rodrigo Freire rfre...@redhat.com
Signed-off-by: Herton Krzesinski her...@redhat.com
---
V3: Separated on a single patch, fixes a compiling warning, cosmethic changes
V2: Uses kstrdup, removed PAGE_MASK.
--- a/drivers/mtd/devices/block2mtd.c   2014-11-07 16:40:14.638676860 -0200
+++ b/drivers/mtd/devices/block2mtd.c   2014-11-07 17:44:45.277769924 -0200
@@ -9,7 +9,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 
+/*
+ * When the first attempt at device initialization fails, we may need to
+ * wait a little bit and retry. This timeout, by default 3 seconds, gives
+ * device time to start up. Required on BCM2708 and a few other chipsets.
+ */
+#define MTD_DEFAULT_TIMEOUT3
+
 #include linux/module.h
+#include linux/delay.h
 #include linux/fs.h
 #include linux/blkdev.h
 #include linux/bio.h
@@ -209,10 +217,14 @@ static void block2mtd_free_device(struct
 }
 
 
-static struct block2mtd_dev *add_device(char *devname, int erase_size)
+static struct block2mtd_dev *add_device(char *devname, int erase_size,
+   int timeout)
 {
+#ifndef MODULE
+   int i;
+#endif
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
-   struct block_device *bdev;
+   struct block_device *bdev = ERR_PTR(-ENODEV);
struct block2mtd_dev *dev;
char *name;
 
@@ -225,15 +237,28 @@ static struct block2mtd_dev *add_device(
 
/* Get a handle on the device */
bdev = blkdev_get_by_path(devname, mode, dev);
-#ifndef MODULE
-   if (IS_ERR(bdev)) {
 
-   /* We might not have rootfs mounted at this point. Try
-  to resolve the device name by other means. */
+#ifndef MODULE
+/*
+ * We might not have the root device mounted at this point.
+ * Try to resolve the device name by other means.
+ */
+   for (i = 0; IS_ERR(bdev)  i = timeout; i++) {
+   dev_t devt;
 
-   dev_t devt = name_to_dev_t(devname);
-   if (devt)
-   bdev = blkdev_get_by_dev(devt, mode, dev);
+   if (i)
+   /*
+* Calling wait_for_device_probe in the first loop
+* was not enough, sleep for a bit in subsequent
+* go-arounds.
+*/
+   msleep(1000);
+   wait_for_device_probe();
+
+   devt = name_to_dev_t(devname);
+   if (!devt)
+   continue;
+   bdev = blkdev_get_by_dev(devt, mode, dev);
}
 #endif
 
@@ -280,6 +305,7 @@ static struct block2mtd_dev *add_device(
/* Device didn't get added, so free the entry */
goto err_destroy_mutex;
}
+
list_add(dev-list, blkmtd_device_list);
pr_info(mtd%d: [%s] erase_size = %dKiB [%d]\n,
dev-mtd.index,
@@ -348,16 +374,19 @@ static inline void kill_final_newline(ch
 
 #ifndef MODULE
 static int block2mtd_init_called = 0;
-static char block2mtd_paramline[80 + 12]; /* 80 for device, 12 for erase size 
*/
+/* 80 for device, 12 for erase size */
+static char block2mtd_paramline[80 + 12];
 #endif
 
 static int block2mtd_setup2(const char *val)
 {
-   char buf[80 + 12]; /* 80 for device, 12 for erase size */
+   /* 80 for device, 12 for erase size, 80 for name, 8 for timeout */
+   char buf[80 + 12 + 80 + 8];
char *str = buf;
char *token[2];
char *name;
size_t erase_size = PAGE_SIZE;
+   unsigned long timeout = MTD_DEFAULT_TIMEOUT;
int i, ret;
 
if (strnlen(val, sizeof(buf)) = sizeof(buf)) {
@@ -395,7 +424,7 @@ static int block2mtd_setup2(const char *
}
}
 
-   add_device(name, erase_size);
+   add_device(name, erase_size, timeout);
 
return 0;
 }
@@ -463,8 +492,7 @@ static void block2mtd_exit(void)
}
 }
 
-
-module_init(block2mtd_init);
+late_initcall(block2mtd_init);
 module_exit(block2mtd_exit);
 
 MODULE_LICENSE(GPL);
---
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 2/3] mtd: block2mtd: Adds a mtd name and a block device timeout option

2014-11-09 Thread Rodrigo Freire
From: Felix Fietkau n...@openwrt.org

mtd: block2mtd: Adds a mtd name and a block device timeout option

This patch adds support to a block2mtd MTD name and allows to specify
a block device timeout when adding a block2mtd MTD drive.
Usage: block2mtd=dev[,erasesize[,name[,timeout]]]
The devices will be identified the following way:
[root@server01 ~]# cat /proc/mtd
dev:size   erasesize  name
mtd0: a000 0001 rootfs
mtd1: 26508 0001 anothername
mtd2: acd0 0001 /dev/mmcblk0p2
Notice that the device mtd2 was added without specifying a name.

Signed-off-by: Felix Fietkau n...@openwrt.org
Signed-off-by: Rodrigo Freire rfre...@redhat.com
Signed-off-by: Herton Krzesinski her...@redhat.com
---
V3: Separated on a single patch
--- a/drivers/mtd/devices/block2mtd.c   2014-11-07 17:16:11.711479312 -0200
+++ b/drivers/mtd/devices/block2mtd.c   2014-11-07 17:15:14.255464054 -0200
@@ -25,6 +25,7 @@
 #include linux/list.h
 #include linux/init.h
 #include linux/mtd/mtd.h
+#include linux/mtd/partitions.h
 #include linux/mutex.h
 #include linux/mount.h
 #include linux/slab.h
@@ -218,7 +219,7 @@ static void block2mtd_free_device(struct
 
 
 static struct block2mtd_dev *add_device(char *devname, int erase_size,
-   int timeout)
+   const char *mtdname, int timeout)
 {
 #ifndef MODULE
int i;
@@ -226,6 +227,7 @@ static struct block2mtd_dev *add_device(
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
struct block_device *bdev = ERR_PTR(-ENODEV);
struct block2mtd_dev *dev;
+   struct mtd_partition *part;
char *name;
 
if (!devname)
@@ -282,7 +284,9 @@ static struct block2mtd_dev *add_device(
 
/* Setup the MTD structure */
/* make the name contain the block device in */
-   name = kasprintf(GFP_KERNEL, block2mtd: %s, devname);
+   if (!mtdname)
+   mtdname = devname;
+   name = kstrdup(mtdname, GFP_KERNEL);
if (!name)
goto err_destroy_mutex;
 
@@ -301,7 +305,11 @@ static struct block2mtd_dev *add_device(
dev-mtd.priv = dev;
dev-mtd.owner = THIS_MODULE;
 
-   if (mtd_device_register(dev-mtd, NULL, 0)) {
+   part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL);
+   part-name = name;
+   part-offset = 0;
+   part-size = dev-mtd.size;
+   if (mtd_device_register(dev-mtd, part, 1)) {
/* Device didn't get added, so free the entry */
goto err_destroy_mutex;
}
@@ -309,8 +317,7 @@ static struct block2mtd_dev *add_device(
list_add(dev-list, blkmtd_device_list);
pr_info(mtd%d: [%s] erase_size = %dKiB [%d]\n,
dev-mtd.index,
-   dev-mtd.name + strlen(block2mtd: ),
-   dev-mtd.erasesize  10, dev-mtd.erasesize);
+   mtdname, dev-mtd.erasesize  10, dev-mtd.erasesize);
return dev;
 
 err_destroy_mutex:
@@ -383,7 +390,7 @@ static int block2mtd_setup2(const char *
/* 80 for device, 12 for erase size, 80 for name, 8 for timeout */
char buf[80 + 12 + 80 + 8];
char *str = buf;
-   char *token[2];
+   char *token[4];
char *name;
size_t erase_size = PAGE_SIZE;
unsigned long timeout = MTD_DEFAULT_TIMEOUT;
@@ -397,7 +404,7 @@ static int block2mtd_setup2(const char *
strcpy(str, val);
kill_final_newline(str);
 
-   for (i = 0; i  2; i++)
+   for (i = 0; i  4; i++)
token[i] = strsep(str, ,);
 
if (str) {
@@ -424,7 +431,13 @@ static int block2mtd_setup2(const char *
}
}
 
-   add_device(name, erase_size, timeout);
+   if (token[2]  (strlen(token[2]) + 1  80))
+   pr_err(mtd device name too long);
+
+
+   if (token[3]  kstrtoul(token[3], 0, timeout))
+   pr_err(invalid timeout);
+   add_device(name, erase_size, token[2], timeout);
 
return 0;
 }
@@ -458,7 +471,7 @@ static int block2mtd_setup(const char *v
 
 
 module_param_call(block2mtd, block2mtd_setup, NULL, NULL, 0200);
-MODULE_PARM_DESC(block2mtd, Device to use. 
\block2mtd=dev[,erasesize]\);
+MODULE_PARM_DESC(block2mtd, Device to use. 
\block2mtd=dev[,erasesize[,name[,timeout]]]\);
 
 static int __init block2mtd_init(void)
 {
---
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 3/3] mtd: block2mtd: Removes PAGE_MASK as a index to partition size

2014-11-09 Thread Rodrigo Freire
From: Felix Fietkau n...@openwrt.org

mtd: block2mtd: Removes PAGE_MASK as a index to partition size

PAGE_MASK is no longer needed with the new term.
This patch keeps the device size aligned with the erase_size.

Signed-off-by: Felix Fietkau n...@openwrt.org
Signed-off-by: Rodrigo Freire rfre...@redhat.com
Signed-off-by: Herton Krzesinski her...@redhat.com
---
V3: Separated on a single patch
--- a/drivers/mtd/devices/block2mtd.c   2014-11-07 17:40:58.688747820 -0200
+++ b/drivers/mtd/devices/block2mtd.c   2014-11-07 17:41:28.054750893 -0200
@@ -291,8 +291,7 @@ static struct block2mtd_dev *add_device(
goto err_destroy_mutex;
 
dev-mtd.name = name;
-
-   dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK;
+   dev-mtd.size = dev-blkdev-bd_inode-i_size  ~(erase_size - 1);
dev-mtd.erasesize = erase_size;
dev-mtd.writesize = 1;
dev-mtd.writebufsize = PAGE_SIZE;
---
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RESEND PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-11-01 Thread Rodrigo Freire
From: Felix Fietkau  

mtd: block2mtd: Ensure that block2mtd is presented timely on boot time

Currently, a block MTD device is not presented to the system on time, in 
order to start mounting the filesystems. This patch ensures that block2mtd 
is presented at the right time, so filesystems can be mounted on boot time. 
This issue is seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2 
over block2mtd devices as the root filesystem.
This patchset also adds a MTD device name and a timeout option to the driver. 
Original patchset: 
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
 
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444
 

Signed-off-by: Felix Fietkau  
Signed-off-by: Herton Ronaldo Krzesinski  
Signed-off-by: Rodrigo Freire  
--- 
V2: Uses kstrdup, removed PAGE_MASK. 
--- a/drivers/mtd/devices/block2mtd.c 2014-09-16 21:38:12.543952627 -0300 
+++ b/drivers/mtd/devices/block2mtd.c 2014-09-17 17:43:21.424944394 -0300 
@@ -9,7 +9,15 @@ 

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt 

+/* 
+* When the first attempt at device initialization fails, we may need to 
+* wait a little bit and retry. This timeout, by default 3 seconds, gives 
+* device time to start up. Required on BCM2835.
+*/ 
+#define MTD_DEFAULT_TIMEOUT 3 
+ 
#include  
+#include  
#include  
#include  
#include  
@@ -17,6 +25,7 @@ 
#include  
#include  
#include  
+#include  
#include  
#include  
#include  
@@ -209,12 +218,14 @@ static void block2mtd_free_device(struct 
} 


-static struct block2mtd_dev *add_device(char *devname, int erase_size) 
+static struct block2mtd_dev *add_device(char *devname, int erase_size, const 
char *mtdname, int timeout) 
{ 
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL; 
- struct block_device *bdev; 
+ struct block_device *bdev = ERR_PTR(-ENODEV); 
struct block2mtd_dev *dev; 
+ struct mtd_partition *part; 
char *name; 
+ int i; 

if (!devname) 
return NULL; 
@@ -225,15 +236,28 @@ static struct block2mtd_dev *add_device( 

/* Get a handle on the device */ 
bdev = blkdev_get_by_path(devname, mode, dev); 
-#ifndef MODULE 
- if (IS_ERR(bdev)) { 

- /* We might not have rootfs mounted at this point. Try 
- to resolve the device name by other means. */ 
- 
- dev_t devt = name_to_dev_t(devname); 
- if (devt) 
- bdev = blkdev_get_by_dev(devt, mode, dev); 
+#ifndef MODULE 
+/* 
+* We might not have the root device mounted at this point. 
+* Try to resolve the device name by other means. 
+*/ 
+ for (i = 0; IS_ERR(bdev) && i <= timeout; i++) { 
+ dev_t devt; 
+ 
+ if (i) 
+ /* 
+ * Calling wait_for_device_probe in the first loop 
+ * was not enough, sleep for a bit in subsequent 
+ * go-arounds. 
+ */ 
+ msleep(1000); 
+ wait_for_device_probe(); 
+ 
+ devt = name_to_dev_t(devname); 
+ if (!devt) 
+ continue; 
+ bdev = blkdev_get_by_dev(devt, mode, dev); 
} 
#endif 

@@ -257,13 +281,14 @@ static struct block2mtd_dev *add_device( 

/* Setup the MTD structure */ 
/* make the name contain the block device in */ 
- name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname); 
+ if (!mtdname) 
+ mtdname = devname; 
+ name = kstrdup (mtdname, GFP_KERNEL); 
if (!name) 
goto err_destroy_mutex; 

dev->mtd.name = name; 
- 
- dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK; 
+ dev->mtd.size = dev->blkdev->bd_inode->i_size & ~(erase_size - 1); 
dev->mtd.erasesize = erase_size; 
dev->mtd.writesize = 1; 
dev->mtd.writebufsize = PAGE_SIZE; 
@@ -276,15 +301,19 @@ static struct block2mtd_dev *add_device( 
dev->mtd.priv = dev; 
dev->mtd.owner = THIS_MODULE; 

- if (mtd_device_register(>mtd, NULL, 0)) { 
+ part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL); 
+ part->name = name; 
+ part->offset = 0; 
+ part->size = dev->mtd.size; 
+ if (mtd_device_register(>mtd, part, 1)) { 
/* Device didn't get added, so free the entry */ 
goto err_destroy_mutex; 
} 
+ 
list_add(>list, _device_list); 
pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n", 
dev->mtd.index, 
- dev->mtd.name + strlen("block2mtd: "), 
- dev->mtd.erasesize >> 10, dev->mtd.erasesize); 
+ mtdname, dev->mtd.erasesize >> 10, dev->mtd.erasesize); 
return dev; 

err_destroy_mutex: 
@@ -353,11 +382,12 @@ static char block2mtd_paramline[80 + 12] 

static int block2mtd_setup2(const char *val) 
{ 
- char buf[80 + 12]; /* 80 for device, 12 for erase size */ 
+ char buf[80 + 12 + 80 + 8]; /* 80 for device, 12 for erase size, 80 for name, 
8 for timeout */ 
char *str = buf; 
- char *token[2]; 
+ char *token[4]; 
char *name; 
size_t erase_size = PAGE_SIZE; 
+ unsigned long timeout = MTD_DEFAULT_TIMEOUT; 
int i, ret; 

if (strnlen(val, sizeof(buf)) >= sizeof(buf)) { 
@@ -368,7 +398,7 @@ static int block2mtd_setup2(const char * 
strcpy(str, val); 
kill_final_newline(str); 

- for (i = 0; i < 

[RESEND PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-11-01 Thread Rodrigo Freire
From: Felix Fietkau n...@openwrt.org 

mtd: block2mtd: Ensure that block2mtd is presented timely on boot time

Currently, a block MTD device is not presented to the system on time, in 
order to start mounting the filesystems. This patch ensures that block2mtd 
is presented at the right time, so filesystems can be mounted on boot time. 
This issue is seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2 
over block2mtd devices as the root filesystem.
This patchset also adds a MTD device name and a timeout option to the driver. 
Original patchset: 
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
 
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444
 

Signed-off-by: Felix Fietkau n...@openwrt.org 
Signed-off-by: Herton Ronaldo Krzesinski her...@redhat.com 
Signed-off-by: Rodrigo Freire rfre...@redhat.com 
--- 
V2: Uses kstrdup, removed PAGE_MASK. 
--- a/drivers/mtd/devices/block2mtd.c 2014-09-16 21:38:12.543952627 -0300 
+++ b/drivers/mtd/devices/block2mtd.c 2014-09-17 17:43:21.424944394 -0300 
@@ -9,7 +9,15 @@ 

#define pr_fmt(fmt) KBUILD_MODNAME :  fmt 

+/* 
+* When the first attempt at device initialization fails, we may need to 
+* wait a little bit and retry. This timeout, by default 3 seconds, gives 
+* device time to start up. Required on BCM2835.
+*/ 
+#define MTD_DEFAULT_TIMEOUT 3 
+ 
#include linux/module.h 
+#include linux/delay.h 
#include linux/fs.h 
#include linux/blkdev.h 
#include linux/bio.h 
@@ -17,6 +25,7 @@ 
#include linux/list.h 
#include linux/init.h 
#include linux/mtd/mtd.h 
+#include linux/mtd/partitions.h 
#include linux/mutex.h 
#include linux/mount.h 
#include linux/slab.h 
@@ -209,12 +218,14 @@ static void block2mtd_free_device(struct 
} 


-static struct block2mtd_dev *add_device(char *devname, int erase_size) 
+static struct block2mtd_dev *add_device(char *devname, int erase_size, const 
char *mtdname, int timeout) 
{ 
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL; 
- struct block_device *bdev; 
+ struct block_device *bdev = ERR_PTR(-ENODEV); 
struct block2mtd_dev *dev; 
+ struct mtd_partition *part; 
char *name; 
+ int i; 

if (!devname) 
return NULL; 
@@ -225,15 +236,28 @@ static struct block2mtd_dev *add_device( 

/* Get a handle on the device */ 
bdev = blkdev_get_by_path(devname, mode, dev); 
-#ifndef MODULE 
- if (IS_ERR(bdev)) { 

- /* We might not have rootfs mounted at this point. Try 
- to resolve the device name by other means. */ 
- 
- dev_t devt = name_to_dev_t(devname); 
- if (devt) 
- bdev = blkdev_get_by_dev(devt, mode, dev); 
+#ifndef MODULE 
+/* 
+* We might not have the root device mounted at this point. 
+* Try to resolve the device name by other means. 
+*/ 
+ for (i = 0; IS_ERR(bdev)  i = timeout; i++) { 
+ dev_t devt; 
+ 
+ if (i) 
+ /* 
+ * Calling wait_for_device_probe in the first loop 
+ * was not enough, sleep for a bit in subsequent 
+ * go-arounds. 
+ */ 
+ msleep(1000); 
+ wait_for_device_probe(); 
+ 
+ devt = name_to_dev_t(devname); 
+ if (!devt) 
+ continue; 
+ bdev = blkdev_get_by_dev(devt, mode, dev); 
} 
#endif 

@@ -257,13 +281,14 @@ static struct block2mtd_dev *add_device( 

/* Setup the MTD structure */ 
/* make the name contain the block device in */ 
- name = kasprintf(GFP_KERNEL, block2mtd: %s, devname); 
+ if (!mtdname) 
+ mtdname = devname; 
+ name = kstrdup (mtdname, GFP_KERNEL); 
if (!name) 
goto err_destroy_mutex; 

dev-mtd.name = name; 
- 
- dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK; 
+ dev-mtd.size = dev-blkdev-bd_inode-i_size  ~(erase_size - 1); 
dev-mtd.erasesize = erase_size; 
dev-mtd.writesize = 1; 
dev-mtd.writebufsize = PAGE_SIZE; 
@@ -276,15 +301,19 @@ static struct block2mtd_dev *add_device( 
dev-mtd.priv = dev; 
dev-mtd.owner = THIS_MODULE; 

- if (mtd_device_register(dev-mtd, NULL, 0)) { 
+ part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL); 
+ part-name = name; 
+ part-offset = 0; 
+ part-size = dev-mtd.size; 
+ if (mtd_device_register(dev-mtd, part, 1)) { 
/* Device didn't get added, so free the entry */ 
goto err_destroy_mutex; 
} 
+ 
list_add(dev-list, blkmtd_device_list); 
pr_info(mtd%d: [%s] erase_size = %dKiB [%d]\n, 
dev-mtd.index, 
- dev-mtd.name + strlen(block2mtd: ), 
- dev-mtd.erasesize  10, dev-mtd.erasesize); 
+ mtdname, dev-mtd.erasesize  10, dev-mtd.erasesize); 
return dev; 

err_destroy_mutex: 
@@ -353,11 +382,12 @@ static char block2mtd_paramline[80 + 12] 

static int block2mtd_setup2(const char *val) 
{ 
- char buf[80 + 12]; /* 80 for device, 12 for erase size */ 
+ char buf[80 + 12 + 80 + 8]; /* 80 for device, 12 for erase size, 80 for name, 
8 for timeout */ 
char *str = buf; 
- char *token[2]; 
+ char *token[4]; 
char *name; 
size_t erase_size = PAGE_SIZE; 
+ unsigned long timeout = MTD_DEFAULT_TIMEOUT; 
int i, ret; 

if (strnlen(val, sizeof(buf)) = sizeof(buf)) { 
@@ -368,7 +398,7 @@ static int block2mtd_setup2(const char * 
strcpy(str, val

[RESEND PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-10-09 Thread Rodrigo Freire
From: Felix Fietkau  

mtd: block2mtd: Ensure that block2mtd is presented in a timely fashion 

Currently, a block MTD device is not presented to the system on time, in 
order to start mounting the filesystems. This patch ensures that block2mtd 
is presented at the right time, so filesystems can be mounted on boot time. 
This issue was seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2 
block2mtd filesystems. 
This patchset also adds a MTD device name and a timeout option to the driver. 
Original patchset: 
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
 
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444
 

Signed-off-by: Felix Fietkau  
Signed-off-by: Rodrigo Freire  
Signed-off-by: Herton Krzesinski  
--- 
V2: Uses kstrdup, removed PAGE_MASK. 
--- a/drivers/mtd/devices/block2mtd.c 2014-09-16 21:38:12.543952627 -0300 
+++ b/drivers/mtd/devices/block2mtd.c 2014-09-17 17:43:21.424944394 -0300 
@@ -9,7 +9,15 @@ 

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt 

+/* 
+* When the first attempt at device initialization fails, we may need to 
+* wait a little bit and retry. This timeout, by default 3 seconds, gives 
+* device time to start up. Required on BCM2708 and a few other chipsets. 
+*/ 
+#define MTD_DEFAULT_TIMEOUT 3 
+ 
#include  
+#include  
#include  
#include  
#include  
@@ -17,6 +25,7 @@ 
#include  
#include  
#include  
+#include  
#include  
#include  
#include  
@@ -209,12 +218,14 @@ static void block2mtd_free_device(struct 
} 


-static struct block2mtd_dev *add_device(char *devname, int erase_size) 
+static struct block2mtd_dev *add_device(char *devname, int erase_size, const 
char *mtdname, int timeout) 
{ 
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL; 
- struct block_device *bdev; 
+ struct block_device *bdev = ERR_PTR(-ENODEV); 
struct block2mtd_dev *dev; 
+ struct mtd_partition *part; 
char *name; 
+ int i; 

if (!devname) 
return NULL; 
@@ -225,15 +236,28 @@ static struct block2mtd_dev *add_device( 

/* Get a handle on the device */ 
bdev = blkdev_get_by_path(devname, mode, dev); 
-#ifndef MODULE 
- if (IS_ERR(bdev)) { 

- /* We might not have rootfs mounted at this point. Try 
- to resolve the device name by other means. */ 
- 
- dev_t devt = name_to_dev_t(devname); 
- if (devt) 
- bdev = blkdev_get_by_dev(devt, mode, dev); 
+#ifndef MODULE 
+/* 
+* We might not have the root device mounted at this point. 
+* Try to resolve the device name by other means. 
+*/ 
+ for (i = 0; IS_ERR(bdev) && i <= timeout; i++) { 
+ dev_t devt; 
+ 
+ if (i) 
+ /* 
+ * Calling wait_for_device_probe in the first loop 
+ * was not enough, sleep for a bit in subsequent 
+ * go-arounds. 
+ */ 
+ msleep(1000); 
+ wait_for_device_probe(); 
+ 
+ devt = name_to_dev_t(devname); 
+ if (!devt) 
+ continue; 
+ bdev = blkdev_get_by_dev(devt, mode, dev); 
} 
#endif 

@@ -257,13 +281,14 @@ static struct block2mtd_dev *add_device( 

/* Setup the MTD structure */ 
/* make the name contain the block device in */ 
- name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname); 
+ if (!mtdname) 
+ mtdname = devname; 
+ name = kstrdup (mtdname, GFP_KERNEL); 
if (!name) 
goto err_destroy_mutex; 

dev->mtd.name = name; 
- 
- dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK; 
+ dev->mtd.size = dev->blkdev->bd_inode->i_size & ~(erase_size - 1); 
dev->mtd.erasesize = erase_size; 
dev->mtd.writesize = 1; 
dev->mtd.writebufsize = PAGE_SIZE; 
@@ -276,15 +301,19 @@ static struct block2mtd_dev *add_device( 
dev->mtd.priv = dev; 
dev->mtd.owner = THIS_MODULE; 

- if (mtd_device_register(>mtd, NULL, 0)) { 
+ part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL); 
+ part->name = name; 
+ part->offset = 0; 
+ part->size = dev->mtd.size; 
+ if (mtd_device_register(>mtd, part, 1)) { 
/* Device didn't get added, so free the entry */ 
goto err_destroy_mutex; 
} 
+ 
list_add(>list, _device_list); 
pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n", 
dev->mtd.index, 
- dev->mtd.name + strlen("block2mtd: "), 
- dev->mtd.erasesize >> 10, dev->mtd.erasesize); 
+ mtdname, dev->mtd.erasesize >> 10, dev->mtd.erasesize); 
return dev; 

err_destroy_mutex: 
@@ -353,11 +382,12 @@ static char block2mtd_paramline[80 + 12] 

static int block2mtd_setup2(const char *val) 
{ 
- char buf[80 + 12]; /* 80 for device, 12 for erase size */ 
+ char buf[80 + 12 + 80 + 8]; /* 80 for device, 12 for erase size, 80 for name, 
8 for timeout */ 
char *str = buf; 
- char *token[2]; 
+ char *token[4]; 
char *name; 
size_t erase_size = PAGE_SIZE; 
+ unsigned long timeout = MTD_DEFAULT_TIMEOUT; 
int i, ret; 

if (strnlen(val, sizeof(buf)) >= sizeof(buf)) { 
@@ -368,7 +398,7 @@ static int block2mtd_setup2(const char * 
strcpy(str, val); 
kill_final_newline(str); 

- for (i = 0; i < 2; i+

[RESEND PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-10-09 Thread Rodrigo Freire
From: Felix Fietkau n...@openwrt.org 

mtd: block2mtd: Ensure that block2mtd is presented in a timely fashion 

Currently, a block MTD device is not presented to the system on time, in 
order to start mounting the filesystems. This patch ensures that block2mtd 
is presented at the right time, so filesystems can be mounted on boot time. 
This issue was seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2 
block2mtd filesystems. 
This patchset also adds a MTD device name and a timeout option to the driver. 
Original patchset: 
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
 
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444
 

Signed-off-by: Felix Fietkau n...@openwrt.org 
Signed-off-by: Rodrigo Freire rfre...@redhat.com 
Signed-off-by: Herton Krzesinski her...@redhat.com 
--- 
V2: Uses kstrdup, removed PAGE_MASK. 
--- a/drivers/mtd/devices/block2mtd.c 2014-09-16 21:38:12.543952627 -0300 
+++ b/drivers/mtd/devices/block2mtd.c 2014-09-17 17:43:21.424944394 -0300 
@@ -9,7 +9,15 @@ 

#define pr_fmt(fmt) KBUILD_MODNAME :  fmt 

+/* 
+* When the first attempt at device initialization fails, we may need to 
+* wait a little bit and retry. This timeout, by default 3 seconds, gives 
+* device time to start up. Required on BCM2708 and a few other chipsets. 
+*/ 
+#define MTD_DEFAULT_TIMEOUT 3 
+ 
#include linux/module.h 
+#include linux/delay.h 
#include linux/fs.h 
#include linux/blkdev.h 
#include linux/bio.h 
@@ -17,6 +25,7 @@ 
#include linux/list.h 
#include linux/init.h 
#include linux/mtd/mtd.h 
+#include linux/mtd/partitions.h 
#include linux/mutex.h 
#include linux/mount.h 
#include linux/slab.h 
@@ -209,12 +218,14 @@ static void block2mtd_free_device(struct 
} 


-static struct block2mtd_dev *add_device(char *devname, int erase_size) 
+static struct block2mtd_dev *add_device(char *devname, int erase_size, const 
char *mtdname, int timeout) 
{ 
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL; 
- struct block_device *bdev; 
+ struct block_device *bdev = ERR_PTR(-ENODEV); 
struct block2mtd_dev *dev; 
+ struct mtd_partition *part; 
char *name; 
+ int i; 

if (!devname) 
return NULL; 
@@ -225,15 +236,28 @@ static struct block2mtd_dev *add_device( 

/* Get a handle on the device */ 
bdev = blkdev_get_by_path(devname, mode, dev); 
-#ifndef MODULE 
- if (IS_ERR(bdev)) { 

- /* We might not have rootfs mounted at this point. Try 
- to resolve the device name by other means. */ 
- 
- dev_t devt = name_to_dev_t(devname); 
- if (devt) 
- bdev = blkdev_get_by_dev(devt, mode, dev); 
+#ifndef MODULE 
+/* 
+* We might not have the root device mounted at this point. 
+* Try to resolve the device name by other means. 
+*/ 
+ for (i = 0; IS_ERR(bdev)  i = timeout; i++) { 
+ dev_t devt; 
+ 
+ if (i) 
+ /* 
+ * Calling wait_for_device_probe in the first loop 
+ * was not enough, sleep for a bit in subsequent 
+ * go-arounds. 
+ */ 
+ msleep(1000); 
+ wait_for_device_probe(); 
+ 
+ devt = name_to_dev_t(devname); 
+ if (!devt) 
+ continue; 
+ bdev = blkdev_get_by_dev(devt, mode, dev); 
} 
#endif 

@@ -257,13 +281,14 @@ static struct block2mtd_dev *add_device( 

/* Setup the MTD structure */ 
/* make the name contain the block device in */ 
- name = kasprintf(GFP_KERNEL, block2mtd: %s, devname); 
+ if (!mtdname) 
+ mtdname = devname; 
+ name = kstrdup (mtdname, GFP_KERNEL); 
if (!name) 
goto err_destroy_mutex; 

dev-mtd.name = name; 
- 
- dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK; 
+ dev-mtd.size = dev-blkdev-bd_inode-i_size  ~(erase_size - 1); 
dev-mtd.erasesize = erase_size; 
dev-mtd.writesize = 1; 
dev-mtd.writebufsize = PAGE_SIZE; 
@@ -276,15 +301,19 @@ static struct block2mtd_dev *add_device( 
dev-mtd.priv = dev; 
dev-mtd.owner = THIS_MODULE; 

- if (mtd_device_register(dev-mtd, NULL, 0)) { 
+ part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL); 
+ part-name = name; 
+ part-offset = 0; 
+ part-size = dev-mtd.size; 
+ if (mtd_device_register(dev-mtd, part, 1)) { 
/* Device didn't get added, so free the entry */ 
goto err_destroy_mutex; 
} 
+ 
list_add(dev-list, blkmtd_device_list); 
pr_info(mtd%d: [%s] erase_size = %dKiB [%d]\n, 
dev-mtd.index, 
- dev-mtd.name + strlen(block2mtd: ), 
- dev-mtd.erasesize  10, dev-mtd.erasesize); 
+ mtdname, dev-mtd.erasesize  10, dev-mtd.erasesize); 
return dev; 

err_destroy_mutex: 
@@ -353,11 +382,12 @@ static char block2mtd_paramline[80 + 12] 

static int block2mtd_setup2(const char *val) 
{ 
- char buf[80 + 12]; /* 80 for device, 12 for erase size */ 
+ char buf[80 + 12 + 80 + 8]; /* 80 for device, 12 for erase size, 80 for name, 
8 for timeout */ 
char *str = buf; 
- char *token[2]; 
+ char *token[4]; 
char *name; 
size_t erase_size = PAGE_SIZE; 
+ unsigned long timeout = MTD_DEFAULT_TIMEOUT; 
int i, ret; 

if (strnlen(val, sizeof(buf)) = sizeof(buf)) { 
@@ -368,7 +398,7 @@ static int block2mtd_setup2(const char * 
strcpy(str, val

Re: [PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-09-17 Thread Rodrigo Freire
Holas Ezequiel,

- Original Message - 
From: "Ezequiel Garcia"  
> On 17 September 2014 21:28, Rodrigo Freire  wrote: 
> 
> Using block2mtd sounds a bit unusual. I see that you are trying to get 
> a more robust fs have you tried using f2fs instead of jffs2? 

I see that it is still marked as Experimental as of latest (3.17-RC5)
kernel. But will take a look in the future, thanks for pointing.


> > Currently, a block MTD device is not presented to the system on time, in 
> > order to start mounting the filesystems. This patch ensures that block2mtd 
> > is presented at the right time, so filesystems can be mounted on boot time. 
> 
> It worries me a bit to add such a long delay to the boot. If for some 
> reason the SD is not working, then the kernel will wait (by default) 3 
> seconds now? 

Not really; see the decision path:
IF block2mtd is not a module (is builtin), AND
   IF there is a valid block2mtd= clause on kernel cmdline, AND
  IF the device specified device on block2mtd= clause is still not present
  THEN wait *up to* 3 seconds (or seconds=n if specified on block2mtd=
   cmdline) to the device to show up. If the device shows up earlier,
   the device is created and boot proceeds.
  ELSE Fail to create the block2mtd device.
ELSE keep booting the kernel normally, without any further delays.

Best regards,

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


[PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-09-17 Thread Rodrigo Freire
From: Felix Fietkau 

mtd: block2mtd: Ensure that block2mtd is presented in a timely fashion

Currently, a block MTD device is not presented to the system on time, in
order to start mounting the filesystems. This patch ensures that block2mtd
is presented at the right time, so filesystems can be mounted on boot time.
This issue was seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
block2mtd filesystems.
This patchset also adds a MTD device name and a timeout option to the driver.
Original patchset:
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444

Signed-off-by: Felix Fietkau 
Signed-off-by: Rodrigo Freire 
Signed-off-by: Herton Krzesinski 
---
V2: Uses kstrdup, removed PAGE_MASK.
--- a/drivers/mtd/devices/block2mtd.c   2014-09-16 21:38:12.543952627 -0300
+++ b/drivers/mtd/devices/block2mtd.c   2014-09-17 17:43:21.424944394 -0300
@@ -9,7 +9,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+/*
+* When the first attempt at device initialization fails, we may need to
+* wait a little bit and retry. This timeout, by default 3 seconds, gives
+* device time to start up. Required on BCM2708 and a few other chipsets.
+*/
+#define MTD_DEFAULT_TIMEOUT3
+
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -17,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -209,12 +218,14 @@ static void block2mtd_free_device(struct
 }
 
 
-static struct block2mtd_dev *add_device(char *devname, int erase_size)
+static struct block2mtd_dev *add_device(char *devname, int erase_size, const 
char *mtdname, int timeout)
 {
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
-   struct block_device *bdev;
+   struct block_device *bdev = ERR_PTR(-ENODEV);
struct block2mtd_dev *dev;
+   struct mtd_partition *part;
char *name;
+   int i;
 
if (!devname)
return NULL;
@@ -225,15 +236,28 @@ static struct block2mtd_dev *add_device(
 
/* Get a handle on the device */
bdev = blkdev_get_by_path(devname, mode, dev);
-#ifndef MODULE
-   if (IS_ERR(bdev)) {
 
-   /* We might not have rootfs mounted at this point. Try
-  to resolve the device name by other means. */
-
-   dev_t devt = name_to_dev_t(devname);
-   if (devt)
-   bdev = blkdev_get_by_dev(devt, mode, dev);
+#ifndef MODULE
+/*
+* We might not have the root device mounted at this point.
+* Try to resolve the device name by other means.
+*/
+   for (i = 0; IS_ERR(bdev) && i <= timeout; i++) {
+   dev_t devt;
+
+   if (i)
+   /*
+* Calling wait_for_device_probe in the first loop 
+* was not enough, sleep for a bit in subsequent
+* go-arounds.
+   */
+   msleep(1000);
+   wait_for_device_probe();
+
+   devt = name_to_dev_t(devname);
+   if (!devt)
+   continue;
+   bdev = blkdev_get_by_dev(devt, mode, dev);
}
 #endif
 
@@ -257,13 +281,14 @@ static struct block2mtd_dev *add_device(
 
/* Setup the MTD structure */
/* make the name contain the block device in */
-   name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
+   if (!mtdname)
+   mtdname = devname;
+   name = kstrdup (mtdname, GFP_KERNEL);
if (!name)
goto err_destroy_mutex;
 
dev->mtd.name = name;
-
-   dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;
+   dev->mtd.size = dev->blkdev->bd_inode->i_size & ~(erase_size - 1);
dev->mtd.erasesize = erase_size;
dev->mtd.writesize = 1;
dev->mtd.writebufsize = PAGE_SIZE;
@@ -276,15 +301,19 @@ static struct block2mtd_dev *add_device(
dev->mtd.priv = dev;
dev->mtd.owner = THIS_MODULE;
 
-   if (mtd_device_register(>mtd, NULL, 0)) {
+   part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL);
+   part->name = name;
+   part->offset = 0;
+   part->size = dev->mtd.size;
+   if (mtd_device_register(>mtd, part, 1)) {
/* Device didn't get added, so free the entry */
goto err_destroy_mutex;
}
+
list_add(>list, _device_list);
pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n",
dev->mtd.index,
-   dev->mtd.name + strlen("block2mtd: "),
-   dev->mtd.erasesize >> 10, dev->mtd.erasesize);
+   mtdname, dev->mtd.erasesize >> 10, dev->mtd.erasesize);
return dev;
 
 err_destroy_m

Re: [PATCH] block2mtd: mtd: Present block2mtd timely on boot time

2014-09-17 Thread Rodrigo Freire
Hi Jörn,

- Original Message - 
From: "Jörn Engel"  
> On Mon, 8 September 2014 16:04:40 -0400, Rodrigo Freire wrote: 
> > 
> > @@ -257,13 +281,15 @@ static struct block2mtd_dev *add_device( 
> > 
> > /* Setup the MTD structure */ 
> > /* make the name contain the block device in */ 
> > - name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname); 
> > + if (!mtdname) 
> > + mtdname = devname; 
> > + name = kmalloc(strlen(mtdname) + 1, GFP_KERNEL); 
> > if (!name) 
> > goto err_destroy_mutex; 
> > 
> > + strcpy(name, mtdname); 
> kstrdup. 
> And see below for the ABI change. 

Thanks for pointing. Fixed.


> > dev->mtd.name = name;
> > -
> > -   dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;
> > +   dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK & 
> > ~(erase_size - 1);
> 
> PAGE_MASK is no longer needed with the new term.  Or does anyone
> seriously want to support erase_size < PAGE_SIZE?

Makes sense. I was talking to Felix and indeed there are some MTD devices 
which have 4k erase page size. Unheard of something smaller.
But it is on the MTD land and not block2mtd.


> Timeout has a default value, but name defaults to NULL. Add three 
> devices without specifying the name and you get funny results. 
> If we handled the NULL case by doing what the driver used to do before 
> this patch, I think this would be fine. 

Please see the fragment below:

@@ -257,13 +281,15 @@ static struct block2mtd_dev *add_device(
 
/* Setup the MTD structure */
/* make the name contain the block device in */
-   name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
+   if (!mtdname)
+   mtdname = devname;
+   name = kstrdup(mtdname, GFP_KERNEL);
if (!name)
goto err_destroy_mutex;

If the name is a NULL or not provided, the mtdname will then become the mtd 
device name.
I also tried mounting several partitions, with both specified name and not and 
everything seemed to work nicely.

See a V2 patch on the next message.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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] block2mtd: mtd: Present block2mtd timely on boot time

2014-09-17 Thread Rodrigo Freire
Hi Jörn,

- Original Message - 
From: Jörn Engel jo...@logfs.org 
 On Mon, 8 September 2014 16:04:40 -0400, Rodrigo Freire wrote: 
  
  @@ -257,13 +281,15 @@ static struct block2mtd_dev *add_device( 
  
  /* Setup the MTD structure */ 
  /* make the name contain the block device in */ 
  - name = kasprintf(GFP_KERNEL, block2mtd: %s, devname); 
  + if (!mtdname) 
  + mtdname = devname; 
  + name = kmalloc(strlen(mtdname) + 1, GFP_KERNEL); 
  if (!name) 
  goto err_destroy_mutex; 
  
  + strcpy(name, mtdname); 
 kstrdup. 
 And see below for the ABI change. 

Thanks for pointing. Fixed.


  dev-mtd.name = name;
  -
  -   dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK;
  +   dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK  
  ~(erase_size - 1);
 
 PAGE_MASK is no longer needed with the new term.  Or does anyone
 seriously want to support erase_size  PAGE_SIZE?

Makes sense. I was talking to Felix and indeed there are some MTD devices 
which have 4k erase page size. Unheard of something smaller.
But it is on the MTD land and not block2mtd.


 Timeout has a default value, but name defaults to NULL. Add three 
 devices without specifying the name and you get funny results. 
 If we handled the NULL case by doing what the driver used to do before 
 this patch, I think this would be fine. 

Please see the fragment below:

@@ -257,13 +281,15 @@ static struct block2mtd_dev *add_device(
 
/* Setup the MTD structure */
/* make the name contain the block device in */
-   name = kasprintf(GFP_KERNEL, block2mtd: %s, devname);
+   if (!mtdname)
+   mtdname = devname;
+   name = kstrdup(mtdname, GFP_KERNEL);
if (!name)
goto err_destroy_mutex;

If the name is a NULL or not provided, the mtdname will then become the mtd 
device name.
I also tried mounting several partitions, with both specified name and not and 
everything seemed to work nicely.

See a V2 patch on the next message.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-09-17 Thread Rodrigo Freire
From: Felix Fietkau n...@openwrt.org

mtd: block2mtd: Ensure that block2mtd is presented in a timely fashion

Currently, a block MTD device is not presented to the system on time, in
order to start mounting the filesystems. This patch ensures that block2mtd
is presented at the right time, so filesystems can be mounted on boot time.
This issue was seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
block2mtd filesystems.
This patchset also adds a MTD device name and a timeout option to the driver.
Original patchset:
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444

Signed-off-by: Felix Fietkau n...@openwrt.org
Signed-off-by: Rodrigo Freire rfre...@redhat.com
Signed-off-by: Herton Krzesinski her...@redhat.com
---
V2: Uses kstrdup, removed PAGE_MASK.
--- a/drivers/mtd/devices/block2mtd.c   2014-09-16 21:38:12.543952627 -0300
+++ b/drivers/mtd/devices/block2mtd.c   2014-09-17 17:43:21.424944394 -0300
@@ -9,7 +9,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 
+/*
+* When the first attempt at device initialization fails, we may need to
+* wait a little bit and retry. This timeout, by default 3 seconds, gives
+* device time to start up. Required on BCM2708 and a few other chipsets.
+*/
+#define MTD_DEFAULT_TIMEOUT3
+
 #include linux/module.h
+#include linux/delay.h
 #include linux/fs.h
 #include linux/blkdev.h
 #include linux/bio.h
@@ -17,6 +25,7 @@
 #include linux/list.h
 #include linux/init.h
 #include linux/mtd/mtd.h
+#include linux/mtd/partitions.h
 #include linux/mutex.h
 #include linux/mount.h
 #include linux/slab.h
@@ -209,12 +218,14 @@ static void block2mtd_free_device(struct
 }
 
 
-static struct block2mtd_dev *add_device(char *devname, int erase_size)
+static struct block2mtd_dev *add_device(char *devname, int erase_size, const 
char *mtdname, int timeout)
 {
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
-   struct block_device *bdev;
+   struct block_device *bdev = ERR_PTR(-ENODEV);
struct block2mtd_dev *dev;
+   struct mtd_partition *part;
char *name;
+   int i;
 
if (!devname)
return NULL;
@@ -225,15 +236,28 @@ static struct block2mtd_dev *add_device(
 
/* Get a handle on the device */
bdev = blkdev_get_by_path(devname, mode, dev);
-#ifndef MODULE
-   if (IS_ERR(bdev)) {
 
-   /* We might not have rootfs mounted at this point. Try
-  to resolve the device name by other means. */
-
-   dev_t devt = name_to_dev_t(devname);
-   if (devt)
-   bdev = blkdev_get_by_dev(devt, mode, dev);
+#ifndef MODULE
+/*
+* We might not have the root device mounted at this point.
+* Try to resolve the device name by other means.
+*/
+   for (i = 0; IS_ERR(bdev)  i = timeout; i++) {
+   dev_t devt;
+
+   if (i)
+   /*
+* Calling wait_for_device_probe in the first loop 
+* was not enough, sleep for a bit in subsequent
+* go-arounds.
+   */
+   msleep(1000);
+   wait_for_device_probe();
+
+   devt = name_to_dev_t(devname);
+   if (!devt)
+   continue;
+   bdev = blkdev_get_by_dev(devt, mode, dev);
}
 #endif
 
@@ -257,13 +281,14 @@ static struct block2mtd_dev *add_device(
 
/* Setup the MTD structure */
/* make the name contain the block device in */
-   name = kasprintf(GFP_KERNEL, block2mtd: %s, devname);
+   if (!mtdname)
+   mtdname = devname;
+   name = kstrdup (mtdname, GFP_KERNEL);
if (!name)
goto err_destroy_mutex;
 
dev-mtd.name = name;
-
-   dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK;
+   dev-mtd.size = dev-blkdev-bd_inode-i_size  ~(erase_size - 1);
dev-mtd.erasesize = erase_size;
dev-mtd.writesize = 1;
dev-mtd.writebufsize = PAGE_SIZE;
@@ -276,15 +301,19 @@ static struct block2mtd_dev *add_device(
dev-mtd.priv = dev;
dev-mtd.owner = THIS_MODULE;
 
-   if (mtd_device_register(dev-mtd, NULL, 0)) {
+   part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL);
+   part-name = name;
+   part-offset = 0;
+   part-size = dev-mtd.size;
+   if (mtd_device_register(dev-mtd, part, 1)) {
/* Device didn't get added, so free the entry */
goto err_destroy_mutex;
}
+
list_add(dev-list, blkmtd_device_list);
pr_info(mtd%d: [%s] erase_size = %dKiB [%d]\n,
dev-mtd.index,
-   dev-mtd.name + strlen(block2mtd: ),
-   dev-mtd.erasesize  10, dev-mtd.erasesize);
+   mtdname, dev-mtd.erasesize  10, dev

Re: [PATCH V2] mtd: block2mtd: Present block2mtd timely on boot time

2014-09-17 Thread Rodrigo Freire
Holas Ezequiel,

- Original Message - 
From: Ezequiel Garcia ezequ...@vanguardiasur.com.ar 
 On 17 September 2014 21:28, Rodrigo Freire rfre...@redhat.com wrote: 
 
 Using block2mtd sounds a bit unusual. I see that you are trying to get 
 a more robust fs have you tried using f2fs instead of jffs2? 

I see that it is still marked as Experimental as of latest (3.17-RC5)
kernel. But will take a look in the future, thanks for pointing.


  Currently, a block MTD device is not presented to the system on time, in 
  order to start mounting the filesystems. This patch ensures that block2mtd 
  is presented at the right time, so filesystems can be mounted on boot time. 
 
 It worries me a bit to add such a long delay to the boot. If for some 
 reason the SD is not working, then the kernel will wait (by default) 3 
 seconds now? 

Not really; see the decision path:
IF block2mtd is not a module (is builtin), AND
   IF there is a valid block2mtd= clause on kernel cmdline, AND
  IF the device specified device on block2mtd= clause is still not present
  THEN wait *up to* 3 seconds (or seconds=n if specified on block2mtd=
   cmdline) to the device to show up. If the device shows up earlier,
   the device is created and boot proceeds.
  ELSE Fail to create the block2mtd device.
ELSE keep booting the kernel normally, without any further delays.

Best regards,

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


[PATCH] block2mtd: mtd: Present block2mtd timely on boot time

2014-09-08 Thread Rodrigo Freire
From: Felix Fietkau 

block2mtd: Ensure that block2mtd is presented in a timely fashion 

Currently, a block MTD device is not presented to the system on time, in 
order to start mounting the filesystems. This patch ensures that block2mtd 
is presented at the right time, so filesystems can be mounted on boot time.
This issue was seen on BCM2708 (Raspberry Pi) systems when mounting JFFS2 
block2mtd filesystems.
This patchset also adds a MTD device name and a timeout option to the driver.
Original patchset:
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444

Signed-off-by: Felix Fietkau 
Signed-off-by: Rodrigo Freire  

--- a/drivers/mtd/devices/block2mtd.c   2014-09-05 11:13:39.143698413 -0300
+++ b/drivers/mtd/devices/block2mtd.c   2014-09-05 17:50:28.107366433 -0300
@@ -9,7 +9,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+/*
+* When the first attempt at device initialization fails, we may need to
+* wait a little bit and retry. This timeout, by default 3 seconds, gives
+* device time to start up. Required on BCM2708 and a few other chipsets.
+*/
+#define MTD_DEFAULT_TIMEOUT3
+
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -17,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -209,12 +218,14 @@ static void block2mtd_free_device(struct
 }
 
 
-static struct block2mtd_dev *add_device(char *devname, int erase_size)
+static struct block2mtd_dev *add_device(char *devname, int erase_size, const 
char *mtdname, int timeout)
 {
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
-   struct block_device *bdev;
+   struct block_device *bdev = ERR_PTR(-ENODEV);
struct block2mtd_dev *dev;
+   struct mtd_partition *part;
char *name;
+   int i;
 
if (!devname)
return NULL;
@@ -225,15 +236,28 @@ static struct block2mtd_dev *add_device(
 
/* Get a handle on the device */
bdev = blkdev_get_by_path(devname, mode, dev);
-#ifndef MODULE
-   if (IS_ERR(bdev)) {
 
-   /* We might not have rootfs mounted at this point. Try
-  to resolve the device name by other means. */
-
-   dev_t devt = name_to_dev_t(devname);
-   if (devt)
-   bdev = blkdev_get_by_dev(devt, mode, dev);
+#ifndef MODULE
+/*
+* We might not have the root device mounted at this point.
+* Try to resolve the device name by other means.
+*/
+   for (i = 0; IS_ERR(bdev) && i <= timeout; i++) {
+   dev_t devt;
+
+   if (i)
+   /*
+* Calling wait_for_device_probe in the first loop 
+* was not enough, sleep for a bit in subsequent
+* go-arounds.
+   */
+   msleep(1000);
+   wait_for_device_probe();
+
+   devt = name_to_dev_t(devname);
+   if (!devt)
+   continue;
+   bdev = blkdev_get_by_dev(devt, mode, dev);
}
 #endif
 
@@ -257,13 +281,15 @@ static struct block2mtd_dev *add_device(
 
/* Setup the MTD structure */
/* make the name contain the block device in */
-   name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
+   if (!mtdname)
+   mtdname = devname;
+   name = kmalloc(strlen(mtdname) + 1, GFP_KERNEL);
if (!name)
goto err_destroy_mutex;
 
+   strcpy(name, mtdname);
dev->mtd.name = name;
-
-   dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;
+   dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK & 
~(erase_size - 1);
dev->mtd.erasesize = erase_size;
dev->mtd.writesize = 1;
dev->mtd.writebufsize = PAGE_SIZE;
@@ -276,15 +302,19 @@ static struct block2mtd_dev *add_device(
dev->mtd.priv = dev;
dev->mtd.owner = THIS_MODULE;
 
-   if (mtd_device_register(>mtd, NULL, 0)) {
+   part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL);
+   part->name = name;
+   part->offset = 0;
+   part->size = dev->mtd.size;
+   if (mtd_device_register(>mtd, part, 1)) {
/* Device didn't get added, so free the entry */
goto err_destroy_mutex;
}
+
list_add(>list, _device_list);
pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n",
dev->mtd.index,
-   dev->mtd.name + strlen("block2mtd: "),
-   dev->mtd.erasesize >> 10, dev->mtd.erasesize);
+   mtdname, dev->mtd.erasesize >> 10, dev->mtd.erasesize);
return dev;
 
 err_destroy_mutex:
@@ -3

[PATCH] block2mtd: mtd: Present block2mtd timely on boot time

2014-09-08 Thread Rodrigo Freire
From: Felix Fietkau n...@openwrt.org

block2mtd: Ensure that block2mtd is presented in a timely fashion 

Currently, a block MTD device is not presented to the system on time, in 
order to start mounting the filesystems. This patch ensures that block2mtd 
is presented at the right time, so filesystems can be mounted on boot time.
This issue was seen on BCM2708 (Raspberry Pi) systems when mounting JFFS2 
block2mtd filesystems.
This patchset also adds a MTD device name and a timeout option to the driver.
Original patchset:
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/440-block2mtd_init.patch?rev=40444
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.12/441-block2mtd_probe.patch?rev=40444

Signed-off-by: Felix Fietkau n...@openwrt.org
Signed-off-by: Rodrigo Freire rfre...@redhat.com 

--- a/drivers/mtd/devices/block2mtd.c   2014-09-05 11:13:39.143698413 -0300
+++ b/drivers/mtd/devices/block2mtd.c   2014-09-05 17:50:28.107366433 -0300
@@ -9,7 +9,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 
+/*
+* When the first attempt at device initialization fails, we may need to
+* wait a little bit and retry. This timeout, by default 3 seconds, gives
+* device time to start up. Required on BCM2708 and a few other chipsets.
+*/
+#define MTD_DEFAULT_TIMEOUT3
+
 #include linux/module.h
+#include linux/delay.h
 #include linux/fs.h
 #include linux/blkdev.h
 #include linux/bio.h
@@ -17,6 +25,7 @@
 #include linux/list.h
 #include linux/init.h
 #include linux/mtd/mtd.h
+#include linux/mtd/partitions.h
 #include linux/mutex.h
 #include linux/mount.h
 #include linux/slab.h
@@ -209,12 +218,14 @@ static void block2mtd_free_device(struct
 }
 
 
-static struct block2mtd_dev *add_device(char *devname, int erase_size)
+static struct block2mtd_dev *add_device(char *devname, int erase_size, const 
char *mtdname, int timeout)
 {
const fmode_t mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
-   struct block_device *bdev;
+   struct block_device *bdev = ERR_PTR(-ENODEV);
struct block2mtd_dev *dev;
+   struct mtd_partition *part;
char *name;
+   int i;
 
if (!devname)
return NULL;
@@ -225,15 +236,28 @@ static struct block2mtd_dev *add_device(
 
/* Get a handle on the device */
bdev = blkdev_get_by_path(devname, mode, dev);
-#ifndef MODULE
-   if (IS_ERR(bdev)) {
 
-   /* We might not have rootfs mounted at this point. Try
-  to resolve the device name by other means. */
-
-   dev_t devt = name_to_dev_t(devname);
-   if (devt)
-   bdev = blkdev_get_by_dev(devt, mode, dev);
+#ifndef MODULE
+/*
+* We might not have the root device mounted at this point.
+* Try to resolve the device name by other means.
+*/
+   for (i = 0; IS_ERR(bdev)  i = timeout; i++) {
+   dev_t devt;
+
+   if (i)
+   /*
+* Calling wait_for_device_probe in the first loop 
+* was not enough, sleep for a bit in subsequent
+* go-arounds.
+   */
+   msleep(1000);
+   wait_for_device_probe();
+
+   devt = name_to_dev_t(devname);
+   if (!devt)
+   continue;
+   bdev = blkdev_get_by_dev(devt, mode, dev);
}
 #endif
 
@@ -257,13 +281,15 @@ static struct block2mtd_dev *add_device(
 
/* Setup the MTD structure */
/* make the name contain the block device in */
-   name = kasprintf(GFP_KERNEL, block2mtd: %s, devname);
+   if (!mtdname)
+   mtdname = devname;
+   name = kmalloc(strlen(mtdname) + 1, GFP_KERNEL);
if (!name)
goto err_destroy_mutex;
 
+   strcpy(name, mtdname);
dev-mtd.name = name;
-
-   dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK;
+   dev-mtd.size = dev-blkdev-bd_inode-i_size  PAGE_MASK  
~(erase_size - 1);
dev-mtd.erasesize = erase_size;
dev-mtd.writesize = 1;
dev-mtd.writebufsize = PAGE_SIZE;
@@ -276,15 +302,19 @@ static struct block2mtd_dev *add_device(
dev-mtd.priv = dev;
dev-mtd.owner = THIS_MODULE;
 
-   if (mtd_device_register(dev-mtd, NULL, 0)) {
+   part = kzalloc(sizeof(struct mtd_partition), GFP_KERNEL);
+   part-name = name;
+   part-offset = 0;
+   part-size = dev-mtd.size;
+   if (mtd_device_register(dev-mtd, part, 1)) {
/* Device didn't get added, so free the entry */
goto err_destroy_mutex;
}
+
list_add(dev-list, blkmtd_device_list);
pr_info(mtd%d: [%s] erase_size = %dKiB [%d]\n,
dev-mtd.index,
-   dev-mtd.name + strlen(block2mtd: ),
-   dev-mtd.erasesize  10, dev-mtd.erasesize);
+   mtdname, dev-mtd.erasesize  10, dev-mtd.erasesize);
return dev