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
|