threads / discuss / 24056

Misterious warning about file system boundaries

Subject: Misterious warning about file system boundaries

## tl;dr

9 messages between Jun 9, 2010 and Jun 12, 2010.

replies: 8people: 6as markdown or json

Michael J Gruber· Jun 9, 2010, 20:21 UTC · lore
Heya,
now what is going on here? After upgrading to current next I get

warning: working tree spans across filesystems but GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.

in several repos, such as my local git.git repo. That is certainly on a single file system only (ext4 over lvm over luks, all on one partition, Fedora 13). I also get this for another repo, but not for every repo. It goes away when I set the var and comes back when I don't set it, of course.

Although I haven't bisected this should be due to 52b98a7 (write-index: check and warn when worktree crosses a filesystem boundary, 2010-04-04).

How does the code detect a file system boundary, and where could it go wrong?

Michael
Andreas Ericsson· Jun 10, 2010, 08:03 UTC · re: Michael J Gruber · lore

Re: Misterious warning about file system boundaries

On 06/09/2010 10:21 PM, Michael J Gruber wrote:
Show 19 quoted lines
> Heya,
> 
> now what is going on here? After upgrading to current next I get
> 
> warning: working tree spans across filesystems but
> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
> 
> in several repos, such as my local git.git repo. That is certainly on a
> single file system only (ext4 over lvm over luks, all on one partition,
> Fedora 13). I also get this for another repo, but not for every repo. It
> goes away when I set the var and comes back when I don't set it, of course.
> 
> Although I haven't bisected this should be due to
> 52b98a7 (write-index: check and warn when worktree crosses a filesystem
> boundary, 2010-04-04).
> 
> How does the code detect a file system boundary, and where could it go
> wrong?
> 

According to the patch, it checks if the device id recorded from stat(2) is the same for all files and, if not, warns about it.

It seems that your interpretation of "one partition" differs from that reported by the kernel. Why that is so, I have no idea.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
Michael J Gruber· Jun 10, 2010, 09:05 UTC · re: Andreas Ericsson · lore

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

Andreas Ericsson venit, vidit, dixit 10.06.2010 10:03:
Show 27 quoted lines
> On 06/09/2010 10:21 PM, Michael J Gruber wrote:
>> Heya,
>>
>> now what is going on here? After upgrading to current next I get
>>
>> warning: working tree spans across filesystems but
>> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>>
>> in several repos, such as my local git.git repo. That is certainly on a
>> single file system only (ext4 over lvm over luks, all on one partition,
>> Fedora 13). I also get this for another repo, but not for every repo. It
>> goes away when I set the var and comes back when I don't set it, of course.
>>
>> Although I haven't bisected this should be due to
>> 52b98a7 (write-index: check and warn when worktree crosses a filesystem
>> boundary, 2010-04-04).
>>
>> How does the code detect a file system boundary, and where could it go
>> wrong?
>>
> 
> According to the patch, it checks if the device id recorded from stat(2)
> is the same for all files and, if not, warns about it.
> 
> It seems that your interpretation of "one partition" differs from that
> reported by the kernel. Why that is so, I have no idea.
> 

I'm sorry, but "my interpretation"? WTF? This is all on /home/mjg/src/git which has no bind mounts whatsoever.

I actually mixed up my / and /home situation above, /home is even simpler: single ext3 over luks dm device over single "real" partition. All of this (except for single ext3 part.) should not matter, of course.

I bisected it just be sure, and it boils down to 9780e62 which is the commit merging 52b98a7 to next.

git ls-files|xargs stat -c "%d %D" |sort|uniq
gives
64772 fd04
which is, in particular, 1 device only. Now, here comes funny. After
changing write_index() to print the two ce_dev's which differ, i.e.
printf("%d %d\n", ce->ce_dev, cache[first_valid_ent]->ce_dev);
 I have:
./git-status -s|sort|uniq -c
warning: working tree spans across filesystems but
GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
    150 64770 64772
    662 64771 64772
      1  M read-cache.c
WTF???
git reset --hard doesn't help this.
rm .git/index && git reset does help.

So, it seems that nowadays not even "reset --hard" refreshes the index completely. After a reboot your device IDs may change, of course, and files on the same file system will have different ce_dev values in the index depending on when their index entry was refreshed last.

I really don't know how to fix this (other than by refreshing stat info in write_index() before warning). Debugging it was quite an exercise already.

Also, having git reset --index do the equivalent of "rm .git/index && git reset" might be good to have.

I'm keeping the bad index, btw, so that I can test a possible future fix.
Michael
Erik Faye-Lund· Jun 10, 2010, 09:39 UTC · re: Michael J Gruber · lore

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

On Thu, Jun 10, 2010 at 11:05 AM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

Show 63 quoted lines
> Andreas Ericsson venit, vidit, dixit 10.06.2010 10:03:
>> On 06/09/2010 10:21 PM, Michael J Gruber wrote:
>>> Heya,
>>>
>>> now what is going on here? After upgrading to current next I get
>>>
>>> warning: working tree spans across filesystems but
>>> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>>>
>>> in several repos, such as my local git.git repo. That is certainly on a
>>> single file system only (ext4 over lvm over luks, all on one partition,
>>> Fedora 13). I also get this for another repo, but not for every repo. It
>>> goes away when I set the var and comes back when I don't set it, of course.
>>>
>>> Although I haven't bisected this should be due to
>>> 52b98a7 (write-index: check and warn when worktree crosses a filesystem
>>> boundary, 2010-04-04).
>>>
>>> How does the code detect a file system boundary, and where could it go
>>> wrong?
>>>
>>
>> According to the patch, it checks if the device id recorded from stat(2)
>> is the same for all files and, if not, warns about it.
>>
>> It seems that your interpretation of "one partition" differs from that
>> reported by the kernel. Why that is so, I have no idea.
>>
>
> I'm sorry, but "my interpretation"? WTF? This is all on
> /home/mjg/src/git which has no bind mounts whatsoever.
>
> I actually mixed up my / and /home situation above, /home is even
> simpler: single ext3 over luks dm device over single "real" partition.
> All of this (except for single ext3 part.) should not matter, of course.
>
> I bisected it just be sure, and it boils down to 9780e62 which is the
> commit merging 52b98a7 to next.
>
> git ls-files|xargs stat -c "%d %D" |sort|uniq
>
> gives
>
> 64772 fd04
>
> which is, in particular, 1 device only. Now, here comes funny. After
> changing write_index() to print the two ce_dev's which differ, i.e.
> printf("%d %d\n", ce->ce_dev, cache[first_valid_ent]->ce_dev);
>  I have:
>
> ./git-status -s|sort|uniq -c
> warning: working tree spans across filesystems but
> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>    150 64770 64772
>    662 64771 64772
>      1  M read-cache.c
>
> WTF???
>
> git reset --hard doesn't help this.
>
> rm .git/index && git reset does help.
>
<snip>
>
> Also, having git reset --index do the equivalent of "rm .git/index &&
> git reset" might be good to have.
>
Doesn't "git update-index --refresh" do the trick?
-- 
Erik "kusma" Faye-Lund
Michael J Gruber· Jun 10, 2010, 10:36 UTC · re: Erik Faye-Lund · lore

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

Erik Faye-Lund venit, vidit, dixit 10.06.2010 11:39:
Show 73 quoted lines
> On Thu, Jun 10, 2010 at 11:05 AM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Andreas Ericsson venit, vidit, dixit 10.06.2010 10:03:
>>> On 06/09/2010 10:21 PM, Michael J Gruber wrote:
>>>> Heya,
>>>>
>>>> now what is going on here? After upgrading to current next I get
>>>>
>>>> warning: working tree spans across filesystems but
>>>> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>>>>
>>>> in several repos, such as my local git.git repo. That is certainly on a
>>>> single file system only (ext4 over lvm over luks, all on one partition,
>>>> Fedora 13). I also get this for another repo, but not for every repo. It
>>>> goes away when I set the var and comes back when I don't set it, of course.
>>>>
>>>> Although I haven't bisected this should be due to
>>>> 52b98a7 (write-index: check and warn when worktree crosses a filesystem
>>>> boundary, 2010-04-04).
>>>>
>>>> How does the code detect a file system boundary, and where could it go
>>>> wrong?
>>>>
>>>
>>> According to the patch, it checks if the device id recorded from stat(2)
>>> is the same for all files and, if not, warns about it.
>>>
>>> It seems that your interpretation of "one partition" differs from that
>>> reported by the kernel. Why that is so, I have no idea.
>>>
>>
>> I'm sorry, but "my interpretation"? WTF? This is all on
>> /home/mjg/src/git which has no bind mounts whatsoever.
>>
>> I actually mixed up my / and /home situation above, /home is even
>> simpler: single ext3 over luks dm device over single "real" partition.
>> All of this (except for single ext3 part.) should not matter, of course.
>>
>> I bisected it just be sure, and it boils down to 9780e62 which is the
>> commit merging 52b98a7 to next.
>>
>> git ls-files|xargs stat -c "%d %D" |sort|uniq
>>
>> gives
>>
>> 64772 fd04
>>
>> which is, in particular, 1 device only. Now, here comes funny. After
>> changing write_index() to print the two ce_dev's which differ, i.e.
>> printf("%d %d\n", ce->ce_dev, cache[first_valid_ent]->ce_dev);
>>  I have:
>>
>> ./git-status -s|sort|uniq -c
>> warning: working tree spans across filesystems but
>> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>>    150 64770 64772
>>    662 64771 64772
>>      1  M read-cache.c
>>
>> WTF???
>>
>> git reset --hard doesn't help this.
>>
>> rm .git/index && git reset does help.
>>
> <snip>
>>
>> Also, having git reset --index do the equivalent of "rm .git/index &&
>> git reset" might be good to have.
>>
> 
> Doesn't "git update-index --refresh" do the trick?
> 
No.
And neither does --really-refresh.
I guess we need --I-really-mean-it-refresh.

In fact, not even after recompiling with USE_STDEV=y that --really-refresh helps which stomps me.But what do I know.

Michael
Ævar Arnfjörð Bjarmason· Jun 10, 2010, 10:52 UTC · re: Michael J Gruber · lore

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

On Thu, Jun 10, 2010 at 10:36, Michael J Gruber <git@drmicha.warpmail.net> wrote:

> I guess we need --I-really-mean-it-refresh.
>
> In fact, not even after recompiling with USE_STDEV=y that
> --really-refresh helps which stomps me.But what do I know.

The real problem here is the assumption in 52b98a7 that stat's st_dev will stay the same for a given device, clearly that's not the case on Linux if you reboot your machine.

Is there some way of working around that?
Jeff King· Jun 10, 2010, 11:02 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

On Thu, Jun 10, 2010 at 10:52:40AM +0000, Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> On Thu, Jun 10, 2010 at 10:36, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
> > I guess we need --I-really-mean-it-refresh.
> >
> > In fact, not even after recompiling with USE_STDEV=y that
> > --really-refresh helps which stomps me.But what do I know.
> 
> The real problem here is the assumption in 52b98a7 that stat's st_dev
> will stay the same for a given device, clearly that's not the case on
> Linux if you reboot your machine.
> 
> Is there some way of working around that?

We could simply drop the warning. The real point of the original patch was to stop cross-device traversal in setup_git_directory, which doesn't care about crossing reboots. I believe the index check and warning are just there as an early suggestion. When you actually fail to cross the device boundary looking for .git in the future, it will complain and mention GIT_DISCOVERY_ACROSS_FILESYSTEM then.

Since the early-warning suggestion is generating false positives, and I don't think there is a portable and reliable way around it, dropping it makes sense to me.

-Peff
Junio C Hamano· Jun 10, 2010, 14:53 UTC · re: Jeff King · lore

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

Jeff King <peff@peff.net> writes:
> Since the early-warning suggestion is generating false positives, and I
> don't think there is a portable and reliable way around it, dropping it
> makes sense to me.

Makes sense. Let's make it so (I won't have time to do that myself until late this evening, though).

Thanks.
Jeff King· Jun 12, 2010, 04:24 UTC · re: Junio C Hamano · lore

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

On Thu, Jun 10, 2010 at 07:53:47AM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > Since the early-warning suggestion is generating false positives, and I
> > don't think there is a portable and reliable way around it, dropping it
> > makes sense to me.
> 
> Makes sense.  Let's make it so (I won't have time to do that myself until
> late this evening, though).
> 
> Thanks.

Here it is with a nice commit message (though since it is only in next, I guess you can just merge the early part and both 52b98a and my revert will be dropped). It's on top of ld/discovery-limit-to-fs, of course.

Note that it would be possible to do a warning in some instances. We just can't trust the on-disk index ce_dev information, so if you actually refreshed two cross-filesystem entries we could detect that. That is such a minority case that I doubt it is worth caring about, though.

-- >8 --
Subject: [PATCH] Revert "write-index: check and warn when worktree crosses a filesystem boundary"
This reverts commit 52b98a7d2f12b5d0dd076221d40f8fa93598e11a.

The goal of that commit was to warn users early when their worktree crossed filesystem boundaries. It worked by comparing the st_dev stat information in the index, and warning when we had more than one device.

However, the stat information may come from multiple runs, and the st_dev field is not necessarily stable. In particular, st_dev will change on Linux across reboots. Index entries from the previous reboot will appear to come from a different device, triggering a false positive.

Since this message is really only an early warning for people who will be bit by the new GIT_DISCOVERY_ACROSS_FILESYSTEM behavior, and because they will get an actual error later on (when we can't find their cross-filesystem .git directory), we can just scrap the early warning.

Signed-off-by: Jeff King <peff@peff.net>
---
 read-cache.c |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index e381ea5..f1f789b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1550,8 +1550,6 @@ int write_index(struct index_state *istate, int newfd)
 	struct cache_entry **cache = istate->cache;
 	int entries = istate->cache_nr;
 	struct stat st;
-	int first_valid_ent = -1;
-	int more_than_one_dev;
 
 	for (i = removed = extended = 0; i < entries; i++) {
 		if (cache[i]->ce_flags & CE_REMOVE)
@@ -1574,7 +1572,6 @@ int write_index(struct index_state *istate, int newfd)
 	if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)
 		return -1;
 
-	more_than_one_dev = 0;
 	for (i = 0; i < entries; i++) {
 		struct cache_entry *ce = cache[i];
 		if (ce->ce_flags & CE_REMOVE)
@@ -1583,19 +1580,8 @@ int write_index(struct index_state *istate, int newfd)
 			ce_smudge_racily_clean_entry(ce);
 		if (ce_write_entry(&c, newfd, ce) < 0)
 			return -1;
-		if (ce_uptodate(ce)) {
-			if (first_valid_ent < 0)
-				first_valid_ent = i;
-			else if (ce->ce_dev != cache[first_valid_ent]->ce_dev)
-				more_than_one_dev = 1;
-		}
 	}
 
-	if (more_than_one_dev &&
-	    !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0))
-		warning("working tree spans across filesystems but "
-			"GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.");
-
 	/* Write extension data here */
 	if (istate->cache_tree) {
 		struct strbuf sb = STRBUF_INIT;
-- 
1.7.1.516.gd5539.dirty

← back to recent threads