Re: [PATCH] recover from iclog allocation failures

[Eric Sandeen <sandeen@sandeen.net>]

Christoph Hellwig wrote:
> On Fri, Feb 08, 2008 at 11:45:53PM -0600, Eric Sandeen wrote:
>>  	mp->m_log = xlog_alloc_log(mp, log_target, blk_offset, num_bblks);
>> +	if (!mp->m_log)
>> +		return ENOMEM;
> 
> Currently there's no allocations in there that should be able to fail.

Well, actually...

> But actually marking these KM_MAYFAIL would be a good idea.
> 
>> @@ -1219,6 +1221,13 @@ xlog_alloc_log(xfs_mount_t	*mp,
>>  		prev_iclog = iclog;
>>  
>>  		bp = xfs_buf_get_noaddr(log->l_iclog_size, mp->m_logdev_targp);

This was failing for him I think.

>> +		if (!iclog || !bp) {
>> +			if (iclog)
>> +				kmem_free(iclog, sizeof(xlog_in_core_t));
>> +			log->l_iclog_bufs = i;
>> +			xlog_dealloc_log(log);
>> +			return NULL;
>> +		}
> 
> Please check for iclog beeing NULL before trying to allocate the buffer,
> and switch it to KM_MAYFAIL.  Given that there are two failing cases now
> it would make sense to have goto-unwinding here.

hmm yeah probably so.

> Also once you touch the memory allocation feel free to remove the
> useless casts of their return values.
> 
>> Index: linux-2.6.24.noarch/fs/xfs/xfs_mount.c
>> ===================================================================
>> --- linux-2.6.24.noarch.orig/fs/xfs/xfs_mount.c
>> +++ linux-2.6.24.noarch/fs/xfs/xfs_mount.c
>> @@ -1007,6 +1007,7 @@ xfs_mountfs(
>>  			error = XFS_ERROR(EINVAL);
>>  			goto error1;
>>  		}
>> +		uuid_mounted = 1;
> 
> How is this related to the rest of the patch?

if we errored out, we returned from mount w/o taking the uuid back out
of the table.

-Eric