(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@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers