[Devel] Re: [RFC][PATCH 4/4] checkpoint/restart: simplify cr_scan_fds()

[Oren Laadan]

(as discussed in the LKML thread) as far as I can see the existing code is
safe, and this code is not more correct (for restart) in terms of races with
changes to the file table ?

Dave Hansen wrote:
> I think having all the allocations in one place, plus the
> reduction in the number of lines speaks for itself.  In
> any case, this is last in the series and can be dropped if
> you don't like it.
> 
> ---
> 
>  linux-2.6.git-dave/checkpoint/ckpt_file.c |   13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff -puN checkpoint/ckpt_file.c~fix-cr_scan_fds-realloc checkpoint/ckpt_file.c
> --- linux-2.6.git/checkpoint/ckpt_file.c~fix-cr_scan_fds-realloc	2008-12-02 10:26:53.000000000 -0800
> +++ linux-2.6.git-dave/checkpoint/ckpt_file.c	2008-12-02 10:26:53.000000000 -0800
> @@ -38,6 +38,7 @@ int cr_scan_fds(struct files_struct *fil
>  	int i, n = 0;
>  	int tot = CR_DEFAULT_FDTABLE;
>  
> +retry:
>  	fds = kmalloc(tot * sizeof(*fds), GFP_KERNEL);
>  	if (!fds)
>  		return -ENOMEM;
> @@ -55,18 +56,12 @@ int cr_scan_fds(struct files_struct *fil
>  		if (!fcheck_files(files, i))
>  			continue;
>  		if (n == tot) {
> -			/*
> -			 * fcheck_files() is safe with drop/re-acquire
> -			 * of the lock, because it tests:  fd < max_fds
> -			 */
> +			/* we undershot the size of fds[] */
>  			spin_unlock(&files->file_lock);
>  			rcu_read_unlock();
>  			tot *= 2;	/* won't overflow: kmalloc will fail */
> -			fds = krealloc(fds, tot * sizeof(*fds), GFP_KERNEL);
> -			if (!fds)
> -				return -ENOMEM;
> -			rcu_read_lock();
> -			spin_lock(&files->file_lock);
> +			kfree(fds);
> +			goto retry;
>  		}
>  		fds[n++] = i;
>  	}
> _
> 
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers