| Message ID | 20151202102339.GB8644@n2100.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > } > >
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; > } >
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.
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
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
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.
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
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!
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
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; }