diff mbox

[v2,1/2] scripts: Add a recorduidiv program

Message ID 20151202102339.GB8644@n2100.arm.linux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Russell King - ARM Linux Dec. 2, 2015, 10:23 a.m. UTC
On Tue, Dec 01, 2015 at 06:16:43PM +0000, Russell King - ARM Linux wrote:
> On Tue, Dec 01, 2015 at 12:22:12PM -0500, Steven Rostedt wrote:
> > I guess another solution is to do a copy instead of modifying in place
> > if it detects the multiple hard link?
> 
> That would be the "transparent" solution.  If you think it's worth
> persuing, I'll have a go at fixing recordmcount to do that.

Well, copying the file is easy - I've tested this and the linker
appears happy with the result:

 scripts/recordmcount.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Steven Rostedt Dec. 2, 2015, 2:05 p.m. UTC | #1
On Wed, 2 Dec 2015 10:23:39 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Tue, Dec 01, 2015 at 06:16:43PM +0000, Russell King - ARM Linux wrote:
> > On Tue, Dec 01, 2015 at 12:22:12PM -0500, Steven Rostedt wrote:  
> > > I guess another solution is to do a copy instead of modifying in place
> > > if it detects the multiple hard link?  
> > 
> > That would be the "transparent" solution.  If you think it's worth
> > persuing, I'll have a go at fixing recordmcount to do that.  
> 
> Well, copying the file is easy - I've tested this and the linker
> appears happy with the result:

Want to make this into a proper patch and I'll start running it through
my tests?

-- Steve

> 
>  scripts/recordmcount.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> index 698768bdc581..91705ef30402 100644
> --- a/scripts/recordmcount.c
> +++ b/scripts/recordmcount.c
> @@ -211,6 +211,20 @@ static void *mmap_file(char const *fname)
>  		addr = umalloc(sb.st_size);
>  		uread(fd_map, addr, sb.st_size);
>  	}
> +	if (sb.st_nlink != 1) {
> +		/* file is hard-linked, break the hard link */
> +		close(fd_map);
> +		if (unlink(fname) < 0) {
> +			perror(fname);
> +			fail_file();
> +		}
> +		fd_map = open(fname, O_RDWR | O_CREAT, sb.st_mode);
> +		if (fd_map < 0) {
> +			perror(fname);
> +			fail_file();
> +		}
> +		uwrite(fd_map, addr, sb.st_size);
> +	}
>  	return addr;
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steven Rostedt Dec. 11, 2015, 2:31 p.m. UTC | #2
On Fri, 11 Dec 2015 12:09:03 +0000
Russell King <rmk+kernel@arm.linux.org.uk> wrote:

> recordmcount edits the file in-place, which can cause problems when
> using ccache in hardlink mode.  Arrange for recordmcount to break a
> hardlinked object.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> Steven, sorry it took a while to get this out...

Should this be for stable, or is it fine to just add this to my 4.5
queue?

-- Steve

> 
>  scripts/recordmcount.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> index 698768bdc581..91705ef30402 100644
> --- a/scripts/recordmcount.c
> +++ b/scripts/recordmcount.c
> @@ -211,6 +211,20 @@ static void *mmap_file(char const *fname)
>  		addr = umalloc(sb.st_size);
>  		uread(fd_map, addr, sb.st_size);
>  	}
> +	if (sb.st_nlink != 1) {
> +		/* file is hard-linked, break the hard link */
> +		close(fd_map);
> +		if (unlink(fname) < 0) {
> +			perror(fname);
> +			fail_file();
> +		}
> +		fd_map = open(fname, O_RDWR | O_CREAT, sb.st_mode);
> +		if (fd_map < 0) {
> +			perror(fname);
> +			fail_file();
> +		}
> +		uwrite(fd_map, addr, sb.st_size);
> +	}
>  	return addr;
>  }
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Dec. 11, 2015, 2:45 p.m. UTC | #3
On Fri, Dec 11, 2015 at 09:31:25AM -0500, Steven Rostedt wrote:
> On Fri, 11 Dec 2015 12:09:03 +0000
> Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> 
> > recordmcount edits the file in-place, which can cause problems when
> > using ccache in hardlink mode.  Arrange for recordmcount to break a
> > hardlinked object.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> > Steven, sorry it took a while to get this out...
> 
> Should this be for stable, or is it fine to just add this to my 4.5
> queue?

I thought you wanted to test it first - although I've been running with
this for a while now, my nightly builds have masked out the mcount
warning, and I suspect it'll take a while for ccache to purge itself
of the modified objects.

If you're happy to add a stable tag to it, then please do so.
Steven Rostedt Dec. 11, 2015, 3:08 p.m. UTC | #4
On Fri, 11 Dec 2015 14:45:41 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Fri, Dec 11, 2015 at 09:31:25AM -0500, Steven Rostedt wrote:
> > On Fri, 11 Dec 2015 12:09:03 +0000
> > Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> >   
> > > recordmcount edits the file in-place, which can cause problems when
> > > using ccache in hardlink mode.  Arrange for recordmcount to break a
> > > hardlinked object.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > ---
> > > Steven, sorry it took a while to get this out...  
> > 
> > Should this be for stable, or is it fine to just add this to my 4.5
> > queue?  
> 
> I thought you wanted to test it first - although I've been running with

You're right. I forgot I said that ;-)


> this for a while now, my nightly builds have masked out the mcount
> warning, and I suspect it'll take a while for ccache to purge itself
> of the modified objects.
> 
> If you're happy to add a stable tag to it, then please do so.
> 

I'm fine with you taking it too, but let me go ahead and run it through
my tests now. I'll let you know the results. Takes several hours.

Thanks,

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steven Rostedt Dec. 11, 2015, 6:10 p.m. UTC | #5
On Fri, 11 Dec 2015 14:45:41 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Fri, Dec 11, 2015 at 09:31:25AM -0500, Steven Rostedt wrote:
> > On Fri, 11 Dec 2015 12:09:03 +0000
> > Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> >   
> > > recordmcount edits the file in-place, which can cause problems when
> > > using ccache in hardlink mode.  Arrange for recordmcount to break a
> > > hardlinked object.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > ---
> > > Steven, sorry it took a while to get this out...  
> > 
> > Should this be for stable, or is it fine to just add this to my 4.5
> > queue?  
> 
> I thought you wanted to test it first - although I've been running with
> this for a while now, my nightly builds have masked out the mcount
> warning, and I suspect it'll take a while for ccache to purge itself
> of the modified objects.
> 
> If you're happy to add a stable tag to it, then please do so.
> 

I ran it through most my tests (it's still running and is at 20 of 33
tests). If there was anything wrong with this patch, I'm sure one of my
tests would have crashed by now.

Do you want to take it, or shall I?

If you want to take it, you can add my:

Reviewed-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Dec. 11, 2015, 6:33 p.m. UTC | #6
On Fri, Dec 11, 2015 at 01:10:29PM -0500, Steven Rostedt wrote:
> I ran it through most my tests (it's still running and is at 20 of 33
> tests). If there was anything wrong with this patch, I'm sure one of my
> tests would have crashed by now.

Thanks for testing.

> Do you want to take it, or shall I?

I'm easy - it probably makes more sense if take it.
Steven Rostedt Dec. 11, 2015, 6:51 p.m. UTC | #7
On Fri, 11 Dec 2015 18:33:27 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Fri, Dec 11, 2015 at 01:10:29PM -0500, Steven Rostedt wrote:
> > I ran it through most my tests (it's still running and is at 20 of 33
> > tests). If there was anything wrong with this patch, I'm sure one of my
> > tests would have crashed by now.  
> 
> Thanks for testing.
> 
> > Do you want to take it, or shall I?  
> 
> I'm easy - it probably makes more sense if take it.
> 

Heh, you got me hanging in suspense. You missed a word. Is it:

 "makes more sense if *you* take it"

or

 "makes more sense if *I* take it"

??

Or you could be replying to my sentence of "take it" or "shall I" in
which it would be *you* take it.

/me confused

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Dec. 11, 2015, 6:58 p.m. UTC | #8
On Fri, Dec 11, 2015 at 01:51:15PM -0500, Steven Rostedt wrote:
> On Fri, 11 Dec 2015 18:33:27 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Fri, Dec 11, 2015 at 01:10:29PM -0500, Steven Rostedt wrote:
> > > I ran it through most my tests (it's still running and is at 20 of 33
> > > tests). If there was anything wrong with this patch, I'm sure one of my
> > > tests would have crashed by now.  
> > 
> > Thanks for testing.
> > 
> > > Do you want to take it, or shall I?  
> > 
> > I'm easy - it probably makes more sense if take it.
> > 
> 
> Heh, you got me hanging in suspense. You missed a word. Is it:
> 
>  "makes more sense if *you* take it"
> 
> or
> 
>  "makes more sense if *I* take it"
> 
> ??

Oops, sorry.  "makes more sense if *you* take it" was what I thought I
typed!
Steven Rostedt Dec. 11, 2015, 7:28 p.m. UTC | #9
On Fri, 11 Dec 2015 18:58:09 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:


> Oops, sorry.  "makes more sense if *you* take it" was what I thought I
> typed!
> 

OK, will do.

Thanks!

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 698768bdc581..91705ef30402 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -211,6 +211,20 @@  static void *mmap_file(char const *fname)
 		addr = umalloc(sb.st_size);
 		uread(fd_map, addr, sb.st_size);
 	}
+	if (sb.st_nlink != 1) {
+		/* file is hard-linked, break the hard link */
+		close(fd_map);
+		if (unlink(fname) < 0) {
+			perror(fname);
+			fail_file();
+		}
+		fd_map = open(fname, O_RDWR | O_CREAT, sb.st_mode);
+		if (fd_map < 0) {
+			perror(fname);
+			fail_file();
+		}
+		uwrite(fd_map, addr, sb.st_size);
+	}
 	return addr;
 }