[PATCH 0/2] ext4: Some MMP fixes

[PATCH 0/2] ext4: Some MMP fixes

[Nikitas Angelinas]

Some small fixes for the multiple mount protection feature in ext4.

Nikitas Angelinas (2):
      ext4: MMP: kmmpd should use nodename from init_uts_ns.name, not sysname
      ext4: MMP: fix error message rate-limiting logic in kmmpd

 fs/ext4/mmp.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

[PATCH 1/2] ext4: MMP: kmmpd should use nodename from init_uts_ns.name, not sysname

[Nikitas Angelinas]

sysname holds "Linux" by default, i.e. what appears when doing a "uname
-s"; nodename should be used to print the machine's hostname, i.e. what
is returned when doing a "uname -n" or "hostname", and what
gethostname(2)/sethostname(2) manipulate, in order to notify the
administrator of the node which is contending to mount the filesystem.

Signed-off-by: Nikitas Angelinas <nikitas_angelinas@xyratex.com>
Signed-off-by: Andrew Perepechko <andrew_perepechko@xyratex.com>
---
 fs/ext4/mmp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 9bdef3f..2fca64e 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -109,7 +109,7 @@ static int kmmpd(void *data)
 	mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval);
 	bdevname(bh->b_bdev, mmp->mmp_bdevname);
 
-	memcpy(mmp->mmp_nodename, init_utsname()->sysname,
+	memcpy(mmp->mmp_nodename, init_utsname()->nodename,
 	       sizeof(mmp->mmp_nodename));
 
 	while (!kthread_should_stop()) {
-- 
1.7.4.4

[PATCH 2/2] ext4: MMP: fix error message rate-limiting logic in kmmpd

[Nikitas Angelinas]

Current logic would print an error message only once, and then
'failed_writes' would stay at 1. Rework the loop to increment
'failed_writes' and print the error message every s_mmp_update_interval
* 60 seconds, as intended according to the comment.

Perhaps it would be better to reset 'failed_writes' to 0 on every
successful write, such that new errors are noticed sooner, but maybe in
the interest of efficiency it would be best not to add an operation on
every kmmpd thread run?

Signed-off-by: Nikitas Angelinas <nikitas_angelinas@xyratex.com>
Signed-off-by: Andrew Perepechko <andrew_perepechko@xyratex.com>
---
 fs/ext4/mmp.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 2fca64e..6b32742 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -125,8 +125,9 @@ static int kmmpd(void *data)
 		 * Don't spew too many error messages. Print one every
 		 * (s_mmp_update_interval * 60) seconds.
 		 */
-		if (retval && (failed_writes % 60) == 0) {
-			ext4_error(sb, "Error writing to MMP block");
+		if (retval) {
+			if ((failed_writes % 60) == 0)
+				ext4_error(sb, "Error writing to MMP block");
 			failed_writes++;
 		}
 
-- 
1.7.4.4

Re: [PATCH 1/2] ext4: MMP: kmmpd should use nodename from init_uts_ns.name, not sysname

[Andrew Perepechko]

Hello Nikitas!

It looks like some bits of your patches somehow got dropped.

Andrew

On 09/23/2011 04:23 AM, Nikitas Angelinas wrote:
> sysname holds "Linux" by default, i.e. what appears when doing a "uname
> -s"; nodename should be used to print the machine's hostname, i.e. what
> is returned when doing a "uname -n" or "hostname", and what
> gethostname(2)/sethostname(2) manipulate, in order to notify the
> administrator of the node which is contending to mount the filesystem.
>
> Signed-off-by: Nikitas Angelinas<nikitas_angelinas@xyratex.com>
> Signed-off-by: Andrew Perepechko<andrew_perepechko@xyratex.com>
> ---
>   fs/ext4/mmp.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 9bdef3f..2fca64e 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -109,7 +109,7 @@ static int kmmpd(void *data)
>   	mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval);
>   	bdevname(bh->b_bdev, mmp->mmp_bdevname);
>
> -	memcpy(mmp->mmp_nodename, init_utsname()->sysname,
> +	memcpy(mmp->mmp_nodename, init_utsname()->nodename,
>   	       sizeof(mmp->mmp_nodename));
>
>   	while (!kthread_should_stop()) {

______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

Re: [PATCH 1/2] ext4: MMP: kmmpd should use nodename from init_uts_ns.name, not sysname

[Andrew Perepechko]

Never mind, they are not relevant.

Andrew

On 09/23/2011 12:24 PM, Andrew Perepechko wrote:
> Hello Nikitas!
>
> It looks like some bits of your patches somehow got dropped.
>
> Andrew
>
> On 09/23/2011 04:23 AM, Nikitas Angelinas wrote:
>> sysname holds "Linux" by default, i.e. what appears when doing a "uname
>> -s"; nodename should be used to print the machine's hostname, i.e. what
>> is returned when doing a "uname -n" or "hostname", and what
>> gethostname(2)/sethostname(2) manipulate, in order to notify the
>> administrator of the node which is contending to mount the filesystem.
>>
>> Signed-off-by: Nikitas Angelinas<nikitas_angelinas@xyratex.com>
>> Signed-off-by: Andrew Perepechko<andrew_perepechko@xyratex.com>
>> ---
>>   fs/ext4/mmp.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
>> index 9bdef3f..2fca64e 100644
>> --- a/fs/ext4/mmp.c
>> +++ b/fs/ext4/mmp.c
>> @@ -109,7 +109,7 @@ static int kmmpd(void *data)
>>       mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval);
>>       bdevname(bh->b_bdev, mmp->mmp_bdevname);
>>
>> -    memcpy(mmp->mmp_nodename, init_utsname()->sysname,
>> +    memcpy(mmp->mmp_nodename, init_utsname()->nodename,
>>              sizeof(mmp->mmp_nodename));
>>
>>       while (!kthread_should_stop()) {
>

______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

Re: [PATCH 1/2] ext4: MMP: kmmpd should use nodename from init_uts_ns.name, not sysname

[Andreas Dilger]

On 2011-09-22, at 6:23 PM, Nikitas Angelinas wrote:
> sysname holds "Linux" by default, i.e. what appears when doing a "uname
> -s"; nodename should be used to print the machine's hostname, i.e. what
> is returned when doing a "uname -n" or "hostname", and what
> gethostname(2)/sethostname(2) manipulate, in order to notify the
> administrator of the node which is contending to mount the filesystem.
> 
> Signed-off-by: Nikitas Angelinas <nikitas_angelinas@xyratex.com>
> Signed-off-by: Andrew Perepechko <andrew_perepechko@xyratex.com>

This looks to have crept in with the port of the patch to RHEL6, which
required the code to stop accessing system_utsname directly and instead
using the init_utsname() helper to access the structure.  That version
of the patch was contributed by an external developer, so I can't really
say why it was done, probably just an oversight.  In our RHEL5 patch it
used to be:

       memcpy(mmp->mmp_nodename, system_utsname.nodename,
              sizeof(mmp->mmp_nodename));

You can add Acked-by: Andreas Dilger <adilger@dilger.ca>.

Thanks for the fix.

> ---
> fs/ext4/mmp.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 9bdef3f..2fca64e 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -109,7 +109,7 @@ static int kmmpd(void *data)
> 	mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval);
> 	bdevname(bh->b_bdev, mmp->mmp_bdevname);
> 
> -	memcpy(mmp->mmp_nodename, init_utsname()->sysname,
> +	memcpy(mmp->mmp_nodename, init_utsname()->nodename,
> 	       sizeof(mmp->mmp_nodename));
> 
> 	while (!kthread_should_stop()) {
> -- 
> 1.7.4.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas

Re: [PATCH 2/2] ext4: MMP: fix error message rate-limiting logic in kmmpd

[Andreas Dilger]

On 2011-09-22, at 6:23 PM, Nikitas Angelinas wrote:
> Current logic would print an error message only once, and then
> 'failed_writes' would stay at 1. Rework the loop to increment
> 'failed_writes' and print the error message every s_mmp_update_interval
> * 60 seconds, as intended according to the comment.
> 
> Perhaps it would be better to reset 'failed_writes' to 0 on every
> successful write, such that new errors are noticed sooner, but maybe in
> the interest of efficiency it would be best not to add an operation on
> every kmmpd thread run?
> 
> Signed-off-by: Nikitas Angelinas <nikitas_angelinas@xyratex.com>
> Signed-off-by: Andrew Perepechko <andrew_perepechko@xyratex.com>

Acked-by: Andreas Dilger <adilger@dilger.ca>

Thanks for the fix.
> ---
> fs/ext4/mmp.c |    5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 2fca64e..6b32742 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -125,8 +125,9 @@ static int kmmpd(void *data)
> 		 * Don't spew too many error messages. Print one every
> 		 * (s_mmp_update_interval * 60) seconds.
> 		 */
> -		if (retval && (failed_writes % 60) == 0) {
> -			ext4_error(sb, "Error writing to MMP block");
> +		if (retval) {
> +			if ((failed_writes % 60) == 0)
> +				ext4_error(sb, "Error writing to MMP block");
> 			failed_writes++;
> 		}
> 
> -- 
> 1.7.4.4
> 


Cheers, Andreas

(no subject)

[Nikitas Angelinas]

Same patches with Andreas' acked-by added.

(unknown),

[Nikitas Angelinas]

Same patches with Andreas' acked-by added.

[PATCH 1/2] ext4: MMP: kmmpd should use nodename from init_uts_ns.name, not sysname

[Nikitas Angelinas]

sysname holds "Linux" by default, i.e. what appears when doing a "uname
-s"; nodename should be used to print the machine's hostname, i.e. what
is returned when doing a "uname -n" or "hostname", and what
gethostname(2)/sethostname(2) manipulate, in order to notify the
administrator of the node which is contending to mount the filesystem.

Signed-off-by: Nikitas Angelinas <nikitas_angelinas@xyratex.com>
Signed-off-by: Andrew Perepechko <andrew_perepechko@xyratex.com>
Acked-by: Andreas Dilger <adilger@dilger.ca>
---
 fs/ext4/mmp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 9bdef3f..2fca64e 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -109,7 +109,7 @@ static int kmmpd(void *data)
 	mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval);
 	bdevname(bh->b_bdev, mmp->mmp_bdevname);
 
-	memcpy(mmp->mmp_nodename, init_utsname()->sysname,
+	memcpy(mmp->mmp_nodename, init_utsname()->nodename,
 	       sizeof(mmp->mmp_nodename));
 
 	while (!kthread_should_stop()) {
-- 
1.7.4.4

[PATCH 2/2] ext4: MMP: fix error message rate-limiting logic in kmmpd

[Nikitas Angelinas]

Current logic would print an error message only once, and then
'failed_writes' would stay at 1. Rework the loop to increment
'failed_writes' and print the error message every s_mmp_update_interval
* 60 seconds, as intended according to the comment.

Perhaps it would be better to reset 'failed_writes' to 0 on every
successful write, such that new errors are noticed sooner, but maybe in
the interest of efficiency it would be best not to add an operation on
every kmmpd thread run?

Signed-off-by: Nikitas Angelinas <nikitas_angelinas@xyratex.com>
Signed-off-by: Andrew Perepechko <andrew_perepechko@xyratex.com>
Acked-by: Andreas Dilger <adilger@dilger.ca>
---
 fs/ext4/mmp.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 2fca64e..6b32742 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -125,8 +125,9 @@ static int kmmpd(void *data)
 		 * Don't spew too many error messages. Print one every
 		 * (s_mmp_update_interval * 60) seconds.
 		 */
-		if (retval && (failed_writes % 60) == 0) {
-			ext4_error(sb, "Error writing to MMP block");
+		if (retval) {
+			if ((failed_writes % 60) == 0)
+				ext4_error(sb, "Error writing to MMP block");
 			failed_writes++;
 		}
 
-- 
1.7.4.4

Re: [PATCH 1/2] ext4: MMP: kmmpd should use nodename from init_uts_ns.name, not sysname

[Ted Ts'o]

On Fri, Sep 30, 2011 at 12:38:19AM +0100, Nikitas Angelinas wrote:
> sysname holds "Linux" by default, i.e. what appears when doing a "uname
> -s"; nodename should be used to print the machine's hostname, i.e. what
> is returned when doing a "uname -n" or "hostname", and what
> gethostname(2)/sethostname(2) manipulate, in order to notify the
> administrator of the node which is contending to mount the filesystem.
> 
> Signed-off-by: Nikitas Angelinas <nikitas_angelinas@xyratex.com>
> Signed-off-by: Andrew Perepechko <andrew_perepechko@xyratex.com>
> Acked-by: Andreas Dilger <adilger@dilger.ca>

Applied, thanks.

						- Ted

Re: [PATCH 2/2] ext4: MMP: fix error message rate-limiting logic in kmmpd

[Ted Ts'o]

On Fri, Sep 30, 2011 at 12:38:20AM +0100, Nikitas Angelinas wrote:
> Current logic would print an error message only once, and then
> 'failed_writes' would stay at 1. Rework the loop to increment
> 'failed_writes' and print the error message every s_mmp_update_interval
> * 60 seconds, as intended according to the comment.
> 
> Perhaps it would be better to reset 'failed_writes' to 0 on every
> successful write, such that new errors are noticed sooner, but maybe in
> the interest of efficiency it would be best not to add an operation on
> every kmmpd thread run?
> 
> Signed-off-by: Nikitas Angelinas <nikitas_angelinas@xyratex.com>
> Signed-off-by: Andrew Perepechko <andrew_perepechko@xyratex.com>
> Acked-by: Andreas Dilger <adilger@dilger.ca>

Thanks, applied.

						- Ted