[RFC] ext3: don't clear orphan list on ro mount with errors

When we have a filesystem with an orphan inode list *and* in error
state, things behave differently if:

1) e2fsck -p is done prior to mount: e2fsck fixes things and exits
   happily (barring other significant problems)

vs.

2) mount is done first, then e2fsck -p: due to the orphan inode
   list removal, more errors are found and e2fsck exits with
   UNEXPECTED INCONSISTENCY.

The 2nd case above, on the root filesystem, has the tendency to halt
the boot process, which is unfortunate.

The situation can be improved by not clearing the orphan
inode list when the fs is mounted readonly.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---


--
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

On Mon 27-08-12 14:30:40, Eric Sandeen wrote:
> When we have a filesystem with an orphan inode list *and* in error
> state, things behave differently if:
> 
> 1) e2fsck -p is done prior to mount: e2fsck fixes things and exits
>    happily (barring other significant problems)
> 
> vs.
> 
> 2) mount is done first, then e2fsck -p: due to the orphan inode
>    list removal, more errors are found and e2fsck exits with
>    UNEXPECTED INCONSISTENCY.
> 
> The 2nd case above, on the root filesystem, has the tendency to halt
> the boot process, which is unfortunate.
> 
> The situation can be improved by not clearing the orphan
> inode list when the fs is mounted readonly.
  Yeah, makes sense. I've added the patch to my tree. Thanks.

								Honza
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index ff9bcdc..485b4fa 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -1490,10 +1490,12 @@ static void ext3_orphan_cleanup (struct super_block * sb,
>  	}
>  
>  	if (EXT3_SB(sb)->s_mount_state & EXT3_ERROR_FS) {
> -		if (es->s_last_orphan)
> +		/* don't clear list on RO mount w/ errors */
> +		if (es->s_last_orphan && !(s_flags & MS_RDONLY)) {
>  			jbd_debug(1, "Errors on filesystem, "
>  				  "clearing orphan list.\n");
> -		es->s_last_orphan = 0;
> +			es->s_last_orphan = 0;
> +		}
>  		jbd_debug(1, "Skipping orphan recovery on fs with errors.\n");
>  		return;
>  	}
>

On 8/28/12 3:02 AM, Jan Kara wrote:
> On Mon 27-08-12 14:30:40, Eric Sandeen wrote:
>> When we have a filesystem with an orphan inode list *and* in error
>> state, things behave differently if:
>>
>> 1) e2fsck -p is done prior to mount: e2fsck fixes things and exits
>>    happily (barring other significant problems)
>>
>> vs.
>>
>> 2) mount is done first, then e2fsck -p: due to the orphan inode
>>    list removal, more errors are found and e2fsck exits with
>>    UNEXPECTED INCONSISTENCY.
>>
>> The 2nd case above, on the root filesystem, has the tendency to halt
>> the boot process, which is unfortunate.
>>
>> The situation can be improved by not clearing the orphan
>> inode list when the fs is mounted readonly.
>   Yeah, makes sense. I've added the patch to my tree. Thanks.
> 
> 								Honza

After a little more investigation, I'm now wondering if this is really
worth doing.

e2fsck zaps the orphan list just like the kernel does:

         * If the filesystem contains errors, don't run the orphan
         * list, since the orphan list can't be trusted; and we're
         * going to be running a full e2fsck run anyway...

and my 1) and 2) differences above were due to testing an older version
of e2fsck which didn't properly propagate the error flag.  (Sorry...)

Since upstream e2fsck will _also_ ignore the orphan inode list, there's
probably no great reason for preserving it on a readonly mount after all,
unless it's just to minimize changes when mounting RO (which may be a
sufficient reason, I suppose).  So feel free to take it or leave it,
I guess.

Thanks,
-Eric
--
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

On Tue 04-09-12 13:51:57, Eric Sandeen wrote:
> On 8/28/12 3:02 AM, Jan Kara wrote:
> > On Mon 27-08-12 14:30:40, Eric Sandeen wrote:
> >> When we have a filesystem with an orphan inode list *and* in error
> >> state, things behave differently if:
> >>
> >> 1) e2fsck -p is done prior to mount: e2fsck fixes things and exits
> >>    happily (barring other significant problems)
> >>
> >> vs.
> >>
> >> 2) mount is done first, then e2fsck -p: due to the orphan inode
> >>    list removal, more errors are found and e2fsck exits with
> >>    UNEXPECTED INCONSISTENCY.
> >>
> >> The 2nd case above, on the root filesystem, has the tendency to halt
> >> the boot process, which is unfortunate.
> >>
> >> The situation can be improved by not clearing the orphan
> >> inode list when the fs is mounted readonly.
> >   Yeah, makes sense. I've added the patch to my tree. Thanks.
> > 
> > 								Honza
> 
> After a little more investigation, I'm now wondering if this is really
> worth doing.
> 
> e2fsck zaps the orphan list just like the kernel does:
> 
>          * If the filesystem contains errors, don't run the orphan
>          * list, since the orphan list can't be trusted; and we're
>          * going to be running a full e2fsck run anyway...
> 
> and my 1) and 2) differences above were due to testing an older version
> of e2fsck which didn't properly propagate the error flag.  (Sorry...)
> 
> Since upstream e2fsck will _also_ ignore the orphan inode list, there's
> probably no great reason for preserving it on a readonly mount after all,
> unless it's just to minimize changes when mounting RO (which may be a
> sufficient reason, I suppose).  So feel free to take it or leave it,
> I guess.
  Since I've already pushed this to Linus and minimizing changes on RO
filesystem makes sense anyway, I'll leave the patch in... Thanks for the
update.

								Honza